FS#186 - autoreplaced trains can leave all wagons in depot

Attached to Project: OpenTTD
Opened by Bjarni (Bjarni) - Wednesday, 24 May 2006, 11:53 GMT
Type Bug
Category Vehicles → Autoreplace
Status Closed
Assigned To Bjarni (Bjarni)
Operating System All
Severity Medium
Priority High
Reported Version 0.4.7
Due in Version 0.4.8
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


the problem is that when mammoth trains are disabled, the max is 9 (it used to be 10. Don't ask me why ludde changed that)
If an engine got 8 wagons, it's a total of 9. If the engine is replaced to a dualheaded one, it mean that a train of two units tries to take 8 units and add them to the train. This will fail due to length and the wagons will be left behind.

1) Make autoreplace remove a wagon. Because ReplaceVehicle() adds the wagons and MaybeReplaceVehicle() removes wagons (to keep length, not number of units) it mean that a change of design is needed (or dublicated code)
I don't like this solution.

2) completely remove max-train-length. Since trains are linked lists, we don't have anything that makes a hard limit. Because it takes time to go through all units in very long trains, this solution is not popular, at least not in multiplayer.

3) a modified version of 2), which takes computer resources into account. Instead of making max-train-length based on single units, we will use it to prevent huge trains (specially in MP). Since it's not important if the limit is 30 or 32 units, we can make one (length)unit to be what you get when you hit the build button once, then a multiheaded pair of engines will only count once, an articulated engine will also only count once (like it does now, so it's inconsistent). This mean that autoreplace will always remove one unit and add one unit and this bug will be gone.
It makes little sense to keep the max-train-length to keep trains to fit on 5 tile stations, both because of the max 9 (not 10, which is 5 tiles) and because newgrf allow us to use shorter wagons.

I'm in favour of 3). I already got it working in my local copy and it works. Now I would like feedback on what you think of such a change
This task depends upon

Closed by  Bjarni (Bjarni)
Thursday, 08 June 2006, 20:12 GMT
Reason for closing:  Fixed
Comment by Bjarni (Bjarni) - Wednesday, 24 May 2006, 12:00 GMT
forgot to mention: if we go with 3), we should make max-train-length an int, not a bool, so some games can have 30 while others can have 70, 200 or completely off depending on what they want (and available CPU power)
Comment by Peter Nelson (peter1138) - Saturday, 03 June 2006, 08:17 GMT
This patch stops an engine being replaced if the train length would be too long.

There is no suppport for wagon removal yet, as, as you said, it would need to be done before the replacement is done, unless some hack is put it in to make the move command ignore length whilst in autoreplace...