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

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
Labels
needs triage

Comments

@James103
Copy link
Contributor

@James103 James103 commented Jan 26, 2021

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 #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 label Feb 28, 2021
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Mar 1, 2021

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 James103 commented Mar 1, 2021

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 #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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants