Skip to content

Adopt fade_in / fade_out helpers in samples#543

Closed
Copilot wants to merge 3 commits into
mainfrom
copilot/adopt-fade-in-fade-out-helpers
Closed

Adopt fade_in / fade_out helpers in samples#543
Copilot wants to merge 3 commits into
mainfrom
copilot/adopt-fade-in-fade-out-helpers

Conversation

Copilot AI commented May 24, 2026

Copy link
Copy Markdown
Contributor

The 0.3.1-beta fade_in() / fade_out() helpers (#450) weren't exercised by any sample. Adopt them in the samples called out by the issue, where appropriate.

Sample changes

  • slideshow — start at pal_bright(0) and fade_in(4) after ppu_on_all() so the CNROM title fades up.
  • scoreboard — swap the static pal_bright(4) for pal_bright(0) + fade_in(4) after ppu_on_all().
  • climbersetup_graphics() now ends at pal_bright(0); main flow calls fade_in(4) once the level is drawn and fade_out(4) when the player reaches the top floor. The per-frame hit-flash (pal_bright(vbright) decrementing from 8 → 4) is left alone — it's a flash, not a fade ramp.

Already done, no change

  • fade — Phase 1 already uses fade_in(4) / fade_out(4). The bg_fade_in / spr_fade_in helpers ramp pal_bg_bright / pal_spr_bright, for which there is no built-in equivalent, so they stay.
  • rletitle — already uses the pal_bright(0) + fade_in(4) pattern.

Example (scoreboard)

pal_col(0, DarkBlue);
pal_col(1, White);
pal_bright(0);          // was: pal_bright(4)
// ...
ppu_on_all();
fade_in(4);             // new

Test data

Rebuilt the three changed samples (Debug + Release), refreshed the embedded src/dotnes.tests/Data/*.{debug,release}.dll, and regenerated the corresponding TranspilerTests.Write.*.verified.bin snapshots.

Copilot AI changed the title [WIP] Update samples to use fade_in and fade_out helpers Adopt fade_in / fade_out helpers in samples May 24, 2026
Copilot AI requested a review from jonathanpeppers May 24, 2026 23:15
@jonathanpeppers jonathanpeppers marked this pull request as ready for review May 26, 2026 20:40
Copilot AI review requested due to automatic review settings May 26, 2026 20:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 several samples to exercise the fade_in(byte delay) / fade_out(byte delay) NESLib helpers (added in #450) by replacing immediate brightness setup with a dark start plus a fade once rendering is enabled, and adding a fade-out at the climber end-of-run transition.

Changes:

  • slideshow: Start at pal_bright(0) and call fade_in(4) after ppu_on_all() to fade up the title screen.
  • scoreboard: Replace pal_bright(4) with pal_bright(0) + fade_in(4) after ppu_on_all().
  • climber: End setup_graphics() at pal_bright(0), fade_in(4) after the stage is drawn, and fade_out(4) when reaching the top floor; test DLLs/snapshots refreshed for the affected samples.

Reviewed changes

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

Show a summary per file
File Description
samples/slideshow/Program.cs Fade in from black after enabling rendering to demonstrate fade_in(4).
samples/scoreboard/Program.cs Start dark and fade in after ppu_on_all() using fade_in(4).
samples/climber/Program.cs Start dark in graphics setup; fade in after world draw; fade out on run completion.
src/dotnes.tests/Data/slideshow.debug.dll Rebuilt test input assembly for updated slideshow sample (Debug).
src/dotnes.tests/Data/slideshow.release.dll Rebuilt test input assembly for updated slideshow sample (Release).
src/dotnes.tests/Data/scoreboard.debug.dll Rebuilt test input assembly for updated scoreboard sample (Debug).
src/dotnes.tests/Data/scoreboard.release.dll Rebuilt test input assembly for updated scoreboard sample (Release).
src/dotnes.tests/Data/climber.debug.dll Rebuilt test input assembly for updated climber sample (Debug).
src/dotnes.tests/Data/climber.release.dll Rebuilt test input assembly for updated climber sample (Release).
src/dotnes.tests/TranspilerTests.Write.slideshow.verified.bin Updated verified ROM snapshot for slideshow due to fade-in behavior change.
src/dotnes.tests/TranspilerTests.Write.scoreboard.verified.bin Updated verified ROM snapshot for scoreboard due to fade-in behavior change.
src/dotnes.tests/TranspilerTests.Write.climber.verified.bin Updated verified ROM snapshot for climber due to fade-in/out behavior change.

Comment thread samples/slideshow/Program.cs Outdated
Sets the iNES header to mapper 3 with 2 CHR banks (16 KB total).
Pressing A on the controller toggles between CHR bank 0 and CHR bank 1,
showing different tile graphics on screen.
Also uses fade_in() to smoothly fade in the title screen.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Done — updated to fade_in(4) in f8dc6bb.

Comment thread samples/scoreboard/Program.cs Outdated
/*
Scoreboard - press A to increment BCD score.
Demonstrates vrambuf_put(), pad_trigger(), ppu_wait_nmi().
Demonstrates vrambuf_put(), pad_trigger(), ppu_wait_nmi(), and fade_in().

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Done — updated to fade_in(4) in f8dc6bb.

Comment thread samples/climber/Program.cs Outdated
bank_bg(0);
vrambuf_clear();
set_vram_update(updbuf);
// Start dark; the main flow calls fade_in() once the world is drawn.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Done — updated to fade_in(4) in f8dc6bb.

Address code review feedback to use fade_in(4) instead of fade_in() in
header comments, since the helper API takes a delay parameter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers

Copy link
Copy Markdown
Owner

Closing — after review, this PR was adding fade_in / fade_out calls to samples that had no manual fade boilerplate to replace:

The only samples with the manual for (vb=0; vb<=4; pal_bright(vb); ppu_wait_frame()) pattern were rletitle and fade, both already migrated in #450. There is no remaining boilerplate to replace, so closing without merging.

@jonathanpeppers jonathanpeppers deleted the copilot/adopt-fade-in-fade-out-helpers branch May 27, 2026 21: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.

Adopt fade_in / fade_out helpers in samples

3 participants