FS#4030 - RV skips "go non-stop to nearest depot" order

Attached to Project: OpenTTD
Opened by Attila7 (Attila7) - Friday, 13 August 2010, 18:21 GMT
Last edited by Remko Bijker (Rubidium) - Sunday, 15 August 2010, 13:17 GMT
Type Bug
Category Vehicles
Status Closed
Assigned To No-one
Operating System All
Severity Critical
Priority Normal
Reported Version trunk
Due in Version 1.0.4
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


There is a bug where an RV will skip an order to go NON-STOP to the nearest depot. This happens if the RV was told to go to a drive-thru station, then to the nearest depot and stop, but upon leaving the station it must turn around at the end of a road. When the RV hits the end of the road, the order list changes to the next order (depot skipped).

The key is that the order must be "non-stop". If the order is just "go to nearest depot", the order will not be skipped.

I watched this for a number of trips of a bus, then I rearranged the roads so the RV could go to the depot without having to turn around and the order was not skipped.

See attached screenshot and savegame file.
This task depends upon

Closed by  Remko Bijker (Rubidium)
Sunday, 15 August 2010, 13:17 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r20498
Comment by Vít Šefl (Vitus) - Friday, 13 August 2010, 21:24 GMT
Very strange indeed. Attached savegame for easier testing.
Comment by Attila7 (Attila7) - Friday, 13 August 2010, 21:26 GMT
I have done some digging in the code, and this bug seems to be a major performance hit, because of the following code at line 118 in file order_cmd.cpp:

bool Order::Equals(const Order &other) const
/* In case of go to nearest depot orders we need "only" compare the flags
* with the other and not the nearest depot order bit or the actual
* destination because those get clear/filled in during the order
* evaluation. If we do not do this the order will continuously be seen as
* a different order and it will try to find a "nearest depot" every tick. */
if ((this->type == OT_GOTO_DEPOT && this->type == other.type) &&
((this->GetDepotActionType() & ODATFB_NEAREST_DEPOT) != 0 ||
(other.GetDepotActionType() & ODATFB_NEAREST_DEPOT) != 0)) {
this->GetDepotOrderType() == other.GetDepotOrderType() &&
(this->GetDepotActionType() & ~ODATFB_NEAREST_DEPOT) == (other.GetDepotActionType() & ~ODATFB_NEAREST_DEPOT);

this->type == other.type &&
this->flags == other.flags &&
this->dest == other.dest;

I am not yet sure what the fix is, but what this code is trying to avoid is exactly what is happening if the order is non-stop goto nearest depot. The game tries to find the nearest depot on every tic (calling FindClosestDepot at line 1740 of order_cmd.cpp). When the bus gets to the reversing point, the pathfinder does not find a path to the nearest depot for some reason (another bug). This triggers the increment of the order number at line 1759 of order_cmd.

When the order does not contain "non-stop", the pathfinder is called and it finds the nearest depot as soon as the bus leaves the station and is not called again because the code above works as intended.

The bug in the pathfinder is that it cannot find the nearest depot when the bus is pointed directly South as it is reversing.