Fix train stations sometimes not detecting an arrived train #9875
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had players report to me that randomly, a train could stop at a station, but the station would act as if there was no train present.
Look at the rear station here, the train is stopped but not accepting the schedule nor is the flag raised.

Demo case for the issue created after completed research
.
After many, many hours of research and adding instrumentation, and with research help from @JensenJ and @cshcrafter , I have finally determined the root cause of this problem.
In brief:
When a train approaches a station it reserves itself as the stations nearestTrain weakReference. Usually when a train leaves the station again this reference is nulled. But, when a train changes its destination station en route, a hanging weakReference to the wrong train remains in place. This in turn exposes another implementation issue where reserveFor() uses distanceToDestination instead of doing a distance check to the actual station. In the case of a hanging weakReference this distance can be relative to any random station, and if the stale train is stopped at its destination station somewhere, reserveFor would always think the stale train is closest and fail to reserve itself properly for the newly approaching train.
Full writeup based on investigating the log with more detail:
Two trains are on approach to the western loading yard (first c1c9d, then 4b5f0). They stop at yard entry where they get their schedule to loader1, which is usually e1e37, but can also be aa4fe if the former is busy.
We see in the log train c1c9d approaching aa4fe and calling reserveFor at aa4fe. But, befure c1c9d reaches aa4fe, it reroutes to e1e37.
This leaves a weakReference of nearestTrain hanging at aa4fe that would never be cleared as cancelReservation as c1c9d leaves e1e37 only clears the weak reference at e1e37.
This hanging reference to c1c9d ordinarily doesn't do anything if the code in reserveFor was working as it was originally intended. However, as reserveFor uses train.navigation.distanceToDestination, this distance is relative to the trains target, which does not have to match the distance we actually want, the distance to the station reserveFor is being called at. Usually the two match as this function is intended for the case of two competing trains trying to approach our station. But in the case of an en route navigation target change, the weakReference is left hanging and thus our distanceToDestination can be relative to any other station.
Our chain of events now continues. c1c9d has rerouted to e1e37 and has stopped there, thus having a distanceToDestination of 0.
Moments later, our second train, 4b5f0 gets its schedule at the yard lead and correctly deduces that e1e37 is busy and it should go to aa4fe. As it starts navigating there, it calls reserveFor at aa4fe. But this call fails to remove the hanging weak reference to c1c9d being the nearest train despite 4b5f0 being the closest to aa4fe because c1c9d is the closest not to aa4fe, but to its current target.
End result is that train 4b5f0 finishes its movement and stops at aa4fe, now no longer calling reserveFor as it is no longer navigating. aa4fe never correcly updats its nearest train and thinks that c1c9d is actually the closest train despite being stopped next door.
This log was created with large amounts of instrumentation added on another branch you can find here
Minimal Testcase
If you want to look at a minimal test case demonstrating the issue, download this world and give both trains a schedule towards 'home' (first slot of your inventory). The setup creates a case where the first train wants to navigate to the left target station when leaving the home station, but is stopped en route and the signal to its target station is pulled to red, which makes the the train pathfind to the right target station. This sets up the stale reference. Then the second train arrives at the left station and the effects of the stale reference combined with the first train being stopped at its destination station show itself.
BlockedStationDemo.zip