From 9b54ff83c092afbab1923113c9b357a6c718e738 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Apr 2026 12:09:46 +0000 Subject: [PATCH] audio: always enable all four tone channels on sfx trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing $07 in `emit_play_triangle` and $0B in `emit_play_noise` meant that a noise play following an in-progress triangle note would clear bit 2 of $4015 and cut the triangle off mid-envelope (and vice versa). Write $0F from both paths so every trigger keeps pulse1, pulse2, triangle, and noise enabled; channels with no active envelope stay silent via the runtime's per-channel counter gating. Also fixes the attribute-byte packing comment in `png_to_nametable` — the code was correct, the doc string had the quadrant order backwards. The only observable ROM change is `examples/noise_triangle_sfx.nes` (two immediate operands shift) and its audio hash golden; the committed PNG golden is byte-identical. Found in independent code review after the section landed. https://claude.ai/code/session_01MaNVcDmK9gsspRkdxowQAM --- examples/noise_triangle_sfx.nes | Bin 24592 -> 24592 bytes src/assets/chr.rs | 2 +- src/codegen/ir_codegen.rs | 18 ++++++++++++------ .../goldens/noise_triangle_sfx.audio.hash | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/examples/noise_triangle_sfx.nes b/examples/noise_triangle_sfx.nes index 00ed76279d2eda1968546f3772641e4187e5b5e7..d96c03a3b2ec81742fb21e8cd3c2c422eec250d0 100644 GIT binary patch delta 22 ecmbPmfN{bB#to|(8Tlu#VKimr-~5PCFAe} Result<([u8; 960], [u8; 64]), // Attribute table: 8×8 bytes, each covering a 32×32 region made // up of four 16×16 quadrants. Each quadrant gets 2 bits - // (0..=3) packed into the byte as `TR<<6 | TL<<4 | BR<<2 | BL` + // (0..=3) packed into the byte as `BR<<6 | BL<<4 | TR<<2 | TL` // per the PPU's documented layout. The 15-row nametable only // half-fills the last attribute byte-row (rows 8..10 of the // bottom attribute byte are unused and stay at 0, matching the diff --git a/src/codegen/ir_codegen.rs b/src/codegen/ir_codegen.rs index 7b004cb..aa176fe 100644 --- a/src/codegen/ir_codegen.rs +++ b/src/codegen/ir_codegen.rs @@ -1407,10 +1407,15 @@ impl<'a> IrCodeGen<'a> { self.emit(STA, AM::Absolute(0x400A)); self.emit(LDA, AM::Immediate(period_hi)); self.emit(STA, AM::Absolute(0x400B)); - // Enable pulse-1 + pulse-2 + triangle in the APU status - // register. We don't bother reading $4015 first — overwriting - // with $07 keeps all currently-used channels enabled. - self.emit(LDA, AM::Immediate(0x07)); + // Enable all four tone channels in the APU status register. + // We always write $0F (pulse1+pulse2+triangle+noise) instead + // of just the channel we're triggering, because per-play + // writes use immediate values and a later noise play with + // $0B would otherwise silence an in-progress triangle note + // by clearing bit 2. Channels with no active envelope stay + // silent via the runtime's per-channel counter gating, so + // enabling them blindly is harmless. + self.emit(LDA, AM::Immediate(0x0F)); self.emit(STA, AM::Absolute(0x4015)); // Main-RAM envelope pointer. self.emit(LDA, AM::SymbolLo(label.to_string())); @@ -1439,8 +1444,9 @@ impl<'a> IrCodeGen<'a> { // $400F: length counter load. self.emit(LDA, AM::Immediate(period_hi)); self.emit(STA, AM::Absolute(0x400F)); - // Enable pulse-1 + pulse-2 + noise channels. - self.emit(LDA, AM::Immediate(0x0B)); + // Enable all four tone channels — see the equivalent write + // in `emit_play_triangle` for why $0F rather than $0B. + self.emit(LDA, AM::Immediate(0x0F)); self.emit(STA, AM::Absolute(0x4015)); // Main-RAM envelope pointer. self.emit(LDA, AM::SymbolLo(label.to_string())); diff --git a/tests/emulator/goldens/noise_triangle_sfx.audio.hash b/tests/emulator/goldens/noise_triangle_sfx.audio.hash index 5f988a9..5c09402 100644 --- a/tests/emulator/goldens/noise_triangle_sfx.audio.hash +++ b/tests/emulator/goldens/noise_triangle_sfx.audio.hash @@ -1 +1 @@ -a82b6ff5 132084 +8fd9f7d2 132084