Fix: Equalise the number of frames needed for road vehicles to traverse different radius curves #8609
Motivation / Problem
There is a story behind all this, before I started working on the PR.
Savegame: vehicle speed test large.zip
For directions along the axis (diagonal), between the 3 vehicle types, I expected the results to match. Alas, they don't. Train took 594 days, ship took 588 days and bus took 592 days. Travelling on the way back, the results maintained consistent within each vehicle type: 594 days for train, 588 days for ship, 592 days for bus.
For the other directions (horizontal and vertical), I expected train and ship times to match, but not the bus, due to it not really having a horizontal/vertical road. I tested the bus with a long curvy zig-zag road from start to finish. Between train and ship, train took 396 days and ship took 392 days. On the way back, the results were the same: 396 for train, 392 for ship.
This discrepancy is what led me to investigate deep into the issue, and I find the problem is due to the different number of frames road vehicles need to go through to cross curved tiles.
Each road piece has two curves: the inner curve and the outer curve. Inner curves have fewer frames that outer curves, that's to be expected. However, different inner curves have less frames than other inner curves. The same with the outer curves, having less frames than other outer curves.
Going back to the zig-zag curves of the speed test, and reducing the number of tiles to just 2, we get:
I wanted to work on a solution to this issue, and what came to mind immediately was to equalise the number of frames needed to traverse the different curves. I had 4 options: "17/10", "18/11", "17/11" or "18/10".
I went on with "18 outer /11 inner" as the solution for all 4 curves, as I thought it would be easier to do savegame conversion based on adding missing frames than removing frames.
To make all curves "18/11", I ended up having to invent values for the missing frames. These values are respectively the x_pos and y_pos of the vehicle. I used my best judgement to make the shift from a frame to another not to break IndividualRoadVehicleController, and the best approach that I went with, is also the one that makes the most sense, which was changing the position values by 1, and accomodate the rest of the frames for this addition/subtraction.
The most complicated part is the savegame conversion. It now has to, not only, update the current positions to match the new values for the movement data, it also has to, in the case of articulated vehicles, play catch-up. In essence, all of the affected curves gain 1 more frame, which means 1 more tick that needs to be catch-up for each trailing part. Luckily this was already done in the past, so I only had to borrow that part of the code, and adapt it to this situation.
Looking at the bus again:
Coincidentally, it is also identical to the diagonal directions, also with 592 days.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.
(This is pseudo-code.)
The text was updated successfully, but these errors were encountered:
The samegame conversion here touches all 4 of "frame", "direction", "x_pos" and "y_pos". On top of that it also calls IndividualRoadVehicleController(), and UpdateInclination().
In general, this PR is way too complex, and impossible to review: road vehicles have very special movement in curves, when reversing, in depots, in wormholes, ... and the consistency of articulated vehicles must be preserved in all cases.
my professional opinion is that the chance of this happening is fairly low. they would have to have set the length of the middle part to a value that exactly matches the distance between two 45° turns, and since you say this is currently inconsistent, that sounds very unlikely.
if they simply copied the CETS approach, that should not be affected by this change.
…se different radius curves