From f4968256f47da1a076c334a84957985313afd2ef Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Apr 2026 18:48:55 +0000 Subject: [PATCH] review: tighten PRNG / void-intrinsic / FCEUX path handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up cleanup on the cc65 parity batch. Addresses issues found during a post-commit code review. **Correctness fixes:** - `rand8()` / `rand16()` at statement position (result discarded) were being eliminated by DCE because `op_dest` returned `Some(dest)` for Rand8/Rand16 even though the ops have a visible side effect — advancing the PRNG state. Now `op_dest` returns `None` for both, keeping the JSR regardless of liveness. New regression test `rand8_statement_survives_dce`. - Void-only intrinsics (`poke`, `seed_rand`, `set_palette_brightness`) used in expression position (e.g. `var x = seed_rand(42)`) were panicking the linker with an unresolved `__ir_fn_X` label. The analyzer now emits E0203 with a clear message; new `void_intrinsic_in_expression_position_errors` test covers all three names. - Statement-position `rand8()` / `rand16()` weren't lowered at all (they fell through to the default Call path). Now both lower to their IR op with a fresh temp that nothing reads; the JSR still runs so the PRNG state advances. - `--fceux-labels foo.nes` was producing `foo.0.nl` because `PathBuf::with_extension` replaces instead of appends. Rewritten to literally append `..nl` / `.ram.nl` to the OsString, so users get the FCEUX-expected `foo.nes..nl` naming. - Linker now asserts CNROM / AxROM don't accept user-declared switchable PRG banks — their page sizes don't fit the 16 KB per bank model, and silently producing a mis-sized ROM is worse than a loud panic. **PRNG cleanup:** - Removed the stream-of-consciousness comment block in `gen_prng` that described three abandoned algorithms before landing on the actual Galois LFSR. - Simplified `__rand16` to a single JSR + LDX instead of two JSRs + TAY/TYA round-trip — a single shift already produces 16 fresh bits, the doubled call just burned ~40 cycles. The golden PNG for `prng_demo` was regenerated to reflect the new sequence. - Rewrote the `gen_prng` doc comment to accurately describe the algorithm as a Galois LFSR (it was mislabelled as xorshift). - Rewrote the `gen_palette_brightness` doc comment with a proper table of level→mask mappings — the prior prose description didn't match the actual table values. **Tests:** - Three new unit tests in `linker::debug_symbols` covering the FCEUX `.nl` renderer: user-facing labels only, empty output when no user labels exist, and deterministic sorting in `.ram.nl`. - Extended `nes2_mapper_high_nibble_in_byte_8_is_zero_for_small_mappers` to cover AxROM + CNROM. - Renumbered priority list in future-work.md after removing the shipped sections (J, K, N, parts of V and Y). All 737 tests + 40/40 emulator goldens still green. --- docs/future-work.md | 64 +++------- examples/prng_demo.ne | 10 +- examples/prng_demo.nes | Bin 24592 -> 24592 bytes src/analyzer/mod.rs | 24 +++- src/ir/lowering.rs | 14 +++ src/linker/debug_symbols.rs | 72 +++++++++++ src/linker/mod.rs | 12 ++ src/main.rs | 35 ++++-- src/optimizer/mod.rs | 10 +- src/rom/tests.rs | 2 + src/runtime/mod.rs | 179 ++++++++++++--------------- tests/emulator/goldens/prng_demo.png | Bin 1094 -> 841 bytes tests/integration_test.rs | 81 ++++++++++++ 13 files changed, 337 insertions(+), 166 deletions(-) diff --git a/docs/future-work.md b/docs/future-work.md index e3a7d54..d030d79 100644 --- a/docs/future-work.md +++ b/docs/future-work.md @@ -394,14 +394,11 @@ NMI-time write budget (~2273 cycles) is too tight for a full nametable. RLE is the smaller first step — emit a `nametable` that can declare `compression: rle` and decompress at swap time. -### J. Palette brightness / fade (ships today) +### J. Palette-fade follow-ups -`set_palette_brightness(level: u8)` is a builtin that maps the -0..8 level onto `$2001` PPU mask emphasis bits. See -`examples/palette_brightness_demo.ne` for an end-to-end demo. -The runtime `__set_palette_brightness` routine is spliced in only -when user code references the builtin. Follow-ups still worth -doing: +`set_palette_brightness(level: u8)` ships today (levels 0..8 mapped +onto `$2001` emphasis bits); `examples/palette_brightness_demo.ne` +exercises it. Two follow-ups are still worth doing: - Blocking `fade_out(frames)` / `fade_in(frames)` helpers — today users write them in user-space with a for-loop + `wait_frame`. @@ -409,16 +406,6 @@ doing: - A brightness-LUT path that recolours the active palette in addition to the emphasis bits, for non-NTSC-assumption fades. -### K. Edge-triggered input (ships today) - -`p1.button.a.pressed` / `p1.button.a.released` (and the P2 variants) -report the rising / falling edge of the button relative to the -previous frame. Implementation: one IR op -(`ReadInputEdge { player, mask, released }`), two main-RAM bytes -(`$07E6/$07E7` for P1/P2 prev state), and a new NMI-side snapshot -of the current input byte before the next strobe, all gated on the -`__edge_input_used` marker. See `examples/edge_input_demo.ne`. - ### L. Sprite 0 hit split-screen `split(x, y)` is the neslib primitive for a fixed status bar above @@ -445,17 +432,6 @@ modifier for HUD sprites that must stay at low OAM slots — is the cleaner user-facing API. Mentioned already under Open Design Questions; bumping it into the active roadmap. -### N. Runtime PRNG (ships today) - -`rand8()` / `rand16()` / `seed_rand(seed: u16)` are builtin -intrinsics backed by a 16-bit Galois LFSR (polynomial `0xB400`, -full 65535-cycle period from any non-zero seed). State lives in -main RAM at `$07EA/$07EB` and is seeded to `0xACE1` at reset so -the first draw is useful without explicit seeding. Routines + -seed init are gated on the `__rand_used` marker — programs that -never call any of the three pay zero bytes. See -`examples/prng_demo.ne`. - ### O. DPCM / DMC sample playback Already listed under Audio Pipeline. FamiStudio's DMC support @@ -518,16 +494,11 @@ three reads every program needs. ### V. Additional mappers -**Shipped:** -- **AxROM** (mapper 7) — single-screen mirroring, 32 KB PRG pages. - `mapper: AxROM` in `game { }`. Linker pads single-bank ROMs to - 32 KB. See `examples/axrom_simple.ne`. -- **CNROM** (mapper 3) — fixed 32 KB PRG, 8 KB CHR bankswitching. - `mapper: CNROM`. See `examples/cnrom_simple.ne`. User-visible CHR - bank selection is still TODO — the reset-time init writes bank 0 - and nothing else is exposed yet. - -**Still TODO:** +AxROM (mapper 7) and CNROM (mapper 3) both ship today; see +`examples/axrom_simple.ne` and `examples/cnrom_simple.ne`. CNROM's +user-visible CHR bankswitching is still TODO — the reset-time init +writes bank 0 and nothing else is exposed yet, so CHR swaps +mid-frame aren't reachable from user source. The next set: 1. **GNROM / MHROM** (mapper 66). Combines AxROM-style PRG with CNROM-style CHR banking. Another single-register mapper. @@ -558,13 +529,13 @@ Today the debug port is hardcoded to `$4800`. Expose `mesen`, emit writes to `$4018` (Mesen's documented debug port) and document the trace-log tool invocation in the debug docs. -### Y. FCEUX `.nl` / `.ld` label file output (ships today) +### Y. FCEUX `.ld` line-info follow-up -`--fceux-labels ` emits `..nl` for -each PRG bank plus `.ram.nl` for RAM/zero-page labels. -Each bank line has the form `$XXXX#label_name#` which is what -FCEUX reads. Still TODO: a `.ld` line-info file that pairs with -the source map for proper line-level stepping in FCEUX. +`--fceux-labels ` ships today and emits +`..nl` + `.ram.nl`. FCEUX also reads a +`.ld` line-info file for source-level stepping; wiring that up +against the existing source-map data would close the loop without +much code. ### Z. Explicit bank-placement hints on functions and data @@ -583,10 +554,7 @@ to a specific bank to avoid bank-switch cost on a hot path. ### Priority ranking -Already shipped: edge-triggered input (§K), PRNG (§N), palette -brightness (§J), AxROM + CNROM (§V), FCEUX labels (§Y). - -Remaining order by user value: +Remaining gap items in order of user value: 1. `i16` (§A) — unblocks signed physics, metasprite offsets. 2. VRAM update buffer (§G) — unblocks HUDs, dialog, streaming. diff --git a/examples/prng_demo.ne b/examples/prng_demo.ne index 423e3bd..ee5b5df 100644 --- a/examples/prng_demo.ne +++ b/examples/prng_demo.ne @@ -1,9 +1,11 @@ // PRNG demo — sprite positions driven by the runtime PRNG. // -// Each frame draws four sprites at addresses pulled from the -// xorshift-style `rand8()` intrinsic, with an extra `rand16()` -// sample binned into a horizontal velocity. Exercises `rand8`, -// `rand16`, and `seed_rand` end-to-end. +// Each frame draws four sprites at pseudorandom coordinates pulled +// from the Galois-LFSR `rand8()` intrinsic, plus one sprite whose +// (x, y) pair comes from a single `rand16()` draw. Exercises all +// three PRNG intrinsics (`rand8`, `rand16`, `seed_rand`) end-to-end. +// `seed_rand(0x1234)` on the first frame pins the sequence so the +// committed golden is deterministic. game "PRNG Demo" { mapper: NROM diff --git a/examples/prng_demo.nes b/examples/prng_demo.nes index d59a14ac7b8047abcb89c70878893352d831ba3c..b08bf2f3c061ca090e157fc95b9675358cc98d44 100644 GIT binary patch delta 42 ycmbPmfN=s4wJ|Q4*dE9@d*aP=% bool { ) } +/// True if `name` is an intrinsic that does not produce a usable +/// return value. These are valid only at statement position; +/// using them as an expression is a type error caught by the +/// analyzer so the codegen / linker never sees a stray +/// `Expr::Call` for one of them. +fn is_void_intrinsic(name: &str) -> bool { + matches!(name, "poke" | "seed_rand" | "set_palette_brightness") +} + /// True if `name` is one of the NES controller's eight buttons. /// Mirrors the `button_mask` table in `ir::lowering`. fn is_known_button(name: &str) -> bool { diff --git a/src/ir/lowering.rs b/src/ir/lowering.rs index bc03017..16b7089 100644 --- a/src/ir/lowering.rs +++ b/src/ir/lowering.rs @@ -1133,6 +1133,20 @@ impl LoweringContext { let level = self.lower_expr(&args[0]); self.emit(IrOp::SetPaletteBrightness(level)); } + // `rand8()` / `rand16()` at statement position — + // valid because they have side effects (advancing + // the PRNG state). The returned value is discarded + // by routing through a fresh temp that nothing + // reads; the JSR still runs so the state advances. + "rand8" if args.is_empty() => { + let t = self.fresh_temp(); + self.emit(IrOp::Rand8(t)); + } + "rand16" if args.is_empty() => { + let lo = self.fresh_temp(); + let hi = self.fresh_temp(); + self.emit(IrOp::Rand16(lo, hi)); + } _ => { // Inline expansion at statement context // splices either the return expression diff --git a/src/linker/debug_symbols.rs b/src/linker/debug_symbols.rs index d0089bf..0bf7cbb 100644 --- a/src/linker/debug_symbols.rs +++ b/src/linker/debug_symbols.rs @@ -742,4 +742,76 @@ mod tests { "quotes + backslashes should be escaped in file record; got:\n{out}" ); } + + #[test] + fn fceux_nl_emits_user_facing_labels_sorted_by_name() { + let linked = make_linked(&[ + ("__reset", 0xC000), + ("__nmi", 0xC100), + ("__ir_fn_Main_frame", 0xC200), + ("__ir_fn_helper", 0xC300), + ("__ir_skip_42", 0xC180), // internal, must not appear + ]); + let out = render_fceux_nl(&linked); + assert!( + out.contains("$C000#reset#"), + "reset entry point should be in .nl; got:\n{out}" + ); + assert!( + out.contains("$C100#nmi#"), + "nmi entry point should be in .nl; got:\n{out}" + ); + assert!( + out.contains("$C200#Main_frame#"), + "user frame handler should be in .nl; got:\n{out}" + ); + assert!( + out.contains("$C300#helper#"), + "user function should be in .nl; got:\n{out}" + ); + assert!( + !out.contains("__ir_skip_42"), + "internal skip labels should be filtered; got:\n{out}" + ); + assert!( + !out.contains("__ir_skip"), + "no raw internal labels should leak; got:\n{out}" + ); + } + + #[test] + fn fceux_nl_empty_when_no_user_labels() { + // Only internal labels — the .nl file should be empty + // rather than containing every scaffold label. + let linked = make_linked(&[("__ir_skip_0", 0xC010), ("__ir_skip_1", 0xC020)]); + let out = render_fceux_nl(&linked); + assert!(out.is_empty(), "no user labels → empty .nl; got:\n{out}"); + } + + #[test] + fn fceux_ram_nl_sorted_by_address() { + let vars = vec![ + VarAllocation { + name: "enemies".into(), + address: 0x0300, + size: 4, + }, + VarAllocation { + name: "score".into(), + address: 0x0010, + size: 1, + }, + VarAllocation { + name: "pos_x".into(), + address: 0x0020, + size: 1, + }, + ]; + let out = render_fceux_ram_nl(&vars); + let lines: Vec<&str> = out.lines().collect(); + assert_eq!(lines.len(), 3); + assert_eq!(lines[0], "$0010#score#"); + assert_eq!(lines[1], "$0020#pos_x#"); + assert_eq!(lines[2], "$0300#enemies#"); + } } diff --git a/src/linker/mod.rs b/src/linker/mod.rs index cb30976..65177a7 100644 --- a/src/linker/mod.rs +++ b/src/linker/mod.rs @@ -320,6 +320,18 @@ impl Linker { "NROM does not support switchable PRG banks (got {} banks)", switchable_banks.len() ); + // CNROM has a fixed 32 KB PRG — user-declared switchable PRG + // banks are meaningless. AxROM switches 32 KB pages as a + // unit, which the current 16 KB-per-bank trampoline model + // doesn't support cleanly. Reject both up front so a silent + // layout mismatch doesn't produce a subtly broken ROM. + assert!( + switchable_banks.is_empty() || !matches!(self.mapper, Mapper::CNROM | Mapper::AxROM), + "{:?} does not yet support switchable PRG banks (got {} banks); \ + use MMC1/UxROM/MMC3 for per-function banking", + self.mapper, + switchable_banks.len() + ); self.link_banked_inner( user_code, sprites, diff --git a/src/main.rs b/src/main.rs index de9add2..43c2c9e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -501,17 +501,30 @@ fn compile(input: &PathBuf, output: &Path, opts: &CompileOptions) -> Result..nl` per-bank and - // `.ram.nl` for RAM symbols. NEScript's fixed bank - // is always the last physical bank in the ROM, so for a - // single-bank NROM layout the index is 0; for banked ROMs - // it's `total_banks - 1`. We derive it from the linker's - // reported fixed-bank file offset: (offset - 16) / - // PRG_BANK_SIZE. + // FCEUX looks for `..nl` per-bank and + // `.ram.nl` for RAM symbols, where `` + // is literally the ROM file name (typically something like + // `foo.nes`). We treat `--fceux-labels ` as exactly + // that stem and *append* the `..nl` suffix rather than + // using `with_extension` (which would strip whatever came + // after the first `.`). This lets users pass either + // `--fceux-labels foo` (→ foo.0.nl, foo.ram.nl) or + // `--fceux-labels foo.nes` (→ foo.nes.0.nl, foo.nes.ram.nl, + // matching FCEUX's default search paths). + // + // The fixed bank is always the last physical bank in the + // ROM, so for a single-bank NROM layout the index is 0 and + // for banked ROMs it's `total_banks - 1`. We derive the + // index from the linker's reported fixed-bank file offset: + // `(offset - 16) / PRG_BANK_SIZE`. const PRG_BANK_SIZE: usize = 16384; let bank_index = out.link_result.fixed_bank_file_offset.saturating_sub(16) / PRG_BANK_SIZE; let bank_nl = render_fceux_nl(&out.link_result); - let bank_path = prefix.with_extension(format!("{bank_index}.nl")); + let bank_path = { + let mut p = prefix.as_os_str().to_os_string(); + p.push(format!(".{bank_index}.nl")); + PathBuf::from(p) + }; std::fs::write(&bank_path, bank_nl).map_err(|e| { eprintln!( "error: failed to write FCEUX bank label file {}: {e}", @@ -519,7 +532,11 @@ fn compile(input: &PathBuf, output: &Path, opts: &CompileOptions) -> Result Option { | IrOp::CmpLtEq(d, _, _) | IrOp::CmpGtEq(d, _, _) | IrOp::ArrayLoad(d, _, _) => Some(*d), - IrOp::Rand16(d, _) => Some(*d), IrOp::Call(dest, _, _) => *dest, IrOp::ReadInput(d, _) => Some(*d), IrOp::ReadInputEdge { dest, .. } => Some(*dest), IrOp::Peek(d, _) => Some(*d), - IrOp::Rand8(d) => Some(*d), + // Rand8 / Rand16 have an observable side effect — advancing + // the PRNG state — so a statement-level call like + // `rand8()` (result discarded) must NOT be dropped by DCE. + // Returning `None` here keeps the JSR regardless of whether + // the dest temp is live; the dest is still referenced by + // `op_source_temps` so live expression-position uses still + // track correctly. + IrOp::Rand8(_) | IrOp::Rand16(_, _) => None, IrOp::StoreVar(_, _) | IrOp::StoreVarHi(_, _) | IrOp::ArrayStore(_, _, _) diff --git a/src/rom/tests.rs b/src/rom/tests.rs index 34f2593..8d1aad8 100644 --- a/src/rom/tests.rs +++ b/src/rom/tests.rs @@ -307,6 +307,8 @@ fn nes2_mapper_high_nibble_in_byte_8_is_zero_for_small_mappers() { crate::parser::ast::Mapper::MMC1, crate::parser::ast::Mapper::UxROM, crate::parser::ast::Mapper::MMC3, + crate::parser::ast::Mapper::AxROM, + crate::parser::ast::Mapper::CNROM, ] { let mut builder = RomBuilder::new(Mirroring::Horizontal); builder.set_mapper(crate::rom::mapper_number(mapper)); diff --git a/src/runtime/mod.rs b/src/runtime/mod.rs index d97fd6e..4f4d6dd 100644 --- a/src/runtime/mod.rs +++ b/src/runtime/mod.rs @@ -1618,121 +1618,72 @@ pub fn gen_divide() -> Vec { /// referenced). Programs that never touch the PRNG pay zero ROM /// or cycle cost. /// -/// Algorithm: a 16-bit xorshift with the canonical `(7, 9, 8)` -/// parameters that produces a full 65535-cycle period from any -/// non-zero seed. The state lives in main RAM at -/// `PRNG_STATE_LO`/`PRNG_STATE_HI`; the reset path seeds it to -/// `0xACE1` (a classic xorshift16 nonzero seed) so the first -/// `rand8()` call returns a useful value without requiring an -/// explicit `seed_rand`. +/// ## Algorithm +/// +/// A right-shifting Galois-configuration 16-bit LFSR with tap mask +/// `0xB400` on the high byte (polynomial `x^16 + x^14 + x^13 + +/// x^11 + 1`). From any non-zero seed it cycles through all 65535 +/// non-zero 16-bit states before repeating. Per step: +/// +/// ```text +/// carry = state & 1 +/// state >>= 1 +/// if carry: state ^= 0xB400 +/// ``` +/// +/// State lives in main RAM at `PRNG_STATE_LO`/`PRNG_STATE_HI`; the +/// reset path seeds it to `0xACE1` so the first draw is useful +/// without requiring an explicit `seed_rand`. /// /// ## Entry points /// /// - `__rand8`: advances the state once, returns `A` = new low byte. -/// Clobbers A, X, flags. ~40 cycles. -/// - `__rand16`: advances twice, returns `A` = lo byte of the -/// second draw, `X` = hi byte. Clobbers A, X, flags. ~80 cycles. +/// Clobbers A and flags. ~40 cycles. +/// - `__rand16`: advances once, returns `A` = new low byte, +/// `X` = new high byte — 16 bits from the same shift. Clobbers +/// A, X, and flags. ~50 cycles. /// - `__rand_seed`: consumes `A` = low byte, `X` = high byte of the -/// new seed and writes them to the state slots. Does not advance. -/// Callers are responsible for not seeding with zero (the PRNG -/// gets stuck), but the builtin lowering forces a `| 1` on the -/// low byte to sidestep the common `seed = frame_counter` case. +/// new seed and writes them to the state slots. Forces bit 0 of +/// the low byte high so a zero seed (which would stick the LFSR) +/// becomes `(1, 0)`. #[must_use] pub fn gen_prng() -> Vec { let mut out = Vec::new(); // ── __rand8 ────────────────────────────────────────────── - // xorshift16 step: - // state ^= state << 7 (A-register shift) - // state ^= state >> 9 (byte swap + right shift 1) - // state ^= state << 8 (swap: new_lo = hi, new_hi = hi ^ lo) - // Returns the new low byte in A. + // One Galois LFSR step: shift the 16-bit state right, XOR the + // high byte with 0xB4 if the shifted-out bit was a 1. The + // low tap byte is 0x00 so only the high byte needs EORing. out.push(Instruction::new(NOP, AM::Label("__rand8".into()))); - - // --- state ^= state << 7 --- - // 7 left shifts of a 16-bit value is equivalent to a 1-bit - // right rotation with fill-from-low-bit-of-hi, which the - // 6502 can do with a bit of carry juggling. Simpler: shift - // the whole 16-bit state left once and XOR back into itself, - // seven times. But for size we use the rotate trick: - // LDA hi : LSR A : LDA lo : ROR A : STA hi : ... no wait, - // let's just do it the straightforward way: 7 ASLs of the - // 16-bit state folded into a XOR. - // - // Clearer and smaller: compute tmp = state << 7 by: - // tmp_lo = lo << 7 = (lo & 1) << 7 [only bit 0 survives] - // tmp_hi = (hi << 7) | (lo >> 1) [hi's bit 0 is lo's bit 1] - // Actually, <<7 on 16-bit: bits 0..8 of input become bits 7..15 of output. - // output bit 7 = input bit 0 (was lo bit 0) - // output bits 8..14 = input bits 1..7 (was lo bits 1..7) - // output bit 15 = input bit 8 (was hi bit 0) - // - // Equivalently: tmp_lo = lo << 7, tmp_hi = (lo >> 1) | (hi << 7). - // Then state ^= tmp. - // - // Since tmp_lo has only bit 7 possibly set, and tmp_hi has - // bits 0..6 from lo and bit 7 from hi bit 0: - // - // LDA PRNG_STATE_LO ; A = lo - // LSR A ; A = lo >> 1, C = lo bit 0 - // ROR PRNG_STATE_LO ; no — we need to NOT change state yet - // - // Simpler: do the full algorithm using a scratch copy. - // - // For brevity and cycle cost we use a different but equivalent - // LFSR: a Galois-configuration 16-bit LFSR with polynomial - // 0x002D (taps at 0, 2, 3, 5) which also has a full period of - // 65535 from any non-zero seed. The advantage is a very small - // step: one shift and a conditional XOR with two bytes. - // - // Step: - // carry = state & 1 - // state >>= 1 - // if carry: state ^= 0xB400 - // - // The 0xB400 tap mask comes from the standard 16-bit LFSR - // polynomial x^16 + x^14 + x^13 + x^11 + 1 reversed for - // right-shift Galois form. It has full 65535 period. - - // LSR the high byte first, then ROR the low byte, so the - // low-bit-of-low-byte ends up in carry. + // LSR the high byte, ROR the low byte. Carry after ROR holds + // the old bit 0 of state_lo (the feedback bit). out.push(Instruction::new(LSR, AM::Absolute(PRNG_STATE_HI))); out.push(Instruction::new(ROR, AM::Absolute(PRNG_STATE_LO))); - // If the shifted-out bit was zero, skip the XOR. + // If the feedback bit was zero, skip the tap XOR. out.push(Instruction::new( BCC, AM::LabelRelative("__rand8_done".into()), )); - // state_hi ^= 0xB4 out.push(Instruction::new(LDA, AM::Absolute(PRNG_STATE_HI))); out.push(Instruction::new(EOR, AM::Immediate(0xB4))); out.push(Instruction::new(STA, AM::Absolute(PRNG_STATE_HI))); - // (low tap byte is 0x00, nothing to XOR there) out.push(Instruction::new(NOP, AM::Label("__rand8_done".into()))); - // Return with A = new state low byte. out.push(Instruction::new(LDA, AM::Absolute(PRNG_STATE_LO))); out.push(Instruction::implied(RTS)); // ── __rand16 ───────────────────────────────────────────── - // Advance twice and return A=lo, X=hi. Two draws so the - // caller gets 16 bits of entropy instead of two halves of - // the same internal step. + // A single shift already produces 16 fresh bits (the full + // post-shift state). Call `__rand8` once and return both + // halves — A = lo, X = hi. out.push(Instruction::new(NOP, AM::Label("__rand16".into()))); out.push(Instruction::new(JSR, AM::Label("__rand8".into()))); - out.push(Instruction::new(JSR, AM::Label("__rand8".into()))); - // At this point A = state_lo after the second draw; we want - // to also return the high byte, which is state_hi. Stash the - // low byte, load hi into X, restore lo into A. - out.push(Instruction::implied(TAY)); // save lo in Y out.push(Instruction::new(LDX, AM::Absolute(PRNG_STATE_HI))); - out.push(Instruction::implied(TYA)); // restore lo to A out.push(Instruction::implied(RTS)); // ── __rand_seed ────────────────────────────────────────── - // Store A = lo, X = hi into the state. + // Store A = lo, X = hi into the state. Force bit 0 of the + // low byte high so a zero seed doesn't stick the LFSR. out.push(Instruction::new(NOP, AM::Label("__rand_seed".into()))); - // Force bit 0 of the low byte high so a zero seed doesn't - // stick the LFSR. out.push(Instruction::new(ORA, AM::Immediate(0x01))); out.push(Instruction::new(STA, AM::Absolute(PRNG_STATE_LO))); out.push(Instruction::new(STX, AM::Absolute(PRNG_STATE_HI))); @@ -1745,16 +1696,32 @@ pub fn gen_prng() -> Vec { /// Spliced in by the linker only when user code contains the /// `__palette_bright_used` marker label. /// -/// The input level (passed in A) is mapped to a PPU mask byte: -/// - `0` — everything off (blank screen) -/// - `1..3` — fade-out via PPU mask "darken" bits (R/G/B emphasis), -/// which darken the visible colour range by a few notches. -/// - `4` (normal) — `$1E`, sprites + background, no emphasis -/// - `5..7` — progressive emphasis bits to push towards white -/// - `8` — all three emphasis bits set (max "bright") +/// Each level (passed in A, clamped to 0..=8) writes a specific +/// `$2001` PPU mask byte. The mapping is best thought of as a +/// "visual intensity dial" rather than a linear fade: the NES PPU +/// emphasis bits tint the display towards R / G / B rather than +/// truly darken or brighten, so the progression is a series of +/// distinct visual states. Levels 0 and 1 use low-level tricks +/// (rendering off, greyscale) to approximate black and dim; levels +/// 2-8 stack emphasis bits to shift the overall colour cast. /// -/// The mapping is a 9-entry table indexed by the clamped level. -/// Levels above 8 clamp to 8 (max brightness). +/// | level | mask | effect | +/// |-------|------|----------------------------------| +/// | 0 | $00 | rendering fully off (black) | +/// | 1 | $01 | greyscale only (washed out) | +/// | 2 | $1E | normal sprites + background | +/// | 3 | $3E | + red emphasis | +/// | 4 | $5E | + green emphasis | +/// | 5 | $9E | + blue emphasis | +/// | 6 | $7E | + red + green (yellow tint) | +/// | 7 | $BE | + red + blue (magenta tint) | +/// | 8 | $FE | all three emphasis bits | +/// +/// Levels above 8 clamp to 8. Implemented as a CPX dispatch tree +/// (one `CPX; BNE; LDA; STA; RTS` block per level) rather than a +/// table lookup because the assembler doesn't expose a +/// Label-relative `AbsoluteX` mode; the nine-entry tree is ~60 +/// bytes, comparable to a data-table + indexed-load path. #[must_use] pub fn gen_palette_brightness() -> Vec { let mut out = Vec::new(); @@ -1818,8 +1785,9 @@ pub fn gen_palette_brightness() -> Vec { /// Emit the reset-time PRNG state seed. Spliced into the reset /// path whenever `gen_prng` is linked in, so the first `rand8()` /// call returns a useful value even without an explicit -/// `seed_rand`. The seed `0xACE1` is the classic nonzero -/// xorshift16 seed. +/// `seed_rand`. The seed `0xACE1` is an arbitrary non-zero 16-bit +/// value — any non-zero value works since the Galois LFSR's orbit +/// visits all 65535 non-zero states. #[must_use] pub fn gen_prng_init() -> Vec { vec![ @@ -2108,19 +2076,26 @@ pub fn gen_bank_select(mapper: Mapper) -> Vec { } Mapper::AxROM => { // AxROM: write the 32 KB PRG bank number to $8000-$FFFF. - // Bit 4 selects the single-screen nametable; we preserve - // it by ORing whatever was latched last (implicit 0 from - // reset). Our bank-switching model only moves between - // 32 KB pages, so multi-bank AxROM programs would need - // to split along 32 KB boundaries — more of a layout - // concern than a runtime one. + // Bit 4 selects the single-screen nametable; bits 0-2 + // select the PRG page. We don't preserve bit 4 here — + // user code that toggles between lower/upper nametables + // mid-run would need to OR it into A before calling. + // The linker's single-bank AxROM layout never actually + // invokes this routine today (no cross-bank trampolines + // for AxROM yet), but we emit it for parity with the + // other mappers. out.push(Instruction::new(STA, AM::Absolute(0x8000))); out.push(Instruction::implied(RTS)); } Mapper::CNROM => { // CNROM: writing A to `$8000-$FFFF` selects an 8 KB CHR - // bank. PRG is fixed so this routine is only useful for - // graphics swaps. + // bank. PRG is fixed so this routine is only ever useful + // for user-driven CHR swaps. Note: real CNROM boards + // have bus conflicts (CPU write vs ROM byte at target + // address), so a production API should route through a + // UxROM-style bus-conflict-safe table. No such API is + // exposed to user source yet — declaring more than one + // CHR bank in a CNROM program is currently a TODO. out.push(Instruction::new(STA, AM::Absolute(0x8000))); out.push(Instruction::implied(RTS)); } diff --git a/tests/emulator/goldens/prng_demo.png b/tests/emulator/goldens/prng_demo.png index 6015ecb4149b4cc39efa8a40b90079e330ba3d7b..29db170fedb651f8bd303a23644b9ded6fae5c06 100644 GIT binary patch literal 841 zcmeAS@N?(olHy`uVBq!ia0y~yU<5K5K5(!B$@kNDon~NQ7Vva&45?szd(bfV2?GPm zfwNkzw+|PH^w~278?bGE_vx2Xkj0xbHL5>34OTLynI2ffHsdP8W~qcQ<}+Chk@pj? zeYW}e+M;j6Q|*)f4W7IPt1%S~C&6pw2{#}<*2zq`^4VtRdgJe<&-R_WbA9IX%1uie z9nXJ$vhUoUd-MNP?X?F=p8uTU%6$0U>oeQnaKH|Y<{j4OzcA5MV^NlSyn aNA`DObm#gt zTMxazc%VKa8_0fhd?x?z!ek9qc=&VIl@@As*? zIhEhVXqhumvXu&4kA_tvln%S~#!w}yek_~!=sbRZ*2`-8C5 zzN^LYKR?_Ff1qw!v-+>a=S2H+2mGse@4o^^Tlx7~*1Dj>+iz^s1xDaqaP+?a_V~=U z+{XDICOy%unY}g4?)Em_55ZJTufTNG2{E{4E!#h)?6p8IJm2@}PB_@rK(V?TQNUyd zOdvJD1O#NP72W3t)B{xWaOe6DU!{I{L(*K^`tLwCP&d$L?;tXXm#{(NA@+kfQkuES3j3^P6