From d07148d8d9e28027cbe1b903f1a99b2fb8004452 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Sun, 16 Aug 2026 21:38:12 -0400 Subject: [PATCH 1/5] =?UTF-8?q?feat(frontend):=20the=20Latency=20Oracle=20?= =?UTF-8?q?panel=20=E2=80=94=20measure=20a=20game's=20own=20input=20lag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every emulator turns "what run-ahead depth should I use?" into a manual ritual: hold a direction, frame-advance until the sprite moves, subtract one. RetroArch documents exactly that procedure. RustyNES's own settings panel offered nothing better than the prose "1 fits most games". This panel measures the number instead. The measurement is `rustynes_probe::latency`, which is the first consumer of the deterministic-probe engine: snapshot an anchor, replay it twice — once with a probe button held from frame 0, once with it never pressed — and report the first frame at which an observable diverges. That frame index IS the game's internal lag, because a deterministic core replaying identical state can differ for exactly one reason. `measure_in_place` is added to the probe crate for this caller. The existing `measure` clones a `Nes` to leave the caller's instance untouched; the frontend already holds `&mut Nes` under the emulator lock and a `Nes` is large enough that cloning it per measurement is a cost with no purpose here, so `measure_in_place` snapshots a restore point, runs the probe against the live instance, and restores it before returning. The live timeline is exactly where it was. Two properties are deliberate and both are pinned by tests. It recommends; it does not apply. 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. `take_pending_apply` is only ever set by the Apply button, and `a_measurement_alone_never_requests_an_apply` fails if storing a report ever queues a config write on its own. It reports its own uncertainty. The probe returns `None` rather than a guess whenever the trial buttons disagree or nothing reacts inside the budget, and the panel renders 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 no tool, because its wrong answers are then indistinguishable from its right ones. The per-button breakdown is shown for confident results too: a tool that publishes only its conclusion cannot be checked. A measurement deeper than the run-ahead range is reported honestly and the recommendation clamped, rather than the measurement being discarded. The panel runs its measurement AFTER the egui render rather than inside the window closure, so `nes` is never captured by the viewport callback — the same deferred-run shape the other `&mut Nes` panels use. It drives several hundred frames under the lock, so the UI pauses briefly and the button says so instead of pretending the work is free. Ships behind no feature gate and default-closed, like every other tool panel. The deterministic core is untouched: the probe only snapshots and restores through the public API. --- Cargo.lock | 1 + crates/rustynes-frontend/Cargo.toml | 4 + .../src/debugger/latency_panel.rs | 275 ++++++++++++++++++ crates/rustynes-frontend/src/debugger/mod.rs | 44 ++- crates/rustynes-probe/src/latency.rs | 90 +++++- 5 files changed, 403 insertions(+), 11 deletions(-) create mode 100644 crates/rustynes-frontend/src/debugger/latency_panel.rs 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/debugger/latency_panel.rs b/crates/rustynes-frontend/src/debugger/latency_panel.rs new file mode 100644 index 00000000..ad1de4f0 --- /dev/null +++ b/crates/rustynes-frontend/src/debugger/latency_panel.rs @@ -0,0 +1,275 @@ +//! 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}; + +/// Highest depth this panel will ever recommend. +/// +/// Matches the Emulation menu's run-ahead range. A game measuring higher than +/// this is reported honestly and the recommendation is clamped, rather than the +/// measurement being silently discarded. +const MAX_DEPTH: u32 = 3; + +/// Persistent panel state. +#[derive(Default)] +pub struct LatencyPanel { + /// The most recent measurement, if one has been run for this session. + report: Option, + /// "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 { + /// 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(); + + if ui + .add_enabled(can_measure, egui::Button::new("\u{23F1} 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, &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, + 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: one + // NTSC frame is 16.639 ms. + #[allow(clippy::cast_precision_loss)] // small counts; display only. + let ms = f64::from(frames) * 16.639; + 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; + }; + // `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)); + } + + /// `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..b2a3e3e0 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, @@ -1468,6 +1478,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 +1753,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 +2102,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-probe/src/latency.rs b/crates/rustynes-probe/src/latency.rs index 7eaec867..a8b3b767 100644 --- a/crates/rustynes-probe/src/latency.rs +++ b/crates/rustynes-probe/src/latency.rs @@ -169,17 +169,47 @@ 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. + let _ = nes.restore(&restore_point); + 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 +496,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] From 5cfe0874353cd37c44b2b9f0002fc903643e3a7e Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Sun, 16 Aug 2026 21:38:45 -0400 Subject: [PATCH 2/5] refactor(frontend): group the Tools and Debug menus by task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools had accreted to twenty flat entries and Debug to a fifteen-item column. Both had grown one item per release since v1.3.0's last reorg, each addition individually reasonable and the aggregate unscannable: Tools put Cheats, TAStudio, Netplay, NSF Player, ROM Database, Pixel Provenance and the HD-pack builder at one level, and Debug put "CPU" and "Lua Script" side by side as peers. The entries are regrouped by the TASK being performed, not alphabetically and not by the release that added them. No entry is removed, no entry changes what it dispatches, and nothing moves between top-level menus — only the depth at which it sits — so no existing muscle memory for WHICH menu holds a thing is broken. Tools becomes: Cheats at the top level (by a wide margin the most-opened panel; burying the common case is how menus get worse), then Movies & Recording, Audio, Input, Game Data, Analysis, HD Pack, then Netplay and RetroAchievements below a separator. Movies & Recording absorbs the whole capture-and-replay surface, which was previously scattered across four separate depths: the movie transport submenu, TAStudio and Replay / TAS as top-level siblings, and the A/V and 30-second-clip exporters interleaved between unrelated inspectors. Its gating changes shape but not effect. Pre-reorg the `rom && !rom_change_restricted` condition decided whether the submenu could be OPENED, with a disabled placeholder button standing in for it during a netplay session; it is now applied per item. The reachable set is identical, but the user can now open the menu and see which specific entries are unavailable rather than facing a single opaque disabled label. The "Export subtitles" item gains an explicit enable condition it previously inherited from that outer gate. Analysis collects the three tools that answer a question ABOUT the running game rather than changing it — Latency Oracle, Pixel Provenance, BasicBot — all of which are output-only. This is also where the Latency Oracle's menu entry lands, rather than under Settings: it is a measurement you run, not a preference you set. HD Pack is deliberately NOT wrapped in a further "Enhancements" level. It would be that category's only member, so the extra hop would buy indirection and no grouping. Netplay and RetroAchievements stay at the top level below a separator because they are not tools pointed at the game — they change what the SESSION is (a lockstep rollback match; an authenticated hardcore run). The separator carries the same `not(wasm32)` gate as the two items it introduces, or the wasm build would render a trailing separator with nothing beneath it. Debug's eleven inspectors split along what is being inspected: Chip State (CPU / PPU / APU / OAM / Mapper), Memory (live view, differ), Execution (trace, breakpoints, events, Lua). The table-driven loop is kept per group via a small local closure, so adding an inspector remains a one-line edit. The header editor stays at the top level — it edits a file on disk rather than inspecting running state, so it belongs to neither group — and the symbol load/clear pair becomes a submenu because it is one lifecycle rather than two independent commands. Emulation gets the one tidy it needed: the FDS swap accelerator and the per-side selector were two sibling entries describing one piece of hardware and are now a single Famicom Disk System submenu. The swap accelerator is global, so nothing becomes slower to reach in practice. Menu-construction only. No `MenuAction` is added, removed, or re-targeted, so the dispatch side is untouched and the emulation core is not involved. Verified across all five native feature combinations (default, scripting, scripting+hd-pack, retroachievements, full) and BOTH wasm32 targets — the latter matters here specifically because this change moves `cfg(not(target_arch = "wasm32"))` blocks between nesting levels, which is the exact shape that broke the wasm build in PR #373. --- crates/rustynes-frontend/src/ui_shell.rs | 687 +++++++++++++---------- 1 file changed, 403 insertions(+), 284 deletions(-) diff --git a/crates/rustynes-frontend/src/ui_shell.rs b/crates/rustynes-frontend/src/ui_shell.rs index 2eb1b117..d834a354 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,286 @@ 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(); + } + 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 +1460,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 +1514,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 +1587,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(); + } + }); } }); From c7f18676de9f43a39c3a79aa21b3feddafaa4363 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Sun, 16 Aug 2026 22:00:36 -0400 Subject: [PATCH 3/5] fix(frontend,probe): three review findings on the Latency Oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three were real, and one of them is a defect class this project has already paid for once. `measure_in_place` restored with `restore`, not `restore_quiet`. 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 apply here: the bytes were snapshotted from this same instance moments earlier and describe the same timeline. The effect was that asking "how much input lag does this game have?" silently destroyed the user's rewind history as the price of the answer. Now `restore_quiet`. The same call discarded its `Result` with `let _ =`. The snapshot comes from `nes.snapshot()` on this instance one call earlier, so a failure would mean the snapshot format cannot round-trip itself — and returning normally would hand back a report while leaving the game several hundred frames ahead, exactly the outcome the restore exists to prevent. It now expects, with the invariant stated. The panel's felt-latency read-out multiplied by a hardcoded 16.639 ms. RustyNES emulates PAL and Dendy, whose frame is 19.9972 ms, so the panel understated their lag by 20.2%. This is 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 — which is why AGENTS.md now says to DERIVE declared values from the core's constants rather than transcribe them. The panel now captures `Nes::frame_duration()` at measurement time. Captured at measurement time, not read at render time, because it is a property of the measurement rather than of the current session: unloading the ROM, or loading a PAL game after measuring an NTSC one, must not silently restate an old result in the new region's units. `felt_milliseconds_track_the_region_not_a_constant` pins it — the test fails if the conversion is ever hardcoded again, because PAL and NTSC would then report identical milliseconds for identical frame counts. The panel's `MAX_DEPTH` was a third independent `3`. `MAX_RUN_AHEAD_DEPTH` exists in `emu.rs` precisely because `effective_run_ahead`'s cap and the throttle's cap were once separate literals that drifted (PR #358), so a third copy reopened that seam. It is now `pub(crate)` and re-exported here. Its `cfg(not(target_arch = "wasm32"))` gate is dropped: it was native-only because both its users were, and the panel compiles everywhere. The fourth finding — that a backticked `basic_bot::search` in a rustdoc comment would trip `rustdoc::private_intra_doc_links` under `-D warnings` — does not reproduce. Rustdoc resolves intra-doc links only in bracketed form; a bare code span is not a link. `RUSTDOCFLAGS="-D warnings" cargo doc -p rustynes-probe --no-deps` is clean. Left as written. --- .../src/debugger/latency_panel.rs | 60 ++++++++++++++++--- crates/rustynes-frontend/src/emu.rs | 10 ++-- crates/rustynes-probe/src/latency.rs | 16 ++++- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/crates/rustynes-frontend/src/debugger/latency_panel.rs b/crates/rustynes-frontend/src/debugger/latency_panel.rs index ad1de4f0..f4702825 100644 --- a/crates/rustynes-frontend/src/debugger/latency_panel.rs +++ b/crates/rustynes-frontend/src/debugger/latency_panel.rs @@ -32,16 +32,28 @@ use rustynes_probe::latency::{self, Confidence, LatencyConfig, LatencyReport}; /// Highest depth this panel will ever recommend. /// -/// Matches the Emulation menu's run-ahead range. A game measuring higher than -/// this is reported honestly and the recommendation is clamped, rather than the -/// measurement being silently discarded. -const MAX_DEPTH: u32 = 3; +/// 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, @@ -114,7 +126,13 @@ fn body(ui: &mut egui::Ui, state: &mut LatencyPanel, can_measure: bool, current: if let Some(report) = &state.report { ui.separator(); - report_body(ui, report, current, &mut state.pending_apply); + report_body( + ui, + report, + current, + state.frame_ms, + &mut state.pending_apply, + ); } if !state.status.is_empty() { @@ -128,15 +146,20 @@ 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: one - // NTSC frame is 16.639 ms. - #[allow(clippy::cast_precision_loss)] // small counts; display only. - let ms = f64::from(frames) * 16.639; + // 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 { @@ -206,6 +229,9 @@ fn run_measurement(state: &mut LatencyPanel, nes: Option<&mut Nes>) { "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()); @@ -261,6 +287,22 @@ mod tests { 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" + ); + } + /// `take_pending_apply` drains, so one Apply click cannot be consumed twice /// and re-applied on a later frame. #[test] 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-probe/src/latency.rs b/crates/rustynes-probe/src/latency.rs index a8b3b767..187efdc7 100644 --- a/crates/rustynes-probe/src/latency.rs +++ b/crates/rustynes-probe/src/latency.rs @@ -192,7 +192,21 @@ pub fn measure_in_place(nes: &mut Nes, cfg: LatencyConfig) -> LatencyReport { 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. - let _ = nes.restore(&restore_point); + // + // `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 } From ae19796eddc4788504abba596b266e154ea0122a Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Sun, 16 Aug 2026 22:30:00 -0400 Subject: [PATCH 4/5] fix(frontend): drop an emoji from the Latency Oracle button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The measure button read `"\u{23F1} Measure now"` — U+23F1 STOPWATCH, an emoji, in code. The project style rule forbids emojis in code, commits, comments, and docs outright, so this is a rule violation and not a preference; it went in because the button was written as a bare string literal instead of going through the icon helper like every other labelled control in the shell. Now `icons::label(glyph::GAUGE, "Measure now")`. `glyph::GAUGE` is a private-use-area codepoint from the bundled icon font rather than a Unicode emoji, and it is the same glyph the Tools -> Analysis menu entry uses, so the button inside the panel now matches the item that opens it. Swept the two new files for any other emoji codepoint across the pictograph, dingbat, misc-symbol, variation-selector, and misc-technical blocks. Clean. Found by the Antigravity reviewer, which posted it as a plain PR comment rather than as a review or a thread — invisible to a resolve-every-thread sweep and to a `reviews[].body` read alike. That is the third distinct place a bot finding has hidden on this project; the ceremony has to check issue comments too, not just review bodies. --- .../rustynes-frontend/src/debugger/latency_panel.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/rustynes-frontend/src/debugger/latency_panel.rs b/crates/rustynes-frontend/src/debugger/latency_panel.rs index f4702825..cd958d6e 100644 --- a/crates/rustynes-frontend/src/debugger/latency_panel.rs +++ b/crates/rustynes-frontend/src/debugger/latency_panel.rs @@ -30,6 +30,8 @@ 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 @@ -113,8 +115,17 @@ fn body(ui: &mut egui::Ui, state: &mut LatencyPanel, can_measure: bool, current: ); 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("\u{23F1} Measure now")) + .add_enabled( + can_measure, + egui::Button::new(ic(glyph::GAUGE, "Measure now")), + ) .clicked() { state.measure_requested = true; From de7c13d7093f542b0f260f376f7dbd26c0369391 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Sun, 16 Aug 2026 22:48:11 -0400 Subject: [PATCH 5/5] fix(frontend): clear the latency report on a ROM change; gate a wasm separator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings, both real. A Latency Oracle report survived a ROM transition. `App::close_rom`, `App::load_rom_from_path`, and the wasm `RomLoaded` path each end a TAStudio session, because it anchored on an emulator instance that no longer exists — and nothing did the equivalent for the latency panel. So a measurement taken on one game stayed on screen as a confident statement about the next one. The worse half is `pending_apply`. It is the queued Apply click, and it survived too, which means a run-ahead depth measured for game A sat one click away from being applied while game B was running. That inverts the panel's central property: the reason it recommends rather than applies is that a wrong depth silently spends frame budget the host may not have, and a depth measured on a different cartridge is exactly a wrong depth. `DebuggerOverlay::clear_latency_report` now sits beside `clear_tas_editor` at all three transition sites, and `clearing_discards_the_report_and_any_queued_apply` pins both halves — the report and the queued depth — rather than only the visible one. Second: an ungated separator directly above the `cfg(not(wasm32))` movie interop block. On wasm those items compile out and the separator collapses onto the one below them, rendering two rules with nothing between. It now carries the same gate as the block it introduces, matching the treatment already applied to the session-services separator in the same menu. That one was gated for exactly this reason during the reorg and this one was missed, which is a fair catch — the reorg moved several `cfg` blocks between nesting levels and the wasm build compiles cleanly either way, so nothing but a reading of the rendered menu would have surfaced it. Verified: frontend suite 501 passing, clippy clean across default, `scripting`, `scripting,hd-pack`, `retroachievements` and `full`, both wasm32 targets, and `RUSTDOCFLAGS="-D warnings" cargo doc --workspace`. --- crates/rustynes-frontend/src/app.rs | 12 ++++++ .../src/debugger/latency_panel.rs | 42 +++++++++++++++++++ crates/rustynes-frontend/src/debugger/mod.rs | 10 +++++ crates/rustynes-frontend/src/ui_shell.rs | 7 ++++ 4 files changed, 71 insertions(+) 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 index cd958d6e..ff2255b9 100644 --- a/crates/rustynes-frontend/src/debugger/latency_panel.rs +++ b/crates/rustynes-frontend/src/debugger/latency_panel.rs @@ -66,6 +66,21 @@ pub struct LatencyPanel { } 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 @@ -314,6 +329,33 @@ mod tests { ); } + /// 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] diff --git a/crates/rustynes-frontend/src/debugger/mod.rs b/crates/rustynes-frontend/src/debugger/mod.rs index b2a3e3e0..5745258a 100644 --- a/crates/rustynes-frontend/src/debugger/mod.rs +++ b/crates/rustynes-frontend/src/debugger/mod.rs @@ -1106,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 diff --git a/crates/rustynes-frontend/src/ui_shell.rs b/crates/rustynes-frontend/src/ui_shell.rs index d834a354..ef7b2162 100644 --- a/crates/rustynes-frontend/src/ui_shell.rs +++ b/crates/rustynes-frontend/src/ui_shell.rs @@ -1182,6 +1182,13 @@ impl UiShell { 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