From 005a2c6291a3a6a8d43812be167b01245b7b85a0 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Wed, 19 Aug 2026 14:41:49 -0400 Subject: [PATCH 1/2] feat(frontend): export a classified address from the RAM Atlas into RAM Watch v2.3.9 item C. The RAM Atlas classifies all 2 KiB of work RAM and can verify one address by perturbing it, but until now a verified address could not leave the panel. The atlas is cleared at every ROM transition by design -- it describes one observation window on one cartridge -- so "the address I just proved matters" had to be transcribed by hand into the tool that keeps it. RAM Watch is that tool, so it is the destination rather than a second list. `MemoryComparePanelState::seed_watch` appends on another panel's behalf and returns which of two things happened. Two outcomes rather than a bool: a second click on an address already present did not grow the list, so reporting "added" would be a false report of work done, and reporting a failure would be false in the other direction. That is the same refusal to collapse distinct answers that keeps `Untested` and `Inert` apart in the classifier itself. The seeded entry is byte-scoped and never inherits the panel's own size selector. The atlas's evidence -- change count, direction, range, distinct values -- is per byte, so seeding a u16 would attach a byte's verdict to a second address nothing was ever observed about. Widening it afterwards is a decision the user has made rather than one made for them. It also arrives unfrozen, since a frozen entry writes to the game and an export must not. The exported label carries the verdict AND its lens. Liveness is relative to the observable -- a byte is Live through work RAM and may be Inert through the screen -- and the watch list is precisely where an unqualified "LIVE" would outlive the panel that qualified it. `Untested` is spelled out rather than left blank, because once the atlas is gone an entry with no verdict and an entry that was never tested look identical, and an untested export cites no lens at all: nothing was observed through one, so naming one would dress a hypothesis as a measurement. The button is offered for every address, including `Inert` and `Untested`. Restricting it to verified-live addresses would be the paternalistic reading of the panel's honesty rule; the rule is that a claim carries its evidence, not that unverified addresses are unusable. It is also not gated on the netplay / TAS / hardcore predicate that gates Observe and Verify, because it writes to another panel's list and never advances the emulator. The dispatch lives in the overlay, which is the only place holding both panels' state, and it opens the destination window as part of the export. Memory Compare is drawn earlier in the same pass, so the seeded row appears on the next frame -- which is why the outcome is also reported on the atlas's own status line, where the user is already looking. An export that succeeds silently and one that silently does nothing are otherwise indistinguishable, which is the defect shape this release keeps finding. Six tests, five of them mutation-checked: removing the duplicate guard, inheriting the size selector, dropping the lens from a Live verdict, labelling an untested address as Live, and cloning the request instead of taking it each fail exactly their own test. --- .../src/debugger/atlas_panel.rs | 153 ++++++++++++++++++ .../src/debugger/memory_compare_panel.rs | 82 ++++++++++ crates/rustynes-frontend/src/debugger/mod.rs | 24 +++ 3 files changed, 259 insertions(+) diff --git a/crates/rustynes-frontend/src/debugger/atlas_panel.rs b/crates/rustynes-frontend/src/debugger/atlas_panel.rs index 90405fd1..2a225ee3 100644 --- a/crates/rustynes-frontend/src/debugger/atlas_panel.rs +++ b/crates/rustynes-frontend/src/debugger/atlas_panel.rs @@ -95,6 +95,11 @@ pub struct AtlasPanel { verify_batch_requested: bool, /// The address whose detail is expanded. selected: Option, + /// An address the user asked to send to RAM Watch, with the label the atlas + /// can honestly attach to it. Drained by the overlay after this panel's + /// render, for the same reason as the two emulator actions: the destination + /// panel's state is not reachable from here. + watch_requested: Option<(u16, String)>, /// Status / cost line. status: String, } @@ -114,6 +119,7 @@ impl Default for AtlasPanel { verify_requested: None, verify_batch_requested: false, selected: None, + watch_requested: None, status: String::new(), } } @@ -129,6 +135,27 @@ impl AtlasPanel { pub fn clear(&mut self) { *self = Self::default(); } + + /// Take a pending RAM Watch export, if one was requested this frame. + /// + /// Taken rather than read so a single click seeds a single entry. The + /// alternative — leaving the request standing and de-duplicating downstream — + /// makes the panel's behaviour depend on the destination list's contents, + /// which is exactly the coupling this hand-off exists to avoid. + pub fn take_watch_request(&mut self) -> Option<(u16, String)> { + self.watch_requested.take() + } + + /// Report the outcome of an export back onto the panel's status line. + /// + /// The destination window is drawn EARLIER in the overlay's pass than this + /// one, so a seed made here does not appear over there until the next frame. + /// Without this line the user's evidence that anything happened is a list + /// they may not be looking at — an export that silently succeeds and one that + /// silently does nothing look identical. + pub fn note_export(&mut self, text: impl Into) { + self.status = text.into(); + } } /// Draw the RAM Atlas window. `nes` is `Some` only when a ROM is loaded under the @@ -438,9 +465,56 @@ fn detail(ui: &mut egui::Ui, state: &mut AtlasPanel, l: &Label, can_run: bool) { } } } + + // Offered for EVERY address, including `Untested` and `Inert`. + // + // Restricting the export to verified-live addresses would be the + // paternalistic reading of this panel's honesty rule. The rule is that a + // claim must carry its evidence, not that unverified addresses are + // unusable — an `Untested` sparse byte is a perfectly good thing to watch + // while forming a hypothesis, and `Inert` is documented here as NOT + // meaning unused. What the rule requires is that the exported entry say + // which of the three it was, which `watch_label` does. + // + // Not gated on `can_run` either: this writes to another panel's list and + // never touches the emulator, so the netplay / TAS / hardcore predicate + // that gates Observe and Verify does not apply to it. + if ui + .button(ic(glyph::EYE, "Send to RAM Watch")) + .on_hover_text( + "Adds this address to Tools -> Analysis -> Memory Compare's RAM \ + Watch list, labelled with the atlas verdict that produced it.", + ) + .clicked() + { + state.watch_requested = Some((l.addr, watch_label(l, state.lens))); + } }); } +/// The label an exported address carries into RAM Watch. +/// +/// Three things, in the order a reader needs them: the address, the behavioural +/// class, and the verification verdict **with its lens**. The lens is not +/// decoration — liveness is relative to the observable, so "live" without it is +/// the over-claim the atlas exists to avoid, and the watch list is precisely +/// where that claim would outlive the panel that qualified it. +/// +/// `Untested` is spelled out rather than omitted. An entry with no verdict and +/// an entry that was never tested look the same once the atlas is gone. +fn watch_label(l: &Label, lens: Observable) -> String { + let verdict = match l.liveness { + Liveness::Untested => "unverified".to_owned(), + Liveness::Live => format!("LIVE via {}", lens_name(lens)), + Liveness::Inert => format!("inert via {}", lens_name(lens)), + }; + format!( + "${:04X} atlas: {}, {verdict}", + l.addr, + behaviour_name(l.behaviour) + ) +} + /// The threshold that produced this label, in words. fn why(l: &Label, frames: u32) -> String { let transitions = frames.saturating_sub(1); @@ -741,6 +815,85 @@ mod tests { } } + /// The exported label must name the LENS a verdict came from. Liveness is + /// relative to the observable — a byte is `Live` through work RAM and may be + /// `Inert` through the screen — and the watch list is exactly where an + /// unqualified "LIVE" would outlive the panel that qualified it. + #[test] + fn an_exported_verdict_carries_its_lens() { + let l = label(0x0071, Behaviour::RisingCounter, Liveness::Live); + let text = watch_label(&l, Observable::Wram); + assert!(text.contains("0071"), "the address is missing: {text}"); + assert!(text.contains("rising"), "the behaviour is missing: {text}"); + assert!(text.contains("LIVE"), "the verdict is missing: {text}"); + assert!( + text.contains(lens_name(Observable::Wram)), + "the lens is missing, so the verdict over-claims: {text}" + ); + // The same label under a different lens must READ differently. Naming + // the lens is only worth anything if the name tracks the argument. + assert_ne!( + text, + watch_label(&l, Observable::Framebuffer), + "the lens name did not change with the lens" + ); + } + + /// `Untested` is spelled out rather than left blank. Once the atlas is gone, + /// an entry with no verdict and an entry that was never tested look the same, + /// which is the exact collapse this panel refuses to make elsewhere. + #[test] + fn an_unverified_export_says_so() { + let l = label(0x0300, Behaviour::Sparse, Liveness::Untested); + let text = watch_label(&l, Observable::Framebuffer); + assert!( + text.contains("unverified"), + "an untested address exported without saying so: {text}" + ); + assert!( + !text.contains("LIVE"), + "an untested address claimed a verdict: {text}" + ); + // And it must NOT name a lens: nothing was observed through one, so + // citing one would dress a hypothesis as a measurement. + assert!( + !text.contains(lens_name(Observable::Framebuffer)), + "an untested address cited a lens it never used: {text}" + ); + } + + /// The request is TAKEN, so one click seeds one entry. Left standing it would + /// re-fire every frame until the destination happened to de-duplicate it, + /// which makes this panel's behaviour depend on another panel's contents. + #[test] + fn a_watch_request_is_drained_by_the_first_taker() { + let mut p = AtlasPanel { + watch_requested: Some((0x0071, "atlas".to_owned())), + ..AtlasPanel::default() + }; + assert_eq!(p.take_watch_request(), Some((0x0071, "atlas".to_owned()))); + assert_eq!( + p.take_watch_request(), + None, + "the request survived being taken and would seed a second entry" + ); + } + + /// A ROM change must discard a pending export too. An address is a fact about + /// one cartridge's memory map; seeding it into a watch list after a different + /// game has loaded is the same class of stale claim as keeping the labels. + #[test] + fn clearing_discards_a_pending_export() { + let mut p = AtlasPanel { + watch_requested: Some((0x0071, "atlas".to_owned())), + status: "stale".to_owned(), + ..AtlasPanel::default() + }; + p.clear(); + assert_eq!(p.take_watch_request(), None); + assert!(p.status.is_empty()); + } + /// A ROM change must discard the whole atlas. Two thousand labels describing /// a cartridge that is no longer loaded is a worse lie than one stale number, /// because it looks like a map. diff --git a/crates/rustynes-frontend/src/debugger/memory_compare_panel.rs b/crates/rustynes-frontend/src/debugger/memory_compare_panel.rs index 53cdba13..ce6a49bb 100644 --- a/crates/rustynes-frontend/src/debugger/memory_compare_panel.rs +++ b/crates/rustynes-frontend/src/debugger/memory_compare_panel.rs @@ -112,6 +112,18 @@ impl CompareTo { } } +/// What [`MemoryComparePanelState::seed_watch`] did. +/// +/// Two outcomes rather than a `bool`, so a caller's status line cannot describe +/// one as the other. Neither is an error. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SeedOutcome { + /// A new entry was appended. + Added, + /// An entry for that address was already present; the list is unchanged. + AlreadyWatched, +} + /// One RAM Watch entry. #[derive(Debug, Clone)] struct WatchEntry { @@ -242,6 +254,38 @@ impl MemoryComparePanelState { self.watch_addr_text.clear(); self.watch_label_text.clear(); } + + /// Add a watch entry on behalf of another panel. + /// + /// The RAM Atlas classifies every byte of work RAM but deliberately offers + /// no way to keep an address once the panel is closed — an atlas is a + /// snapshot of one observation window, and it is cleared at every ROM + /// transition. RAM Watch is where an address the user cares about already + /// lives, so the export target is this list rather than a second one. + /// + /// `Size::U8` is not a parameter: the atlas classifies **bytes**, and its + /// evidence (change count, direction, range) is per byte. Seeding a `u16` + /// from a byte-scoped verdict would attach that evidence to a second address + /// nothing was ever observed about. The user can widen the entry afterwards, + /// which is a decision they have made rather than one made for them. + /// + /// Returns which of the two things happened, because they are different and + /// the caller has to say which. A second click on an address already watched + /// must not report "added" (the list did not grow) and must not report a + /// failure (nothing went wrong) — the same refusal to collapse distinct + /// outcomes that keeps `Untested` and `Inert` apart in the atlas itself. + pub fn seed_watch(&mut self, addr: u16, label: String) -> SeedOutcome { + if self.watches.iter().any(|w| w.addr == addr) { + return SeedOutcome::AlreadyWatched; + } + self.watches.push(WatchEntry { + addr, + size: Size::U8, + label, + frozen: None, + }); + SeedOutcome::Added + } } /// Copy the 2 KB work RAM via the (logically side-effect-free) peek. @@ -581,6 +625,44 @@ fn parse_wch(text: &str) -> Vec { mod tests { use super::*; + /// The export path from the RAM Atlas. A second click on the same address + /// must be reported as "already there", never as an addition — the list did + /// not grow, and a status line saying it did is a false report of work done. + #[test] + fn seeding_the_same_address_twice_adds_one_entry() { + let mut s = MemoryComparePanelState::default(); + assert_eq!(s.seed_watch(0x0071, "first".to_owned()), SeedOutcome::Added); + assert_eq!(s.watches.len(), 1); + assert_eq!( + s.seed_watch(0x0071, "second".to_owned()), + SeedOutcome::AlreadyWatched + ); + assert_eq!(s.watches.len(), 1, "a duplicate address grew the list"); + assert_eq!( + s.watches[0].label, "first", + "the duplicate overwrote the original entry's label" + ); + } + + /// A seeded entry is byte-scoped, because the atlas's evidence is per byte. + /// Seeding a `u16` would attach a byte's verdict to a second address nothing + /// was ever observed about. + #[test] + fn a_seeded_entry_is_one_byte_and_unfrozen() { + // The panel's own size selector is deliberately set to something else: + // the seed must not inherit whatever the user last picked in the Add row. + let mut s = MemoryComparePanelState { + watch_size: Size::U32, + ..MemoryComparePanelState::default() + }; + s.seed_watch(0x0300, "atlas".to_owned()); + assert_eq!(s.watches[0].size, Size::U8); + assert!( + s.watches[0].frozen.is_none(), + "an exported watch arrived frozen, which would write to the game" + ); + } + #[test] fn op_predicates() { assert!(Op::Eq.apply(5, 5)); diff --git a/crates/rustynes-frontend/src/debugger/mod.rs b/crates/rustynes-frontend/src/debugger/mod.rs index 760cfa1c..e20916ac 100644 --- a/crates/rustynes-frontend/src/debugger/mod.rs +++ b/crates/rustynes-frontend/src/debugger/mod.rs @@ -2305,6 +2305,30 @@ impl DebuggerOverlay { nes.as_deref_mut(), atlas_writes_locked, ); + // The atlas -> RAM Watch export (v2.3.9 item C). Dispatched here + // rather than inside the panel because the destination is another + // panel's private state, and the overlay is the only place that holds + // both. + // + // The destination window is opened as part of the export. Landing an + // entry in a list the user cannot see is the shape of defect this + // release keeps finding: it succeeds, reports nothing, and is + // indistinguishable from having done nothing at all. Memory Compare + // is drawn EARLIER in this pass, so the seeded row itself appears on + // the next frame — which is why the outcome is also reported on the + // atlas's own status line, where the user is already looking. + if let Some((addr, label)) = self.atlas_ui.take_watch_request() { + let outcome = self.memory_compare_ui.seed_watch(addr, label); + self.show_memory_compare = true; + self.atlas_ui.note_export(match outcome { + memory_compare_panel::SeedOutcome::Added => { + format!("${addr:04X} added to RAM Watch (Memory Compare).") + } + memory_compare_panel::SeedOutcome::AlreadyWatched => { + format!("${addr:04X} was already in the RAM Watch list.") + } + }); + } } // v2.3.8 "Parallax" — the Divergence Lens. Gated on the SAME predicate as // the RAM Atlas above, and for identical reasons rather than by analogy: From f3877cbe2e678fd2e7859ba6014c1297db4bb14a Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Wed, 19 Aug 2026 14:48:45 -0400 Subject: [PATCH 2/2] docs(ram-atlas): the export path, and why persistence is not it Docs-as-spec: the panel gained a behaviour and its spec did not describe it, which is the drift this project's contribution gate exists to stop. Documents the four properties of the export as the honesty rule applied to four different edges -- the label carries its lens, every address is exportable, the entry is byte-scoped and unfrozen, and a duplicate is reported as a duplicate -- each cross-referenced to the section of this document that establishes the rule it applies. Also narrows two "deliberately not implemented" entries rather than deleting them. The remaining export paths are the Cheat panel, Lua and RetroAchievements authoring, and the note now records the one way a cheat export differs: a cheat is a WRITE, so it needs the locked-session predicate the watch export correctly does without. Per-game persistence stays deferred, with the reason stated instead of implied. The obvious form is unsafe -- a restored verdict without its evidence is a claim that cannot be checked, and this panel's whole argument is that its output can be. A restored verdict WITH its evidence is still a statement about the game state the observation ran in, which the next session does not share. The export is the durable path in the meantime, and RAM Watch already has .wch save and load. --- docs/ram-atlas.md | 61 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/docs/ram-atlas.md b/docs/ram-atlas.md index c9da72f9..72362d06 100644 --- a/docs/ram-atlas.md +++ b/docs/ram-atlas.md @@ -240,16 +240,67 @@ measurement. - An atlas is ROM-bound and is discarded on every ROM transition via `DebuggerOverlay::clear_rom_bound_analysis`. 2048 stale labels are a worse lie than one stale number, because they look like a map. +- **Send to RAM Watch** exports the selected address into Memory Compare's watch + list. See below. + +## Export to RAM Watch + +An atlas is deliberately discarded at every ROM transition, which is correct and +has a cost: the address a user just *verified* by spending trials on it +disappears with everything else. `Send to RAM Watch` is the way out — the +destination is Memory Compare's existing watch list rather than a second list, +because that is already the tool that keeps addresses. + +Four properties, each of which is the honesty rule applied to a different edge: + +- **The label carries the verdict AND its lens.** Liveness is relative to the + observable (`## Liveness is relative to its lens` above), so an unqualified + "LIVE" in a list that outlives this panel is exactly the over-claim the panel + exists to avoid. `Untested` is spelled out rather than left blank — once the + atlas is gone, an entry with no verdict and an entry that was never tested look + identical — and an untested export names **no** lens, because nothing was + observed through one. +- **Every address is exportable**, including `Inert` and `Untested`. Restricting + the button to verified-live addresses would be the paternalistic reading of the + rule; the rule is that a claim carries its evidence, not that unverified + addresses are unusable. An `Untested` sparse byte is a good thing to watch while + forming a hypothesis, and `Inert` does not mean unused (`## What a label does + not mean`). +- **The entry is byte-scoped and unfrozen.** The atlas's evidence is per byte, so + seeding a `u16` would attach a byte's verdict to a second address nothing was + observed about; the user can widen it afterwards, which is a decision they made. + A frozen watch entry writes to the game, and an export must not. +- **A duplicate is reported as a duplicate.** A second click on an address already + watched did not grow the list. `SeedOutcome` keeps "added" and "already there" + apart, because reporting either as the other is a false report — the same + refusal that keeps `Untested` and `Inert` apart. + +The button is **not** gated on the netplay / TAS / hardcore predicate that gates +Observe and Verify (`## Unavailable during locked sessions`): it writes to another +panel's list and never advances the emulator. + +Memory Compare is drawn earlier in the overlay's pass than the atlas, so the +seeded row itself appears on the **next** frame. The outcome is therefore also +reported on the atlas's own status line, where the user is already looking — an +export that silently succeeds and one that silently does nothing are otherwise +indistinguishable. ## Deliberately not implemented Named here so they read as decisions rather than oversights: -- **Export paths** — seeding the Watch and Cheat panels, the Lua API, and - RetroAchievements authoring. Additive on top of the labels; better shaped once - the labels have been used in anger. -- **Per-game persistence** — an atlas keyed on the ROM hash via the - `rustynes-gamedb` overlay. +- **The remaining export paths** — the Cheat panel, the Lua API, and + RetroAchievements authoring. RAM Watch is implemented (above); the other three + are additive on top of the same request/dispatch shape and better decided once + the first one has been used in anger. A cheat is also a *write*, so it needs the + locked-session predicate the watch export correctly does without. +- **Per-game persistence** — an atlas keyed on the ROM hash. The obvious form + is unsafe: a restored verdict without its evidence is a claim that cannot be + checked, and this panel's whole argument is that its output can be. A restored + verdict *with* its evidence is still a statement about the game state the + observation ran in, which the next session does not share. The RAM Watch export + above is the durable path in the meantime, and RAM Watch already has `.wch` + save and load. - **Coordinate cross-referencing against OAM** — the original design sketched correlating candidate addresses against sprite X/Y. It is not implemented, and the `Behaviour` set makes no coordinate claim, so nothing currently over-states