Skip to content

Fix climber jump sprite disappearing (#276)#291

Merged
jonathanpeppers merged 3 commits intomainfrom
fix/climber-jump-sprite-disappears
Mar 20, 2026
Merged

Fix climber jump sprite disappearing (#276)#291
jonathanpeppers merged 3 commits intomainfrom
fix/climber-jump-sprite-disappears

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Owner

Problem

The player sprite disappeared during jumps in the climber sample (#276).

Root Cause

The transpiler's HandleStelemI1 in IL2NESWriter.ArrayHandling.cs only tracked ONE standalone value local when analyzing the value expression for array stores. For the pattern:

byte prev_lo = actor_yy_lo[pi];
actor_yy_lo[pi] = (byte)(prev_lo + dv);

The second ldloc (dv) overwrote the first (prev_lo) in valueLocalIdx, and addValue stayed 0 because the add instruction wasn't preceded by a constant. This caused the transpiler to emit:

LDA dv        ; loads dv
CLC
ADC #$00      ; adds 0 instead of prev_lo!
STA arr,X

Instead of the correct:

LDA prev_lo   ; loads first local
CLC
ADC dv         ; adds second local
STA arr,X

This made actor_yy_lo[pi] = dv instead of prev_lo + dv, causing the Y position to diverge wildly from the scroll position. After a few frames, rel_hi != 0 triggered the off-screen check, hiding the player sprite.

Fix

Track both value locals (valueLocalIdx + valueLocalIdx2) in the stelem handler's backward IL scan. When both are present with an add or sub operation, emit LDA local1; CLC; ADC local2 (or SEC; SBC local2).

Verification

  • All 545 tests pass (481 dotnes.tests + 64 analyzer tests)
  • Climber verified.bin updated to match the corrected output
  • Emulator debug confirms correct jump physics: Y increments by 4, 4, 4, 3, 3... (vel>>2) instead of being set to the raw dv value
  • Player sprite (tiles $E8-$EB) remains visible throughout the jump animation

HandleStelemI1 only tracked one standalone value local (valueLocalIdx),
so arr[i] = (byte)(prev_lo + dv) was emitted as LDA dv; CLC; ADC #
instead of LDA prev_lo; CLC; ADC dv. The second ldloc overwrote the
first, and addValue stayed 0 because the add wasn't preceded by a
constant.

Track both locals (valueLocalIdx + valueLocalIdx2) and emit the correct
LDA local1; CLC; ADC local2 / SEC; SBC local2 pattern.

This caused the climber sample's jump physics to set actor_yy_lo = dv
instead of prev_lo + dv, making the Y position diverge wildly and
triggering the off-screen check (rel_hi != 0), hiding the player sprite.

Fixes #276

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 20, 2026 01:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a transpiler codegen bug that caused incorrect byte-array element updates in the climber sample, which in turn made the player sprite disappear during jumps.

Changes:

  • Extend HandleStelemI1 value-expression analysis to track a second standalone ldloc operand.
  • Emit LDA local1; CLC/SEC; ADC/SBC local2 for arr[i] = (byte)(local1 +/- local2) patterns.
  • Update the climber verified.bin snapshot to reflect corrected ROM output.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs Updates stelem.i1 pattern detection/emission to support two-local add/sub value expressions.
src/dotnes.tests/TranspilerTests.Write.climber.verified.bin Snapshot update for climber ROM bytes after corrected codegen.

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs Outdated
Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs
Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs Outdated
jonathanpeppers and others added 2 commits March 19, 2026 21:15
- Only set valueLocalIdx2 when still unset, ignoring 3rd+ locals
- Use hasAdd != hasSub (XOR) so both flags set falls through
- Add RoslynTests for arr[i]=(byte)(local1+local2) and subtraction

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers merged commit 6274343 into main Mar 20, 2026
1 check passed
@jonathanpeppers jonathanpeppers deleted the fix/climber-jump-sprite-disappears branch March 20, 2026 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants