FS#6142 - Incorrect signal states while placing path-signals .

Attached to Project: OpenTTD
Opened by Jaap (Jaap) - Tuesday, 14 October 2014, 21:32 GMT
Type Bug
Category Core
Status New
Assigned To No-one
Operating System All
Severity High
Priority Normal
Reported Version 1.4.3
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


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.

This task depends upon

Comment by Stefan Krug (estys) - Thursday, 01 January 2015, 12:58 GMT
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.
Comment by Remko Bijker (Rubidium) - Saturday, 03 January 2015, 10:00 GMT
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.
(application/octet-stream)    fs6142.sav (507.1 KiB)