FS#2130 - Restoring a conditional order jump may fail

Attached to Project: OpenTTD
Opened by Swallow (Swallow) - Saturday, 05 July 2008, 12:43 GMT
Last edited by Remko Bijker (Rubidium) - Saturday, 05 July 2008, 23:44 GMT
Type Bug
Category Vehicles
Status Closed
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version 0.7.0
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Bug is present in r13677

When one replaces a vehicle in a depot, the order list is normally preserved. This order backup and restoration may fail for conditional order jumps in two cases:

a) The comparator used is not 'equal to' or 'not equal to'
b) The order to jump to has a higher number than the conditional jump itself.

As far I can tell, this is caused by these two lines (459/460) in order_cmd.cpp:

if (skip_to >= v->num_orders) return CMD_ERROR;
if (new_order.GetNonStopType() != ONSF_STOP_EVERYWHERE) return CMD_ERROR;

These checks may be needed when inserting orders, they cause trouble when restoring them. The order to skip to may be higher than the total number of orders because not all of them are inserted yet. Non stop flags use the same bits as the conditional comparator, which causes the second check to give problems.

Note: while I could find out what causes this. I'm not sure of the best solution. Therefore I decided to not to create a fix (yet)
This task depends upon

Closed by  Remko Bijker (Rubidium)
Sunday, 20 July 2008, 07:41 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r13752.
Comment by Remko Bijker (Rubidium) - Wednesday, 09 July 2008, 10:19 GMT
The major problem is that you can't allow inserting orders that are invalid at the moment of insertion. I'm sure there's a proper way around it but as you already mentioned, it needs quite a lot of thinking for solutions before finding one that will actually work correctly.
Comment by Yorick (yorick) - Tuesday, 15 July 2008, 17:32 GMT
What about a DC_AUTOREPLACE flag that would allow the orders to be inserted?
Comment by Remko Bijker (Rubidium) - Tuesday, 15 July 2008, 17:34 GMT
DC_AUTOREPLACE isn't send over the network and that is exactly what is needed to restore the backup.
Comment by Yorick (yorick) - Tuesday, 15 July 2008, 17:35 GMT
DC_AUTOREPLACE isn't, but the autoreplace itself should be done everywhere...