fix: prevent tilemap coordinate overflow - #2009
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Reviewed the complete fixed range 03576e6c0bbead9e73736482539dd321f5521206...21e96c06419853a66ce74ac93566582bf1a0e3fb, including the generated runtime metadata. Focused validation passes with go test ./internal/tilemap and go test .; the attempted 386 test build is blocked by pre-existing missing native binding symbols outside this diff.
No concrete, actionable correctness or regression issues were found in the changed tilemap arithmetic, bounds handling, generated metadata, or call-site integration. The patch is correct based on the fixed diff and the passing focused host tests.
songweijun1203
approved these changes
Sep 28, 2026
joeykchen
force-pushed
the
fix/tilemap-coordinate-overflow
branch
from
September 28, 2026 03:39
21e96c0 to
8e0874f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tile placement multiplied int32 coordinates before widening:
67108864 * 64became zero instead of4294967296. Bounds also overflowed atMaxInt32+1andMinInt32-1.Perform coordinate and bounds arithmetic in int64, then check that returned dimensions and runtime-consumed edges fit int before narrowing. Unrepresentable bounds return the existing false result. A second cleanup replaces four repeated min/max branches with built-ins. Compact records, sorting, callback order, and negative tile-size conventions remain unchanged.
Validation: new placement and extreme-boundary regressions fail on the old implementation and pass after the fix. Native tilemap/root tests, pure tilemap tests/root compilation, wasm root compilation, and Linux 386 pure tilemap compilation pass with Go 1.26.5. A native-only overlay additionally exercises simulated 32-bit range rejection; no execution of a 386 binary is claimed.
Runtime metadata was regenerated twice. Source patches combine cleanly with #2003; regenerate
export.typeswithmake generate-runtimewhen combining pending runtime PRs.Combined validation of all five fixes passed with Go 1.26.5: 79 host packages, codegen and interpreter submodules, pure/wasm root compilation, and actual Web binding wasm tests. Two full generation runs produced identical diffs; all six external Godot outputs were unchanged.