v1.3.0 Workstream C — devtools (Memory Compare) + menu/Settings UX + auto-save - #84
Conversation
… C3) GeraNES-class developer tooling. A classic emulator memory search over the 2 KB CPU work RAM ($0000-$07FF): snapshot a baseline, then iteratively narrow a candidate set by how each byte moved since the last snapshot (changed / unchanged / increased / decreased / equals-value) until one address remains — the standard "find the value that dropped when you lost a life" workflow, feeding the raw-RAM cheat panel. New `debugger/memory_compare_panel.rs` + `ChipPanel::MemoryCompare`, wired exactly like the Memory viewer (show-bool, state, open_chip_panel, Debug-menu entry, the Debug-checkbox row, and the menu-bar list). Read-only: samples via the side-effect-free `cpu_bus_peek`, holds its own baseline copy, never writes the core (determinism unaffected). Disabled under RA hardcore mode like the Memory viewer + cheat panel. Unit tests for the filter predicates + value parser. Verified: clippy -D warnings clean (default + scripting,hd-pack + retroachievements + wasm); fmt; frontend builds; panel tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
….3.0 menu reorg) Part of the approved v1.3.0 menu reorganization (Workstream C / UX): - File menu gains a "Close ROM" item (MenuAction::CloseRom + App::close_rom): tears down the Nes, clears perf/buffers/label, stops emu-thread production, returns to the no-ROM state. Gated on a loaded ROM + no netplay session. - Removed the debugger overlay's 19-checkbox panel-toggle toolbar (it duplicated the always-visible menu-bar Debug + Tools menus). The overlay top bar keeps only the live read-outs the menu bar lacks (frame/cycle, fps, movie status); panels now open solely from the menu bar. Verified: clippy -D warnings (default + wasm) clean; fmt; builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (v1.3.0) Workstream C menu/UX polish (GUI-unverifiable by CI; verify on-device): - Top-level order is now File / Emulation / View / Tools / Debug / Help. - File: add Close ROM; group save-state actions under a Save States submenu (Save/Load State, Active Slot, Save-/Load-to-Slot, Manage States). - Swap Disk Side moved File -> Emulation; NSF Player moved Debug -> Tools; Performance Monitor moved Tools -> Debug; the standalone "Mod" menu folded into a Tools "HD Pack" submenu (still hd-pack-feature + native-gated). - Settings window: split the composable shader stack into a dedicated Shaders tab and rename "Advanced" -> "Emulation" (run-ahead + rewind). The window now snapshots the config each frame and persists on any change, so every setting in every tab auto-saves (per-control save_config calls kept as a backstop). - Document the egui-0.34 menu close model in ui_shell::menu_bar; the "menu lingers until several clicks" report (BUG-3) needs an on-device repro to pin the MenuState trigger and is NOT hacked blind (default is CloseOnClick). Pure UI, no determinism surface. Gates green: fmt; clippy -D warnings (default, retroachievements, scripting+hd-pack, both wasm flavours); markdownlint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a new Memory Compare (cheat-hunt) debugger panel, reorganizes the menu bar layout, adds a feature to close the currently loaded ROM, splits the shader stack into its own Settings tab, and implements auto-saving for settings. The reviewer provided several constructive suggestions: clearing additional staging buffers in close_rom to prevent stale state, optimizing the memory search apply function by avoiding redundant cpu_bus_peek calls, resolving inconsistent radix parsing for bare numbers in parse_byte, and avoiding per-frame allocations caused by cloning the entire Config structure while the Settings window is open.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn close_rom(&mut self) { | ||
| { | ||
| let mut guard = self.emu.lock(); | ||
| let emu = &mut *guard; | ||
| emu.nes = None; | ||
| emu.perf.clear(); | ||
| emu.present_fb.clear(); | ||
| emu.audio_buf.clear(); | ||
| emu.next_frame_time = None; | ||
| } | ||
| // Stop the dedicated emulation thread from producing frames. | ||
| #[cfg(all(not(target_arch = "wasm32"), feature = "emu-thread"))] | ||
| if let Some(thread) = self.emu_thread.as_ref() { | ||
| thread.control().set_has_rom(false); | ||
| } | ||
| self.rom_label = String::new(); | ||
| self.rom_bytes = Vec::new(); | ||
| self.present_staging.clear(); | ||
| self.ui | ||
| .set_status(crate::ui_shell::StatusMessage::info("ROM closed")); | ||
| } |
There was a problem hiding this comment.
In close_rom, while self.present_staging is cleared, other staging buffers like self.present_index_staging (and self.present_hd_tiles / self.present_chr_snapshot if the hd-pack feature is enabled) are not cleared. To prevent stale state from lingering and to free up allocated memory when no ROM is loaded, these buffers should also be cleared.
fn close_rom(&mut self) {
{
let mut guard = self.emu.lock();
let emu = &mut *guard;
emu.nes = None;
emu.perf.clear();
emu.present_fb.clear();
emu.audio_buf.clear();
emu.next_frame_time = None;
}
// Stop the dedicated emulation thread from producing frames.
#[cfg(all(not(target_arch = | fn apply(&mut self, nes: &mut Nes) { | ||
| let (Some(base), Some(cands)) = (self.baseline.as_ref(), self.candidates.as_mut()) else { | ||
| return; | ||
| }; | ||
| let eq = parse_byte(&self.eq_text).unwrap_or(0); | ||
| let filter = self.filter; | ||
| cands.retain(|&addr| { | ||
| let cur = nes.cpu_bus_peek(addr); | ||
| filter.matches(base[addr as usize], cur, eq) | ||
| }); | ||
| self.baseline = Some(snapshot(nes)); | ||
| self.steps += 1; | ||
| } |
There was a problem hiding this comment.
Instead of calling cpu_bus_peek individually for each candidate (up to 2048 times) and then calling snapshot (which reads all 2048 bytes again), you can take the snapshot once at the beginning of apply. This avoids redundant cpu_bus_peek calls and improves efficiency.
fn apply(&mut self, nes: &mut Nes) {
let (Some(base), Some(cands)) = (self.baseline.as_ref(), self.candidates.as_mut()) else {
return;
};
let cur_ram = snapshot(nes);
let eq = parse_byte(&self.eq_text).unwrap_or(0);
let filter = self.filter;
cands.retain(|&addr| {
let cur = cur_ram[addr as usize];
filter.matches(base[addr as usize], cur, eq)
});
self.baseline = Some(cur_ram);
self.steps += 1;
}| fn parse_byte(s: &str) -> Option<u8> { | ||
| let t = s.trim(); | ||
| t.strip_prefix('$') | ||
| .or_else(|| t.strip_prefix("0x")) | ||
| .map_or_else( | ||
| || { | ||
| t.parse::<u8>() | ||
| .ok() | ||
| .or_else(|| u8::from_str_radix(t, 16).ok()) | ||
| }, | ||
| |hex| u8::from_str_radix(hex, 16).ok(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
The parse_byte function has inconsistent parsing behavior for bare numbers (without $ or 0x prefixes). A bare input like "10" is parsed as decimal 10 because t.parse::<u8>() succeeds first, whereas "1A" is parsed as hex 26 (0x1A) via the fallback. This means the radix of bare inputs implicitly changes depending on whether they contain non-decimal hex characters (A-F). It is recommended to either treat all bare inputs consistently (e.g., always decimal, or always hex) or remove the bare hex fallback entirely to enforce explicit prefixes.
| // This guarantees persistence for every setting in every tab even where | ||
| // an individual control only flags a live-apply (`state.apply.*`) without | ||
| // its own `save_config` call. (`Config: Clone + PartialEq`.) | ||
| let config_before = config.clone(); |
There was a problem hiding this comment.
Cloning the entire Config structure (which contains heap-allocated Vecs, Strings, and HashMaps) on every single frame while the Settings window is open introduces unnecessary allocation and CPU overhead. Since this project prioritizes high-performance and avoiding frame-rate stutter, we should avoid per-frame allocations. Consider only performing the clone/comparison when there is active user interaction in the settings window (e.g., ui.ctx().input(|i| i.pointer.any_down() || !i.keys_down.is_empty())), or detecting changes via state.apply.any().
Cut the v1.3.0 "Bedrock" foundation + breadth release on the cycle-accurate v1.0.0 core. Bumps workspace + crate versions 1.2.0 -> 1.3.0, finalizes CHANGELOG [Unreleased] -> [1.3.0], and updates README / STATUS / CLAUDE. Shipped (all merged, additive/off-by-default, AccuracyCoin 100% 139/139 held): edition 2024 / Rust 1.96 / egui 0.34.3 + wgpu 29.0.3 + rfd 0.17.2 toolchain (#79/#80); frame-pacing fix (#82); Memory Compare + menu/Settings reorg + per-setting auto-save (#84); mapper coverage 87 -> 101 BestEffort sweep (#85) + Vs. DualSystem header detection (#86) + the m218 16K-PRG fix + BestEffort boot-smoke screenshots (#87); HD-pack <condition>/<background> rules (#88, ADR 0014); netplay desync diagnostics + niche peripheral aliases (#89). PGO/BOLT gate exercised. C1 hard-tier residuals re-baselined (cpu_interrupts_v2 closed; 3 deferred to v2.0). Carryover (maintainer-manual): casual-mode browser RetroAchievements (ADR 0015 — needs Emscripten/pure-Rust rcheevos->wasm + live-browser verify; native RA unaffected); plus the v1.2.0-era F1 on-device touch + F3 live-netplay matrix. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
v1.3.0 Workstream C — developer tooling + menu/Settings UX overhaul
Additive, native + UI only; no determinism surface (AccuracyCoin / oracle untouched).
Memory Compare panel (C3)
A cheat-hunt panel: snapshot the 2 KB work RAM and filter by changed / unchanged /
increased / decreased / equals across snapshots (the classic Cheat Engine workflow).
Read-only via
cpu_bus_peek; integrity-guarded and RA-hardcore-gated like theexisting cheat tooling. New
ChipPanel::MemoryCompare.Menu-bar reorganization
submenu (Save/Load State, Active Slot, Save-/Load-to-Slot, Manage States).
Performance Monitor Tools → Debug; the standalone "Mod" menu folded into a
Tools HD Pack submenu (still
hd-pack-feature + native-gated).every panel).
Settings window
"Advanced" renamed Emulation (run-ahead + rewind).
change, so every setting in every tab is sticky (per-control
save_configcallskept as a redundant backstop).
egui-0.34 menu close behavior (BUG-3)
Documented the egui-0.34 close model (default
CloseOnClick, gated by egui'sdeepest-submenu tracking) in
ui_shell::menu_bar. The "menu lingers until severalclicks" report needs an on-device repro to pin the exact
MenuStatetrigger and isnot changed blind (a wrong change would regress menu selection).
Gates
cargo fmt --check;clippy -D warnings(default +retroachievements+scripting,hd-pack+ both wasm flavours); markdownlint. Docs (docs/frontend.md)and
CHANGELOG.md [Unreleased]updated.