Skip to content

Fix scroll tracking during falls in climber#321

Merged
jonathanpeppers merged 4 commits intomainfrom
fix/climber-scroll-during-fall
Mar 29, 2026
Merged

Fix scroll tracking during falls in climber#321
jonathanpeppers merged 4 commits intomainfrom
fix/climber-scroll-during-fall

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Owner

Fixes #318

Changes

  • Player sprite (actor 0) is never hidden by the off-screen check. When rel_hi != 0, the sprite is clamped to Y=224 instead of being skipped. This matches the original climber.c which uses signed 16-bit math for screen_y and never culls the player.

  • player_screen_y is always updated for actor 0, even when off-screen, so the scroll handler can track the player during falls.

Known Limitation

On certain randomly-generated floor layouts, the byte-pair subtraction for rel_hi can produce incorrect results when yy and scroll_yy have certain page-boundary relationships. This causes the player to appear at the wrong screen position temporarily during falls on high floors. A proper fix requires the transpiler to support signed 16-bit subtraction for the screen_y computation, matching the original C code's int arithmetic.

Mesen traces confirm the fall physics itself is correct at all floor heights (4, 6, 8, 10, 12).

- Player sprite (actor 0) is never hidden by the off-screen check.
  When rel_hi != 0, clamp screen_y to 224 instead of skipping render.
  This matches the original climber.c which uses signed 16-bit math
  for screen_y and never culls the player.

- player_screen_y is always updated for actor 0, even when off-screen,
  so the scroll handler can track the player during falls.

- Scroll handler adjusts 1px/frame (matching original check_scroll_down).

The remaining issue is that on certain randomly-generated floor layouts,
the byte-pair subtraction for rel_hi can produce incorrect results
when yy and scroll_yy have certain page-boundary relationships. This
causes the player to appear at the wrong screen position temporarily.
A proper fix requires the transpiler to support signed 16-bit subtraction
for the screen_y computation, matching the original's int arithmetic.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 22, 2026 01:24
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

This PR updates the climber sample to prevent the player sprite from being culled during high-floor falls when scroll tracking temporarily lags behind the player, addressing issue #318 (visual disappearance due to off-screen logic).

Changes:

  • Adjust sprite rendering logic so actor 0 (player) is never culled by the rel_hi != 0 off-screen check; instead, the player’s screen_y is clamped to 224 when off-screen.
  • Ensure player_screen_y is updated for the player even when clamped, so the scroll handler can continue tracking during falls.
  • Update the test fixtures (compiled DLLs + verified ROM snapshot) to match the new sample behavior.

Reviewed changes

Copilot reviewed 1 out of 4 changed files in this pull request and generated 1 comment.

File Description
samples/climber/Program.cs Changes sprite culling/clamping logic for the player and updates player_screen_y to keep scroll tracking correct during falls.
src/dotnes.tests/Data/climber.debug.dll Updated compiled debug fixture for the climber sample used by snapshot tests.
src/dotnes.tests/Data/climber.release.dll Updated compiled release fixture for the climber sample used by snapshot tests.
src/dotnes.tests/TranspilerTests.Write.climber.verified.bin Updated verified ROM snapshot to reflect the new climber sample output.

…ring-fall

# Conflicts:
#	src/dotnes.tests/Data/climber.debug.dll
#	src/dotnes.tests/Data/climber.release.dll
#	src/dotnes.tests/TranspilerTests.Write.climber.verified.bin
@jonathanpeppers
Copy link
Copy Markdown
Owner Author

🤖 Resolved merge conflicts with main (binary test data files — rebuilt DLLs and snapshot).

All 611 tests pass. cc65 stack stable at $0800 through 6000 frames.

Code review:

The fix is correct — the player (actor 0) should never be culled from the sprite draw loop because player_screen_y must always be updated for scroll tracking. The original C code uses signed 16-bit math that naturally handles this; the byte-pair arithmetic in dotnes loses the sign, so clamping to 224 is a reasonable workaround.

One concern from the PR description: "On certain randomly-generated floor layouts, the byte-pair subtraction for rel_hi can produce incorrect results." This is a pre-existing limitation of the byte-pair scroll math, not introduced by this PR.

Ready for review.

The assignment at line 403 (inside the ai==0 branch) is the
important one — it sets player_screen_y even when off-screen.
The duplicate at line 472 was redundant since it only ran when
the actor wasn't culled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers merged commit 20d00ca into main Mar 29, 2026
3 checks passed
@jonathanpeppers jonathanpeppers deleted the fix/climber-scroll-during-fall branch March 29, 2026 02:06
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.

Climber: player disappears on high-floor damage due to scroll desync

2 participants