-
-
Notifications
You must be signed in to change notification settings - Fork 929
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
Fix #7043, Fix #7274: Delete town owned bridge based on adjacent tile ownership, not nearest town. #7284
Conversation
… adjacent tile ownership, not nearest town. The existing test performed badly with a large number of towns due to having to calculate the nearest town, and also by its nature assumed a bridge was built by the nearest town, leading to bridges of nearby large towns be removed incorrectly.
Due to no longer using CalcClosestTownFromTile(), worldgen performance on large maps is increased substantially. A quick worldgen test with
Worldgen settings: 4096x4096, TGP, High, Mountainous, None, High, Improved, Manual, 31, Jan 1950, Funding only, Very Rough, Many, Freeform. |
b6dab5a
to
4ab4d7d
Compare
I think I would still prefer a solution that stores the owning town in the map array, also to avoid the double map array traversal. |
I think this is a very nice stepping stone towards an even better solution (like #7282). For now I suggest we reschedule this to 1.10, as that allows us to accept this PR as first step to improve performance, and work on another before 1.10 is released. |
Note that town bridges are only deleted by a town on map generation when a town fails to build. This is normally quite rare but some settings can cause it to happen enough that it's noticeably slow. During the game, towns will never delete their own bridges, and even if they did it would not be an intensive loop, just a one-off. |
… adjacent tile ownership, not nearest town. (OpenTTD#7284) This only affects failed town generation, as towns do not delete bridges under any other circumstances. The existing test performed badly with a large number of towns due to having to calculate the nearest town, and also by its nature assumed a bridge was built by the nearest town, leading to bridges of nearby large towns be removed incorrectly. If we gain the ability to quickly retrieve the correct town (which is _not_ the nearest town) from the bridge, this change should be reviewed.
… adjacent tile ownership, not nearest town. (OpenTTD#7284) This only affects failed town generation, as towns do not delete bridges under any other circumstances. The existing test performed badly with a large number of towns due to having to calculate the nearest town, and also by its nature assumed a bridge was built by the nearest town, leading to bridges of nearby large towns be removed incorrectly. If we gain the ability to quickly retrieve the correct town (which is _not_ the nearest town) from the bridge, this change should be reviewed.
The existing test performed badly with a large number of towns due to having to calculate the
nearest town, and also by its nature assumed a bridge was built by the nearest town, leading
to bridges of nearby large towns be removed incorrectly.
Two birds with one stone...
This is an alternative solution to #7282.