Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signals: incorrect signal states while placing path-signals . #6142

Closed
DorpsGek opened this issue Oct 14, 2014 · 3 comments
Closed

Signals: incorrect signal states while placing path-signals . #6142

DorpsGek opened this issue Oct 14, 2014 · 3 comments
Labels
bug Something isn't working flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) stale Stale issues

Comments

@DorpsGek
Copy link
Member

Jaap opened the ticket and wrote:

I built a railway track around the map with block-signals and a lot of trains where running on this track. I stopped all trains after a while.
Next, I removed all block-signals and replaced with path-signals.

Now, several signals are showing a green light while no save path is available to the next signal. (Of course, several train crashes will occur when I let the trains running again. )
I had this problem several times in normal games, but it was hard to reproduce this issue.
My OS: Windows 7 Pro 64bit with all updates installed.

I attached my save game before I did the signal replacements, so you should be able to reproduce this problem easily.
To make this problem clear, a created some screen prints and watch at the signal at the right bottom of the screen.

Attachments

Reported version: 1.4.3
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6142
@DorpsGek
Copy link
Member Author

DorpsGek commented Jan 1, 2015

estys wrote:

The behavior is the same on trunk r27100.

When removing signal S0, the call to TryPathReserve in CmdRemoveSingleSignal is reserving a path for train T0 to the next signal at S1.
This sounds already a little bit strange, since now we have a reserved path which does not end at a PBS but a normal signal.
What makes things worse is that the signal tile is included in the reservation and now the reserved paths of train T0 and T1 are connected.

So, now, when placing a PBS signal at S0, GetTrainForReservation in CmdBuildSignal is returning train T1 (due to the connected paths and since (in this case) SE direction is checked before NW). Thus, the subsequent call to FreeTrainTrackReservation(T1) will free the reservation of the wrong train.
But even if it was returning the expected T0, the subsequent call to FreeTrainTrackReservation(T0) would follow the reserved path until it ends. Since the path actually only ends at T1's end of reserved path (since they are connected), the call would free also the reserved path of T1, which is not nice either.

One attempt which did solve this particular issue was to FreeTrainTrackReservation and not to TryPathReserve for CmdRemoveSingleSignal. This is also in line with what happens in CmdRemoveSingleRail. The reservation in this case is handled by the TrainController.

Another attempt is to actually exclude the signal tile from the path reservation, avoiding merging of reservations. This seems to be the usual case for PBS signals as well, not sure why non-PBS signals should behave differently.

Both do solve the particular issue, however, I am not familiar enough with the code to judge on potential side-effects.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6142#comment13684

@DorpsGek
Copy link
Member Author

DorpsGek commented Jan 3, 2015

Rubidium wrote:

Thank you for this attempt at fixing this bug, sadly enough both fixes break other things.

The exclude signal on path reservation patch definitely seems to break at least signals on diagonals; just perform the tasks to reproduce this issue with the patch applied and then start all trains. You'll notice they'll get stuck on the diagonal bits.

The no try reserve patch seems to work better, but I'm not fond of removing already existing reservations upon removing signals. This means that a train that has a route through a junction gets its reservation cancelled and another train may pick that reservation up. For example, in the attached savegame: remove the signal that train # 2 has just passed and then unpause the game. With the no try reserve patch you'll get a crash, without it you don't. Sadly enough removing the clearing of the reservation from your patch doesn't solve the issue.

To make matters worse, the code is written with the idea that yellow aspects of signals could be introduced so a reservation may specifically run through a path signal until another signal (especially so trains don't go into an infinite G deceleration when stopping at a red signal). So simply terminating a reservation at newly built signals is not really a nice feature.

Having said that, removing signals is a so-called dangerous action that can trigger train crashes. Just imagine that people started to remove signals on a live real world track. Adding signals is safe since it just adds more signalling blocks, but it keeps the current reservations in place, i.e. the new signals act after the state of the current reservations.

So in the end the removal of the signals could possibly be made slightly safer by not reserving through the next train but I have no real idea on how to do that right now.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6142#comment13690

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. I'm closing it as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun. Feel free to discuss in irc or request re-opening if you disagree. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) stale Stale issues
Projects
None yet
Development

No branches or pull requests

4 participants