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

Cleanup: Use vectors for station platform and sprite layouts #9177

Merged
merged 3 commits into from May 2, 2021

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented May 2, 2021

Motivation / Problem

Loading of station platform layouts and station sprite layouts currently requires memory management and loops, and storing of sizes (and memory management of storing of sizes).

Description

Replace custom management with std::vectors. Loops used during loading have been simplified.

Limitations

Copying another station's layouts now makes an actual copy instead of making a reference to it. I'm not sure how often this is used for the extra memory usage to be a problem though.

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 affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@michicc
Copy link
Member

michicc commented May 2, 2021

You produced a few new warnings, see 064dade. Otherwise it seems reasonable, but I only skimmed it so far.

@PeterN
Copy link
Member Author

PeterN commented May 2, 2021

You produced a few new warnings, see 064dade. Otherwise it seems reasonable, but I only skimmed it so far.

Seems better now, and no cast in sight :D

michicc
michicc previously approved these changes May 2, 2021
PeterN added 3 commits May 2, 2021 14:37
This avoids the need to custom memory management and additional members.

This also resolves use-after-free if modifying copied layouts, so presumably nobody has ever done that.
@PeterN
Copy link
Member Author

PeterN commented May 2, 2021

I noticed that I had new StationSpec rather than new StationSpec(). Seems to potentially affect initialisation...

@PeterN PeterN merged commit 756034f into OpenTTD:master May 2, 2021
@PeterN PeterN deleted the station-layouts branch May 2, 2021 16:15
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

2 participants