Fix #9407: desync when founding a town nearby a station #9526
+7
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TrueBrain
force-pushed
the
fix-create-town
branch
from
69515c5 to
04cae39
Compare
|
The The reason for the slightly more complicated implementation in my branch it to avoid repeating the station tile scan for every new house in the founded town, and the catchment area scan for each found station x each house which found it. The scans only need to be done once after the town is founded. I did not make any measurements at the time but it may be worth bearing in mind. |
"stations_near" wasn't updated when founding a town near a station. As this variable is not saved, any client joining after the town is founded has a different value for "stations_near", potentially causing desyncs. As the intention of this if() statement was to skip an expensive calculation when there are clearly no stations, better to move that check inside the function, so other places also enjoy the speedup.
TrueBrain
force-pushed
the
fix-create-town
branch
from
04cae39 to
99562d6
Compare
LordAro
approved these changes
Aug 31, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation / Problem
When a town is founded during a game, we misuse
_generating_worldto bypass a bunch of validation that normally should happen when towns grow etc.This is also done to skip updating
stations_near, as it is a pretty expensive operation, and when generating a new map there is of course never a station around a town to start with. So this is safe to skip in that scenario. Not when founding a new town.As I wrote in the commit message:
This was reported in #9407.
Description
JGR also made a solution for this (JGRennison/OpenTTD-patches@b23ba0c), and although effective, it felt a bit like bringing a gun to a pillow fight.
I was also mostly wondering why
_generating_worldwas misused like this, and why it would skip thestations_nearupdate.Long story short: the check on
_generating_worldis an optimization, to avoid wasting a lot of CPU on something that is never going to happen during map generation. So I looked for the next-best-thing that does the same but differently, and settled on checking if there is any station at all. During map generation of course there isn't.In result, after this PR the code does the same for new map generation (skipping the expensive station check), but fixes the bug in #9407.
JGR's solution is also a fine solution, but it introduces several globals and more complexity that can break over time. On the other hand, his solution might fix issues we were not even aware of. I leave it to the reviewer to balance this.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.