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

Possible overflow in landscaping rate limit handling can introduce randomness into whether landscaping succeeds #8611

Closed
James103 opened this issue Jan 26, 2021 · 2 comments · Fixed by #8782
Closed

Possible overflow in landscaping rate limit handling can introduce randomness into whether landscaping succeeds #8611

James103 opened this issue Jan 26, 2021 · 2 comments · Fixed by #8782
Labels
needs triage This issue needs further investigation before it becomes actionable

Comments

@James103
Copy link
Contributor

Version of OpenTTD

1.11.0 beta 1

Expected result

Large terraform operations should deterministically succeed or fail based on the rate limits, for which some of the variables thereof may need to be widened (16 bit --> 32 bit, 32-bit --> 64 bit)

Actual result

Large terraform operations can randomly succeed or fail when the rate limit settings are maxed out. For example, right after performing multiple large terraform operations in a row, there is an approx. 1 in 4 chance that the next terraform operation will fail completely.

Steps to reproduce

  1. Load the attached savegame (the included game script does not matter here)
  2. set terraform_per_64k_frames 1073741824
  3. set terraform_frame_burst 65535 (not 65536, see terraform_frame_burst (and similar settings) are stored to 16-bit integers instead of 32 bits. #8610)
  4. Open viewports at the top corner of the map and at the bottom corner of the map.
  5. Drag from the top corner of the map to the bottom corner of the map (as indicated by the signs) using the level land tool.
  6. Repeat step 5 a few times.
  7. Using the raise land or lower land tool, shift-click on a tile repeatedly. Sometimes it will show "Estimated Cost", other times it will error with "landscaping limit reached".

Savegame

test-8611.zip
@TrueBrain TrueBrain added the needs triage This issue needs further investigation before it becomes actionable label Feb 28, 2021
@TrueBrain
Copy link
Member

This one puzzled me for a bit to understand, but as I fixed that terraform_frame_burst cannot be above 32k now, it is a lot harder to hit this bug. So be lucky you could test this while it could still be UINT16_MAX, as the bug is really cute :)

Fix incoming, if I can figure out how to fix this properly :P

@James103
Copy link
Contributor Author

James103 commented Mar 1, 2021 •
edited

Alternative steps to reproduce in 1.11.0-beta2:

  1. Load the attached savegame (the included game script does not matter here)
  2. set clear_per_64k_frames 1073741824
  3. set clear_frame_burst 65535 (not 65536, see terraform_frame_burst (and similar settings) are stored to 16-bit integers instead of 32 bits. #8610)
  4. Open viewports at the top corner of the map and at the bottom corner of the map.
  5. Drag from the top corner of the map to the bottom corner of the map (as indicated by the signs) using the clear area tool.
  6. Repeat step 5 a few times.
  7. Using the clear land area tool, shift-click on a tile repeatedly. Sometimes it will show "Estimated Cost", other times it will error with "tile clearing limit reached".

@TrueBrain TrueBrain mentioned this issue Mar 1, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs triage This issue needs further investigation before it becomes actionable
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix two problems with terraform limits TrueBrain/OpenTTD
2 participants
@TrueBrain @James103