Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: CmdFoundTown may cause multiplayer desyncs due to not initialising Town::stations_near #9407

Closed
JGRennison opened this issue Jun 30, 2021 · 2 comments
Closed

[Bug]: CmdFoundTown may cause multiplayer desyncs due to not initialising Town::stations_near #9407

JGRennison opened this issue Jun 30, 2021 · 2 comments
Labels
bug Something isn't working
Milestone
12.0

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

master

Expected result

No multiplayer desyncs or incorrect caches

Actual result

CmdFoundTown may be called by the player in game, in the scenario editor, or by a GS.

In the event that a town is founded in range of existing stations (i.e. where one or more new town house tiles are within the station's catchment area), the town's stations_near will be incorrectly empty, instead of containing the stations within range.
In the event that this occurs in multiplayer, players which join after the town founding event will not have an incorrect cache and this may result in a state divergence. Most notably this is used by StationFinder::GetStations.

Steps to reproduce

  1. Found a town near to an existing station
  2. Note that Town::stations_near is incorrect, CheckCaches can be used to verify this
JGRennison added a commit to JGRennison/OpenTTD-patches that referenced this issue Jun 30, 2021
@JGRennison
Fix founding towns not filling the nearby station cache, causing desyncs
b23ba0c 
See: OpenTTD/OpenTTD#9407
@TrueBrain TrueBrain added the bug Something isn't working label Jun 30, 2021
@glx22
Copy link
Contributor

glx22 commented Jul 7, 2021 •
edited

The issue is

OpenTTD/src/town_cmd.cpp

Lines 2313 to 2318 in a7fabe4

if (!_generating_world) {
ForAllStationsAroundTiles(TileArea(t, (size & BUILDING_2_TILES_X) ? 2 : 1, (size & BUILDING_2_TILES_Y) ? 2 : 1), [town](Station *st, TileIndex tile) {
town->stations_near.insert(st);
return true;
});
}
in MakeTownHouse() (added with 8b1b3fd) being skipped because

OpenTTD/src/town_cmd.cpp

Line 1993 in a7fabe4

Backup<bool> old_generating_world(_generating_world, true, FILE_LINE);
in CmdFoundTown() (added with ffec9b4 present since at least r1)

@glx22 glx22 mentioned this issue Jul 8, 2021
Merged
@LordAro LordAro added this to the 12.0 milestone Aug 14, 2021
TrueBrain added a commit to TrueBrain/OpenTTD that referenced this issue Aug 31, 2021
@TrueBrain
Fix OpenTTD#9407: desync when founding a town nearby a station
1b5aa46 
"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.
TrueBrain added a commit to TrueBrain/OpenTTD that referenced this issue Aug 31, 2021
@TrueBrain
Fix OpenTTD#9407: desync when founding a town nearby a station
69515c5 
"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.
@TrueBrain TrueBrain mentioned this issue Aug 31, 2021
Merged
@TrueBrain
Copy link
Member

Tnx for the analysis @glx22 , that really helped as lot :) @JGRennison : I want for a slightly different solution than your commit, as I explain in #9526. Let me know if you foresee any issue with that.

TrueBrain added a commit to TrueBrain/OpenTTD that referenced this issue Aug 31, 2021
@TrueBrain
Fix OpenTTD#9407: desync when founding a town nearby a station
04cae39 
"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.
TrueBrain added a commit to TrueBrain/OpenTTD that referenced this issue Aug 31, 2021
@TrueBrain
Fix OpenTTD#9407: desync when founding a town nearby a station
99562d6 
"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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
12.0
Development

No branches or pull requests
4 participants
@TrueBrain @glx22 @JGRennison @LordAro