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

Fix: allow only 255 league tables, as 255 is the invalid id sentinel #12545

Merged
merged 1 commit into from
Apr 20, 2024

Conversation

rubidium42
Copy link
Contributor

Motivation / Problem

While reviewing #12541, the maximum size check and comment for pool size were not in sync which yielded a search for what is valid.

The result: with the league table pool you can add/allocate 256 elements. These will be numbered 0 to 255. 0xFF (or 255) is also used for as sentinel for an invalid league table pool id and as such should not be allowed to be allocated by the pool.

Description

Reduce the league table pool size by 1.

Limitations

Might, in theory, cause some issues with scripts that can't create 256 elements any more. However, they were not allowed to change or use the 256th element afterwards, as that would be flagged as an invalid id.

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')

@rubidium42 rubidium42 added the backport requested This PR should be backport to current release (RC / stable) label Apr 20, 2024
@PeterN
Copy link
Member

PeterN commented Apr 20, 2024

If a savegame exists with the affected ID, can it still be loaded?

@rubidium42
Copy link
Contributor Author

If a savegame exists with the affected ID, can it still be loaded?

No. But I also doubt there are any non-synthetic vanilla saves that have 256 league tables. If there are, then we might consider adding code to handle such cases I'd say.

@rubidium42 rubidium42 merged commit 1dfd0c1 into OpenTTD:master Apr 20, 2024
15 checks passed
@rubidium42 rubidium42 deleted the invalid-league-table-id branch April 20, 2024 14:53
@PeterN PeterN added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants