feat(v2.0.0): beta.5 — Vs. DualSystem dual-console support + R1/R2 closure campaign - #221
Merged
Merged
Conversation
Teach the Vs. System board (iNES mapper 99) the three hardware facts a
DualSystem cabinet adds on top of a UniSystem cart, behind default-no-op
Mapper trait hooks so every other board (and every single-console
mapper-99 boot) is byte-identical:
- Shared 2 KiB work RAM at $6000-$7FFF, mirrored across the 8 KiB window
(MAME vsnes.cpp: `map(0x6000, 0x67ff).mirror(0x1800).ram()`). Each
console's mapper instance holds a COPY plus a write log
(`take_vs_dual_wram_writes` / `apply_vs_dual_wram_write`); the
VsDualSystem wrapper converges the copies after every stepped
instruction. This realizes MAME's fully-shared `.share("nvram")`
memory model at soft-lockstep granularity WITHOUT aliasing (no
Rc/RefCell in the no_std chip stack). The model choice is
evidence-driven: nesdev/Mesen2 document a $4016-bit-1 access mux
(exclusive ownership), but Balloon Fight's boot handshake polls a
shared-WRAM mailbox ($6220) while the mux would deny its partner
access — under exclusive routing the boot provably deadlocks, and
MAME (where the four DualSystem games verifiably run) shares the RAM
unconditionally.
- Sub-console PRG/CHR banking (`set_vs_dual_sub`): the cabinet's two
CPUs run DIFFERENT programs. MAME `balonfgt` loads distinct ROMs into
its `prg` and `sub` regions (the sub's 6d/6a chips differ from the
main's 1d/1a by CRC); Mesen2 banks `prgOuter = main ? 0 : 4` in 8 KiB
pages. The sub instance offsets PRG reads by 32 KiB and ORs CHR page
bit 1, both modulo the ROM size — so a 32 KiB (main-half-only) dump
wraps onto the same program and a proper 64 KiB dual dump splits.
- A versioned save-state: UniSystem carts keep emitting the v1 layout
byte-identically; a provisioned dual WRAM bumps to v2 (v1 + the 2 KiB
tail). The write log is transient (always drained within the stepping
loop) and deliberately not serialized; the sub identity is cabinet
wiring re-applied by the wrapper, like the bus's $4016 bit-7 flag.
Part of v2.0.0 beta.5 (plan Workstream C, ADR 0002).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Give LockstepBus the cabinet-side half of the DualSystem cross-wiring, strictly following the design rule from docs/audit/vs-dualsystem-design-2026-06-11.md: the two consoles' buses NEVER reference each other — each bus only reports its own signals and accepts externally-driven levels, and the VsDualSystem wrapper owns all routing. Everything is dead on a single console (two field writes on the $4016 store, an OR with an always-false flag in irq_level), so the deterministic single-console path is behavior-identical — AccuracyCoin holds 139/139 and nestest stays 0-diff. - $4016 write: latch the bit-1 (main/sub comms) LEVEL on EVERY write for the wrapper to poll (`take_vs_mainsub_edge`, poll-and-clear). Deliberately not edge-filtered: the wrapper seeds the reset-time levels itself (Mesen2 `VsControlManager::Reset` seeds main LOW / sub HIGH), so a bus-side edge filter starting from a false latch would swallow the genuine seeded-HIGH -> written-LOW transition Balloon Fight's reset performs ($8009: STA $4016 with $00 on both CPUs) and deadlock the boot handshake. Applying an unchanged level is idempotent in the wrapper. - $4016 read overlay: bit 7 = sub-console identity (`set_vs_sub`; main reads 0, sub reads $80 — MAME `ret |= Side << 7`, Mesen2 `IsVsMainConsole() ? 0x00 : 0x80`). This is how the shared program ROM decides which half it is. - External /IRQ: `set_vs_external_irq` drives a level the partner's bit-1 write controls (LOW asserts, HIGH releases — MAME `set_input_line(0, (data & 2) ? CLEAR_LINE : ASSERT_LINE)`); OR'd into `irq_level()` alongside the mapper and APU lines. - Passthroughs for the mapper-99 dual-WRAM hooks (enable / write-log drain+replay / take+set / sub banking), keeping the mapper field encapsulated. - `Nes::is_jammed()` + `Nes::step_instruction()`: the wrapper's soft-lockstep steps each console one INSTRUCTION at a time (Mesen2 `RunFrame` + `RunVsSubConsole`), so the debugger-oriented single-step needed a public, jam-guarded form that leaves the frame-complete latch for the wrapper to consume. - vs_db: the `dual_system` flag's doc now reflects that Emu::from_rom routes flagged carts to the full two-console wrapper (it was the "show a needs-support note" placeholder from v2.7.1); the SHA-keyed db is the load-bearing detection source because the circulating DualSystem dumps are iNES 1.0 with no NES 2.0 byte-13 hardware type. Part of v2.0.0 beta.5 (plan Workstream C, ADR 0002). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add rustynes-core::vs_dualsystem — the Vs. DualSystem cabinet as a
wrapper over two complete Nes instances, plus the `Emu` enum frontends
construct through (`Single(Box<Nes>)` / `Dual(Box<VsDualSystem>)`).
Architecture (docs/audit/vs-dualsystem-design-2026-06-11.md): the
wrapper owns ALL cross-wiring; the two consoles never hold references
to each other. After every stepped instruction, `pump_comms` drains
each bus's $4016 bit-1 level (driving the partner's external /IRQ —
LOW asserts, per Mesen2 `UpdateMainSubBit` / MAME
`(data & 2) ? CLEAR_LINE : ASSERT_LINE`) and each mapper's shared-WRAM
write log (replaying it into the partner's copy — MAME's
fully-shared `.share("nvram")` model; see the mapper commit for why
the nesdev/Mesen2 exclusive-access mux provably deadlocks Balloon
Fight's boot and is not used).
Stepping mirrors Mesen2 `NesConsole::RunFrame` + `RunVsSubConsole`:
the main console steps one instruction, then the sub drains until it
is within a 5-CPU-cycle gap (or has caught the main's frame). The gap
comparison is overshoot-safe (`main > sub.saturating_add(5)`): an
instruction advances 2..=8 cycles, so the sub routinely lands AHEAD of
the main — a naive `wrapping_sub(..) > 5` wraps to a huge unsigned
value there and runs the sub away unboundedly (the first
implementation of this loop did exactly that: ~68 CPU-minutes and
5.5 GiB of undrained audio before diagnosis).
Reset-time seed per Mesen2 `VsControlManager::Reset`
(`UpdateMainSubBit(main ? 0x00 : 0x02)`): the main boots bit-1 LOW
(sub /IRQ asserted), the sub HIGH (main /IRQ clear) — Wrecking Crew's
handshake requires it.
Cabinet routing: controller ports 0/1 -> main, 2/3 -> sub P1/P2;
coin acceptors 0/1 -> main, 2/3 -> sub; per-panel service buttons;
per-console DIP banks fall out structurally (two buses, two vs_dip
bytes — Mesen2's `dipSwitches >> 8`).
Save states: an `RVSD` + u16-version container nesting the two
u32-length-prefixed Nes snapshots plus the wrapper's bit-1 latch byte.
Restore re-drives the cross-IRQ levels from the latch and re-converges
the two shared-WRAM copies from one buffer (main's copy is
authoritative), so a cross-restore can never leave the cabinet with
diverged RAMs.
`Emu::from_rom` detection ORs the NES 2.0 byte-13 hardware type
(5/6 = DualSystem) with the SHA-keyed vs_db `dual_system` flag; the
db is load-bearing because the circulating dumps are iNES 1.0.
Out of scope by design (plan + design doc): netplay rollback and
RetroAchievements do not support the dual path; the frontend drains
the main console's mixer only.
Part of v2.0.0 beta.5 (plan Workstream C, ADR 0002).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three-layer verification for the beta.5 dual-console work, honest about what each layer can and cannot prove: - tests/vs_dualsystem_synth.rs (CI-committable, no external assets): builds a synthetic NES 2.0 DualSystem cart in-code — 64 KiB PRG carrying two DIFFERENT hand-assembled 6502 programs (byte-13 hardware type 5) — and proves every wire of the cabinet model end-to-end: header-based dual detection through Emu::from_rom, sub-console second-PRG-half banking (the sub's identity check fail-marks if it ran the main's program), $4016 bit-7 identity (main 0 / sub $80), shared-WRAM convergence (an $11/$22 mailbox exchange visible from both sides), the cross-IRQ protocol in both directions (main asserts the sub's /IRQ via bit-1 LOW; the sub's handler answers through WRAM and pulses the main back), and the RVSD snapshot round-trip continuing cycle-identically. 3/3 green. - tests/vs_dualsystem.rs (commercial-roms gated): boots the four staged GVS dumps through Emu::from_rom. The snapshot round-trip passes; the four boot tests are #[ignore]d WITH the evidence in the reason — the circulating 32 KiB GVS dumps are the MAME maincpu region ONLY (byte-for-byte: GVS Balloon Fight's PRG chunks CRC32-match balonfgt's mds-bf4 a-3.1d/1c/1b/1a; the cabinet's sub CPU runs the different .6d/.6a ROMs, absent from the dumps; GVS Tennis matches vstennisa's main region the same way). The main program's boot handshake waits forever on a sub-side answer ($AA at shared-WRAM offset $220) only the missing sub program can write, so these dumps cannot boot dual on ANY emulator. Re-enable when combined 64 KiB dual dumps are staged. - src/bin/vs_dual_trace (diagnostic): boots a dump through VsDualSystem, prints per-console PC histograms, framebuffer-colour checkpoints, shared-mailbox samples, and a dense two-CPU instruction trace with every comms level + WRAM write annotated. This is the tool that localized the boot deadlock to the $81CF mailbox exchange and disproved the exclusive-WRAM model (both CPUs polling $6220 for a value with no writer anywhere in the 32 KiB binary). Part of v2.0.0 beta.5 (plan Workstream C, ADR 0002). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…core The R1/R2 closure campaign (the bounded effort the beta.3 escape hatch deferred, run on the fully promoted one-clock/every-cycle core) regenerated the mmc3_test_2/4-scanline_timing irq_trace goldens: the committed set was captured on the pre-promote core and no longer reflects the shipping timeline. The fresh capture (fixture green) records the mapper irq_pending asserts at (frame 43, scanline 0, dot 260) / (frame 71, 0, 261) with services at dots 279/280 — the ground truth for the campaign's analysis. Campaign outcome (full record: docs/audit/r1r2-closure-campaign-2026-07-02.md, local): both structural hypotheses FALSIFIED with clean gates — (1) the sprite-fetch A12 emission-dot shift (260->259) is absorbed by CPU-cycle batch quantization; (2) Mesen2's do-while catch-up boundary semantics (the exact-boundary dot executing in the current batch — a real structural difference vs our check-first run_ppu_to) held AccuracyCoin 139/139 + the C1 trio + nestest AND left all four target brackets unchanged. Mechanism identified: the blargg bracket measures the interval between two same-timeline observations ($2002 VBL read -> IRQ window), so ANY consistent batch re-phasing shifts both legs together and cancels — the residual is differential and unreachable on any 3-dots-per-cycle-batched catch-up model. The next credible attempt is the per-dot interleaved scheduler (the documented post-v2.0.0 axis). The four pins stay by-design #[ignore]'d; the two falsified levers join the DO-NOT-RETRY list via the rc.1 ADR-0002 update. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dump
The four Vs. DualSystem boards (Tennis / Mahjong / Wrecking Crew /
Balloon Fight) run two complete NES consoles sharing a comms latch and a
2 KiB WRAM mailbox. Every previously-staged "GVS" dump at
tests/roms/external/vs-system/ is 32 KiB of PRG -- the MAME `maincpu`
region only. VsSystem::cpu_read's dual_sub branch banks the SUB
console's PRG from offset 0x8000 into the cart's PRG array, so with only
32 KiB present the sub console silently re-runs a wrapped copy of the
MAIN program instead of its own code, and the boot handshake -- which
polls for a sub-side mailbox answer -- deadlocks instantly (flat,
single-colour framebuffer on both sides, as previously documented in the
four #[ignore]'d tests in vs_dualsystem.rs).
Two of the four titles' missing sub-CPU program ROMs were located in a
legitimately-owned MAME arcade romset (balonfgt.zip / wrecking.zip;
Tennis and Mahjong were not present in that set and remain out of
scope). A combined 64 KiB-PRG dump was assembled locally for each
(gitignored under tests/roms/external/, never committed) by
concatenating the existing 32 KiB main-half PRG with the sub-CPU PRG
chunks extracted from the zip, in the same empirically-confirmed
d,c,b,a chunk order the main half already uses, plus the shared CHR
verbatim. Full assembly method, CRC32/SHA-256 verification chain, and
boot-outcome writeup: docs/audit/vs-dualsystem-combined-dumps-2026-07-02.md
(local-only per this repo's docs/audit/ convention -- see .gitignore).
Findings, verified via crates/rustynes-test-harness/src/bin/vs_dual_trace.rs
and the new tests below:
- Balloon Fight boots for real on the combined dump. The handshake
completes (neither CPU jams over 1200 frames / 20 simulated seconds),
and both consoles render an identical, legible attract-mode menu
("1PLAYER VS. COMPUTER" / "2PLAYERS MUST USE BOTH SCREENS" / a credit
counter that exactly tracks the 10 simulated coin pulses the harness
performs). This is qualitatively different from the 32 KiB dump's
total deadlock, not just "less broken."
- Wrecking Crew is inconclusive. The wrapper's cross-wiring is
demonstrably active (bidirectional $4016 bit-1 /IRQ toggling and
shared-WRAM writes observed in a dense instruction trace -- a real
improvement over instant deadlock), but the framebuffer never exceeds
3 distinct colours and oscillates on a stable ~600-frame period
whether or not simulated coins are inserted (re-verified with coin
injection disabled), so simulated-input timing is ruled out as the
blocker. This does not distinguish a missing input sequence from a
residual cross-wiring bug specific to this title's handshake shape,
and per this project's testing discipline the corresponding test
stays #[ignore]'d rather than asserting a false positive.
vs_dualsystem.rs changes:
- New passing test `gvs_balloon_fight_dual_combined_boots`, which
deliberately does NOT reuse the existing `assert_dual_alive` helper:
that helper's `> 4 distinct colours` heuristic and `main != sub`
framebuffer check are tuned for typical multi-colour commercial titles
with diverged player views, both of which this legitimate two-colour,
pre-divergence attract screen violates by design. Instead it asserts
the CPUs aren't jammed and pins an `insta` hash-based text snapshot
(cycle counts, distinct-colour counts, FNV-1a64 framebuffer hashes)
following this crate's existing external_coverage.rs convention, after
visually confirming the dumped PNGs show real, legible game text.
- New `#[ignore]`d diagnostic `diag_gvs_wrecking_crew_dual_combined`,
asserting only the confirmed-true fact (neither CPU jams) with a
reason string pointing at the audit doc for the next investigation
session.
- Module docs updated with the 2026-07-02 finding; the original four
32 KiB-only boot tests are untouched (still #[ignore]'d, still
expected-fail on those specific dumps).
vs_db.rs changes:
- Two new `entry_dual(...)` SHA-256 rows for the combined dumps
(Balloon Fight vs_dip=0x00/Rp2C04_0003, Wrecking Crew
vs_dip=0xF8/Rp2C04_0002 -- identical DIP/PPU values to the existing
32 KiB-only entries for the same games, since those describe the
cabinet, not the dump completeness), inserted at the correct sorted
position (enforced by db_is_sorted_by_sha256). The pre-existing
32 KiB-only entries are untouched and still resolve correctly; loading
that specific incomplete dump still flags dual_system (still routes to
VsDualSystem) and simply can't complete the handshake, which remains
expected/harmless.
- `exactly_the_four_dualsystem_carts_are_flagged` renamed to
`exactly_six_dualsystem_rows_across_the_four_carts_are_flagged` and
its assertion updated from 4 to 6: still exactly 4 DualSystem GAMES,
but 2 of them now have both an incomplete and a complete dump row.
No changes to crates/rustynes-core/src/vs_dualsystem.rs (the wrapper's
cross-wiring logic) -- both the Balloon Fight success and the Wrecking
Crew partial result were produced by the existing model against newly
supplied ROM data; no bug was found or fixed in the wrapper itself
during this investigation. Vs. Tennis and Vs. Mahjong are completely
untouched.
Verification: cargo fmt --all --check, cargo clippy --workspace
--all-targets -D warnings (plus the scripting / hd-pack / retroachievements
frontend feature combos), cargo test --workspace (full dev-profile run,
zero failures), and targeted re-runs of
crates/rustynes-test-harness/tests/vs_dualsystem.rs (2 passed / 5
ignored) and vs_dualsystem_synth.rs (3/3, confirming the synthetic-cart
protocol test that CI actually gates on is untouched) under
--features commercial-roms,test-roms.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…isibility (default-off, falsified) The maintainer asked for one more bounded attempt at the R1/R2 residual (the MMC3 1-CPU-cycle IRQ-timing bracket in mmc3_test_2/4-scanline_timing sub-test #3 and its mmc3_test_v1/{4,5,6} siblings), chartered as implementing a "genuine per-dot interleaved CPU/PPU scheduler" per Session-18's Mesen2 StartCpuCycle/Read/EndCpuCycle comparison in ADR-0002. Investigation showed that model already shipped: the v2.0.0 beta.1-beta.4 promote made Cpu::start_cycle -> access -> Cpu::end_cycle (each half calling Bus::run_ppu_to, which ticks the PPU one whole dot per iteration) the ONLY scheduler path, unconditionally. There is no remaining coarse-batch axis to split further. What this commit actually adds is the one concrete, previously-untested lever that remained: real M2-phase (pre-access/low vs post-access/high) visibility for MMC3's IRQ-pending-line assertion, gated behind two paired default-off features (rustynes-core/mmc3-m2-phase-irq, rustynes-mappers/mmc3-m2-phase-irq, forwarded through rustynes-test-harness). Instrumentation this session found the M2-phase plumbing ADR-0002 describes (Mapper::notify_a12_at_sub_dot's "sub-dot 0/1=low, 2=high" convention) was never actually wired to carry real phase data on the live R1 scheduler path -- LockstepBus::run_ppu_to constructed its PpuBusAdapter from a call-LOCAL sub_dot counter that resets to 0 on every invocation, and since run_ppu_to is called twice per CPU cycle (once per half) with each half typically ticking at most one dot, the value threaded to the mapper was almost always 0 regardless of which half produced the transition. Bus::run_ppu_to gained an is_post_access: bool parameter (threaded from the two call sites in Cpu::start_cycle/end_cycle); LockstepBus::run_ppu_to now seeds sub_dot from that real phase under the feature (0 pre-access, 2 post-access, matching the documented convention) instead of the always-reset counter. Mmc3::notify_a12_at_sub_dot uses this to defer a qualifying A12 rise's irq_pending_line assertion by exactly one notify_cpu_cycle boundary when the rise lands post-access (M2-high), while asserting synchronously for pre-access (M2-low) rises -- modeling the propagation-delay asymmetry ADR-0002 hypothesizes. An $E000 ack/disable write cancels an in-flight deferred assertion. This is structurally distinct from every one of the 17+ prior rolled-back attempts (constant-cycle pipelines, gap-threshold tuning, global batch-boundary re-phasings) -- it is a per-rise-property- conditional visibility deferral, evaluated on its own merits against today's promoted core rather than re-deriving a documented dead end. Three new unit tests in rustynes-mappers::mmc3::tests prove the deferral mechanism itself works correctly in isolation (M2-high defers one cycle, M2-low asserts synchronously, ack cancels a pending deferral). But regenerating the irq_trace_fixture for mmc3_test_2/4-scanline_timing with the feature ON vs OFF produces a byte-for-byte identical run (83 frames, 2,203,768 trace records, final $6000=$03) in both configurations -- meaning no qualifying A12 rise this ROM's actual execution produces ever lands during the post-access half of a CPU cycle under the current scheduler, so the phase-conditional lever has zero differential effect on this specific bracket. The two *_currently_fails fail-loud probes (mmc3_test_2/4, mmc3_test_v1/4) both still correctly detect the unmoved failure with the feature on. This is a clean, mechanism-verified falsification, not a regression -- the four target brackets remain by-design #[ignore]'d, unchanged. The code is kept (not reverted) because it is fully feature-gated, confirmed byte-identical to the pre-attempt default build (fmt/clippy/ AccuracyCoin 139/139/cpu_interrupts_v2 5/5/nestest 0-diff/the R5 DMC-DMA pin/mmc3 18-pass-5-ignored/one_clock_invariants 2/2/save_state 9/9 all held with the feature off), and fixes a genuine ADR-0002 documentation gap (real M2-phase data was never actually reachable from the live R1 path before this commit) -- useful infrastructure for whoever tests the gap-accounting M2-edge-precision axis the same-day closure campaign flagged as the next credible lever. Full record, evidence, and the new falsifiable hypothesis for the next attempt in docs/audit/r1r2-per-dot-scheduler-attempt-2026-07-02.md (gitignored, local to this worktree per project convention). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidates the two 2026-07-02 R1/R2 bounded-effort campaigns (Session A: batch-boundary re-phasing, both experiments falsified; Session B: real M2-phase-conditional MMC3 IRQ visibility, mechanism-verified falsification plus a fixed dead-plumbing bug) into a single ADR-0002 decision-update section, per the maintainer's explicit direction to fold both findings into one entry rather than two. Marks the C1/MMC3 axis by-design-deferred beyond v2.0.0 with 21+ documented rollbacks (17 historical + 4 new), records the four new DO-NOT-RETRY levers, and flags the one genuinely untested axis (falling-edge gap>=3 low-time accounting) for a future dedicated session rather than continued spend within this release. CHANGELOG's [Unreleased] section gains the beta.5 entry: the Vs. DualSystem dual-console feature (mapper 99 board support, the VsDualSystem wrapper's shared-WRAM/cross-IRQ model, the Emu construction front door, synth-proven protocol tests) plus the real commercial-boot result (Vs. Balloon Fight verified booting on a combined dump assembled from a legitimately-owned MAME romset; Vs. Wrecking Crew documented honestly as inconclusive rather than forced to a false-positive pass) and a summary of the R1/R2 campaign disposition cross-referencing the ADR update. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for Nintendo Vs. DualSystem dual-console cabinets, adding a VsDualSystem wrapper that manages two Nes instances in a soft-lockstep with shared WRAM and cross-wired IRQ communication. It also updates the Emu entry point to route dual-system ROMs automatically and adds a default-off experiment for M2-phase-aware MMC3 IRQ visibility. Feedback on these changes highlights a performance concern regarding high-frequency Vec allocations in the hot path of pump_comms, recommending a pre-allocated buffer instead, and a portability issue in the test harness where hardcoded /tmp paths should be replaced with std::env::temp_dir().
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.
There was a problem hiding this comment.
Pull request overview
Adds core support and test coverage for Nintendo Vs. DualSystem (dual-console) cabinets, alongside a documented MMC3 IRQ residual closure campaign update and supporting feature-gated plumbing for an additional timing hypothesis.
Changes:
- Introduces
rustynes-core::VsDualSystemplus anEmu“front door” (Single/Dual) to construct/run dual-console cabinets with wrapper-owned cross-wiring and shared-WRAM convergence. - Extends mapper 99 (Vs. System) to support DualSystem sub-console PRG/CHR outer banking,
$4016bit-7 identity, and shared 2 KiB WRAM with snapshot support. - Adds synthetic and external (commercial-roms-gated) test harness coverage plus diagnostics, and updates ADR/CHANGELOG + refreshed
irq_tracegoldens.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/adr/0002-irq-timing-coordination.md | Updates ADR status + records the 2026-07-02 closure campaign disposition and DO-NOT-RETRY additions. |
| crates/rustynes-test-harness/tests/vs_dualsystem.rs | New commercial-roms-gated boot + snapshot tests for DualSystem external dumps, including an insta pin for Balloon Fight combined dump. |
| crates/rustynes-test-harness/tests/vs_dualsystem_synth.rs | New synthetic DualSystem NES 2.0 protocol/cart test suite validating detection, banking, identity, WRAM, IRQ, and snapshot behavior. |
| crates/rustynes-test-harness/tests/snapshots/vs_dualsystem__gvs_balloon_fight_dual_combined_boots.snap | Adds the pinned snapshot text record for the combined-dump Balloon Fight boot test. |
| crates/rustynes-test-harness/src/bin/vs_dual_trace.rs | Adds a diagnostic CLI tool for tracing DualSystem handshake/PC histograms (non-CI). |
| crates/rustynes-test-harness/golden/irq_trace/mmc3_test_2_4_scanline_timing.svc.csv | Re-baselines stale pre-promote IRQ service trace goldens. |
| crates/rustynes-test-harness/Cargo.toml | Wires the new mmc3-m2-phase-irq feature forward and registers the vs_dual_trace diagnostic binary. |
| crates/rustynes-mappers/src/vs_system.rs | Implements DualSystem shared WRAM, sub-console PRG/CHR outer banking, and save-state v2 layout for mapper 99. |
| crates/rustynes-mappers/src/mmc3.rs | Adds default-off mmc3-m2-phase-irq phase-conditional IRQ visibility deferral plus unit tests under the feature. |
| crates/rustynes-mappers/src/mapper.rs | Extends the Mapper trait with DualSystem shared-WRAM and sub-console wiring hooks (default no-op). |
| crates/rustynes-mappers/Cargo.toml | Adds the mmc3-m2-phase-irq feature gate and documents its intent. |
| crates/rustynes-cpu/src/cpu.rs | Updates CPU cycle halves to pass pre/post-access phase to run_ppu_to. |
| crates/rustynes-cpu/src/bus.rs | Extends Bus::run_ppu_to signature to include is_post_access for phase labeling. |
| crates/rustynes-core/src/vs_dualsystem.rs | New dual-console wrapper implementation plus Emu enum constructor routing DualSystem carts. |
| crates/rustynes-core/src/vs_db.rs | Updates DualSystem caveats and adds combined-dump SHA rows; adjusts unit test expectations accordingly. |
| crates/rustynes-core/src/nes.rs | Exposes Nes::is_jammed() for DualSystem soft-lockstep control. |
| crates/rustynes-core/src/lib.rs | Exposes vs_dualsystem module and re-exports Emu/VsDualSystem. |
| crates/rustynes-core/src/bus.rs | Adds DualSystem bus wiring/state (sub identity, external IRQ, $4016 bit-1 latch) and phase-aware run_ppu_to behavior under feature gate. |
| crates/rustynes-core/Cargo.toml | Adds mmc3-m2-phase-irq feature forwarding to mappers. |
| CHANGELOG.md | Documents beta.5 DualSystem support and the R1/R2 closure campaign disposition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Six findings from the initial bot review pass, all adopted:
- **Hot-path allocation (gemini high, Copilot)**: `pump_comms` called
`Mapper::take_vs_dual_wram_writes`, which used `mem::take` to hand the
caller a fresh Vec every call -- after every stepped instruction on a
DualSystem cart, this is a real hot-path allocation, not a theoretical
one. Replaced with a new `Mapper::drain_vs_dual_wram_writes(&mut self,
dst: &mut Vec<(u16, u8)>)` primitive: `VsSystem`'s override uses
`Vec::append` (clippy's own preferred form over `extend(drain(..))`),
which empties `dual_wram_log` while retaining ITS capacity; the wrapper
gained a reusable `comms_scratch: Vec<(u16, u8)>` field so its side of
the exchange is allocation-free too, once warmed up. The old
`take_vs_dual_wram_writes` stays as a convenience default method (built
on the new primitive) for non-hot-path callers (`vs_dual_trace.rs`,
tests) -- unchanged call sites there.
- **Save-state restore leaves stale WRAM-log entries (Copilot)**:
`VsSystem::load_state` never cleared the transient `dual_wram_log`,
so writes logged before a restore point would replay into the partner
console AFTER the restore, corrupting its shared-WRAM copy with
pre-restore data. Now cleared unconditionally on every restore. Also:
a v1 (UniSystem) load no longer leaves a stale `dual_wram` allocation
behind if the live instance had previously been dual-provisioned --
it's explicitly dropped to match the versioned layout just loaded.
- **Two doc/implementation mismatches (Copilot)**: `vs_4016_bit1_dirty`'s
field doc and `take_vs_mainsub_edge`'s doc both described an edge-
filtered ("only when bit 1 changed") protocol; the actual, intentional
design is level-driven (dirty on EVERY `$4016` write -- required so the
reset-time seeded levels aren't swallowed by an edge filter starting
from a false latch, per the existing `cpu_write` comment). Rewrote both
doc comments to describe the real, level-driven semantics.
- **Frontend-integration overclaim (Copilot)**: the `Emu` module doc and
the original PR description both said "frontends construct via
Emu::from_rom" -- but `rustynes-frontend` still constructs `Nes`
directly everywhere (`app.rs`, `emu.rs`) and does not consume `Emu` at
all, so the DualSystem path is unreachable from the shipped desktop/
mobile UI in this release. Corrected the module doc and the CHANGELOG
entry to state this plainly as a known, explicitly deferred gap rather
than implying frontend wiring already landed.
- **Non-portable hardcoded /tmp path (gemini medium)**: the test harness's
best-effort PNG dump helper hardcoded `/tmp/RustyNES/vs-dualsystem`,
which doesn't exist on Windows. Switched to `std::env::temp_dir()`;
dropped the now-unused `Path` import (`PathBuf` still used elsewhere
in the file).
Verified: fmt clean; clippy clean across workspace + the 3 frontend
feature combos; full dev-profile `cargo test --workspace` 111/111 green;
release-profile `vs_dualsystem` (2 passed/5 ignored, `mmc3` (18/5-ignored),
and `vs_dualsystem_synth` (3/3) all still pass identically to before
these fixes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…CI gate The prior commit's take_vs_mainsub_edge doc fix linked to [`Self::vs_4016_bit1_dirty`] -- a private field -- from a pub method's doc comment. rustdoc's -D warnings gate (RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps, the exact CI command) correctly flags public-documentation-links-to-private-item as an error. Swapped the intra-doc link for a plain code span; verified locally with the exact CI invocation plus fmt/clippy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Workstream C (Vs.
DualSystem) + the beta.3-authorized R1/R2 bounded-effort closure campaign, both on the fully-promoted (beta.4) v2.0.0 core.Vs.
DualSystemdual-console supportDualSystemcabinet boards (Vs. Tennis, Vs. Mahjong, Vs. Wrecking Crew, Vs. Balloon Fight — two complete NES consoles sharing a 2 KiB WRAM mailbox and a cross-wired$4016bit-1/IRQline) now construct and run as a genuine two-console pair via the newEmuenum front door (Emu::Single/Emu::Dual).VsDualSystem(rustynes-core) owns bothNesinstances and applies a MAME.share("nvram")-style level-driven WRAM/comms convergence model — chosen over the nesdev/Mesen2-documented exclusive-access mux after disassembly proved the mux model deadlocks Balloon Fight's real boot handshake.$4016bit-7 identity.vs_dualsystem_synth, 3/3: banking, identity, WRAM convergence, bidirectional cross-IRQ, snapshot round-trip — independent of any commercial ROM).Real commercial boot result: two of the four titles' missing sub-CPU program ROMs were located in a legitimately-owned MAME arcade romset and combined into proper 64 KiB dual dumps (never committed —
tests/roms/external/stays gitignored).gvs_balloon_fight_dual_combined_boots, insta-snapshot pinned).#[ignore]'d diagnostic rather than forced to a false-positive pass.R1/R2 bounded-effort closure campaign (2026-07-02, two sessions)
Per the beta.3 escape hatch (plan Risks #3), one dedicated closure attempt at the MMC3 IRQ-timing bracket (
mmc3_test_2/4-scanline_timingsub-test #3 + siblings) on the promoted core. Both sessions are clean falsifications — zero regression, all sacred gates held throughout:run_ppu_todo-while catch-up-boundary conversion; established the mechanism finding that the ROM measures a differential interval invariant to any consistent batch re-phasing — explaining why 15+ prior levers were absorbed. Re-baselined the stale pre-promoteirq_tracegoldens.mmc3-m2-phase-irqfeature), then proved the resulting lever never engages for this ROM (byte-for-byte identical trace on/off).Full disposition + the DO-NOT-RETRY additions:
docs/adr/0002-irq-timing-coordination.md. Axis is now by-design-deferred beyond v2.0.0 (21+ documented rollbacks). The four target brackets remain#[ignore]'d, zero production-ROM impact.Test plan
cargo fmt --all --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo clippy -p rustynes-frontendunderscripting,scripting,hd-pack,retroachievements— cleancargo test --workspace(the CI mirror) — 111 test-result lines, all greentest-romssacred-gate suite (AccuracyCoin 139/139,cpu_interrupts_v25/5, nestest 0-diff, R5 pin,mmc318/5-ignored,one_clock_invariants,save_state) — all greenvs_dualsystem+vs_dualsystem_synthundercommercial-roms,test-roms— 2 passed/5 ignored + 3/3 passed.nesROM files staged or tracked (verified viagit status)🤖 Generated with Claude Code