diff --git a/AGENTS.md b/AGENTS.md index 5742b2f5..374bc208 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -222,6 +222,7 @@ These cross-cutting decisions span multiple files. Reading individual chip docs - **There is ONE toolchain, `rust-toolchain.toml`'s `channel`, and no version literal anywhere in `.github/` — don't add one.** `.github/actions/rust-setup` parses the channel out of that file and fails closed if it can't, so a toolchain bump is a one-line edit there. Pass the composite's `toolchain:` input only to install something *deliberately* different from the project pin. **The resolver is table-scoped `awk` on purpose — do NOT "simplify" it back to a one-line `sed`.** Matching the first `channel = "..."` *anywhere* in the file (the first implementation, caught in review on PR #322) resolves `nightly` if any other table carries a `channel` key ahead of `[toolchain]` — silently installing the very toolchain this setup exists to keep out, while the step still reports success. `awk` rather than `tomllib` because the step runs on Windows and macOS runners too and Python ≥3.11 is not a safe assumption there; only double-quoted TOML strings are accepted, and anything else (missing table, single-quoted value, empty file) aborts the job rather than being guessed at. The old `stable` default was misleading rather than wrong: `rust-toolchain.toml` is a directory override that outranks the `rustup default` the action performs, so every job was already compiling on 1.96.0 (rustup logs `overridden by .../rust-toolchain.toml`) — `stable` just downloaded a second toolchain nothing used and made the workflows *read* as though they tested latest stable, which they never did. **Nightly is used in exactly one place, not a gate:** `cargo fuzz` (hard requirement — libFuzzer's sanitizer flags are nightly-only). If you think a CI job needs nightly, it doesn't. - **`rust-libretro 0.3.2` is unmaintained (no commit since 2023-02) and has a MinGW bug we work around.** It casts a keycode with `cfg(target_family = "windows")`, but C enum signedness follows the *ABI*: only **MSVC** gives plain enums `int` — under **MinGW** (`x86_64-pc-windows-gnu`, what the buildbot builds) bindgen emits `c_uint` and the crate fails `E0308`. `.cargo/config.toml`'s `[env] BINDGEN_EXTRA_CLANG_ARGS_x86_64_pc_windows_gnu = "--target=x86_64-pc-windows-msvc"` fixes it; the generated-bindings diff is 28 lines, all enum signedness. Don't "clean up" that env var without rebuilding for `x86_64-pc-windows-gnu`. - **CodeRabbit is now a 3rd automated PR review bot** (`.coderabbit.yaml`, added 2026-07-20 in PR #316), alongside gemini-code-assist and copilot-pull-request-reviewer — same reply-and-resolve-every-thread ceremony applies before any merge. Configured `profile: assertive` (not the "chill" default) and a `tools{}`/`path_instructions`/custom-checks set audited against this repo's actual file footprint, not guessed. `tone_instructions` has a hard 250-character schema limit that fails validation silently on the CodeRabbit side — after editing `.coderabbit.yaml`, verify with a `@coderabbitai configuration` PR comment and confirm every changed field shows `Source: Repository YAML (base)`. +- **The bot-comment ceremony must read the review BODIES, not just the resolvable threads.** CodeRabbit posts "Outside diff range" and other suppressed findings **inside the review body**, where they are invisible to a resolve-every-thread sweep — and Copilot does the same. This has now cost the project three times: issue #360 (an untested attestation path) reached `main` unaddressed; two findings of the same class on #357 were genuine defects, **one critical** (two threads producing frames during fast-forward under threaded display-sync, fixed in #358); and a **use-after-free** in the v2.3.5 libretro controller tables was caught only because the review body was read. A green "all threads resolved" is not evidence the review was addressed. Fetch the bodies explicitly — `gh pr view --json reviews --jq '.reviews[].body'` — and triage every finding in them before merging. - **lz4_flex 0.14+ requires the crate's own `alloc` feature explicitly** for `compress_prepend_size`/`decompress_size_prepended` (used by `rewind.rs`/`zwinder.rs`) — it split real no_std support into an `alloc`-vs-`std` distinction that didn't exist in 0.13. A `cargo build --workspace` will NOT catch a missing `alloc` feature here because `rustynes-core`'s own default-on `std` feature implies it via cargo's feature unification; only a standalone `cargo build -p rustynes-core --target thumbv7em-none-eabihf --no-default-features` (the exact CI `no_std build` job) will. Run that command locally before pushing any bump that touches this dependency. - **The libretro `.info` RetroArch reads is a DIFFERENT FILE from this repo's, and it went stale for eleven days.** RetroArch downloads `dist/info/rustynes_libretro.info` from `libretro/libretro-super`; `crates/rustynes-libretro/rustynes_libretro.info` is an unrelated copy that nothing syncs and nothing compared. So the v2.2.9 GPL relicense reached `Cargo.toml`, `NOTICE`, `deny.toml`, the SPDX headers and the local `.info` — and **not** the file users actually see, which went on advertising "MIT OR Apache-2.0" at `display_version = v2.2.1`. Both upstream PRs had merged *exactly two weeks before* the relicense, so no sync could have carried it. **A license change is now a mandatory upstream-sync trigger**, on the same footing as a release. `crates/rustynes-test-harness/tests/libretro_info_audit.rs` pins the local file against the workspace manifest so the sync is a *copy*, never a re-derivation; it cannot see upstream, so the sync itself stays a human step. libretro `.info` uses short license tokens, not SPDX, and marks "or later" with a trailing `+` (tallied across all 316 upstream cores: `GPLv2` x100, `GPLv3` x64, `GPLv2+` x19, `GPLv3+` x5) — RustyNES is **`GPLv3+`**; a bare `GPLv3` understates it as GPL-3.0-only. Full detail + the surface table: `docs/libretro/UPSTREAM_SYNC.md`. diff --git a/Cargo.lock b/Cargo.lock index 24e04257..cc2ba0b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4503,6 +4503,13 @@ dependencies = [ "thiserror 2.0.19", ] +[[package]] +name = "rustynes-probe" +version = "2.3.5" +dependencies = [ + "rustynes-core", +] + [[package]] name = "rustynes-ra" version = "2.3.5" diff --git a/Cargo.toml b/Cargo.toml index 8225a0d6..e718de09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ members = [ "crates/rustynes-cheevos", "crates/rustynes-script", "crates/rustynes-gamedb", + "crates/rustynes-probe", "crates/rustynes-frontend", "crates/rustynes-test-harness", "crates/rustynes-mobile", diff --git a/VERSION-PLAN.md b/VERSION-PLAN.md index 71fd26b7..e2a72679 100644 --- a/VERSION-PLAN.md +++ b/VERSION-PLAN.md @@ -1,6 +1,6 @@ # RustyNES Version Plan -**Current release: v2.3.4 "Ledger"** — the coverage release: three boards (mapper 176 submapper 2 WAIXING-FS005, 154 NAMCOT-3453, 243 Sachen SA-020A, breadth **172 → 174 families**), the coverage harness moved onto the frontend's real load path, and the defect that exposed — the per-game database reading a `0` Mapper column as "force NROM" and overwriting correct headers, leaving **every Sachen cartridge** unloadable since **v1.2.0**. **This release touches the emulation core**, so AccuracyCoin exactly 141/141 is **verified, not asserted by construction**. Carried to v2.3.5 unstarted: the APU at 18.7% of frame time (Workstream C, not delivered — its bench was never built). Built on **v2.3.3 "Cadence"** — the display-pacing release: the run-ahead throttle oscillation traced to a stale median (a gate counting 120 frames of a 600-sample ring), a predictive engage arm that converges a `run_ahead = 3` host in 2.8 s instead of 12.1 s, and the `wp_presentation` measurement apparatus that made the diagnosis possible. **No emulation-core changes** (AccuracyCoin exactly 141/141). Built on **v2.3.2 "Lucid"** (pixel provenance + deterministic replay attestation), **v2.3.1 "Plumb Line"** (ten measured rejections), and **v2.3.0 "Datum II"**, the capstone that **closed** the v2.2.6 → v2.3.0 line (true multi-viewport OS-window detach, the emulator-lock frame-pacing fix, a −5.1% byte-identical PPU optimization, and both forum-reported accuracy items verified already-correct) — all on the **v2.0.0 "Timebase"** MAJOR base (the one-clock / every-cycle-bus-access scheduler rewrite). **v1.0.0** was the first stable, production cut. As of **v2.2.9**, RustyNES is **GPL-3.0-or-later** — a derivative work of GPL-licensed emulators (ADR 0036); a licensing correction, **not** a SemVer break (no public-API or save-state change). `docs/STATUS.md` is the authoritative current-state record; `CHANGELOG.md` carries the full per-release history. +**Current release: v2.3.5 "Manifest"** — the declaration release: what the core says about itself. A user reported RetroArch still showing the pre-relicense MIT/Apache-2.0 terms, and it was: RetroArch reads `dist/info/` from **libretro/libretro-super**, a SEPARATE copy nothing synced, so the v2.2.9 GPL relicense never reached the file users see. Corrected to `GPLv3+` with a standing `libretro_info_audit.rs` that makes the upstream sync a **copy** rather than a re-derivation, and a licence change is now a mandatory upstream-sync trigger. Auditing the wrapper then found **five further defects, every one with correct emulation behind it** — PAL ran 20.2% fast, Reset did nothing ever, unload leaked Game Genie indices, the aspect ratio assumed square pixels, and the Zapper was unreachable — plus a **use-after-free** in the controller tables caught in review. The crate went from zero tests to eight. The APU also gained its first throughput bench and a default-configuration mix specialization (−3.3% to −4.2% on `nes_run_frame_nestest`), so **AccuracyCoin 141/141 was VERIFIED, not asserted**. Built on **v2.3.4 "Ledger"** — the coverage release: three boards (mapper 176 submapper 2 WAIXING-FS005, 154 NAMCOT-3453, 243 Sachen SA-020A, breadth **172 → 174 families**), the coverage harness moved onto the frontend's real load path, and the defect that exposed — the per-game database reading a `0` Mapper column as "force NROM" and overwriting correct headers, leaving **every Sachen cartridge** unloadable since **v1.2.0**. **This release touches the emulation core**, so AccuracyCoin exactly 141/141 is **verified, not asserted by construction**. Its Workstream C (the APU at 18.7% of frame time) was carried to v2.3.5 and delivered there. Built on **v2.3.3 "Cadence"** — the display-pacing release: the run-ahead throttle oscillation traced to a stale median (a gate counting 120 frames of a 600-sample ring), a predictive engage arm that converges a `run_ahead = 3` host in 2.8 s instead of 12.1 s, and the `wp_presentation` measurement apparatus that made the diagnosis possible. **No emulation-core changes** (AccuracyCoin exactly 141/141). Built on **v2.3.2 "Lucid"** (pixel provenance + deterministic replay attestation), **v2.3.1 "Plumb Line"** (ten measured rejections), and **v2.3.0 "Datum II"**, the capstone that **closed** the v2.2.6 → v2.3.0 line (true multi-viewport OS-window detach, the emulator-lock frame-pacing fix, a −5.1% byte-identical PPU optimization, and both forum-reported accuracy items verified already-correct) — all on the **v2.0.0 "Timebase"** MAJOR base (the one-clock / every-cycle-bus-access scheduler rewrite). **v1.0.0** was the first stable, production cut. As of **v2.2.9**, RustyNES is **GPL-3.0-or-later** — a derivative work of GPL-licensed emulators (ADR 0036); a licensing correction, **not** a SemVer break (no public-API or save-state change). `docs/STATUS.md` is the authoritative current-state record; `CHANGELOG.md` carries the full per-release history. RustyNES follows [Semantic Versioning 2.0.0](https://semver.org/). @@ -55,7 +55,7 @@ The cycle-accurate engine was integrated as the core in a sequence of documentar | **v0.9.7** | Performance pass (display-sync pacing, dedicated emu thread, audio DRC, run-ahead) | | **v1.0.0** | Production cut — engine + ported desktop UX shell + documentation synthesis | -> **Engine lineage note.** The deep technical history under `docs/` (the `v2.0` master-clock refactor, ADRs, audit logs, the long accuracy program) describes the **upstream engine lineage**. Those old "v1.x"/"v2.x" anchors are engineering history, **not** RustyNES release versions. RustyNES's own release line is v0.1.0 → v0.8.6 → (documentary v0.9.0–v0.9.7) → **v1.0.0** → the v1.1.0–v1.10.0 additive feature line → **v2.0.0 "Timebase"** (the designated MAJOR break) → the v2.0.x "Harbor" line → the v2.1.x "Fathom" accuracy line → the v2.2.x line → v2.3.0 "Datum II" → v2.3.1 "Plumb Line" → v2.3.2 "Lucid" → v2.3.3 "Cadence" → **v2.3.4 "Ledger"** (current). +> **Engine lineage note.** The deep technical history under `docs/` (the `v2.0` master-clock refactor, ADRs, audit logs, the long accuracy program) describes the **upstream engine lineage**. Those old "v1.x"/"v2.x" anchors are engineering history, **not** RustyNES release versions. RustyNES's own release line is v0.1.0 → v0.8.6 → (documentary v0.9.0–v0.9.7) → **v1.0.0** → the v1.1.0–v1.10.0 additive feature line → **v2.0.0 "Timebase"** (the designated MAJOR break) → the v2.0.x "Harbor" line → the v2.1.x "Fathom" accuracy line → the v2.2.x line → v2.3.0 "Datum II" → v2.3.1 "Plumb Line" → v2.3.2 "Lucid" → v2.3.3 "Cadence" → v2.3.4 "Ledger" → **v2.3.5 "Manifest"** (current). ### Post-1.0 release line (v1.1.0 → current) @@ -80,9 +80,10 @@ The 1.x line was **additive / off-by-default** — every release stayed byte-ide | **v2.3.1 "Plumb Line"** | Measurement apparatus made trustworthy, then used: a harness-free frame probe, per-source-file subsystem attribution (which recovers the **APU at 18.7% of frame**, invisible in the symbol profile), an adoption A/B with an A/B/A order-bias control, and a contention-aware relative gate. **Ten core hot-path candidates measured, all ten rejected** via six distinct mechanisms — **no emulation-core changes**, AccuracyCoin exactly 141/141 — see `CHANGELOG.md` `[2.3.1]` | | **v2.3.2 "Lucid"** | Pixel provenance — click any pixel for its full causal chain, down to **the CPU instruction and cycle that last wrote each byte** — plus deterministic replay attestation (`rustynes verify`). All `debug-hooks`-gated and output-only, so AccuracyCoin holds exactly 141/141 — see `CHANGELOG.md` `[2.3.2]` | | **v2.3.3 "Cadence"** | Display pacing. The run-ahead throttle oscillation attributed to a **stale median** — the gate counted 120 frames of a 600-sample ring, so a p50 at index 300 could not leave the previous depth; **6-7 transitions per 24 s → 1**, spurious releases **2 → 0**. The engage arm now predicts instead of waiting (`run_ahead = 3` converges in **2.8 s vs 12.1 s**, 5/5 paired rounds, p = 0.0312) while releasing still demands a real measurement. Compositor refresh via `wp_presentation`, divisor display-sync, and a validity gate that fails closed; dropped frames **135-254 → 1-9**. Two arms measured and **rejected** with their numbers. No emulation-core changes — see `CHANGELOG.md` `[2.3.3]` | -| **v2.3.4 "Ledger"** (current) | Mapper coverage. Three boards — **176 submapper 2** (WAIXING-FS005), **154** (NAMCOT-3453), **243** (Sachen SA-020A) — breadth **172 → 174** (51 Core + 95 Curated + 28 BestEffort), all implemented from the NESdev wiki with no reference-emulator source consulted. The coverage harness moved onto the frontend's real load path, which exposed a **v1.2.0-era defect reaching users**: the per-game database read a `0` Mapper column as "force NROM" and overwrote correct headers, leaving **12 ROMs — every Sachen board in the corpus** — unable to load. Also a Bandai FCG EEPROM debug panic, CLI launches skipping header overrides, mapper 15 PRG-RAM/CHR-RAM, save-state back-compat for 15/88/176, and #360. **Touches the core**, so AccuracyCoin 141/141 is verified, not construction. Workstream C (the APU at 18.7%) **not delivered**, carried to v2.3.5 — see `CHANGELOG.md` `[2.3.4]` | +| **v2.3.4 "Ledger"** | Mapper coverage. Three boards — **176 submapper 2** (WAIXING-FS005), **154** (NAMCOT-3453), **243** (Sachen SA-020A) — breadth **172 → 174** (51 Core + 95 Curated + 28 BestEffort), all implemented from the NESdev wiki with no reference-emulator source consulted. The coverage harness moved onto the frontend's real load path, which exposed a **v1.2.0-era defect reaching users**: the per-game database read a `0` Mapper column as "force NROM" and overwrote correct headers, leaving **12 ROMs — every Sachen board in the corpus** — unable to load. Also a Bandai FCG EEPROM debug panic, CLI launches skipping header overrides, mapper 15 PRG-RAM/CHR-RAM, save-state back-compat for 15/88/176, and #360. **Touches the core**, so AccuracyCoin 141/141 is verified, not construction. Workstream C (the APU at 18.7%) **not delivered**, carried to v2.3.5 — see `CHANGELOG.md` `[2.3.4]` | +| **v2.3.5 "Manifest"** (current) | What the core declares about itself. RetroArch reads `dist/info/rustynes_libretro.info` from **libretro/libretro-super**, a SEPARATE copy from this repo's that nothing synced — so the v2.2.9 GPL relicense reached `Cargo.toml`, `NOTICE`, `deny.toml` and the SPDX headers, and **not the file users see**, which advertised MIT/Apache-2.0 at `v2.2.1` for eleven days. Corrected to **`GPLv3+`** (libretro uses short tokens and marks "or later" with a trailing `+`, tallied across all 316 upstream cores) and pinned by a standing `libretro_info_audit.rs`, so the sync is a copy rather than a re-derivation; **a licence change is now a mandatory upstream-sync trigger**. The wrapper audit that followed found **five defects, each with correct emulation behind it**: a hardcoded 60.0988 fps for every cartridge with `retro_get_region` unimplemented (**PAL ran 20.2% fast**), `retro_reset` unimplemented (**RetroArch's Reset did nothing, ever** — the library default is a literal no-op), `retro_unload_game` unimplemented (Game Genie indices leaked across cartridges), `aspect_ratio = 0.0` (square pixels against the desktop frontend's 8:7), and no controller info (**the Zapper was unreachable** despite `Nes::set_zapper` being fully implemented). Review caught a **use-after-free**: RetroArch shallow-`memcpy`s the outer `retro_controller_info` array but RETAINS each `types` pointer, so those tables must be `'static` — `SET_INPUT_DESCRIPTORS` is different and safe, and the two must never be generalized between. The crate went **0 tests → 8**. Separately the APU (18.7% of frame time, invisible to a symbol profile because fat LTO inlines it into `cpu_clock`) gained its first throughput bench and a default-configuration mix specialization, **−3.3% to −4.2%** on `nes_run_frame_nestest`. Declared values are now DERIVED from `rustynes_core` constants rather than transcribed. **The APU implementation changed**, so AccuracyCoin 141/141 and nestest 0-diff are **verified, not true by construction**. NOT fixed here: RetroArch shows the right licence only once libretro merges, and iOS/iPadOS/tvOS availability is a hardcoded `appstore_cores` list in `libretro/RetroArch` — both upstream — see `CHANGELOG.md` `[2.3.5]` | -> **Forward path.** The v2.0.x "Harbor", v2.1.x "Fathom", and v2.2.x lines have all shipped; the v2.2.6 → v2.3.0 line has now **closed** with v2.3.0 "Datum II"; the v2.3.x performance campaign has now **shipped in full**, as three releases: **v2.3.1 "Plumb Line"** absorbed both the measurement apparatus and the core hot-path campaign, whose ten items were all measured and all rejected and so had no shippable content of their own; **v2.3.2 "Lucid"** the novel features (pixel provenance + replay attestation); and **v2.3.3 "Cadence"** the display-pacing work — the run-ahead throttle oscillation traced to a stale median, the predictive engage arm, and the `wp_presentation` measurement apparatus that made the diagnosis possible. The campaign closed there; **v2.3.4 "Ledger"** (current) opens the next line with mapper coverage — three boards to **174 families**, and the coverage harness moved onto the frontend's real load path, which exposed a per-game-database defect that had left every Sachen cartridge unloadable since v1.2.0. Its Workstream C, the APU at 18.7% of frame time, was **not delivered** and carries to v2.3.5. Note the codenames diverged from this plan as written: what shipped as v2.3.2 took "Lucid" rather than the planned "Grain"/"Conduit II", and v2.3.3 is "Cadence". RustyNES is **permanently open-source and income-free** (ADR 0035): the earlier "joint Google Play + App Store + AltStore + F-Droid launch" is **withdrawn** — any store listing is a **free** app with **no monetization** (no ads, tracking, or paid unlock), an unversioned later step. `to-dos/ROADMAP.md` is the authoritative forward roadmap. +> **Forward path.** The v2.0.x "Harbor", v2.1.x "Fathom", and v2.2.x lines have all shipped; the v2.2.6 → v2.3.0 line has now **closed** with v2.3.0 "Datum II"; the v2.3.x performance campaign has now **shipped in full**, as three releases: **v2.3.1 "Plumb Line"** absorbed both the measurement apparatus and the core hot-path campaign, whose ten items were all measured and all rejected and so had no shippable content of their own; **v2.3.2 "Lucid"** the novel features (pixel provenance + replay attestation); and **v2.3.3 "Cadence"** the display-pacing work — the run-ahead throttle oscillation traced to a stale median, the predictive engage arm, and the `wp_presentation` measurement apparatus that made the diagnosis possible. The campaign closed there; **v2.3.4 "Ledger"** opened the next line with mapper coverage — three boards to **174 families**, and the coverage harness moved onto the frontend's real load path, which exposed a per-game-database defect that had left every Sachen cartridge unloadable since v1.2.0. Its Workstream C, the APU at 18.7% of frame time, was not delivered there and landed in **v2.3.5 "Manifest"** (current), which is otherwise about what the core declares about itself: the libretro `.info` licence drift a user reported, and the five wrapper defects auditing it uncovered. Note the codenames diverged from this plan as written: what shipped as v2.3.2 took "Lucid" rather than the planned "Grain"/"Conduit II", and v2.3.3 is "Cadence". RustyNES is **permanently open-source and income-free** (ADR 0035): the earlier "joint Google Play + App Store + AltStore + F-Droid launch" is **withdrawn** — any store listing is a **free** app with **no monetization** (no ads, tracking, or paid unlock), an unversioned later step. `to-dos/ROADMAP.md` is the authoritative forward roadmap. ## Versioning guidelines diff --git a/crates/rustynes-probe/Cargo.toml b/crates/rustynes-probe/Cargo.toml new file mode 100644 index 00000000..f5b88e5e --- /dev/null +++ b/crates/rustynes-probe/Cargo.toml @@ -0,0 +1,26 @@ +# v2.3.6 — the deterministic-probe engine. +# +# Three planned tools reduce to the same primitive: take a snapshot anchor, +# re-simulate N times under controlled variation, and find the first observable +# divergence. RustyNES can do that soundly because its determinism contract is a +# hard guarantee rather than an aspiration, so the answer is a property of the +# ROM rather than of the run. +# +# Kept dependency-light on purpose, like `rustynes-gamedb`: it depends only on +# `rustynes-core`, so it is headless-testable and CI can gate it without pulling +# in winit/wgpu. +[package] +name = "rustynes-probe" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +authors.workspace = true +repository.workspace = true +description = "Deterministic re-simulation probe: anchor, replay under variation, locate the first divergence" + +[dependencies] +rustynes-core = { path = "../rustynes-core" } + +[lints] +workspace = true diff --git a/crates/rustynes-probe/src/latency.rs b/crates/rustynes-probe/src/latency.rs new file mode 100644 index 00000000..7eaec867 --- /dev/null +++ b/crates/rustynes-probe/src/latency.rs @@ -0,0 +1,539 @@ +//! Measuring a game's **own** input lag. +//! +//! Most NES titles sample the controller in their NMI handler and act on it one +//! or more frames later. That delay is real on hardware, and run-ahead removes +//! it by simulating those frames in advance — but only if you tell it how many. +//! +//! Every emulator makes finding that number a manual ritual: hold a direction, +//! frame-advance until the sprite moves, subtract one. `RetroArch` documents +//! exactly that procedure; `RustyNES`'s own settings panel says only "1 fits most +//! games". Nothing measures it. +//! +//! This does, by asking the question directly: replay the same anchor twice, +//! once with a button held and once with nothing pressed, and find the first +//! frame at which the two runs differ. That index **is** the game's internal +//! lag, because it is the first frame on which pressing the button could have +//! changed anything. +//! +//! # Being honest is the hard part +//! +//! A latency number is acted on — it sets run-ahead depth, which costs real +//! frame budget. So this module is built to **decline** rather than guess: +//! +//! - It probes several buttons, because a given game may ignore most of them, +//! and requires them to *agree* before reporting a value. +//! - It falls back across observables, because a reaction may be audible or +//! internal before it is visible — a menu that commits a highlight to a +//! variable one frame before drawing it would otherwise read as slower than +//! it is. +//! - It reports [`Confidence`] alongside the number, and returns +//! [`LatencyReport::frames`] as `None` whenever the trials disagree or nothing +//! reacted inside the budget. +//! +//! "I could not tell" is a valid, useful answer. A wrong depth silently spends +//! frame budget the host may not have. + +use rustynes_core::{Buttons, Nes}; + +use crate::{Budget, Observable, Probe}; + +/// Buttons worth probing, in the order tried. +/// +/// Directions first: they are what most games act on soonest and what a player +/// is holding when latency matters. `A`/`B` next, since action games respond to +/// them. `START` last and deliberately — it pauses many games, which is a +/// reaction, but a reaction to a *menu*, not to gameplay input, and treating a +/// pause as gameplay latency would over-report. +const PROBE_BUTTONS: [Buttons; 6] = [ + Buttons::RIGHT, + Buttons::LEFT, + Buttons::DOWN, + Buttons::UP, + Buttons::A, + Buttons::B, +]; + +/// Observables tried in order until one produces agreeing answers. +/// +/// Framebuffer first — a visible reaction is what a player perceives as latency. +/// Audio next: a sound effect often fires the same frame the input is accepted, +/// before anything is drawn. Work RAM last, because it detects a reaction the +/// player cannot yet perceive; useful as evidence the game read the pad at all, +/// but the least representative of *felt* latency. +const OBSERVABLE_ORDER: [Observable; 3] = [ + Observable::Framebuffer, + Observable::AudioEnergy, + Observable::Wram, +]; + +/// How much to trust a measurement. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Confidence { + /// Every button that reacted agreed on the same frame. + Unanimous, + /// A majority agreed; at least one reacting button disagreed. Usable, but a + /// caller applying it automatically should say it is approximate. + Majority, + /// No value: nothing reacted inside the budget, or the reacting buttons did + /// not agree closely enough to pick one. + Inconclusive, +} + +/// The result of a measurement. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct LatencyReport { + /// Measured internal lag in frames, or `None` when inconclusive. + /// + /// `None` and `Some(0)` are **different answers** and must not be collapsed: + /// `Some(0)` means the game reacted on the very next frame, `None` means the + /// probe could not tell. Reporting the second as the first is how a latency + /// tool starts lying. + pub frames: Option, + /// How much to trust [`Self::frames`]. + pub confidence: Confidence, + /// Buttons that produced any divergence at all. + pub reacting_buttons: u32, + /// Buttons probed. + pub probed_buttons: u32, + /// Which observable produced the answer, when there is one. + pub observable: Option, + /// Per-button first-divergence frames, in `PROBE_BUTTONS` order (a plain + /// code span: that item is private and `rustdoc::private_intra_doc_links` is + /// denied), for a UI that wants to show the evidence rather than just the + /// conclusion. + pub per_button: Vec>, + /// Trials the measurement actually spent. + /// + /// Useful to a UI ("measured in 7 trials"), and load-bearing for the test + /// that proves the trial budget is binding rather than merely declared. + pub trials_used: u32, +} + +impl LatencyReport { + /// An inconclusive report, with the evidence that produced it. + fn inconclusive(per_button: Vec>, probed: u32, trials_used: u32) -> Self { + let reacting = + u32::try_from(per_button.iter().filter(|d| d.is_some()).count()).unwrap_or(u32::MAX); + Self { + frames: None, + confidence: Confidence::Inconclusive, + reacting_buttons: reacting, + probed_buttons: probed, + observable: None, + per_button, + trials_used, + } + } + + /// A run-ahead depth this measurement supports, clamped to `max_depth`. + /// + /// Returns `None` for an inconclusive report: **leave the user's setting + /// alone rather than guess**. Run-ahead depth is linear in the core's frame + /// cost (roughly 34% / 52% / 78% of the NTSC budget at depth 0 / 1 / 2), so + /// applying a fabricated depth spends real budget for nothing. + #[must_use] + pub fn suggested_run_ahead(&self, max_depth: u32) -> Option { + self.frames.map(|f| f.min(max_depth)) + } +} + +/// How a measurement is run. +#[derive(Clone, Copy, Debug)] +pub struct LatencyConfig { + /// Frames to simulate per trial. A game that has not reacted within this + /// many frames is reported as inconclusive rather than as zero-lag. + pub frames_per_trial: u32, + /// Largest lag treated as a real measurement. A divergence beyond this is + /// far more likely to be the game's own animation or a timer than a reaction + /// to input, so it is discarded rather than reported. + pub max_plausible_lag: u32, +} + +impl Default for LatencyConfig { + fn default() -> Self { + Self { + frames_per_trial: 20, + // Games buffer input by a frame or three. Ten is generous; past it, + // "the screen changed" almost certainly means something else moved. + max_plausible_lag: 10, + } + } +} + +/// Measure the game's internal input lag from the emulator's current state. +/// +/// `nes` is a scratch instance the probe replays into — it is rewound to the +/// anchor repeatedly and left wherever the last trial ended, so do not pass the +/// live emulator. +/// +/// Returns as soon as an observable yields agreeing answers, so the common case +/// costs one observable's worth of trials rather than all three. +pub fn measure(nes: &mut Nes, anchor: &Nes, cfg: LatencyConfig) -> LatencyReport { + // EXACTLY the trials this loop can run: one idle baseline plus one held + // trial per button, per observable. Not "plus headroom" — a ceiling with + // slack in it is not a ceiling, and `run_counted` below makes it binding, so + // a future edit that adds a trial fails closed here rather than silently + // spending more of the caller's time than the budget advertises. + let budget = Budget { + max_frames_per_trial: cfg.frames_per_trial, + max_trials: u32::try_from((PROBE_BUTTONS.len() + 1) * OBSERVABLE_ORDER.len()) + .unwrap_or(u32::MAX), + }; + let mut probe = Probe::anchor(anchor, budget); + let probed = u32::try_from(PROBE_BUTTONS.len()).unwrap_or(u32::MAX); + let mut last_evidence = vec![None; PROBE_BUTTONS.len()]; + + for observable in OBSERVABLE_ORDER { + // The idle baseline is the same for every button under one observable, + // so it is run once rather than per button. + // + // A `None` from `run_counted` means the budget is spent. Report what has + // been gathered rather than continuing unbudgeted: the honest answer to + // "I ran out of trials" is inconclusive, never a verdict from partial + // evidence. + let Some(idle) = probe.run(nes, cfg.frames_per_trial, observable, |_| { + (Buttons::empty(), Buttons::empty()) + }) else { + return LatencyReport::inconclusive(last_evidence, probed, probe.trials_used()); + }; + + let mut per_button = Vec::with_capacity(PROBE_BUTTONS.len()); + for button in PROBE_BUTTONS { + let Some(held) = probe.run(nes, cfg.frames_per_trial, observable, move |_| { + (button, Buttons::empty()) + }) else { + return LatencyReport::inconclusive(last_evidence, probed, probe.trials_used()); + }; + let d = Probe::first_divergence(&held, &idle).filter(|f| *f <= cfg.max_plausible_lag); + per_button.push(d); + } + + if let Some(report) = conclude(&per_button, probed, observable, probe.trials_used()) { + return report; + } + // Keep the RICHEST evidence, not the most recent. Raised in review on + // #384: blindly overwriting meant an inconclusive report could end up + // claiming `reacting_buttons: 0` because the last observable saw nothing, + // discarding a framebuffer round that had six reactions and merely failed + // to agree. The evidence is what a user is shown when the probe declines, + // so throwing away the informative half makes the decline useless. + let richer = per_button.iter().filter(|d| d.is_some()).count() + > last_evidence.iter().filter(|d| d.is_some()).count(); + if richer { + last_evidence = per_button; + } + } + + LatencyReport::inconclusive(last_evidence, probed, probe.trials_used()) +} + +/// Turn per-button divergences into a verdict, or `None` if this observable +/// cannot support one. +fn conclude( + per_button: &[Option], + probed: u32, + observable: Observable, + trials_used: u32, +) -> Option { + let reacting: Vec = per_button.iter().filter_map(|d| *d).collect(); + if reacting.is_empty() { + return None; + } + + // Pick the most common answer. Ties resolve to the SMALLEST frame, which is + // the conservative direction: under-reporting lag sets a lower run-ahead + // depth, which costs the user less frame budget than over-reporting. + let mut best = (0usize, u32::MAX); + for &candidate in &reacting { + let votes = reacting.iter().filter(|f| **f == candidate).count(); + if votes > best.0 || (votes == best.0 && candidate < best.1) { + best = (votes, candidate); + } + } + let (votes, frames) = best; + + let confidence = if votes == reacting.len() { + Confidence::Unanimous + } else if votes * 2 > reacting.len() { + Confidence::Majority + } else { + // A plurality is not agreement. Two buttons saying "1" and two saying + // "4" is a game doing something this probe does not understand, and the + // honest output is no number at all. + return None; + }; + + Some(LatencyReport { + frames: Some(frames), + confidence, + reacting_buttons: u32::try_from(reacting.len()).unwrap_or(u32::MAX), + probed_buttons: probed, + observable: Some(observable), + per_button: per_button.to_vec(), + trials_used, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// An NROM that never reads the controller — the honesty fixture. + fn spin_rom() -> Vec { + let mut prg = vec![0u8; 16 * 1024]; + prg[0] = 0x4C; // JMP $C000 + prg[1] = 0x00; + prg[2] = 0xC0; + wrap_nrom(prg) + } + + /// An NROM that latches the controller **once per frame in its NMI handler** + /// — the structure essentially every real NES game uses — and shifts the + /// eight bits into `$0300`. The main loop does nothing. + /// + /// Frame quantisation is the point. An earlier version of this fixture + /// polled `$4016` continuously from the main loop, and the measurement came + /// back split 3-3 between frames 0 and 5: sampling at end-of-frame caught + /// that loop at a different point in its eight-bit shift for buttons read + /// early (`A`, `B`) versus late (`Right`, `Left`, `Down`), so *which bit + /// position a button occupies* leaked into the answer. That is an artefact of + /// a ROM no real game resembles, not a property of the measurement — and the + /// split correctly produced "inconclusive", which is the algorithm behaving + /// as designed on nonsense input. + fn polling_rom() -> Vec { + let mut prg = vec![0u8; 16 * 1024]; + // $C000: enable NMI, forever. + // + // Re-asserted in the loop rather than written once, because the PPU + // IGNORES `$2000` writes for roughly its first 29,658 CPU cycles after + // reset. A single write at reset lands inside that window, is discarded, + // and NMI never fires — which presented here as every button reporting + // "no reaction", i.e. a fixture that silently tested nothing. + let reset: &[u8] = &[ + 0xA9, 0x80, // LDA #$80 + 0x8D, 0x00, 0x20, // STA $2000 (NMI on VBlank) + 0x4C, 0x00, 0xC0, // JMP $C000 + ]; + prg[..reset.len()].copy_from_slice(reset); + + // $C020: the NMI handler — strobe, read 8 bits into $0300, return. + let nmi: &[u8] = &[ + 0xA9, 0x01, // LDA #$01 + 0x8D, 0x16, 0x40, // STA $4016 (strobe on) + 0xA9, 0x00, // LDA #$00 + 0x8D, 0x16, 0x40, // STA $4016 (strobe off -> latch) + 0xA2, 0x08, // LDX #$08 + 0xA9, 0x00, // LDA #$00 + 0x8D, 0x00, 0x03, // STA $0300 + // read loop @ $C02F + 0xAD, 0x16, 0x40, // LDA $4016 + 0x4A, // LSR A (bit 0 -> carry) + 0x2E, 0x00, 0x03, // ROL $0300 + 0xCA, // DEX + 0xD0, 0xF6, // BNE -10 -> back to LDA $4016 + 0x40, // RTI + ]; + prg[0x20..0x20 + nmi.len()].copy_from_slice(nmi); + wrap_nrom_with_nmi(prg, 0xC020) + } + + fn wrap_nrom(prg: Vec) -> Vec { + wrap_nrom_with_nmi(prg, 0xC000) + } + + fn wrap_nrom_with_nmi(prg: Vec, nmi_addr: u16) -> Vec { + let mut prg = prg; + let len = prg.len(); + prg[len - 6] = (nmi_addr & 0xFF) as u8; // NMI + prg[len - 5] = (nmi_addr >> 8) as u8; + prg[len - 4] = 0x00; // RESET-> $C000 + prg[len - 3] = 0xC0; + prg[len - 2] = 0x00; // IRQ -> $C000 + prg[len - 1] = 0xC0; + let mut bytes = Vec::new(); + bytes.extend_from_slice(b"NES\x1A"); + bytes.push(1); + bytes.push(1); + bytes.push(0); + bytes.push(0); + bytes.extend_from_slice(&[0u8; 8]); + bytes.extend_from_slice(&prg); + bytes.extend_from_slice(&vec![0u8; 8 * 1024]); + bytes + } + + fn warmed(rom: &[u8], frames: u32) -> Nes { + let mut nes = Nes::from_rom(rom).expect("fixture parses"); + for _ in 0..frames { + nes.run_frame(); + } + nes + } + + /// THE honesty property. A ROM that never reads the controller must be + /// reported as **inconclusive**, never as zero-lag. + /// + /// `Some(0)` would be acted on: it sets run-ahead to 0 and tells the user the + /// game has no internal lag, which is a claim the probe has no evidence for. + /// A latency tool that cannot say "I don't know" is worse than none. + #[test] + fn a_game_that_ignores_input_is_inconclusive_not_zero() { + let rom = spin_rom(); + let anchor = warmed(&rom, 20); + let mut scratch = Nes::from_rom(&rom).expect("fixture parses"); + + let report = measure(&mut scratch, &anchor, LatencyConfig::default()); + assert_eq!( + report.frames, None, + "reported a lag it could not have measured" + ); + assert_eq!(report.confidence, Confidence::Inconclusive); + assert_eq!(report.reacting_buttons, 0); + assert_eq!( + report.probed_buttons, + u32::try_from(PROBE_BUTTONS.len()).unwrap_or(u32::MAX) + ); + assert_eq!( + report.suggested_run_ahead(3), + None, + "an inconclusive report must not move the user's run-ahead setting" + ); + } + + /// The complement: a ROM that DOES read the pad must produce a measurement. + /// Without this, the honesty test above would also pass on a probe that + /// always answers "inconclusive". + #[test] + fn a_polling_game_is_measured() { + let rom = polling_rom(); + let anchor = warmed(&rom, 20); + let mut scratch = Nes::from_rom(&rom).expect("fixture parses"); + + let report = measure(&mut scratch, &anchor, LatencyConfig::default()); + assert!( + report.frames.is_some(), + "a continuously-polling ROM produced no measurement: {report:?}" + ); + assert!(report.reacting_buttons > 0); + assert_ne!(report.confidence, Confidence::Inconclusive); + assert!(report.observable.is_some()); + } + + /// The trial budget must be **binding**, not decorative. + /// + /// `measure` sizes `Budget::max_trials` to exactly the trials it can run — + /// one idle baseline plus one held trial per button, per observable — and + /// spends them through `run_counted`. Before review this used `Probe::run`, + /// which does not consume trials, so the budget was computed, documented, + /// and never enforced: a stated contract that nothing checked. + /// + /// This pins the arithmetic, because that is what a future edit breaks. A + /// loop that adds a trial without widening the budget now fails closed at + /// the last observable rather than quietly running over. + #[test] + fn the_trial_budget_is_exactly_what_the_loop_spends() { + let expected = (PROBE_BUTTONS.len() + 1) * OBSERVABLE_ORDER.len(); + + // Drive a full three-observable run: a ROM that never reacts exhausts + // every observable, which is the worst case and the one the budget must + // accommodate exactly. + let rom = spin_rom(); + let anchor = warmed(&rom, 20); + let mut scratch = Nes::from_rom(&rom).expect("fixture parses"); + let report = measure(&mut scratch, &anchor, LatencyConfig::default()); + + // Inconclusive because nothing reacted — NOT because the budget ran out + // mid-run. If the budget were even one trial short, the run would bail + // early through the `run_counted` -> `None` path and still report + // inconclusive, so the count is what distinguishes the two. + assert_eq!(report.confidence, Confidence::Inconclusive); + + // THE assertion. `per_button.len()` cannot distinguish these two cases — + // a budget one trial short still bails on the LAST trial of the LAST + // observable and still returns the previous observable's full six-entry + // evidence, so the first version of this test passed under exactly the + // mutation it existed to catch. `trials_used` is the quantity that + // actually moves. + assert_eq!( + usize::try_from(report.trials_used).unwrap_or(usize::MAX), + expected, + "the run did not spend exactly its budget: either it bailed early \ + (budget too small) or the loop and the budget have drifted apart" + ); + assert_eq!( + expected, 21, + "the trial arithmetic changed; re-check Budget::max_trials in `measure`" + ); + } + + /// A divergence past `max_plausible_lag` is discarded: at that distance it is + /// far likelier to be the game's own animation than a reaction to the pad. + #[test] + fn an_implausibly_late_divergence_is_not_a_measurement() { + let per_button = [Some(40), Some(40), None, None, None, None]; + // `measure` filters before `conclude` sees them, so model that here. + let filtered: Vec> = per_button + .iter() + .map(|d| d.filter(|f| *f <= LatencyConfig::default().max_plausible_lag)) + .collect(); + assert!(conclude(&filtered, 6, Observable::Framebuffer, 0).is_none()); + } + + /// Unanimity, majority and a bare plurality must be told apart — a plurality + /// is not agreement, and must yield no number. + #[test] + fn agreement_is_graded_and_a_plurality_is_not_agreement() { + let unanimous = [Some(2), Some(2), None, None, None, None]; + let r = conclude(&unanimous, 6, Observable::Framebuffer, 0).expect("a verdict"); + assert_eq!(r.frames, Some(2)); + assert_eq!(r.confidence, Confidence::Unanimous); + + let majority = [Some(1), Some(1), Some(4), None, None, None]; + let r = conclude(&majority, 6, Observable::Framebuffer, 0).expect("a verdict"); + assert_eq!(r.frames, Some(1)); + assert_eq!(r.confidence, Confidence::Majority); + + // Two against two: no majority, so no number. + let split = [Some(1), Some(1), Some(4), Some(4), None, None]; + assert!( + conclude(&split, 6, Observable::Framebuffer, 0).is_none(), + "a plurality was reported as a measurement" + ); + } + + /// An even split is inconclusive at every size, not just at four buttons. + /// + /// One-vs-one is the case that looks most like "nearly agreed" and is the + /// easiest to talk oneself into reporting. It carries exactly as much + /// evidence as two-vs-two: none. The smallest-value tie-break inside + /// `conclude` exists only to make candidate selection deterministic — it can + /// never decide a *reported* number, because equal top votes and a majority + /// are mutually exclusive. + #[test] + fn an_even_split_is_inconclusive_at_any_size() { + let one_v_one = [Some(3), Some(1), None, None, None, None]; + assert!( + conclude(&one_v_one, 6, Observable::Framebuffer, 0).is_none(), + "a 1-1 split was reported as a measurement" + ); + let two_v_two = [Some(3), Some(3), Some(1), Some(1), None, None]; + assert!(conclude(&two_v_two, 6, Observable::Framebuffer, 0).is_none()); + } + + /// The suggested depth is clamped, so a measurement cannot ask for more + /// run-ahead than the caller is willing to afford. + #[test] + fn suggested_run_ahead_is_clamped() { + let r = LatencyReport { + frames: Some(7), + confidence: Confidence::Unanimous, + reacting_buttons: 6, + probed_buttons: 6, + observable: Some(Observable::Framebuffer), + per_button: vec![Some(7); 6], + trials_used: 0, + }; + assert_eq!(r.suggested_run_ahead(3), Some(3)); + assert_eq!(r.suggested_run_ahead(0), Some(0)); + } +} diff --git a/crates/rustynes-probe/src/lib.rs b/crates/rustynes-probe/src/lib.rs new file mode 100644 index 00000000..805de056 --- /dev/null +++ b/crates/rustynes-probe/src/lib.rs @@ -0,0 +1,578 @@ +//! Deterministic re-simulation probing: **anchor, replay under variation, +//! locate the first divergence**. +//! +//! # What this is for +//! +//! Several questions about a running game have the same shape: +//! +//! - *How many frames of input lag does this game have?* — replay from one +//! anchor with a button held and with it never pressed, and see which frame +//! first differs. That index **is** the game's internal lag. +//! - *What is this RAM byte for?* — replay with the byte perturbed and see what +//! changes. +//! - *Where do these two configurations disagree?* — replay both and find the +//! first frame that differs. +//! +//! Each is "take a snapshot, re-simulate under controlled variation, find the +//! first observable difference". This crate is that primitive, written once. +//! +//! # Why `RustyNES` can do this and most emulators cannot +//! +//! The answers are only meaningful if a replay from the same anchor with the +//! same inputs produces the same frames *every time*. That is `RustyNES`'s +//! determinism contract (`docs/testing-strategy.md`), a hard guarantee rather +//! than an aspiration — the same property save-states, TAS replay, and netplay +//! rollback already rely on. A probe result is therefore a property of the ROM, +//! not of the run, and [`Probe::run`] re-asserts it rather than assuming it. +//! +//! # What it deliberately does not do +//! +//! It does not own a [`Nes`]. The caller passes a scratch instance in, so a +//! frontend can reuse one across probes and keep the live emulator untouched. +//! It does not spawn threads, and it does not interpret results — deciding that +//! "frame 1 differed, therefore the lag is 1 frame" belongs to the tool, which +//! knows what it asked. +//! +//! # Example +//! +//! ```no_run +//! use rustynes_core::{Buttons, Nes}; +//! use rustynes_probe::{Budget, Observable, Probe}; +//! +//! # fn demo(live: &Nes, scratch: &mut Nes) { +//! let mut probe = Probe::anchor(live, Budget::default()); +//! +//! // Trial A: hold Right from the first frame. Trial B: never press anything. +//! // `run` is budgeted: `None` means the trial ceiling is spent. +//! let held = probe.run(scratch, 16, Observable::Framebuffer, |_| { +//! (Buttons::RIGHT, Buttons::empty()) +//! }).expect("within budget"); +//! let idle = probe.run(scratch, 16, Observable::Framebuffer, |_| { +//! (Buttons::empty(), Buttons::empty()) +//! }).expect("within budget"); +//! +//! match Probe::first_divergence(&held, &idle) { +//! Some(frame) => println!("reacted on frame {frame}"), +//! None => println!("no reaction inside the budget"), +//! } +//! # } +//! ``` + +pub mod latency; + +use rustynes_core::{Buttons, Nes, ROM_HASH_TAG_LEN}; + +/// Bounds on what a single probe may spend. +/// +/// A probe runs the emulator many times over, from a UI thread in the frontend's +/// case, so it needs a ceiling that does not depend on the game cooperating. A +/// game that never reacts must make the probe *stop and say so* rather than run +/// until something else notices. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Budget { + /// Hard cap on frames simulated per trial. Reached, [`Probe::run`] returns + /// the samples it has; the caller sees a short vector and must treat that as + /// "inconclusive", never as "no divergence". + pub max_frames_per_trial: u32, + /// Hard cap on trials a caller may run against one anchor. Enforced by + /// [`Probe::trials_remaining`]; the engine cannot enforce it alone because + /// it does not drive the loop. + pub max_trials: u32, +} + +impl Default for Budget { + fn default() -> Self { + Self { + // ~2 s of NTSC. Long enough for any input-lag question (games react + // within a handful of frames) and short enough that a probe cannot + // stall a UI frame budget for a noticeable time. + max_frames_per_trial: 120, + max_trials: 64, + } + } +} + +/// What a trial observes at the end of each frame. +/// +/// Every variant reduces its observation to one `u64` so trials compare cheaply +/// and a divergence search is a linear scan of two slices. Hashing loses the +/// ability to say *how* two frames differ — deliberately: this engine answers +/// *when*, and the caller that wants *what* already has Pixel Provenance and the +/// debugger for that. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Observable { + /// The RGBA framebuffer. The default question — "did the screen change?". + Framebuffer, + /// The palette-index framebuffer. Cheaper than [`Self::Framebuffer`] and + /// immune to a palette or filter change that leaves the rendered indices + /// identical. + IndexFramebuffer, + /// The 2 KiB work RAM. Catches a game that reacted internally without + /// drawing anything yet — a menu highlight committed to a variable one frame + /// before it is rendered, for instance. + Wram, + /// Coarse energy of the audio produced this frame. The fallback for a + /// reaction that is audible before it is visible; quantised, because exact + /// float equality across a resample would be noise, not signal. + AudioEnergy, +} + +/// An anchor plus the budget probes taken from it must respect. +/// +/// Cloning is cheap relative to re-deriving the state, but the snapshot is a +/// real allocation — hold one anchor and run many trials against it rather than +/// re-anchoring per trial. +#[derive(Clone, Debug)] +pub struct Probe { + snapshot: Vec, + rom_tag: [u8; ROM_HASH_TAG_LEN], + budget: Budget, + trials_used: u32, +} + +impl Probe { + /// Capture the anchor: the emulator's full state plus the identity of the + /// ROM it is running. + /// + /// The ROM tag is recorded so [`Self::run`] can refuse to replay into an + /// instance running a different game — restoring a snapshot across ROMs + /// would produce confident nonsense rather than an error. + #[must_use] + pub fn anchor(nes: &Nes, budget: Budget) -> Self { + Self { + snapshot: nes.snapshot(), + rom_tag: nes.rom_hash_tag(), + budget, + trials_used: 0, + } + } + + /// Trials still allowed against this anchor under its [`Budget`]. + #[must_use] + pub const fn trials_remaining(&self) -> u32 { + self.budget.max_trials.saturating_sub(self.trials_used) + } + + /// Trials spent through [`Self::run`] so far. + /// + /// Reported so a caller can say how much work a measurement cost, and so a + /// test can assert that a budget is actually *binding* rather than merely + /// declared — the two are easy to confuse, and a test that cannot tell them + /// apart is decoration. + #[must_use] + pub const fn trials_used(&self) -> u32 { + self.trials_used + } + + /// The budget this anchor was taken under. + #[must_use] + pub const fn budget(&self) -> Budget { + self.budget + } + + /// Restore the anchor into `nes` and run `frames` frames, sampling + /// `observable` after each one. + /// + /// `input` is called once per frame with the zero-based frame index and + /// returns the buttons for controller 1 and 2. It is the only thing that + /// varies between trials; everything else is re-derived from the anchor, + /// which is what makes two trials comparable. + /// + /// Returns one sample per frame actually run. A vector shorter than `frames` + /// means the budget stopped it — treat that as **inconclusive**, not as + /// evidence of no divergence. + /// + /// # Panics + /// + /// Panics if `nes` is running a different ROM than the anchor was taken + /// from, or if the anchor fails to restore. Both are caller errors that + /// would otherwise yield a plausible, wrong answer, and this engine exists + /// to produce answers people will act on. + fn run_uncounted( + &self, + nes: &mut Nes, + frames: u32, + observable: Observable, + mut input: F, + ) -> Vec + where + F: FnMut(u32) -> (Buttons, Buttons), + { + assert_eq!( + nes.rom_hash_tag(), + self.rom_tag, + "probe anchor belongs to a different ROM than the emulator it was \ + replayed into; restoring across ROMs yields confident nonsense" + ); + nes.restore(&self.snapshot) + .expect("probe anchor round-trips: it came from Nes::snapshot"); + + let n = frames.min(self.budget.max_frames_per_trial); + let mut samples = Vec::with_capacity(n as usize); + // Generously sized so one frame always fits: an NTSC frame at 192 kHz is + // ~3,200 samples. Allocated once per trial, not per frame. + let mut audio = vec![0.0f32; 8192]; + for f in 0..n { + let (p1, p2) = input(f); + nes.set_buttons(0, p1); + nes.set_buttons(1, p2); + nes.run_frame(); + // Drain EVERY frame, whatever the observable. `Nes::restore` does + // drop the blip's pending queue (verified by + // `tests/restore_audio_pin.rs`), so trials cannot contaminate each + // other through it — but draining only for `AudioEnergy` made that + // safety depend on restore's audio semantics staying as they are, + // and let a 120-frame framebuffer trial pile up ~88k samples for + // nothing. Raised in review on #384. + audio.clear(); + + samples.push(sample(nes, observable, &audio)); + } + samples + } + + /// Restore the anchor and run one **budgeted** trial. + /// + /// Returns `None` once [`Self::trials_remaining`] reaches zero, so a search + /// loop terminates on the budget rather than on the caller remembering to + /// check. + /// + /// This is the only public way to run a trial, deliberately. An earlier + /// version also exposed an uncounted `run`, which made the unbudgeted choice + /// the convenient default — and `latency::measure` duly used it, so the + /// budget was computed, documented and never enforced. Raised in review on + /// #384; the uncounted path is now private. + pub fn run( + &mut self, + nes: &mut Nes, + frames: u32, + observable: Observable, + input: F, + ) -> Option> + where + F: FnMut(u32) -> (Buttons, Buttons), + { + if self.trials_remaining() == 0 { + return None; + } + self.trials_used += 1; + Some(self.run_uncounted(nes, frames, observable, input)) + } + + /// The first frame index at which two trials differ, or `None` if they agree + /// over their common length. + /// + /// Comparing only the common prefix is deliberate: a shorter trial means its + /// budget ran out, and "one ran longer" is not a divergence. + #[must_use] + pub fn first_divergence(a: &[u64], b: &[u64]) -> Option { + a.iter() + .zip(b.iter()) + .position(|(x, y)| x != y) + .and_then(|i| u32::try_from(i).ok()) + } + + /// Whether two trials agree over their whole common prefix, and that prefix + /// is non-empty. + /// + /// Distinct from `first_divergence(..).is_none()`, which is also true for + /// two empty trials — a case that means "nothing ran", not "they agree". + #[must_use] + pub fn agree(a: &[u64], b: &[u64]) -> bool { + let common = a.len().min(b.len()); + common > 0 && Self::first_divergence(a, b).is_none() + } +} + +/// Reduce the emulator's current state to one comparable value. +fn sample(nes: &Nes, observable: Observable, audio: &[f32]) -> u64 { + match observable { + Observable::Framebuffer => fnv1a64(nes.framebuffer()), + Observable::IndexFramebuffer => { + // The index framebuffer is `u16` per pixel; fold it through the same + // byte hash so every variant shares one mixing function. + let mut h = FNV_OFFSET; + for px in nes.index_framebuffer() { + h = fnv1a64_step(h, px.to_le_bytes().as_slice()); + } + h + } + Observable::Wram => fnv1a64(nes.wram()), + Observable::AudioEnergy => { + // Quantised sum of |amplitude|. Exact float equality across a + // resampled stream would compare noise; this asks the coarser + // question the fallback is for — "did this frame make a + // meaningfully different sound?". + let energy: f32 = audio.iter().map(|s| s.abs()).sum(); + #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] + let q = (energy * 64.0) as u64; + q + } + } +} + +const FNV_OFFSET: u64 = 0xcbf2_9ce4_8422_2325; +const FNV_PRIME: u64 = 0x0000_0100_0000_01b3; + +fn fnv1a64_step(mut hash: u64, bytes: &[u8]) -> u64 { + for &b in bytes { + hash ^= u64::from(b); + hash = hash.wrapping_mul(FNV_PRIME); + } + hash +} + +fn fnv1a64(bytes: &[u8]) -> u64 { + fnv1a64_step(FNV_OFFSET, bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A minimal NROM that spins forever, mirroring the core's `synth_nrom` + /// fixture. Enough to exercise the engine's contract without a real game. + fn synth_nrom() -> Vec { + let mut bytes = Vec::new(); + bytes.extend_from_slice(b"NES\x1A"); + bytes.push(1); // 16 KiB PRG + bytes.push(1); // 8 KiB CHR + bytes.push(0); + bytes.push(0); + bytes.extend_from_slice(&[0u8; 8]); + let mut prg = vec![0u8; 16 * 1024]; + prg[0] = 0x4C; // JMP $C000 + prg[1] = 0x00; + prg[2] = 0xC0; + let len = prg.len(); + prg[len - 6] = 0x00; // NMI + prg[len - 5] = 0xC0; + prg[len - 4] = 0x00; // RESET + prg[len - 3] = 0xC0; + prg[len - 2] = 0x00; // IRQ + prg[len - 1] = 0xC0; + bytes.extend_from_slice(&prg); + bytes.extend_from_slice(&vec![0u8; 8 * 1024]); + bytes + } + + fn nes() -> Nes { + Nes::from_rom(&synth_nrom()).expect("fixture parses") + } + + fn idle(_: u32) -> (Buttons, Buttons) { + (Buttons::empty(), Buttons::empty()) + } + + /// THE contract the whole engine rests on: two trials with identical inputs + /// produce identical samples, frame for frame. If this fails, nothing else + /// here means anything. + #[test] + fn identical_trials_are_identical() { + let mut n = nes(); + for _ in 0..10 { + n.run_frame(); + } + let mut probe = Probe::anchor(&n, Budget::default()); + + let a = probe + .run(&mut n, 20, Observable::Framebuffer, idle) + .expect("within budget"); + let b = probe + .run(&mut n, 20, Observable::Framebuffer, idle) + .expect("within budget"); + assert_eq!(a, b, "the determinism contract failed under replay"); + assert_eq!(Probe::first_divergence(&a, &b), None); + assert!(Probe::agree(&a, &b)); + } + + /// The anchor must actually restore: a trial run after another trial has + /// advanced the emulator has to produce the same samples as the first. + /// Without the restore, trial 2 would start 20 frames later and differ. + #[test] + fn each_trial_restarts_from_the_anchor() { + let mut n = nes(); + let mut probe = Probe::anchor(&n, Budget::default()); + let first = probe + .run(&mut n, 8, Observable::Wram, idle) + .expect("within budget"); + // Advance well past the anchor between trials. + for _ in 0..50 { + n.run_frame(); + } + let second = probe + .run(&mut n, 8, Observable::Wram, idle) + .expect("within budget"); + assert_eq!(first, second, "the anchor did not restore between trials"); + } + + /// All four observables must be usable and self-consistent. + #[test] + fn every_observable_is_deterministic() { + let mut n = nes(); + let mut probe = Probe::anchor(&n, Budget::default()); + for obs in [ + Observable::Framebuffer, + Observable::IndexFramebuffer, + Observable::Wram, + Observable::AudioEnergy, + ] { + let a = probe.run(&mut n, 6, obs, idle).expect("within budget"); + let b = probe.run(&mut n, 6, obs, idle).expect("within budget"); + assert_eq!(a, b, "{obs:?} was not deterministic under replay"); + assert_eq!(a.len(), 6, "{obs:?} produced the wrong sample count"); + } + } + + /// The budget must cap a trial, and the short result must be distinguishable + /// from a completed one — the caller has to be able to tell "inconclusive" + /// from "no divergence". + #[test] + fn budget_caps_the_trial_length() { + let mut n = nes(); + let budget = Budget { + max_frames_per_trial: 3, + ..Budget::default() + }; + let mut probe = Probe::anchor(&n, budget); + let samples = probe + .run(&mut n, 100, Observable::Framebuffer, idle) + .expect("within budget"); + assert_eq!(samples.len(), 3, "budget did not cap the trial"); + } + + /// `run_counted` must stop handing out trials once the budget is spent, + /// so a search loop terminates on the budget rather than on discipline. + #[test] + fn trial_budget_is_enforced_and_then_refuses() { + let mut n = nes(); + let budget = Budget { + max_trials: 2, + ..Budget::default() + }; + let mut probe = Probe::anchor(&n, budget); + assert_eq!(probe.trials_remaining(), 2); + assert!(probe.run(&mut n, 2, Observable::Wram, idle).is_some()); + assert!(probe.run(&mut n, 2, Observable::Wram, idle).is_some()); + assert_eq!(probe.trials_remaining(), 0); + assert!( + probe.run(&mut n, 2, Observable::Wram, idle).is_none(), + "the engine handed out a trial past its budget" + ); + } + + /// A divergence must be located at the exact frame it first appears, not + /// merely detected. The fixture is synthetic so the answer is known: two + /// sample streams that agree for three entries and then differ. + #[test] + fn first_divergence_reports_the_exact_frame() { + let a = [1u64, 2, 3, 4, 5]; + let b = [1u64, 2, 3, 9, 5]; + assert_eq!(Probe::first_divergence(&a, &b), Some(3)); + assert!(!Probe::agree(&a, &b)); + } + + /// Two trials of different length agree if their common prefix does — a + /// budget-truncated trial is not a divergence. + #[test] + fn a_shorter_trial_is_not_a_divergence() { + let long = [1u64, 2, 3, 4]; + let short = [1u64, 2]; + assert_eq!(Probe::first_divergence(&long, &short), None); + assert!(Probe::agree(&long, &short)); + } + + /// Two empty trials must NOT read as agreement: nothing ran, so there is + /// nothing to agree about. This is the distinction that stops a probe + /// reporting "no reaction" when it in fact never simulated anything. + #[test] + fn empty_trials_do_not_count_as_agreement() { + assert_eq!(Probe::first_divergence(&[], &[]), None); + assert!( + !Probe::agree(&[], &[]), + "empty trials must not report agreement" + ); + } + + /// Input must actually reach the emulator: a trial that presses buttons and + /// one that does not must differ in WRAM on a ROM that stores the pad. + /// + /// The spin-loop fixture never reads the controller, so this asserts the + /// weaker but still meaningful property that the input closure is invoked + /// once per frame with ascending indices — without which two "different" + /// trials would be silently identical and every probe would answer "no + /// reaction". + #[test] + fn the_input_closure_is_called_once_per_frame_in_order() { + let mut n = nes(); + let mut probe = Probe::anchor(&n, Budget::default()); + let mut seen = Vec::new(); + let _ = probe.run(&mut n, 5, Observable::Wram, |f| { + seen.push(f); + (Buttons::empty(), Buttons::empty()) + }); + assert_eq!(seen, vec![0, 1, 2, 3, 4]); + } + + /// A real state difference must propagate through the replay into a detected + /// divergence. + /// + /// The tests above prove the comparator and the replay path separately; this + /// closes the loop. Two anchors differing only in one work-RAM byte must + /// produce sample streams that diverge at frame 0 under the `Wram` + /// observable — which is the mechanism every consumer of this crate relies + /// on, and the one a comparator test alone cannot demonstrate. + /// + /// The fixture ROM never reads the controller, so perturbing memory is the + /// available way to introduce a genuine difference; the Latency Oracle will + /// introduce its difference through input instead, on ROMs that do read it. + #[test] + fn a_real_state_difference_is_detected_end_to_end() { + let mut n = nes(); + for _ in 0..10 { + n.run_frame(); + } + + n.poke_ram(0x0200, 0x00); + let mut probe_a = Probe::anchor(&n, Budget::default()); + let a = probe_a + .run(&mut n, 4, Observable::Wram, idle) + .expect("within budget"); + + // Restore to the same point, change ONE byte, and re-anchor. + probe_a + .run(&mut n, 0, Observable::Wram, idle) + .expect("within budget"); // restore only + n.poke_ram(0x0200, 0xA5); + let mut probe_b = Probe::anchor(&n, Budget::default()); + let b = probe_b + .run(&mut n, 4, Observable::Wram, idle) + .expect("within budget"); + + assert_eq!( + Probe::first_divergence(&a, &b), + Some(0), + "a one-byte work-RAM difference did not reach the observable" + ); + assert!(!Probe::agree(&a, &b)); + } + + /// Replaying an anchor into an emulator running a different ROM must fail + /// loudly. A snapshot restored across ROMs would produce a confident, wrong + /// answer, which is worse than no answer for a tool people act on. + #[test] + #[should_panic(expected = "different ROM")] + fn replaying_into_a_different_rom_panics() { + let n = nes(); + let mut probe = Probe::anchor(&n, Budget::default()); + + // A ROM with different PRG contents => a different hash tag. + let mut other_bytes = synth_nrom(); + let prg_start = 16; + other_bytes[prg_start + 8] = 0xEA; // NOP somewhere harmless + let mut other = Nes::from_rom(&other_bytes).expect("fixture parses"); + let _ = probe + .run(&mut other, 1, Observable::Wram, idle) + .expect("within budget"); + } +} diff --git a/crates/rustynes-probe/tests/restore_audio_pin.rs b/crates/rustynes-probe/tests/restore_audio_pin.rs new file mode 100644 index 00000000..0f5d3b81 --- /dev/null +++ b/crates/rustynes-probe/tests/restore_audio_pin.rs @@ -0,0 +1,62 @@ +//! Pins an assumption the probe engine depends on, raised in review on #384. +//! +//! A reviewer flagged possible **cross-trial audio contamination**: `sample` +//! drained audio only for the `AudioEnergy` observable, so if `Nes::restore` kept +//! the pending presentation queue — as save-states in many emulators do — the +//! undrained framebuffer trials would pile up hundreds of frames, and the first +//! `AudioEnergy` trial would drain them all on frame 0 and diverge falsely +//! against every later trial. +//! +//! It does not happen here: `restore` replaces the blip wholesale and drops the +//! pending queue, which `rustynes-apu`'s snapshot module documents deliberately. +//! This test is the evidence for that claim rather than the claim itself — +//! measured, not read off a comment. The engine now also drains unconditionally, +//! so the property is belt-and-braces, but if `restore` ever starts preserving +//! audio this test says so directly instead of the failure surfacing as a +//! mysterious "every game has zero input lag". + +#[test] +fn restore_drops_pending_audio_so_trials_cannot_contaminate_each_other() { + use rustynes_core::Nes; + let mut prg = vec![0u8; 16 * 1024]; + prg[0] = 0x4C; + prg[1] = 0x00; + prg[2] = 0xC0; + let len = prg.len(); + for (i, b) in [0x00u8, 0xC0, 0x00, 0xC0, 0x00, 0xC0].iter().enumerate() { + prg[len - 6 + i] = *b; + } + let mut bytes = Vec::new(); + bytes.extend_from_slice(b"NES\x1A"); + bytes.extend_from_slice(&[1, 1, 0, 0]); + bytes.extend_from_slice(&[0u8; 8]); + bytes.extend_from_slice(&prg); + bytes.extend_from_slice(&vec![0u8; 8 * 1024]); + + let mut nes = Nes::from_rom(&bytes).unwrap(); + let snap = nes.snapshot(); + // Accumulate WITHOUT draining, exactly as a Framebuffer-observable trial does. + for _ in 0..30 { + nes.run_frame(); + } + let accumulated = nes.drain_audio().len(); + assert!( + accumulated > 10_000, + "premise: 30 undrained frames accumulate (got {accumulated})" + ); + + // Re-accumulate, then restore and drain: if restore keeps the queue, this + // matches `accumulated`; if it drops it, this is ~one frame's worth. + for _ in 0..30 { + nes.run_frame(); + } + nes.restore(&snap).unwrap(); + nes.run_frame(); + let after_restore = nes.drain_audio().len(); + println!("accumulated={accumulated} after_restore={after_restore}"); + assert!( + after_restore < accumulated / 10, + "restore did NOT drop pending audio: {after_restore} vs {accumulated} — \ + cross-trial audio contamination is real" + ); +} diff --git a/docs/performance.md b/docs/performance.md index 3ca57c75..e1383476 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -3597,6 +3597,58 @@ Prediction recorded and wrong, for the record: this campaign expected the shorter, rendering-heavy `flowing_palette` frame to show the *larger* relative win, since the APU should be a bigger fraction of it. It showed essentially none. +### v2.3.6 D3 — caching the C1 fast-path gain predicate (decision: REJECTED, reverted) + +**The change.** v2.3.5's C1 fast path tests +`mask == CHANNEL_MASK_ALL && channel_gain == CHANNEL_GAIN_UNITY` once per CPU +cycle. The second half is a 6-wide `f32` array comparison evaluated **1.789 +million times a second** to answer a question that can only change when a user +drags a mixer slider. D3 cached it in a `gain_is_unity: bool`, reducing the +per-cycle test to a `u8` compare plus a `bool` load. Byte-identical by +construction: same predicate over the same array, same branch taken. + +**Adjudicated with `scripts/perf/ab_check.sh --base --bench nes_run_frame_nestest`, +two independent runs, quiet host.** + +| workload | run 1 | run 2 | +|---|---:|---:| +| `nes_run_frame_nestest` | **+1.72%** (p = 0.00) | **−0.91%** (p = 0.01) | +| `nes_run_frame_nestest_fast` (shipped default) | −0.45% (p = 0.31) | −0.70% (p = 0.05) | +| order-bias control, `_fast` | **−2.53% (p = 0.00) — FAILED** | clean | + +**Rejected**, on three independent grounds, any one of which suffices: + +1. **The sign flips between independent runs** on `nes_run_frame_nestest`: + +1.72% then −0.91%, both nominally significant. Mixed signs are a rejection, + never something to average — and mixed signs *across runs* mean the effect is + not reproducible at all. +2. **Run 1's order-bias control failed** (`_fast` drifted −2.53% from position in + the run alone), so run 1's candidate numbers carry at least that much + systematic error and its small result is not interpretable. +3. **The shipped `_fast` variant never moved significantly** (p = 0.31, then + p = 0.05). `fast_dotloop` has been default-on since v2.2.3, so a change that + does not move `_fast` moves nothing a user runs. + +This is the shape v2.3.1 G2 recorded: a textbook single-run result that +evaporates on re-run. A third run was not pursued — even the most favourable +reading is under 1%, and the change is not free: the cache is derived state that +must be kept in sync with `channel_gain`, which cost a dedicated desync test and +an entry in `snapshot_schema_audit`. Two standing obligations for an effect +indistinguishable from zero is a bad trade, so the code was reverted rather than +kept as a simplification. + +**What this does not say.** "Not measurable here" is not "no difference". The +instrument's resolution on this host is roughly ±1-2%, so a sub-1% effect is +invisible to it. The honest claim is that D3 has no *demonstrated* benefit, and +the project does not carry core state on undemonstrated benefit. + +**Still open from the v2.3.4 Workstream C list**, unmeasured: D1 (gating the DMC +end-of-cycle pair, ~23% of per-cycle cost and never optimized — the largest +remaining target, and the hardest byte-identity proof), D2 (`FrameCounter::tick` +as a countdown rather than a 6-arm match per cycle), D4 (`Pulse::muted()` +caching), D5 (hoisting `add_sample`'s finite-check), D6 (gating the four +unconditional `length.reload()` calls). + ## Things explicitly *not* in scope for v1.0 - **JIT recompilation** of CPU code. NES games are small enough that interpretation suffices; JIT complicates everything. (Higan/ares don't JIT either.)