OpenTTD

Tasklist

FS#5859 - Calling VehicleEnterDepot for a train frees the reservation of the depot

Attached to Project: OpenTTD
Opened by Juanjo (juanjo) - Saturday, 11 January 2014, 14:48 GMT
Last edited by andythenorth (andythenorth) - Saturday, 02 September 2017, 16:48 GMT
Type Patch
Category Core
Status New   Reopened
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version Version?
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

This is just a small simplification of CheckTrainStayInDepot().

There is no need to reserve the depot for servicing in a depot. Moreover, servicing a train automatically frees the reservation.
This task depends upon

Comment by Juanjo (juanjo) - Saturday, 11 January 2014, 14:50 GMT
I forgot to add the description of the task. This is not a bug but a codechange and the patch is applied against r26234. Sorry.
Comment by Ingo von Borstel (planetmaker) - Wednesday, 11 February 2015, 09:19 GMT
Are you sure it needs not reservation on the depot tile, thus that it cannot collide with a train leaving it?
Comment by andythenorth (andythenorth) - Saturday, 02 September 2017, 12:31 GMT
Closed and reopened - frosch disagreed with my closure reasons.
Comment by Juanjo (juanjo) - Saturday, 02 September 2017, 14:07 GMT
I'm sorry I didn't answer. I have checked the patch again.

Comparison before-after:

Situation: Checking if it can leave the depot. Train is inside the depot and next order tries to re-enter the same depot.

Before:
Entering the if-statement, always leads to a "return true".
If depot has no reservation, VehicleEnterDepot is called.
If depot has no reservation, a SetReservation on tile is called. But with no effect, because VehicleEnterDepot is called immediately, clearing the reservation of the depot.
The comment says we need a reservation, but it is not true (it is just a hidden workaround).

After:
Entering the if-statement, always leads to a "return true".
If depot has no reservation, VehicleEnterDepot is called.
So... the lines inside the if-statement could be read as "service the train only if there is no reservation (there is no other train in the middle)".

Both versions do the same, but the second version is clearer: Only re-enter if no other train has a reservation of the depot, because calling VehicleEnterDepot would remove the reservation of the second train (causing a bug).

Of course, we could move some lines from VehicleEnterDepot to rail_cmd.cpp just before VehicleEnterDepot is called. This way, we wouldn't need the "check depot has no reservation" workaround.
Comment by Juanjo (juanjo) - Saturday, 02 September 2017, 14:11 GMT
The issue description is wrong.
"Servicing a train automatically frees the reservation" should have been "calling VehicleEnterDepot for a train frees the reservation of the depot".

Loading...