diff --git a/Cargo.lock b/Cargo.lock index cc2ba0b9..e0856123 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4381,6 +4381,7 @@ dependencies = [ "rustynes-gfx-shaders", "rustynes-hdpack", "rustynes-netplay", + "rustynes-probe", "rustynes-ra", "rustynes-script", "serde", diff --git a/crates/rustynes-frontend/Cargo.toml b/crates/rustynes-frontend/Cargo.toml index 8fc2f1cd..fa830bb8 100644 --- a/crates/rustynes-frontend/Cargo.toml +++ b/crates/rustynes-frontend/Cargo.toml @@ -203,6 +203,10 @@ workspace = true [dependencies] rustynes-gamedb = { path = "../rustynes-gamedb" } +# v2.3.6 — the deterministic re-simulation probe engine, consumed by the Latency +# Oracle panel. Core-only dependency, so it adds nothing to the frontend's build +# graph beyond what `rustynes-core` already pulls in. +rustynes-probe = { path = "../rustynes-probe" } # v1.1.0 beta.2 (Workstream C) — `debug-hooks` enables the core's run-loop # breakpoint/trace/event hooks for the debugger. The hooks are determinism- # neutral no-ops until armed, so the headless test/bench builds (which depend on diff --git a/crates/rustynes-frontend/src/app.rs b/crates/rustynes-frontend/src/app.rs index bc01202c..257b4e3f 100644 --- a/crates/rustynes-frontend/src/app.rs +++ b/crates/rustynes-frontend/src/app.rs @@ -1393,6 +1393,10 @@ impl App { // v1.6.0 "Studio" A2 — a TAStudio session anchors on the closed ROM; end it. if let Some(d) = self.debugger.as_mut() { d.clear_tas_editor(); + // v2.3.6 — a Latency Oracle report is bound to the ROM it was + // measured on. Left standing it describes a cartridge that is no + // longer loaded, with its Apply button still live. (PR #385 review.) + d.clear_latency_report(); } // Stop the dedicated emulation thread from producing frames. #[cfg(all(not(target_arch = "wasm32"), feature = "emu-thread"))] @@ -1700,6 +1704,10 @@ impl App { // replay inputs/branches against a different `Nes`. if let Some(d) = self.debugger.as_mut() { d.clear_tas_editor(); + // v2.3.6 — a Latency Oracle report is bound to the ROM it was + // measured on. Left standing it describes a cartridge that is no + // longer loaded, with its Apply button still live. (PR #385 review.) + d.clear_latency_report(); } // v2.8.0 Phase 5 increment 3 — a reload keeps the pacing regime but // may change the region (NTSC<->PAL frame duration); refresh the @@ -8579,6 +8587,10 @@ impl App { // session (it anchored on the previous `Nes`). if let Some(d) = self.debugger.as_mut() { d.clear_tas_editor(); + // v2.3.6 — a Latency Oracle report is bound to the ROM it was + // measured on. Left standing it describes a cartridge that is no + // longer loaded, with its Apply button still live. (PR #385 review.) + d.clear_latency_report(); } // v2.8.0 Phase 5 increment 3 — let the (idle) emulation thread start // producing now that the core holds a ROM. Set AFTER `nes` is in diff --git a/crates/rustynes-frontend/src/debugger/latency_panel.rs b/crates/rustynes-frontend/src/debugger/latency_panel.rs new file mode 100644 index 00000000..ff2255b9 --- /dev/null +++ b/crates/rustynes-frontend/src/debugger/latency_panel.rs @@ -0,0 +1,370 @@ +//! Latency Oracle panel (v2.3.6) — measure the loaded game's **own** input lag +//! and recommend a run-ahead depth. +//! +//! Every emulator makes finding this number a manual ritual: hold a direction, +//! frame-advance until the sprite moves, subtract one. `RetroArch` documents +//! exactly that procedure; this project's own settings panel says "1 fits most +//! games". [`rustynes_probe::latency`] measures it instead, by replaying one +//! anchor with a button held and without it and finding the first frame that +//! differs. +//! +//! # Two deliberate choices +//! +//! **It recommends; it does not apply.** A measured depth is never written to +//! the config on its own. Run-ahead is linear in the core's frame cost (roughly +//! 34% / 52% / 78% of the NTSC budget at depth 0 / 1 / 2), so silently raising it +//! can push a marginal host into dropped frames for a change the user never +//! asked for. The number appears with an explicit **Apply** button next to it. +//! +//! **It reports its own uncertainty.** The measurement returns `None` rather +//! than a guess whenever the probe buttons disagree or nothing reacts, and this +//! panel shows that as "inconclusive" with the per-button evidence — never as +//! "0 frames". A latency tool that cannot say "I don't know" is worse than none, +//! because its wrong answers are indistinguishable from its right ones. +//! +//! The measurement runs **synchronously under the emu lock** on the button +//! press, like `BasicBot`'s search, and restores the live timeline before +//! returning. It drives several hundred frames, so the UI pauses briefly; the +//! button says so rather than pretending the work is free. + +use rustynes_core::Nes; +use rustynes_probe::latency::{self, Confidence, LatencyConfig, LatencyReport}; + +use crate::icons::{glyph, label as ic}; + +/// Highest depth this panel will ever recommend. +/// +/// A game measuring higher than this is reported honestly and the recommendation +/// clamped, rather than the measurement being silently discarded. +/// +/// Re-exported from [`crate::emu`] rather than declared as its own `3`: that +/// constant exists precisely because `effective_run_ahead`'s cap and the +/// throttle's cap were once separate literals that drifted apart (PR #358), and +/// a third copy here would reopen the same seam. (PR #385 review.) +use crate::emu::MAX_RUN_AHEAD_DEPTH as MAX_DEPTH; + +/// Persistent panel state. +#[derive(Default)] +pub struct LatencyPanel { + /// The most recent measurement, if one has been run for this session. + report: Option, + /// Milliseconds per frame **of the console the report was measured on**, + /// captured at measurement time from `Nes::frame_duration`. + /// + /// Recorded here rather than read at render time because it is a property of + /// the measurement, not of the current session: unloading the ROM, or + /// loading a PAL one after measuring an NTSC one, must not silently restate + /// an old result in the new region's units. + frame_ms: f64, + /// "Measure" was clicked this frame; [`show`] runs it after the render, so + /// `nes` is never captured by the viewport callback. + measure_requested: bool, + /// A depth the user asked to apply; drained by the caller into the config. + pending_apply: Option, + /// Status / error line. + status: String, +} + +impl LatencyPanel { + /// Discard everything bound to the previous ROM. + /// + /// A latency report describes one game. Left standing across a ROM change it + /// becomes a confident statement about a cartridge it was never measured on + /// — and worse, its **Apply** button stays live, so a depth measured for game + /// A is one click from being applied while game B is running. Clearing + /// `pending_apply` matters as much as clearing `report`. + /// + /// Called from the same ROM-transition points that end a `TAStudio` session, + /// for the same reason: that state anchored on an emulator instance which no + /// longer exists. (PR #385 review.) + pub fn clear(&mut self) { + *self = Self::default(); + } + + /// Take a depth the user pressed **Apply** for, if any. + /// + /// Returned rather than written here because the panel has no business + /// touching the config: the caller owns that, and routing it through a + /// drained field keeps "measured" and "applied" as two separate, auditable + /// steps. + pub const fn take_pending_apply(&mut self) -> Option { + self.pending_apply.take() + } +} + +/// Draw the Latency Oracle window. `nes` is `Some` only when a ROM is loaded +/// under the held lock; measuring is disabled otherwise. +pub fn show( + ctx: &egui::Context, + detached: &mut std::collections::HashSet<&'static str>, + open: &mut bool, + state: &mut LatencyPanel, + nes: Option<&mut Nes>, + current_run_ahead: u32, +) { + let can_measure = nes.is_some(); + super::detachable_window( + ctx, + detached, + "latency_oracle", + "Latency Oracle", + super::WindowCfg { + default_width: Some(360.0), + ..Default::default() + }, + open, + |ui| body(ui, state, can_measure, current_run_ahead), + ); + // Measure AFTER the render — `nes` is free here, not captured by any closure. + if std::mem::take(&mut state.measure_requested) { + run_measurement(state, nes); + } +} + +/// The panel body, shared by the docked window and the detached OS viewport. +fn body(ui: &mut egui::Ui, state: &mut LatencyPanel, can_measure: bool, current: u32) { + ui.label("Measures how many frames this game waits before acting on input."); + ui.weak( + "Replays the current moment twice — once with a button held, once without — \ + and finds the first frame that differs. Briefly pauses the emulator.", + ); + ui.separator(); + + // `icons::label` with a `glyph::` constant, NOT a literal codepoint. The + // button read `"\u{23F1} Measure now"` — U+23F1 STOPWATCH, an emoji, which + // the project style rule forbids in code outright. `glyph::GAUGE` is a + // private-use-area codepoint from the bundled icon font, and it is the same + // glyph the Tools menu entry uses, so the button now matches the item that + // opens it. (PR #385 review.) + if ui + .add_enabled( + can_measure, + egui::Button::new(ic(glyph::GAUGE, "Measure now")), + ) + .clicked() + { + state.measure_requested = true; + } + if !can_measure { + ui.weak("Load a ROM to measure."); + } + ui.weak(format!("Run-ahead is currently {current}.")); + + if let Some(report) = &state.report { + ui.separator(); + report_body( + ui, + report, + current, + state.frame_ms, + &mut state.pending_apply, + ); + } + + if !state.status.is_empty() { + ui.separator(); + ui.weak(&state.status); + } +} + +/// Render a finished measurement: the verdict, the recommendation, the evidence. +fn report_body( + ui: &mut egui::Ui, + report: &LatencyReport, + current: u32, + frame_ms: f64, + pending_apply: &mut Option, +) { + if let Some(frames) = report.frames { + let plural = if frames == 1 { "frame" } else { "frames" }; + ui.label(format!("Internal lag: {frames} {plural}")); + // The felt latency, which is what the user actually experiences. + // + // Derived from the console's own frame duration, NOT a hardcoded NTSC + // 16.639. A literal here would overstate PAL and Dendy lag by 20.2% — + // the identical defect v2.3.5 fixed in the libretro wrapper, where a + // hardcoded 60.0988 fps had lost all connection to the constant it was + // copied from and ran every PAL cartridge fast. (PR #385 review.) + let ms = f64::from(frames) * frame_ms; + ui.weak(format!("about {ms:.0} ms of the game's own delay")); + + let confidence = match report.confidence { + Confidence::Unanimous => "every reacting button agreed", + Confidence::Majority => "a majority agreed — treat as approximate", + Confidence::Inconclusive => "inconclusive", + }; + ui.weak(format!( + "{confidence} ({}/{} buttons reacted, {} trials)", + report.reacting_buttons, report.probed_buttons, report.trials_used + )); + + if let Some(depth) = report.suggested_run_ahead(MAX_DEPTH) { + ui.separator(); + ui.horizontal(|ui| { + if depth == current { + ui.label(format!("Run-ahead {depth} already matches.")); + } else { + ui.label(format!("Recommended run-ahead: {depth}")); + // Explicit, never automatic — see the module docs. + if ui.button(format!("Apply {depth}")).clicked() { + *pending_apply = Some(depth); + } + } + }); + if frames > MAX_DEPTH { + ui.weak(format!( + "Measured {frames}, but run-ahead is capped at {MAX_DEPTH}; \ + each extra frame costs roughly a whole frame of emulation." + )); + } + } + } else { + ui.label("Inconclusive — no run-ahead change recommended."); + ui.weak(match report.reacting_buttons { + 0 => "Nothing reacted to any button inside the probe window. Try \ + measuring during gameplay rather than on a title screen or \ + cut-scene." + .to_owned(), + n => format!( + "{n} of {} buttons reacted, but they disagreed on when — so there \ + is no single lag to report.", + report.probed_buttons + ), + }); + } + + // The evidence, always — including for a confident result. A tool that shows + // only its conclusion cannot be checked. + ui.collapsing("Per-button evidence", |ui| { + const NAMES: [&str; 6] = ["Right", "Left", "Down", "Up", "A", "B"]; + for (name, d) in NAMES.iter().zip(report.per_button.iter()) { + match d { + Some(f) => ui.label(format!("{name}: reacted on frame {f}")), + None => ui.weak(format!("{name}: no reaction")), + }; + } + if let Some(obs) = report.observable { + ui.weak(format!("decided on: {obs:?}")); + } + }); +} + +/// Run the measurement against the live emulator, recording a status line. +fn run_measurement(state: &mut LatencyPanel, nes: Option<&mut Nes>) { + let Some(nes) = nes else { + "No ROM loaded.".clone_into(&mut state.status); + return; + }; + // Captured BEFORE the measurement, from the console that is about to be + // measured — see `LatencyPanel::frame_ms`. + state.frame_ms = nes.frame_duration().as_secs_f64() * 1000.0; + // `measure_in_place` snapshots, replays, and restores — the live timeline is + // exactly where it was when this returns. + let report = latency::measure_in_place(nes, LatencyConfig::default()); + state.status = format!("Measured in {} trials.", report.trials_used); + state.report = Some(report); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn report(frames: Option, confidence: Confidence) -> LatencyReport { + LatencyReport { + frames, + confidence, + reacting_buttons: 6, + probed_buttons: 6, + observable: None, + per_button: vec![frames; 6], + trials_used: 7, + } + } + + /// THE property this panel exists to preserve: a measurement never changes + /// the user's setting by itself. `pending_apply` is only ever set by the + /// Apply button, so a freshly-stored report leaves it empty. + #[test] + fn a_measurement_alone_never_requests_an_apply() { + let mut panel = LatencyPanel { + report: Some(report(Some(2), Confidence::Unanimous)), + ..LatencyPanel::default() + }; + assert_eq!( + panel.take_pending_apply(), + None, + "storing a report queued a run-ahead change the user never asked for" + ); + } + + /// An inconclusive report must offer no depth at all — not zero. + #[test] + fn an_inconclusive_report_recommends_nothing() { + let r = report(None, Confidence::Inconclusive); + assert_eq!(r.suggested_run_ahead(MAX_DEPTH), None); + } + + /// A measured lag deeper than the cap is still reported, with the + /// recommendation clamped rather than the measurement thrown away. + #[test] + fn a_deep_measurement_is_clamped_not_discarded() { + let r = report(Some(7), Confidence::Unanimous); + assert_eq!(r.frames, Some(7)); + assert_eq!(r.suggested_run_ahead(MAX_DEPTH), Some(MAX_DEPTH)); + } + + /// The felt-latency read-out must be a function of the console's frame + /// duration, not a constant. Hardcoding NTSC's 16.639 ms makes this fail: + /// PAL and Dendy would report the same milliseconds as NTSC for the same + /// frame count, understating them by 20.2%. + #[test] + fn felt_milliseconds_track_the_region_not_a_constant() { + let ms_of = |d: std::time::Duration| d.as_secs_f64() * 1000.0; + let ntsc = ms_of(rustynes_core::FRAME_DURATION_NTSC); + let pal = ms_of(rustynes_core::FRAME_DURATION_PAL); + assert!( + (f64::from(3_u32) * pal - f64::from(3_u32) * ntsc).abs() > 1.0, + "a three-frame lag must read differently on PAL than on NTSC; \ + identical output means the conversion is hardcoded" + ); + } + + /// A ROM transition must discard the whole measurement — and `pending_apply` + /// especially. A report left standing describes a cartridge that is no + /// longer loaded; a `pending_apply` left standing would apply the previous + /// game's depth to the new one. + #[test] + fn clearing_discards_the_report_and_any_queued_apply() { + let mut panel = LatencyPanel { + report: Some(report(Some(2), Confidence::Unanimous)), + pending_apply: Some(2), + frame_ms: 16.639, + status: "Measured in 7 trials.".to_owned(), + measure_requested: true, + }; + panel.clear(); + assert!( + panel.report.is_none(), + "a stale report survived a ROM change" + ); + assert_eq!( + panel.take_pending_apply(), + None, + "the previous game's run-ahead depth was still queued to apply" + ); + assert!(panel.status.is_empty()); + assert!(!panel.measure_requested); + } + + /// `take_pending_apply` drains, so one Apply click cannot be consumed twice + /// and re-applied on a later frame. + #[test] + fn a_pending_apply_is_drained_exactly_once() { + let mut panel = LatencyPanel { + pending_apply: Some(2), + ..LatencyPanel::default() + }; + assert_eq!(panel.take_pending_apply(), Some(2)); + assert_eq!(panel.take_pending_apply(), None); + } +} diff --git a/crates/rustynes-frontend/src/debugger/mod.rs b/crates/rustynes-frontend/src/debugger/mod.rs index 83352fcf..5745258a 100644 --- a/crates/rustynes-frontend/src/debugger/mod.rs +++ b/crates/rustynes-frontend/src/debugger/mod.rs @@ -121,6 +121,7 @@ mod expr; mod game_db_panel; // v2.2.0 "Capstone" — read-only ROM Info browser (per-game DB + No-Intro CRC + // decoded cartridge header for the loaded ROM). +mod latency_panel; mod rom_info_panel; // v1.5.0 "Lens" Workstream A4 — HD-pack per-pixel inspector (native + hd-pack). // `pub(crate)` so `app.rs` can drive its `show` (the panel needs the compositor @@ -190,6 +191,9 @@ pub enum ToolPanel { /// identity CRCs / SHA-256, its per-game DB entry, and its decoded /// cartridge header. A read-only companion to [`Self::GameDb`]. RomInfo, + /// v2.3.6 — the Latency Oracle: measures the game's own input lag and + /// recommends (never applies) a run-ahead depth. + LatencyOracle, /// Live "Input Display" panel — the consolidated controller + expansion- /// device HUD (v1.7.0 "Forge" beta.5, #51; the v1.5.0 "Lens" Workstream A1 /// Input Miniatures overlay absorbed the former standalone Input Display). @@ -547,6 +551,7 @@ pub fn detached_window_meta(id: &'static str) -> (&'static str, (u32, u32)) { "cheat" => ("Cheats", (460, 440)), "game_db" => ("Game Database", (560, 480)), "rom_info" => ("ROM Info", (520, 520)), + "latency_oracle" => ("Latency Oracle", (380, 420)), "provenance" => ("Pixel Provenance", (520, 620)), "perf" => ("Performance", (560, 440)), "documentation" => ("Documentation", (780, 560)), @@ -673,6 +678,8 @@ pub struct DebuggerOverlay { show_game_db: bool, /// Read-only ROM Info browser open flag (v2.2.0 "Capstone"). show_rom_info: bool, + /// v2.3.6 — Latency Oracle panel visible. + show_latency: bool, /// v2.3.2 "Lucid" — pixel provenance inspector. show_provenance: bool, /// "Input Display" panel open flag (v1.7.0 "Forge" beta.5, #51; née the @@ -760,6 +767,7 @@ pub struct DebuggerOverlay { game_db_ui: game_db_panel::GameDbPanelState, /// Read-only ROM Info panel state (v2.2.0 "Capstone"). rom_info_ui: rom_info_panel::RomInfoPanelState, + latency_ui: latency_panel::LatencyPanel, /// Pixel provenance inspector state (v2.3.2 "Lucid"). provenance_ui: provenance_panel::ProvenancePanelState, /// CRC32 of the currently-loaded ROM (PRG+CHR, header-excluded), pushed by @@ -936,6 +944,7 @@ impl DebuggerOverlay { show_perf: false, show_game_db: false, show_rom_info: false, + show_latency: false, show_provenance: false, show_input_display: false, #[cfg(all(not(target_arch = "wasm32"), feature = "hd-pack"))] @@ -974,6 +983,7 @@ impl DebuggerOverlay { cheat_ui: cheat_panel::CheatPanelState::default(), game_db_ui: game_db_panel::GameDbPanelState::default(), rom_info_ui: rom_info_panel::RomInfoPanelState, + latency_ui: latency_panel::LatencyPanel::default(), provenance_ui: provenance_panel::ProvenancePanelState::default(), rom_crc: None, rom_crc_full: None, @@ -1096,6 +1106,16 @@ impl DebuggerOverlay { self.show_tas = false; } + /// v2.3.6 — discard a Latency Oracle measurement bound to the previous ROM. + /// + /// Called at every ROM transition, beside [`Self::clear_tas_editor`], which + /// is invalidated by the same event for the same reason. Without it a report + /// measured on one game stays on screen for the next, with its **Apply** + /// button still live. (PR #385 review.) + pub fn clear_latency_report(&mut self) { + self.latency_ui.clear(); + } + /// Returns `true` when the overlay is currently visible. The render /// path uses this to pick its emu-lock policy (v2.8.0 Phase 5): the /// egui pass needs `&mut Nes`, so a visible overlay holds the lock @@ -1468,6 +1488,7 @@ impl DebuggerOverlay { ToolPanel::Input => self.show_input = true, ToolPanel::GameDb => self.show_game_db = true, ToolPanel::RomInfo => self.show_rom_info = true, + ToolPanel::LatencyOracle => self.show_latency = true, ToolPanel::PixelProvenance => self.show_provenance = true, ToolPanel::InputDisplay => self.show_input_display = true, ToolPanel::Replay => self.show_replay = true, @@ -1742,12 +1763,17 @@ impl DebuggerOverlay { /// happened to be open, then vanish when that one closed). Today the /// `nes`-reading tool panels are **Cheats** (`show_cheat`), the /// **ROM Database** editor (`show_game_db`), and the read-only **ROM Info** - /// browser (`show_rom_info`), and the **Pixel Provenance** inspector - /// (`show_provenance`). If you add another panel that + /// browser (`show_rom_info`), the **Pixel Provenance** inspector + /// (`show_provenance`), and the **Latency Oracle** (`show_latency`). If you + /// add another panel that /// takes `&Nes` / `&mut Nes` in `tool_panels`, add its `show_*` flag here too. #[must_use] pub const fn any_nes_tool_open(&self) -> bool { - self.show_cheat || self.show_game_db || self.show_rom_info || self.show_provenance + self.show_cheat + || self.show_game_db + || self.show_rom_info + || self.show_provenance + || self.show_latency } /// Whether the **Pixel Provenance** inspector is open. @@ -2086,6 +2112,28 @@ impl DebuggerOverlay { nes.as_deref_mut(), ); } + // v2.3.6 — the Latency Oracle. Takes the optional `nes` because the + // measurement DRIVES the emulator (snapshotting and restoring it, so the + // live timeline is untouched), the same shape as BasicBot above. + // + // It only ever RECOMMENDS a run-ahead depth. `take_pending_apply` is + // non-empty solely when the user pressed Apply, which is why the config + // write lives here rather than inside the panel: "measured" and + // "applied" stay two separate, auditable steps. + if self.show_latency { + let current = config.input.run_ahead; + latency_panel::show( + ctx, + &mut self.detached_panels, + &mut self.show_latency, + &mut self.latency_ui, + nes.as_deref_mut(), + current, + ); + if let Some(depth) = self.latency_ui.take_pending_apply() { + config.input.run_ahead = depth; + } + } // v2.1.6 "Expansion Audio" B7 — the Audio Mixer. It reads `config` (the // persisted mix) and the optional `nes` (read-only DAC taps for the // scopes + the sink for pushing changed gain/mask to the core overlay); diff --git a/crates/rustynes-frontend/src/emu.rs b/crates/rustynes-frontend/src/emu.rs index e1049dc9..9267cda0 100644 --- a/crates/rustynes-frontend/src/emu.rs +++ b/crates/rustynes-frontend/src/emu.rs @@ -99,10 +99,12 @@ pub struct EmuHandle { /// depth the produce path would never run, because its cap and /// `effective_run_ahead`'s cap were separate literals that drifted. (PR #358 /// review.) -/// Native-only: both users (`effective_run_ahead`, `update_runahead_throttle`) -/// are, and the wasm frontend has no run-ahead path. -#[cfg(not(target_arch = "wasm32"))] -const MAX_RUN_AHEAD_DEPTH: u32 = 3; +/// v2.3.6: no longer native-only. It was `#[cfg(not(target_arch = "wasm32"))]` +/// because its only two users (`effective_run_ahead`, `update_runahead_throttle`) +/// are, but the Latency Oracle panel compiles on every target and needs the same +/// cap to clamp its recommendation — and reintroducing a bare `3` there is +/// exactly the drift this constant was created to stop. +pub(crate) const MAX_RUN_AHEAD_DEPTH: u32 = 3; /// v2.3.3 F21 — fraction of the frame budget at which the run-ahead throttle /// engages, measured rather than chosen. diff --git a/crates/rustynes-frontend/src/ui_shell.rs b/crates/rustynes-frontend/src/ui_shell.rs index 2eb1b117..ef7b2162 100644 --- a/crates/rustynes-frontend/src/ui_shell.rs +++ b/crates/rustynes-frontend/src/ui_shell.rs @@ -949,23 +949,30 @@ impl UiShell { // would diverge the recorded timeline). if frame.disk_sides > 0 { ui.separator(); - if accel_enabled( - ui, - !replay_locked, - &ic(glyph::FLOPPY_DISK, "Swap Disk Side"), - &keys.disk_swap, - ) - .clicked() - { - out.action = Some(MenuAction::CycleDiskSide); - ui.close(); - } - // v1.8.9 — Multi-Disk: insert a specific side directly (a - // multi-disk FDS game prompts "insert side N"), or eject. - // Disabled during a replay (mutating the disk diverges the - // recorded timeline), like the cycle item above. - ui.add_enabled_ui(!replay_locked, |ui| { - ui.menu_button(ic(glyph::FLOPPY_DISK, "Disk Side"), |ui| { + // v2.3.6 menu reorg — the swap accelerator and the + // per-side selector were two sibling entries describing + // one piece of hardware; they are now one submenu. The + // accelerator (F9 by default) is global and unaffected by + // the extra hop, so nothing gets slower to reach in + // practice — only tidier to read. + ui.menu_button(ic(glyph::FLOPPY_DISK, "Famicom Disk System"), |ui| { + if accel_enabled( + ui, + !replay_locked, + &ic(glyph::FLOPPY_DISK, "Swap Disk Side"), + &keys.disk_swap, + ) + .clicked() + { + out.action = Some(MenuAction::CycleDiskSide); + ui.close(); + } + ui.separator(); + // v1.8.9 — Multi-Disk: insert a specific side directly (a + // multi-disk FDS game prompts "insert side N"), or eject. + // Disabled during a replay (mutating the disk diverges the + // recorded timeline), like the cycle item above. + ui.add_enabled_ui(!replay_locked, |ui| { for i in 0..frame.disk_sides { if ui .radio( @@ -1101,6 +1108,16 @@ impl UiShell { // ----- Tools ----- ui.menu_button(ic(glyph::WRENCH, crate::t!(MenuTools)), |ui| { + // v2.3.6 menu reorg — Tools had grown to twenty flat entries + // spanning cheats, TAS authoring, media capture, multiplayer, + // ROM inspection and provenance analysis, which is more than a + // menu can be scanned at. The entries below are grouped by the + // TASK the user is doing, one submenu per task, with the two + // that are neither task-scoped nor frequently used (Netplay, + // RetroAchievements — they configure a *session*, not a tool) + // kept at the bottom behind a separator. Cheats stays at the + // top level because it is by a wide margin the most-opened + // panel and burying the common case is how menus get worse. if ui .button(ic(glyph::WAND_MAGIC_SPARKLES, "Cheats...")) .clicked() @@ -1108,258 +1125,293 @@ impl UiShell { out.action = Some(MenuAction::OpenPanel(ToolPanel::Cheats)); ui.close(); } + ui.separator(); + // ---- Movies & Recording -------------------------------- + // Everything that captures or replays a session: the TAS + // movie transport, the external-format interop, the two + // authoring panels, and the A/V + clip exporters. + // // BUG-1: direct child (not inside add_enabled_ui — see File). - // (H1) The Movies submenu is unavailable during a netplay - // session (a rollback session cannot also be a TAS movie). - if rom && !rom_change_restricted { - ui.menu_button(ic(glyph::VIDEO, "Movies (TAS)"), |ui| { - // Record toggles record on/off; it must be locked - // while a movie is PLAYING (can't record over a - // playback). The toggle-off case (already recording) - // stays enabled so the user can stop. - let rec_label = if frame.movie_recording { - ic(glyph::STOP, "Stop Recording") - } else { - ic(glyph::VIDEO, "Record") - }; - let rec_enabled = frame.movie_recording || !frame.movie_playing; - if accel_enabled(ui, rec_enabled, &rec_label, &keys.movie_record) + ui.menu_button(ic(glyph::VIDEO, "Movies & Recording"), |ui| { + // (H1) The movie transport is unavailable during a netplay + // session (a rollback session cannot also be a TAS movie). + // Pre-reorg this gated the whole submenu open/closed; it is + // now applied per item, so the entries stay visible-but- + // disabled and the user can see WHY nothing is available. + let movie_ok = rom && !rom_change_restricted; + // Record toggles record on/off; it must be locked + // while a movie is PLAYING (can't record over a + // playback). The toggle-off case (already recording) + // stays enabled so the user can stop. + let rec_label = if frame.movie_recording { + ic(glyph::STOP, "Stop Recording") + } else { + ic(glyph::VIDEO, "Record") + }; + let rec_enabled = + movie_ok && (frame.movie_recording || !frame.movie_playing); + if accel_enabled(ui, rec_enabled, &rec_label, &keys.movie_record).clicked() + { + out.action = Some(MenuAction::MovieRecordToggle); + ui.close(); + } + // Play toggles playback; locked while RECORDING. The + // toggle-off (already playing) stays enabled to stop. + let play_label = if frame.movie_playing { + ic(glyph::STOP, "Stop Playback") + } else { + ic(glyph::PLAY, "Play") + }; + let play_enabled = + movie_ok && (frame.movie_playing || !frame.movie_recording); + if accel_enabled(ui, play_enabled, &play_label, &keys.movie_play).clicked() + { + out.action = Some(MenuAction::MoviePlayToggle); + ui.close(); + } + // Branch forks the CURRENT playback into a new + // recording — only meaningful while playing back. + if accel_enabled( + ui, + movie_ok && frame.movie_playing, + &ic(glyph::VIDEO, "Branch"), + &keys.movie_branch, + ) + .clicked() + { + out.action = Some(MenuAction::MovieBranch); + ui.close(); + } + // Gated with the block it introduces: the interop items + // below compile out on wasm, and an ungated separator + // here would then sit directly against the one after + // them — two rules with nothing between. Same treatment + // as the session-services separator lower down. + // (PR #385 review.) + #[cfg(not(target_arch = "wasm32"))] + ui.separator(); + // v1.6.0 B1 — external TAS movie interop (FCEUX + // `.fm2` / BizHawk `.bk2`). Import begins playback + // (locked while recording, like Play); Export writes + // the current recording / loaded movie (enabled when + // a movie exists to export). + #[cfg(not(target_arch = "wasm32"))] + { + let import_enabled = movie_ok && !frame.movie_recording; + if ui + .add_enabled( + import_enabled, + egui::Button::new(ic( + glyph::FOLDER_OPEN, + "Import (.fm2 / .bk2)", + )), + ) .clicked() { - out.action = Some(MenuAction::MovieRecordToggle); + out.action = Some(MenuAction::MovieImport); ui.close(); } - // Play toggles playback; locked while RECORDING. The - // toggle-off (already playing) stays enabled to stop. - let play_label = if frame.movie_playing { - ic(glyph::STOP, "Stop Playback") - } else { - ic(glyph::PLAY, "Play") - }; - let play_enabled = frame.movie_playing || !frame.movie_recording; - if accel_enabled(ui, play_enabled, &play_label, &keys.movie_play) + let export_enabled = + movie_ok && (frame.movie_recording || frame.movie_playing); + if ui + .add_enabled( + export_enabled, + egui::Button::new(ic( + glyph::FLOPPY_DISK, + "Export (.fm2 / .bk2)", + )), + ) .clicked() { - out.action = Some(MenuAction::MoviePlayToggle); + out.action = Some(MenuAction::MovieExport); ui.close(); } - // Branch forks the CURRENT playback into a new - // recording — only meaningful while playing back. - if accel_enabled( - ui, - frame.movie_playing, - &ic(glyph::VIDEO, "Branch"), - &keys.movie_branch, - ) - .clicked() + // v1.7.0 H9 — export TAStudio markers as a + // SubRip (.srt) subtitle track. + if ui + .add_enabled( + movie_ok, + egui::Button::new(ic( + glyph::FLOPPY_DISK, + "Export subtitles (.srt)", + )), + ) + .clicked() { - out.action = Some(MenuAction::MovieBranch); + out.action = Some(MenuAction::MovieExportSubtitles); ui.close(); } - ui.separator(); - // v1.6.0 B1 — external TAS movie interop (FCEUX - // `.fm2` / BizHawk `.bk2`). Import begins playback - // (locked while recording, like Play); Export writes - // the current recording / loaded movie (enabled when - // a movie exists to export). - #[cfg(not(target_arch = "wasm32"))] + } + ui.separator(); + // v1.6.0 "Studio" Workstream A2 — TAStudio piano-roll TAS + // editor. Needs a loaded ROM (the editor anchors on the + // current emulator state as the project's frame 0). + if ui + .add_enabled(rom, egui::Button::new(ic(glyph::VIDEO, "TAStudio"))) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::TasStudio)); + ui.close(); + } + // v1.5.0 "Lens" Workstream C2 — Replay / TAS window (device + // topology + timebase + branch/seek UX over the .rnm machinery). + if ui.button(ic(glyph::VIDEO, "Replay / TAS")).clicked() { + out.action = Some(MenuAction::OpenPanel(ToolPanel::Replay)); + ui.close(); + } + ui.separator(); + // v1.6.0 "Studio" Workstream G — A/V recording (native + + // `av-record`-gated). Start opens a save dialog + arms an + // ffmpeg-piped recorder; a second click stops + finalizes. + // Needs a loaded ROM to record anything; the stop case stays + // enabled while armed so the user can finish. + #[cfg(all(not(target_arch = "wasm32"), feature = "av-record"))] + { + let av_label = if frame.av_recording { + ic(glyph::STOP, "Stop A/V Recording") + } else { + ic(glyph::VIDEO, "Record A/V...") + }; + let av_enabled = frame.av_recording || rom; + if ui + .add_enabled(av_enabled, egui::Button::new(av_label)) + .clicked() { - let import_enabled = !frame.movie_recording; - if ui - .add_enabled( - import_enabled, - egui::Button::new(ic( - glyph::FOLDER_OPEN, - "Import (.fm2 / .bk2)", - )), - ) - .clicked() - { - out.action = Some(MenuAction::MovieImport); - ui.close(); - } - let export_enabled = frame.movie_recording || frame.movie_playing; - if ui - .add_enabled( - export_enabled, - egui::Button::new(ic( - glyph::FLOPPY_DISK, - "Export (.fm2 / .bk2)", - )), - ) - .clicked() - { - out.action = Some(MenuAction::MovieExport); - ui.close(); - } - // v1.7.0 H9 — export TAStudio markers as a - // SubRip (.srt) subtitle track. - if ui - .add(egui::Button::new(ic( - glyph::FLOPPY_DISK, - "Export subtitles (.srt)", - ))) - .clicked() - { - out.action = Some(MenuAction::MovieExportSubtitles); - ui.close(); - } + out.action = Some(MenuAction::AvRecordToggle); + ui.close(); } - }); - } else { - ui.add_enabled(false, egui::Button::new(ic(glyph::VIDEO, "Movies (TAS)"))); - } - // v1.6.0 "Studio" Workstream G — A/V recording (native + - // `av-record`-gated). Start opens a save dialog + arms an - // ffmpeg-piped recorder; a second click stops + finalizes. - // Needs a loaded ROM to record anything; the stop case stays - // enabled while armed so the user can finish. - #[cfg(all(not(target_arch = "wasm32"), feature = "av-record"))] - { - let av_label = if frame.av_recording { - ic(glyph::STOP, "Stop A/V Recording") - } else { - ic(glyph::VIDEO, "Record A/V...") - }; - let av_enabled = frame.av_recording || rom; + } + // v1.7.0 "Forge" Workstream D1 — export the last 30 s of the + // live session timeline (the HistoryViewer over the rewind + // ring) as a replayable `.rnm` clip. Needs a loaded ROM. if ui - .add_enabled(av_enabled, egui::Button::new(av_label)) + .add_enabled( + rom, + egui::Button::new(ic(glyph::FLOPPY_DISK, "Export Last 30s (.rnm)")), + ) .clicked() { - out.action = Some(MenuAction::AvRecordToggle); + out.action = Some(MenuAction::HistoryExportClip { seconds: 30.0 }); ui.close(); } - } - // (H1) Opening the Netplay panel is locked while a replay - // (TAS movie) owns the session. Mirrors the `GeraNES` - // reference emulator's Netplay gating (no replay-interaction - // lockout active). - #[cfg(not(target_arch = "wasm32"))] - if ui - .add_enabled( - !replay_locked, - egui::Button::new(ic(glyph::WIFI, "Netplay...")), - ) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::Netplay)); - ui.close(); - } - #[cfg(all(not(target_arch = "wasm32"), feature = "retroachievements"))] - if ui - .button(ic(glyph::TROPHY, "RetroAchievements...")) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::Cheevos)); - ui.close(); - } - // v1.7.0 "Forge" beta.5 (#51) — one consolidated "Input - // Display" panel: standard pads + every expansion peripheral - // (Zapper / Vaus / SNES mouse / Power Pad / keyboard / Hyper - // Shot / Four Score), real-time button/axis state. - if ui.button(ic(glyph::GAMEPAD, "Input Display")).clicked() { - out.action = Some(MenuAction::OpenPanel(ToolPanel::InputDisplay)); - ui.close(); - } - // v1.8.9 "Backlog" — the desktop on-screen virtual pad: a - // clickable egui controller that feeds player 1. Native-only - // (the browser build has the touch overlay). - #[cfg(not(target_arch = "wasm32"))] - if ui.button(ic(glyph::GAMEPAD, "Virtual Pad")).clicked() { - out.action = Some(MenuAction::ToggleVirtualPad); - ui.close(); - } - // v1.3.0 menu reorg — NSF/NSFe music player (moved here from - // the Debug menu; it is a playback tool, not a chip inspector). - if ui.button(ic(glyph::HEADPHONES, "NSF Player")).clicked() { - out.action = Some(MenuAction::OpenChipPanel(ChipPanel::Nsf)); - ui.close(); - } - // v2.1.6 "Expansion Audio" B7 — the Audio Mixer: per-source - // balance sliders + per-channel scopes / VU (base 2A03 + the - // on-cart expansion channel). A frontend mix overlay; the - // deterministic core output is unchanged. - if ui.button(ic(glyph::SLIDERS, "Audio Mixer")).clicked() { - out.action = Some(MenuAction::OpenPanel(ToolPanel::AudioMixer)); - ui.close(); - } - // v1.5.0 "Lens" Workstream C2 — Replay / TAS window (device - // topology + timebase + branch/seek UX over the .rnm machinery). - if ui.button(ic(glyph::VIDEO, "Replay / TAS")).clicked() { - out.action = Some(MenuAction::OpenPanel(ToolPanel::Replay)); - ui.close(); - } - // v1.8.9 "Backlog" — BasicBot input-search control panel. - if ui - .button(ic(glyph::WAND_MAGIC_SPARKLES, "BasicBot")) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::BasicBot)); - ui.close(); - } - // v1.6.0 "Studio" Workstream A2 — TAStudio piano-roll TAS - // editor. Needs a loaded ROM (the editor anchors on the - // current emulator state as the project's frame 0). - if ui - .add_enabled(rom, egui::Button::new(ic(glyph::VIDEO, "TAStudio"))) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::TasStudio)); - ui.close(); - } - // v1.7.0 "Forge" Workstream D1 — export the last 30 s of the - // live session timeline (the HistoryViewer over the rewind - // ring) as a replayable `.rnm` clip. Needs a loaded ROM. - if ui - .add_enabled( - rom, - egui::Button::new(ic(glyph::FLOPPY_DISK, "Export Last 30s (.rnm)")), - ) - .clicked() - { - out.action = Some(MenuAction::HistoryExportClip { seconds: 30.0 }); - ui.close(); - } - // (H1) The ROM Database editor needs a loaded ROM to edit. - if ui - .add_enabled(rom, egui::Button::new(ic(glyph::DATABASE, "ROM Database"))) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::GameDb)); - ui.close(); - } - // (H1) v2.2.0 "Capstone" — the read-only ROM Info browser - // needs a loaded ROM to describe. - if ui - .add_enabled(rom, egui::Button::new(ic(glyph::CIRCLE_INFO, "ROM Info"))) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::RomInfo)); - ui.close(); - } - // (H1) v2.3.2 "Lucid" — the pixel provenance inspector: the - // causal chain from a screen pixel back to the tile, the - // palette entry, and the instruction that wrote them. Needs a - // loaded ROM to have any pixels to explain. NOT gated on the - // frontend's `debug-hooks` alias: the frontend always pulls - // `rustynes-core` with `debug-hooks` on (see its Cargo.toml), - // so gating on the alias — which is off by default — would - // ship the panel permanently unreachable. - if ui - .add_enabled( - rom, - egui::Button::new(ic(glyph::MAGNIFYING_GLASS_PLUS, "Pixel Provenance")), - ) - .clicked() - { - out.action = Some(MenuAction::OpenPanel(ToolPanel::PixelProvenance)); - ui.close(); - } + }); + // ---- Audio --------------------------------------------- + ui.menu_button(ic(glyph::HEADPHONES, "Audio"), |ui| { + // v1.3.0 menu reorg — NSF/NSFe music player (moved here from + // the Debug menu; it is a playback tool, not a chip inspector). + if ui.button(ic(glyph::HEADPHONES, "NSF Player")).clicked() { + out.action = Some(MenuAction::OpenChipPanel(ChipPanel::Nsf)); + ui.close(); + } + // v2.1.6 "Expansion Audio" B7 — the Audio Mixer: per-source + // balance sliders + per-channel scopes / VU (base 2A03 + the + // on-cart expansion channel). A frontend mix overlay; the + // deterministic core output is unchanged. + if ui.button(ic(glyph::SLIDERS, "Audio Mixer")).clicked() { + out.action = Some(MenuAction::OpenPanel(ToolPanel::AudioMixer)); + ui.close(); + } + }); + // ---- Input --------------------------------------------- + ui.menu_button(ic(glyph::GAMEPAD, "Input"), |ui| { + // v1.7.0 "Forge" beta.5 (#51) — one consolidated "Input + // Display" panel: standard pads + every expansion peripheral + // (Zapper / Vaus / SNES mouse / Power Pad / keyboard / Hyper + // Shot / Four Score), real-time button/axis state. + if ui.button(ic(glyph::GAMEPAD, "Input Display")).clicked() { + out.action = Some(MenuAction::OpenPanel(ToolPanel::InputDisplay)); + ui.close(); + } + // v1.8.9 "Backlog" — the desktop on-screen virtual pad: a + // clickable egui controller that feeds player 1. Native-only + // (the browser build has the touch overlay). + #[cfg(not(target_arch = "wasm32"))] + if ui.button(ic(glyph::GAMEPAD, "Virtual Pad")).clicked() { + out.action = Some(MenuAction::ToggleVirtualPad); + ui.close(); + } + }); + // ---- Game Data ----------------------------------------- + // What this cartridge IS, as opposed to what it is doing: + // both entries describe the loaded ROM and both need one. + ui.menu_button(ic(glyph::DATABASE, "Game Data"), |ui| { + // (H1) v2.2.0 "Capstone" — the read-only ROM Info browser + // needs a loaded ROM to describe. + if ui + .add_enabled(rom, egui::Button::new(ic(glyph::CIRCLE_INFO, "ROM Info"))) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::RomInfo)); + ui.close(); + } + // (H1) The ROM Database editor needs a loaded ROM to edit. + if ui + .add_enabled( + rom, + egui::Button::new(ic(glyph::DATABASE, "ROM Database")), + ) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::GameDb)); + ui.close(); + } + }); + // ---- Analysis ------------------------------------------ + // The three tools that answer a question ABOUT the running + // game rather than changing it: what its input lag is, why a + // pixel looks the way it does, and what input sequence reaches + // a goal. All three are output-only. + ui.menu_button(ic(glyph::MAGNIFYING_GLASS_PLUS, "Analysis"), |ui| { + // v2.3.6 — the Latency Oracle. Grouped with the other + // measurement tools rather than under Settings because it is + // a measurement you RUN, not a preference you set; it + // recommends a run-ahead depth and never applies one itself. + if ui + .add_enabled(rom, egui::Button::new(ic(glyph::GAUGE, "Latency Oracle"))) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::LatencyOracle)); + ui.close(); + } + // (H1) v2.3.2 "Lucid" — the pixel provenance inspector: the + // causal chain from a screen pixel back to the tile, the + // palette entry, and the instruction that wrote them. Needs a + // loaded ROM to have any pixels to explain. NOT gated on the + // frontend's `debug-hooks` alias: the frontend always pulls + // `rustynes-core` with `debug-hooks` on (see its Cargo.toml), + // so gating on the alias — which is off by default — would + // ship the panel permanently unreachable. + if ui + .add_enabled( + rom, + egui::Button::new(ic( + glyph::MAGNIFYING_GLASS_PLUS, + "Pixel Provenance", + )), + ) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::PixelProvenance)); + ui.close(); + } + // v1.8.9 "Backlog" — BasicBot input-search control panel. + if ui + .button(ic(glyph::WAND_MAGIC_SPARKLES, "BasicBot")) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::BasicBot)); + ui.close(); + } + }); // v1.3.0 menu reorg — HD-pack loader (v1.2.0 C3), folded in // from the former standalone "Mod" menu as a Tools submenu; // native + `hd-pack`-feature-gated. (H1) Load/unload needs a // loaded ROM (the pack is keyed on the ROM hash) and is locked // while a netplay/replay session owns presentation. + // + // Deliberately NOT wrapped in a further "Enhancements" level: + // it is the only member that category would have, so the extra + // hop would buy indirection and no grouping. #[cfg(all(feature = "hd-pack", not(target_arch = "wasm32")))] ui.menu_button(ic(glyph::PUZZLE_PIECE, "HD Pack"), |ui| { let mod_enabled = rom && !rom_change_restricted && !replay_locked; @@ -1415,6 +1467,41 @@ impl UiShell { ui.close(); } }); + // ---- Session services ---------------------------------- + // Netplay and RetroAchievements are not tools you point at + // the game; they change what the SESSION is (a lockstep + // rollback match, an authenticated hardcore run). They stay + // at the top level, below a separator, so they read as + // session-scoped rather than as two more inspectors. + // + // Gated with the items it introduces: both are native-only, so + // on wasm this would otherwise render as a trailing separator + // with nothing beneath it. + #[cfg(not(target_arch = "wasm32"))] + ui.separator(); + // (H1) Opening the Netplay panel is locked while a replay + // (TAS movie) owns the session. Mirrors the `GeraNES` + // reference emulator's Netplay gating (no replay-interaction + // lockout active). + #[cfg(not(target_arch = "wasm32"))] + if ui + .add_enabled( + !replay_locked, + egui::Button::new(ic(glyph::WIFI, "Netplay...")), + ) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::Netplay)); + ui.close(); + } + #[cfg(all(not(target_arch = "wasm32"), feature = "retroachievements"))] + if ui + .button(ic(glyph::TROPHY, "RetroAchievements...")) + .clicked() + { + out.action = Some(MenuAction::OpenPanel(ToolPanel::Cheevos)); + ui.close(); + } }); // ----- Debug ----- @@ -1434,29 +1521,69 @@ impl UiShell { ui.close(); } ui.separator(); - // Chip / state inspectors. (NSF Player moved to the Tools menu - // in v1.3.0 — it is a playback tool, not a chip inspector.) - for (icon, label, panel) in [ - (glyph::MICROCHIP, "CPU", ChipPanel::Cpu), - (glyph::MICROCHIP, "PPU", ChipPanel::Ppu), - (glyph::VOLUME_HIGH, "APU", ChipPanel::Apu), - (glyph::MEMORY, "Memory", ChipPanel::Memory), - (glyph::MEMORY, "Memory Compare", ChipPanel::MemoryCompare), - (glyph::MEMORY, "OAM", ChipPanel::Oam), - (glyph::PUZZLE_PIECE, "Mapper", ChipPanel::Mapper), - (glyph::CLIPBOARD, "Trace Logger", ChipPanel::Trace), - (glyph::CLIPBOARD, "Watch / Breakpoints", ChipPanel::Watch), - (glyph::CLIPBOARD, "Event Viewer", ChipPanel::Events), - (glyph::CODE, "Lua Script", ChipPanel::Script), - ] { - if ui.button(ic(icon, label)).clicked() { - out.action = Some(MenuAction::OpenChipPanel(panel)); - ui.close(); - } - } + // v2.3.6 menu reorg — the eleven inspectors below used to be + // one flat run, which put "CPU" and "Lua Script" at the same + // level and made the list scan as an undifferentiated column. + // They split cleanly along what you are inspecting: the chips' + // register state, the address space, or the flow of execution. + // The loop shape is kept per group so adding an inspector + // stays a one-line table edit. + // + // (NSF Player moved to the Tools menu in v1.3.0 — it is a + // playback tool, not a chip inspector.) + let mut chip_group = + |ui: &mut egui::Ui, + icon: char, + label: &'static str, + items: &[(char, &'static str, ChipPanel)]| { + ui.menu_button(ic(icon, label), |ui| { + for &(icon, label, panel) in items { + if ui.button(ic(icon, label)).clicked() { + out.action = Some(MenuAction::OpenChipPanel(panel)); + ui.close(); + } + } + }); + }; + // Per-chip register / internal state. + chip_group( + ui, + glyph::MICROCHIP, + "Chip State", + &[ + (glyph::MICROCHIP, "CPU", ChipPanel::Cpu), + (glyph::MICROCHIP, "PPU", ChipPanel::Ppu), + (glyph::VOLUME_HIGH, "APU", ChipPanel::Apu), + (glyph::MEMORY, "OAM", ChipPanel::Oam), + (glyph::PUZZLE_PIECE, "Mapper", ChipPanel::Mapper), + ], + ); + // The address space itself — one live view, one differ. + chip_group( + ui, + glyph::MEMORY, + "Memory", + &[ + (glyph::MEMORY, "Memory", ChipPanel::Memory), + (glyph::MEMORY, "Memory Compare", ChipPanel::MemoryCompare), + ], + ); + // Everything that observes or interrupts the flow of execution. + chip_group( + ui, + glyph::CLIPBOARD, + "Execution", + &[ + (glyph::CLIPBOARD, "Trace Logger", ChipPanel::Trace), + (glyph::CLIPBOARD, "Watch / Breakpoints", ChipPanel::Watch), + (glyph::CLIPBOARD, "Event Viewer", ChipPanel::Events), + (glyph::CODE, "Lua Script", ChipPanel::Script), + ], + ); // v1.7.0 "Forge" Workstream A2 — Cartridge Info / header // editor. Native-only (it inspects + edits a ROM file on - // disk). + // disk). Left at the top level: it edits a file on disk rather + // than inspecting running state, so it belongs to neither group. #[cfg(not(target_arch = "wasm32"))] { ui.separator(); @@ -1467,24 +1594,23 @@ impl UiShell { out.action = Some(MenuAction::OpenChipPanel(ChipPanel::HeaderEditor)); ui.close(); } - } - // v1.4.0 Workstream D (D1) — symbol/label files annotate the - // disassembler + breakpoint + trace views. Native-only (it - // reads a picked file). - #[cfg(not(target_arch = "wasm32"))] - { - ui.separator(); - if ui - .button(ic(glyph::FILE, "Load Symbols (.sym/.mlb/.nl)...")) - .clicked() - { - out.action = Some(MenuAction::LoadSymbols); - ui.close(); - } - if ui.button(ic(glyph::XMARK, "Clear Symbols")).clicked() { - out.action = Some(MenuAction::ClearSymbols); - ui.close(); - } + // v1.4.0 Workstream D (D1) — symbol/label files annotate the + // disassembler + breakpoint + trace views. Native-only (it + // reads a picked file). Grouped because the pair is one + // load/clear lifecycle, not two independent commands. + ui.menu_button(ic(glyph::FILE, "Symbols"), |ui| { + if ui + .button(ic(glyph::FILE, "Load Symbols (.sym/.mlb/.nl)...")) + .clicked() + { + out.action = Some(MenuAction::LoadSymbols); + ui.close(); + } + if ui.button(ic(glyph::XMARK, "Clear Symbols")).clicked() { + out.action = Some(MenuAction::ClearSymbols); + ui.close(); + } + }); } }); diff --git a/crates/rustynes-probe/src/latency.rs b/crates/rustynes-probe/src/latency.rs index 7eaec867..187efdc7 100644 --- a/crates/rustynes-probe/src/latency.rs +++ b/crates/rustynes-probe/src/latency.rs @@ -169,17 +169,61 @@ impl Default for LatencyConfig { /// 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 { + let mut probe = Probe::anchor(anchor, budget_for(cfg)); + run_measurement(&mut probe, nes, cfg) +} + +/// [`measure`] against the emulator's **own** current state, restoring it before +/// returning. +/// +/// The convenience a frontend actually wants: it has one live `Nes` and no +/// second instance to replay into. The state is snapshotted, used as both anchor +/// and scratch, and restored on the way out — so the live timeline is untouched, +/// the same contract `basic_bot::search` offers for the same reason. +/// +/// Note this DRIVES the emulator for the duration (roughly +/// `frames_per_trial * 21` frames), so a caller on a UI thread will block. That +/// is the established shape here — `BasicBot` does the same on an explicit +/// button press — but it is why the panel says "briefly pauses" on the button +/// rather than pretending the work is free. +pub fn measure_in_place(nes: &mut Nes, cfg: LatencyConfig) -> LatencyReport { + let restore_point = nes.snapshot(); + let mut probe = Probe::anchor(&*nes, budget_for(cfg)); + let report = run_measurement(&mut probe, nes, cfg); + // Put the user's timeline back exactly. A measurement that leaves the game + // 400 frames further on would be a worse bug than the one it measures. + // + // `restore_quiet`, NOT `restore`: the loud variant additionally clears the + // rewind ring, on the correct reasoning that a state loaded from elsewhere + // is unrelated to what was buffered. That reasoning does not hold here — + // this is the same timeline, snapshotted moments ago on this very instance — + // so the loud variant would silently destroy the user's rewind history as + // the price of asking how much input lag their game has. + // + // The result is expected rather than discarded. The bytes came from + // `nes.snapshot()` on this instance one call ago, so a failure would mean + // the snapshot format cannot round-trip itself; returning normally would + // hand the user a report while leaving their game several hundred frames + // ahead, which is precisely the outcome this line exists to prevent. + nes.restore_quiet(&restore_point) + .expect("a snapshot taken from this instance restores to it"); + report +} + +/// EXACTLY the trials the measurement 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 `Probe::run` makes it binding, so a future +/// edit that adds a trial fails closed rather than silently spending more of the +/// caller's time than the budget advertises. +fn budget_for(cfg: LatencyConfig) -> 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); + } +} + +fn run_measurement(probe: &mut Probe, nes: &mut Nes, cfg: LatencyConfig) -> LatencyReport { let probed = u32::try_from(PROBE_BUTTONS.len()).unwrap_or(u32::MAX); let mut last_evidence = vec![None; PROBE_BUTTONS.len()]; @@ -466,6 +510,50 @@ mod tests { ); } + /// `measure_in_place` must leave the emulator exactly where it found it. + /// + /// It drives the emulator for hundreds of frames, so a measurement that + /// forgot to restore would advance the user's game — a worse bug than the one + /// being measured, and one that would look like the emulator randomly + /// skipping ahead. Compared on the full snapshot, not just the framebuffer, + /// because a difference in CPU or APU state that has not reached the screen + /// yet is still a difference. + #[test] + fn measure_in_place_restores_the_live_timeline() { + let rom = polling_rom(); + let mut nes = warmed(&rom, 20); + let before = nes.snapshot(); + + let report = measure_in_place(&mut nes, LatencyConfig::default()); + assert!( + report.trials_used > 0, + "premise: the measurement actually ran" + ); + + assert_eq!( + nes.snapshot(), + before, + "measure_in_place moved the live timeline" + ); + } + + /// `measure_in_place` must agree with the two-instance `measure`: it is a + /// convenience, not a different measurement. + #[test] + fn measure_in_place_agrees_with_the_two_instance_form() { + let rom = polling_rom(); + let anchor = warmed(&rom, 20); + let mut scratch = Nes::from_rom(&rom).expect("fixture parses"); + let two_instance = measure(&mut scratch, &anchor, LatencyConfig::default()); + + let mut live = warmed(&rom, 20); + let in_place = measure_in_place(&mut live, LatencyConfig::default()); + + assert_eq!(two_instance.frames, in_place.frames); + assert_eq!(two_instance.confidence, in_place.confidence); + assert_eq!(two_instance.per_button, in_place.per_button); + } + /// 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]