Add optional debug rasters; anchor music channels at fixed addresses#7
Open
mattgodbolt-molty wants to merge 1 commit into
Open
Add optional debug rasters; anchor music channels at fixed addresses#7mattgodbolt-molty wants to merge 1 commit into
mattgodbolt-molty wants to merge 1 commit into
Conversation
`debugrasters = TRUE` in constants.asm enables horizontal raster bars
showing where each frame's CPU time goes:
BLUE = VSYNC IRQ (sound + palette cycling)
RED = update_frog_tile (map redraw under frog)
GREEN = tile_render (frog overlay sprite)
BLACK = idle in wait_vsync
The IRQ saves/restores the main-thread phase colour via a new
`zp_raster_colour` byte so its BLUE bar doesn't clobber the main
thread's RED/GREEN. Default is FALSE — emits no extra bytes.
The bars only become visible once the per-frame work is pushed past
VBLANK into the visible display window. The current per-frame work
fits entirely in border, so on the live screen they're hidden — a
short busy-wait stub in wait_vsync exposes them; intentionally not
included in this PR.
While adding the green raster I found that adding any bytes to
engine.asm corrupted music ("melody right, accompaniment random").
Root cause: the disassembly's `music_data` / `music_data_2` /
`music_data_3` labels landed at addresses that were a *consequence*
of where engine code happened to end, not anchored to the page-aligned
addresses the IRQ actually reads from (`&0C80` / `&0D80` / `&0E80`).
Fix: anchor each channel explicitly with `ORG` in music.asm and rename
to `music_ch1` / `music_ch2` / `music_ch3` plus `anim_timing_const` at
`&0EF7`. Engine and game code now reference the labels rather than
literals (`LDA #LO(music_ch1)`, `EQUB &00, HI(music_ch1), HI(music_ch2),
HI(music_ch3)`, `STA music_ch1,Y`). Engine code can grow freely up to
`&0C80` without disturbing music.
Also dropped the static placeholder music EQUBs in music.asm — they
were overwritten by Level?T before the engine ever read them, so they
were never heard. music.asm is now ~25 lines of pure ORG anchors.
Removed dead `tile_addr_custom` (10 bytes, defined but never called).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds optional raster-based profiling hooks and replaces hard-coded music channel addresses with named, page-anchored labels so the engine layout can change without sliding the music streams. It fits the codebase’s broader goal of turning disassembly-era literals into explicit symbols while preserving the BBC Micro runtime memory map.
Changes:
- Added optional debug raster instrumentation around IRQ, idle wait, frog tile redraw, and overlay rendering.
- Replaced hard-coded music channel page literals with
music_ch1/music_ch2/music_ch3labels anchored at fixed addresses. - Removed the old inline music byte blobs from
music.asmand updated project documentation to describe the layout.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| zero_page.asm | Adds a zero-page byte to remember the current raster colour across IRQs. |
| music.asm | Replaces embedded music bytes with fixed-address label anchors for the three channels and timing byte. |
| game.asm | Initializes raster state, adds raster markers in frame flow, and switches level-copy logic to symbolic music labels. |
| engine.asm | Uses symbolic music addresses in the sequencer and adds a raster marker in tile rendering. |
| constants.asm | Defines the debug toggle, palette constants, and raster helper macros. |
| DIARY.md | Documents the raster work and the music-channel anchoring rationale. |
| CLAUDE.md | Adds a repository note describing the anchored music layout and expected label usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+34
| ORG &0C80 | ||
| .music_ch1 ; Channel 1 base — 256 bytes from Level?T file | ||
|
|
||
| ; --- Channel 1 data (overwritten by Level?T on load) --- | ||
| .music_data | ||
| EQUB &4F, &56, &20, &52 | ||
| EQUB &30, &2C, &07, &F1 | ||
| ORG &0D80 | ||
| .music_ch2 ; Channel 2 base — 256 bytes from Level?T file | ||
|
|
||
| ; Note/duration pairs for channel 1 melody | ||
| EQUB &2E, &1E, &2E, &1E | ||
| EQUB &34, &1E, &34, &1E | ||
| EQUB &3E, &1E, &BE, &B8 | ||
| EQUB &3E, &1E, &3C, &1E | ||
| EQUB &38, &1E, &30, &1E | ||
| EQUB &2E, &1E, &26, &1E | ||
| EQUB &2E, &1E, &2A, &1E | ||
| EQUB &2A, &3C, &34, &1E | ||
| EQUB &34, &1E, &3E, &1E | ||
| EQUB &3E, &1E, &48, &1E | ||
| EQUB &C6, &C2, &46, &1E | ||
| EQUB &3E, &1E, &42, &1E | ||
| EQUB &3C, &1E, &3E, &1E | ||
| EQUB &C2, &BE, &3C, &3C | ||
| EQUB &46, &3C, &3E, &1E | ||
| EQUB &3E, &1E, &40, &1E | ||
| EQUB &40, &1E, &42, &1E | ||
| EQUB &42, &1E, &44, &1E | ||
| EQUB &44, &1E, &46, &1E | ||
| EQUB &BE, &B8, &46, &1E | ||
| EQUB &46, &1E, &46, &3C | ||
| EQUB &00, &1E, &46, &1E | ||
| EQUB &48, &1E, &42, &1E | ||
| EQUB &BE, &BC, &B8, &B4 | ||
| EQUB &46, &1E, &3E, &1E | ||
| EQUB &B8, &B4, &B0, &B8 | ||
| EQUB &42, &1E, &38, &1E | ||
| EQUB &BE, &BC, &B8, &B6 | ||
| EQUB &38, &3C, &38, &1E | ||
| EQUB &34, &1E | ||
| EQUB &FE, &FF ; End of channel 1 | ||
| ORG &0E80 | ||
| .music_ch3 ; Channel 3 base — first 119 bytes are channel | ||
| ; data (FE/FF terminator + residual); byte 119 | ||
| ; (= file offset 631) is the timing constant. | ||
|
|
||
| ; Residual data — overwritten by Level?T on load. | ||
| EQUB &DD, &07, &4F, &47 | ||
| EQUB &B2, &DA | ||
| EQUB &35, &D7, &2D, &49, &04, &BC, &E5, &79 | ||
| EQUB &71, &90, &32, &E7, &91, &5D, &9E, &5A | ||
| EQUB &41, &3A, &96, &48, &5E, &01, &EF, &C7 | ||
| EQUB &91, &61, &E2, &13, &36, &22, &E8, &2F | ||
| EQUB &46, &70, &D7, &15, &FD, &DF, &EF, &7D | ||
| EQUB &CE, &33, &08, &C3, &CC, &FF, &B8, &62 | ||
| EQUB &0C, &B4, &A0, &BE, &47, &9F, &10, &54 | ||
| EQUB &9C, &3D, &FC, &23, &EC, &DB, &27, &52 | ||
| EQUB &BC, &28, &AD, &54, &D3, &5B, &DD, &F8 | ||
| EQUB &74, &97, &D3, &A9, &63, &85, &9E, &ED | ||
| EQUB &58, &7B, &99, &9E, &23, &38, &F9, &58 | ||
| EQUB &EE, &A7, &FE, &BF, &AF, &B0, &81, &35 | ||
| EQUB &4B, &28, &CA, &01, &03, &A6, &61, &22 | ||
| EQUB &B4, &EB, &50, &8B, &6B, &39, &13, &CB | ||
| EQUB &D4, &29, &1F, &56, &73, &D1, &92, &59 | ||
| EQUB &A4, &58, &DE, &01, &04, &6A, &65, &65 | ||
|
|
||
| ; --- Channel 2 data (overwritten by Level?T on load) --- | ||
| .music_data_2 | ||
| EQUB &07, &E1 | ||
| ; Note/duration pairs for channel 2 | ||
| EQUB &A6, &9C, &A6, &9C | ||
| EQUB &AA, &9C, &AA, &9C | ||
| EQUB &B8, &AE, &B8, &AE | ||
| EQUB &B8, &AE, &B4, &AE | ||
| EQUB &B0, &A6, &A6, &A0 | ||
| EQUB &A6, &9C, &A6, &9C | ||
| EQUB &A0, &92, &A6, &A0 | ||
| EQUB &A6, &9C, &A4, &9C | ||
| EQUB &AE, &A4, &AE, &A4 | ||
| EQUB &B4, &AE, &B4, &AE | ||
| EQUB &BE, &B8, &3C, &1E | ||
| EQUB &BC, &B4, &B8, &AE | ||
| EQUB &B0, &B8, &B6, &AA | ||
| EQUB &B8, &B0, &B0, &B8 | ||
| EQUB &36, &3C, &36, &3C | ||
| EQUB &B8, &AE, &B8, &AE | ||
| EQUB &B8, &AE, &B8, &AE | ||
| EQUB &B8, &B0, &B8, &B0 | ||
| EQUB &B8, &B0, &B8, &B0 | ||
| EQUB &BE, &AE, &B8, &AE | ||
| EQUB &B8, &B6, &B2, &B6 | ||
| EQUB &B8, &B4, &B8, &BA | ||
| EQUB &B8, &B4, &B0, &AE | ||
| EQUB &C2, &B8, &B0, &B8 | ||
| EQUB &2A, &1E, &2A, &1E | ||
| EQUB &BE, &B4, &AE, &B4 | ||
| EQUB &26, &1E, &26, &1E | ||
| EQUB &B0, &B8, &B0, &AA | ||
| EQUB &2A, &1E, &2A, &1E | ||
| EQUB &A6, &AE, &B8, &BE | ||
| EQUB &BC, &B8, &B4, &B0 | ||
| EQUB &FE, &FF ; End of channel 2 | ||
|
|
||
| ; Residual data — overwritten by Level?T on load | ||
| EQUB &83, &BD, &17, &06, &4C, &E1, &D3, &F8 | ||
| EQUB &81, &3C, &7D, &C2, &6F, &85, &F6, &B2 | ||
| EQUB &A5, &B9, &2D, &B1, &E2, &8C, &0C, &03 | ||
| EQUB &E5, &62, &48, &49, &5A, &6B, &8A, &A2 | ||
| EQUB &D2, &92, &F0, &77, &44, &6A, &D5, &32 | ||
| EQUB &DE, &F1, &CE, &4F, &60, &94, &FA, &C8 | ||
| EQUB &E3, &46, &3B, &CD, &5E, &9E, &F3, &D2 | ||
| EQUB &26, &22, &9C, &D0, &B5, &7B, &6C, &AA | ||
| EQUB &F2, &64, &8D, &92, &89, &93, &09, &1F | ||
| EQUB &AF, &50, &BC, &96, &B2, &D8, &54, &0E | ||
| EQUB &F3, &40, &A1, &CA, &6C, &97, &5F, &71 | ||
| EQUB &AB, &0F, &D9, &41, &D4, &3C, &16, &B0 | ||
| EQUB &D6, &55, &C6, &BB, &60, &49, &BE, &38 | ||
| EQUB &3C, &2B, &C9, &F8, &73, &5B, &D7, &EE | ||
| EQUB &94, &9D, &DC, &02, &41, &0A, &B7, &48 | ||
| EQUB &6F, &90, &FF, &54, &00, &3D, &79, &53 | ||
|
|
||
| ; --- Channel 3 data (overwritten by Level?T on load) --- | ||
| .music_data_3 | ||
| EQUB &07, &D1 | ||
| ; Note/duration pairs for channel 3 | ||
| EQUB &0E, &3C, &0C, &3C | ||
| EQUB &08, &3C, &16, &3C | ||
| EQUB &18, &3C, &1C, &3C | ||
| EQUB &1A, &3C, &1C, &3C | ||
| EQUB &16, &3C, &20, &3C | ||
| EQUB &12, &1E, &1C, &1E | ||
| EQUB &16, &1E, &20, &1E | ||
| EQUB &12, &1E, &16, &1E | ||
| EQUB &18, &1E, &12, &1E | ||
| EQUB &96, &92, &96, &98 | ||
| EQUB &96, &92, &8E, &8C | ||
| EQUB &20, &3C, &1C, &3C | ||
| EQUB &18, &3C, &18, &1E | ||
| EQUB &B0, &98, &16, &3C | ||
| EQUB &12, &1E, &12, &1E | ||
| EQUB &10, &3C, &10, &3C | ||
| EQUB &12, &3C, &1C, &3C | ||
| EQUB &26, &3C, &18, &3C | ||
| EQUB &12, &3C, &16, &3C | ||
| EQUB &20, &3C, &24, &3C | ||
| EQUB &FE, &FF ; End of channel 3 | ||
|
|
||
| ; Residual data — overwritten by Level?T on load | ||
| EQUB &2E, &63, &45, &1E | ||
| EQUB &06, &54, &10, &59 | ||
| EQUB &9F, &48, &C8, &29 | ||
| EQUB &49, &DD, &6D, &90 | ||
| EQUB &FA, &7F, &E2, &15 | ||
| EQUB &2D, &95, &0E, &AD | ||
| EQUB &6B, &91, &D5, &BF | ||
| EQUB &DA, &70, &FC, &86 | ||
| EQUB &2A, &98, &73 | ||
| .anim_timing_const ; Note duration base — level-specific (overwritten by Level?T) | ||
| EQUB &0F, &36, &34, &30, &38 | ||
| EQUB &38, &29, &34, &65 | ||
| ORG &0EF7 | ||
| .anim_timing_const ; Per-level note duration (Level?T file offset 631) |
Comment on lines
+376
to
+381
| A busy-wait inside `wait_vsync` (≈70 scanlines after VSYNC IRQ exit) | ||
| deliberately stalls the main thread until visible display starts, so the | ||
| render phases fall inside the visible window rather than during VBLANK. | ||
| Without it, the game's per-frame work is so well under budget that it all | ||
| fits in vertical blanking and the bars are never seen on screen — useful | ||
| profile result by itself. |
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
debugrasters = TRUEtoggle inconstants.asmthat paints horizontal colour bars showing where each frame's CPU time is going (BLUE = VSYNC IRQ, RED =update_frog_tile, GREEN =tile_render, BLACK = idle inwait_vsync). IRQ uses a newzp_raster_colourbyte to save/restore the main-thread phase colour. Default is FALSE — emits no extra bytes.music_data/music_data_2/music_data_3→music_ch1/music_ch2/music_ch3, anchored explicitly withORGat&0C80/&0D80/&0E80(andanim_timing_constat&0EF7). Engine and game code now reference these labels rather than the literals the disassembler had baked in. Fixes a latent fragility where adding bytes toengine.asmwould slide channels 2/3 off their pages and corrupt music ("melody right, accompaniment random").music.asm— they were overwritten by Level?T before the engine ever read them, so were never heard.music.asmis now ~25 lines of pure ORG anchors.tile_addr_custom(10 bytes, defined but never called).CLAUDE.md, diary entry 23 inDIARY.md.Note on visibility
The bars only become visible once per-frame work is pushed past VBLANK into visible display. Currently the work fits entirely in border, so on the live screen the bars are hidden — a short busy-wait stub in
wait_vsyncexposes them; not included here. The fact that you can't see them is itself a useful profile result (loads of frame budget headroom).Test plan
makebuilds cleanfrogman_rebuilt.ssdin jsbeeb — game runs, music plays correctly on level 1debugrasters = TRUEinconstants.asm, rebuild — should still build and run; rasters won't be visible without a busy-wait but the macros emit the right bytes (verifiable viamcp__jsbeeb__disassembleat the four sites)🤖 Generated with Claude Code