Fix 8x16 sprite tile fetch for odd tile numbers - #784
Open
IronCodeStudios wants to merge 1 commit into
Open
Conversation
In 8x16 sprite mode, an odd tile index selects the $1000 pattern table, whose tiles live at ptTile[] indices 256..511 — so the top tile should be `topTileNum + 256`. The code used `topTileNum - 1 + 256`, which is off by one and fetches the wrong tile, scrambling 8x16 sprites in MMC3 games such as Super Mario Bros. 3 and Kirby's Adventure (the sprite-layer corruption reported in bfirsh#771). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
In 8×16 sprite mode, a sprite whose tile index is odd selects the
$1000pattern table. Tiles for that table live at
ptTile[]indices256..511, sothe top tile of the pair should map to
topTileNum + 256.The current code computes
topTileNum - 1 + 256, which is off by one forodd tile numbers and fetches the wrong pattern tile — scrambling 8×16 sprites in
MMC3 games that lean on them, most visibly Super Mario Bros. 3 and Kirby's
Adventure. This is the sprite-layer corruption reported in #771.
Fix
Even tiles (
$0000) are unchanged; odd tiles ($1000) now indextopTileNum + 256.The
latchAccesscalls below already usetopTileNumagainstsprBaseAddr, sothey were correct and are untouched.
Testing
npm run buildclean;npm test→ 583 passed / 0 failed / 29 skipped,AccuracyCoin 107/27 (unchanged from
main),prettier --checkpasses.the scramble is gone (compared against stock and 2.0.0).
Relationship to #666
#666 also fixes this same off-by-one but bundles it with a CPU IRQ-latching
change and MMC5 nametable changes. This PR isolates just the sprite fix.
Heads-up for maintainers: #666's IRQ-latching change (leaving masked IRQs latched
until serviced) regresses the APU "Frame Counter IRQ" AccuracyCoin test — the
frame-counter IRQ is a level line the APU de-asserts on acknowledgement, so
latching leaves a stale request that fires spuriously. This sprite-only PR avoids
that area. (#771 also reports an audio symptom that's out of scope here.)