Fix pre-commit Formatting#711
Closed
Sebastian-Griesbach wants to merge 6 commits into
Closed
Conversation
Kallinteris-Andreas
force-pushed
the
main
branch
from
July 13, 2026 04:59
a42e8ad to
4642385
Compare
Member
|
@Sebastian-Griesbach can you please also rebase off main? |
…dation#11) The player position registers (myPOSP0, myPOSP1) are int16_t but used as indices into ourPlayerPositionResetWhenTable[8][160][160]. If a position escapes the valid [0, 159] range, the table access is out-of-bounds, causing a segfault. Three changes: 1. Clamp positions before the table lookups in RESP0/RESP1 (cases 0x10, 0x11 in TIA::poke). This is the direct crash site from issue Farama-Foundation#11. 2. Replace the single-step HMOVE wrapping (if >= 160 then -= 160) with proper modular arithmetic that handles any value. The original correction only works for deltas in [-15, +8]; modulo is safe for all inputs. 3. Validate positions after state restore (TIA::load). The deserialized values were cast to int16_t with no bounds check, so a corrupted or stale save state could inject invalid positions. All five position registers (P0, P1, M0, M1, BL) are fixed in each location. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Injects out-of-bounds values into serialized TIA position registers and verifies the emulator survives after restoring the corrupted state. Without the bounds-validation fix in TIA::load, this would segfault. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert the HMOVE position wrapping from modulo arithmetic back to the original if/else single-step correction. Motion table values are bounded to [-15, +8], so positions in [0, 159] land in [-15, 167] after motion — always within one correction of the valid range. The RESP and load() clamps guarantee the [0, 159] precondition, making modulo unnecessary on this hot path. Expand test_restore_state_with_corrupted_positions with 11 comprehensive edge cases (boundary values, int16_t extremes, exact multiples) and add test_hmove_single_step_wrapping_bounds to exhaustively verify the single-step wrapping contract and document its limits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sebastian-Griesbach
force-pushed
the
mr_bugfix
branch
from
July 13, 2026 12:51
373a575 to
6562749
Compare
Author
|
Done |
Member
|
Closing in favor of the base PR, I really apologize for the confusion |
Member
|
Thanks so much for the help with it though :) |
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
3 participants
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.
This PR is an edit of a preexisting PR: #668
Please cherry pick commit 373a575
The relevant commit only fixes formatting but does not change the code.