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

Codechange: Scan station catchment tiles when removing station from nearby towns/industries. #12129

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented Feb 19, 2024

Motivation / Problem

On a map with lots of towns and industries, along with lots of stations, iterating the complete town and industry pool to (possibly) remove the station from town and industry nearby stations lists can take some time.

This is most noticeable when a company goes bankrupt as lots of updates are done together, but also has a small impact on removing stations parts.

Description

Stations also have a list of catchment tiles covered, which is updated after the town and industry nearby station lists are cleared.

So we can avoid iterating all towns and industries when updating station catchment, and scan a limited portion of the map instead.

This provides a modest performance benefit when many towns/industries exist:

game stations towns industries action before after
reddit 1 591 512 1,560 RemoveFromAllNearbyLists() x100 2,420µs 61µs
Wentbourne 2,648 368 1,309 RemoveFromAllNearbyLists() x100 3,417µs 111µs
Xarick_4kx4k 20,856 12,538 20,873 RemoveFromAllNearbyLists() x100 823,153µs 645µs
Wentbourne reset_company 1 499,610µs 458,537µs
Xarick_4kx4k stop_ai 2 2,284,738µs 604,671µs

Limitations

Relies on catchment_tiles not missing any tile. It shouldn't do as that would cause other issues too.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, game_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

…earby towns/industries.

Avoid iterating all towns and industries when updating station catchment, and scan a limited portion of the map instead.

This provides a modest performance benefit when many towns/industries exist.
@JGRennison
Copy link
Contributor

Players can (and often do) bulldoze whole towns. If a town has no house tiles it wouldn't be found and the reference would be left dangling.

@PeterN
Copy link
Member Author

PeterN commented Feb 19, 2024

ClearTownHouse() removes the station from Town::stations_near via RemoveNearbyStations(), so I don't think that would be affected.

@PeterN PeterN merged commit 2d48829 into OpenTTD:master Mar 5, 2024
19 checks passed
@PeterN PeterN deleted the update-catchment-scan-tiles branch March 5, 2024 18:34
TrueBrain pushed a commit to TrueBrain/OpenTTD that referenced this pull request Mar 16, 2024
…earby towns/industries. (OpenTTD#12129)

Avoid iterating all towns and industries when updating station catchment, and scan a limited portion of the map instead.

This provides a modest performance benefit when many towns/industries exist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants