feat: v2.2.9 "Studio II" — TAStudio wiring, .bk2 playback, detachable tool windows - #346
feat: v2.2.9 "Studio II" — TAStudio wiring, .bk2 playback, detachable tool windows#346doublegate wants to merge 10 commits into
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesRustyNES v2.2.9
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Not ready to approve
The new detachable_window helper drops existing per-panel window defaults (size/position/resizability), which is a user-visible regression for several tool panels.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR bumps RustyNES to v2.2.9 “Studio II” and implements three frontend-focused UX fixes: TAStudio edits are now applied to the running emulator, BizHawk .bk2 playback now honors LogKey column order, and debugger/tool panels can detach into native OS windows via egui multi-viewport.
Changes:
- Added a shared
detachable_windowhelper and routed many debugger/tool panels through it to enable native multi-window detaching. - Updated
.bk2import to parse and apply the declaredLogKey:column order (with fallback behavior) and added regression tests. - Updated release/version documentation and workspace versioning to 2.2.9.
File summaries
| File | Description |
|---|---|
| README.md | Updates “Current Release” text to v2.2.9 and summarizes the Studio II changes. |
| docs/STATUS.md | Updates the status header to reflect v2.2.9 as current release. |
| docs/frontend.md | Documents detachable tool windows as shipped in v2.2.9 and updates deferred list accordingly. |
| crates/rustynes-frontend/src/debugger/trace_panel.rs | Routes Trace panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/rom_info_panel.rs | Routes ROM Info panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/replay_panel.rs | Routes Replay/TAS panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/ppu_panel.rs | Routes PPU panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/oam_panel.rs | Routes OAM panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/nsf_panel.rs | Routes NSF panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/mod.rs | Introduces detachable_window, adds detached_panels tracking, and updates panel call sites. |
| crates/rustynes-frontend/src/debugger/memory_compare_panel.rs | Routes Memory Compare panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/mapper_panel.rs | Routes Mapper panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/input_miniatures_panel.rs | Routes Input Display panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/game_db_panel.rs | Routes ROM Database panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/event_panel.rs | Routes Event Viewer panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/doc_panel.rs | Routes Documentation panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/cheat_panel.rs | Routes Cheats panel through detachable_window and threads detached state on both native/wasm. |
| crates/rustynes-frontend/src/debugger/audio_mixer.rs | Routes Audio Mixer panel through detachable_window and threads detached state. |
| crates/rustynes-frontend/src/debugger/apu_panel.rs | Routes APU panel through detachable_window and threads detached state. |
| crates/rustynes-core/src/bk2_interop.rs | Implements LogKey: parsing for per-port column mapping and updates input parsing + tests. |
| CHANGELOG.md | Adds v2.2.9 release notes describing the new behaviors and constraints. |
| Cargo.toml | Bumps workspace package version to 2.2.9. |
| Cargo.lock | Updates crate versions to 2.2.9 across workspace packages. |
Review details
- Files reviewed: 27/28 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…ilot #346) Copilot flagged a real UX regression in the v2.2.9 detachable-window conversion: routing every tool panel through the shared `detachable_window` helper dropped each panel's bespoke `egui::Window` builder options — `default_pos`, `default_size`, `default_width` / `min_width`, and `resizable(false)` on ROM Info / Input Display / ROM Database / Performance. Losing the `default_pos` values in particular collapsed the debugger's designed workspace layout into egui's default overlap cascade on first open, and four fixed-size panels silently became resizable. `detachable_window` now takes a `WindowCfg { default_pos, default_size, default_width, min_width, resizable }` (all `Option`, `Copy + Default`) and applies each set field to the docked `egui::Window`; all 19 call sites (18 panels; `cheat_panel` has a native + a wasm variant) pass back their exact prior values, so first-open placement/size and the four non-resizable panels are restored. The config applies to the docked form only — a detached panel is a real OS window the window manager sizes and places (egui persists the docked window's own position/size by id after first open, so `WindowCfg` only seeds the first appearance). Native + wasm32 `clippy -D warnings` clean on both feature sets. Also (proactive, matching the CodeRabbit finding already fixed on #345): the v2.2.4 entry in the AGENTS.md lineage paragraph still called v2.2.4 "the current release" — reworded to point at the actual current-release paragraph so AGENTS.md carries a single current-release record. Frontend-only; the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…2 import (v2.2.9)
Two of the v2.2.9 'Studio II' items, both objectively verified:
TAStudio inputs now drive the emulator. handle_tas_requests (the piano-roll panel
path) only mutated the editor's input_log on a SetInput and never re-derived the
running Nes, so a cell edit looked disconnected from emulation (the NESdev-forum
'TAStudio inputs do not seem to be connected up' report). It now tracks an
input_dirty flag across the batch and does a single deterministic re-seek to the
cursor afterward, exactly like the scripting path (apply_tas_commands). InsertFrame
/ DeleteFrame / StampMacro also mark dirty; Seek / CreateBranch / LoadBranch reseat
the Nes themselves.
.bk2 import honors the LogKey column order. The parser ignored the LogKey: line and
mapped pad columns by fixed U D L R S s B A position, so a BizHawk movie authored
with a different column order or extra columns mapped every button to the wrong bit
('.bk2 did not play back'). parse_log_key now reads the per-port column order from
the LogKey (# groups, | columns), maps each column by its button name (ignoring the
'Pn ' prefix), and falls back to the standard order when a group is truncated/exotic
(preserving the existing tests). parse_pad maps by that column list and tolerates a
group LONGER than the modeled columns (extra buttons like a mic are ignored). A new
test proves a non-standard order + an extra column.
.bk2 import feedback is on-screen. handle_movie_import surfaced every outcome via
eprintln! to a terminal nobody sees (so a failed import looked like nothing
happened). It now sets the on-screen status line for each path (no ROM, parse error,
wrong-ROM seek failure, success) via StatusMessage. Consolidated the file's nine
per-function StatusMessage imports into one module-level use.
Verification: bk2 tests 7/7 (incl. the new order test), rustynes-frontend 464/464,
core no_std cross-compile clean, clippy -D warnings + fmt clean on both crates.
Remaining v2.2.9 item: floating tool windows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the NESdev-forum report that tool windows are trapped inside the main OS window on Windows 10: every debugger/tool panel used `egui::Window::new(...)` inside the single central viewport, so it could never leave the host window. Adds a shared `detachable_window` helper in `debugger/mod.rs` that gives each panel a "⧉ Detach" button. Detached, the panel renders in a real OS window via `ctx.show_viewport_immediate` (the same egui multi-viewport mechanism `basic_bot_panel` already used) with a "⧉ Reattach" button; the OS window's close button reattaches too. A `DebuggerOverlay::detached_panels: HashSet<&'static str>` (keyed by each panel's stable id) tracks which panels are floating across frames. **Native-only by construction.** egui multi-viewport needs winit multi-window, absent on wasm, so the detached branch and the Detach button are `#[cfg(not(target_arch = "wasm32"))]`; on wasm the panel always renders docked in an `egui::Window`, unchanged. The helper carries a wasm-scoped `allow(clippy::needless_pass_by_ref_mut)` plus a `let _ = (&detached, id)` discard so both the rustc `unused_variables` and clippy `needless_pass_by_ref_mut` lints stay green there without desyncing the native signature (verified: `cargo clippy -p rustynes-frontend --target wasm32-unknown-unknown --lib --bins` clean for both the default and `wasm-canvas` feature sets). 17 panels are routed through the helper (PPU, OAM, APU, Memory, Event Viewer, NSF, Mapper, Watch, Trace, Cheats [native + wasm cfg variants], ROM Database, Performance, Documentation, Input Display, Audio Mixer, Replay/TAS, Memory Compare, ROM Info), each dropping its bespoke `.resizable()/.default_pos()/ .default_size()/.min_width()` builder options for the shared affordance. Panels whose `show()` returns a value (`cpu_panel`) or that already own multi-window / config-heavy bodies (settings, netplay, cheevos, input-rebind, tastudio, basic_bot) are intentionally left for a follow-up. Frontend-only — the deterministic core, save-states, and every golden vector are untouched (AccuracyCoin 141/141, nestest 0-diff). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Version bump 2.2.8 → 2.2.9 (workspace `version`, Cargo.lock) and the docs-as-spec sync for the "Studio II" release — the fourth step of the v2.2.6 → v2.3.0 NESdev-remediation line, capping the TAStudio-wiring, `.bk2` playback, and detachable-tool-window work committed earlier on this branch. - **CHANGELOG.md** — new `[2.2.9]` section (Fixed: TAStudio piano-roll edits now drive the emulator, `.bk2` playback honors the `LogKey` column order; Added: detachable/floating tool windows across 17 panels, native-only). - **docs/STATUS.md** (single source of truth) — current-release lead reset to v2.2.9, demoting v2.2.8 to "Built on". - **README.md** — Current Release lead updated to v2.2.9. - **AGENTS.md** — both the top current-release block and the operating-note paragraph lead with v2.2.9; the "never claim a version later than …" guard and the v2.2.6 → v2.3.0 line-summary bump to mark v2.2.8/v2.2.9 shipped. - **docs/frontend.md** — detachable multi-viewport tool windows moved out of the Deferred list into shipped (v2.2.9), with the `detachable_window` / `show_viewport_immediate` mechanism noted. Frontend-only across the whole release, so the deterministic core, save-states, and every golden vector are byte-identical: **AccuracyCoin 141/141**, nestest 0-diff. The detached-window behavior itself awaits an on-device (ideally Windows-10) visual check; the mechanism compiles + clippy-passes on native and both wasm feature sets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilot #346) Copilot flagged a real UX regression in the v2.2.9 detachable-window conversion: routing every tool panel through the shared `detachable_window` helper dropped each panel's bespoke `egui::Window` builder options — `default_pos`, `default_size`, `default_width` / `min_width`, and `resizable(false)` on ROM Info / Input Display / ROM Database / Performance. Losing the `default_pos` values in particular collapsed the debugger's designed workspace layout into egui's default overlap cascade on first open, and four fixed-size panels silently became resizable. `detachable_window` now takes a `WindowCfg { default_pos, default_size, default_width, min_width, resizable }` (all `Option`, `Copy + Default`) and applies each set field to the docked `egui::Window`; all 19 call sites (18 panels; `cheat_panel` has a native + a wasm variant) pass back their exact prior values, so first-open placement/size and the four non-resizable panels are restored. The config applies to the docked form only — a detached panel is a real OS window the window manager sizes and places (egui persists the docked window's own position/size by id after first open, so `WindowCfg` only seeds the first appearance). Native + wasm32 `clippy -D warnings` clean on both feature sets. Also (proactive, matching the CodeRabbit finding already fixed on #345): the v2.2.4 entry in the AGENTS.md lineage paragraph still called v2.2.4 "the current release" — reworded to point at the actual current-release paragraph so AGENTS.md carries a single current-release record. Frontend-only; the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
16e18ac to
7aa5836
Compare
…k of GPL emulators
RustyNES incorporates and is derived from code from GPL-licensed NES emulators.
It is therefore a derivative work distributable only under the GPL, and this
commit relicenses it from `MIT OR Apache-2.0` to **GPL-3.0-or-later**, credits
the derived-from sources per subsystem, and withdraws the incorrect "no GPL
source incorporated" position taken in v2.2.5 "Colophon".
Context. A NESdev community review found that the codebase contains bugs,
constants, variable names, code ordering, and comments referencing specific
upstream files, functions, and line numbers that go well beyond using an
emulator as a testing oracle. That is correct. The project's own in-source
comments, before a v2.2.5 edit reworded them, said as much: "Faithful port of
Mesen2's `ProcessSpriteEvaluation` (`NesPpu.cpp:1015-1141`)", "Ported bit-for-bit
from puNES `JV001.c`", "numeric tables ported verbatim from Bisqwit's C", and
~12 "Ported from Mesen2 `<file>.h`" mapper comments. v2.2.5 reframed that code as
"oracle cross-checks" and kept a permissive license the combined work was not
entitled to use. Laundering GPL code through AI tooling does not change its
license, and responsibility for what landed in the tree rests with the project.
Derived-from sources and their licenses (full file-by-file table in
docs/originality-and-provenance.md Section 1):
- Mesen2 (GPL-3.0-or-later): CPU unstable-store opcodes; the PPU
sprite-evaluation FSM + OAM-data-bus model; ~15 mapper boards (Bandai EEPROM,
JY Company, Waixing, Sachen, Txc, NTDEC, Kaiser, MMC3 variants, FK23C,
CoolBoy); the Bisqwit NTSC filter tables; the UNIF tables; the debug-symbol
importer; the PGO harness.
- puNES (GPL-2.0-or-later): JV001 / mapper 147 (bit-for-bit); the FDS per-CRC
drive-timing table.
- FCEUX (GPL-2.0-or-later): UNIF handling; some mapper banking.
- Nestopia UE (GPL-2.0-or-later): FME-7 / 5B audio detail.
Every upstream grants "or (at your option) any later version", so the
GPL-2.0-or-later material upgrades to v3 and GPL-3.0-or-later is the correct,
consistent expression for the combined work. GeraNES (GPL-3.0-only) was used as
an oracle only, with no code derived, so it does not force `-only`.
Changes:
- LICENSE is now the GPLv3 text; LICENSE-MIT and LICENSE-APACHE are removed;
the workspace + rustynes-cheevos `license` fields become GPL-3.0-or-later;
deny.toml allows GPL-3.0-or-later for the project's own crates (cargo-deny
`check licenses` = ok); release.yml packages LICENSE instead of the two
removed files.
- docs/originality-and-provenance.md is rewritten to lead with the derivation
table and the derivative-work declaration; NOTICE attributes each GPL
upstream and the code derived from it; README, AGENTS, CONTRIBUTING, SUPPORT,
ROADMAP, the in-app About/CLI/doc-panel strings, the Android about_body
(EN + ES), and the libretro `.info` license field all state GPL-3.0-or-later.
- New ADR 0036 records the decision, the SPDX rationale, and the GPLv3/App-Store
distribution caveat. The scattered "port of" comments are deliberately NOT
restored (they were imprecise; the audited derivation table supersedes them),
but the derivation is now stated plainly and completely.
- Incorporated permissive components (emu2413/MIT, TriCNES/MIT, rcheevos/MIT,
blip_buf/LGPL-2.1-or-later, fonts) are GPL-compatible and keep their notices.
Zero emulation-core behavior change: AccuracyCoin holds 141/141 and nestest is
0-diff by construction. This is a licensing and documentation correction.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Carry-over fix: the version badge still read v2.2.8 after the v2.2.9 doc bump. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ived source
Follows the v2.2.9 relicense: now that the accurate license and attribution are
established, mark the derived source itself. Each of the 23 files that contains
code derived from a GPL emulator gains a top-of-file header:
// SPDX-License-Identifier: GPL-3.0-or-later
//
// Provenance: <what is derived, from which upstream file/function>. See
// docs/originality-and-provenance.md (Section 1) and NOTICE ...
so the license and the specific upstream are discoverable at the point of use —
e.g. `rustynes-ppu/src/ppu.rs` names Mesen2 `NesPpu.cpp`
(`ProcessSpriteEvaluation` / `ReadSpriteRam`) plus the TriCNES (MIT) octal-latch
model; `rustynes-mappers/src/fds.rs` names puNES `fds.c`;
`rustynes-frontend/src/ntsc_bisqwit.rs` records the verbatim-ported Bisqwit
tables via Mesen2. The ~15 Mesen2-derived mapper boards, the CPU unstable-store
opcodes, the emu2413/blip_buf audio, the CRT-shader reimplementations, the debug-
symbol importer, and the PGO harness are all likewise marked.
This is the accurate replacement for the old scattered, imprecise per-line "port
of" comments (not restored verbatim); the SPDX + provenance headers plus the
audited §1 derivation table are the discoverable record. CHANGELOG, ADR 0036, and
docs/originality-and-provenance.md §8 are updated to describe this approach.
Comments only — `cargo fmt --all --check` clean, `cargo check --workspace`
compiles, zero behavior change (AccuracyCoin 141/141, nestest 0-diff).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/app/src/main/res/values/strings.xml`:
- Line 188: The AccuracyCoin release counts are outdated in the user-facing
accuracy text. Update about_body in android/app/src/main/res/values/strings.xml
at lines 188-188 and android/app/src/main/res/values-es/strings.xml at lines
168-168 from 139/139 to 141/141, and update the frontend accuracy text in
crates/rustynes-frontend/src/debugger/doc_panel.rs at lines 1192-1192 to 100%
(141/141).
In `@CHANGELOG.md`:
- Around line 75-80: Count the actual detachable_window call sites, then use
that verified count and complete panel list consistently in CHANGELOG.md lines
75-80, AGENTS.md lines 30 and 188, and docs/frontend.md lines 1743-1747; update
each affected summary/specification without removing a valid panel name or
introducing inconsistent counts.
In `@crates/rustynes-core/src/bk2_interop.rs`:
- Around line 365-370: Update the parsing logic around the groups and cols
closures to preserve empty interior fields while removing only the
syntax-defined terminal delimiter, so empty controller columns and console
groups retain their positional indexes. Ensure malformed or unsupported layouts
are rejected with Bk2Error rather than silently remapped, and add regression
tests covering an empty controller column and empty console group.
In `@crates/rustynes-frontend/src/app.rs`:
- Around line 3036-3044: Before handling TasRequest::CreateBranch or
TasRequest::LoadBranch, check input_dirty and call ed.seek(nes, ed.cursor()) to
flush pending edits; only then invoke create_branch or load_branch, preserving
the flushed state instead of unconditionally clearing the flag. Add a regression
covering pending edit requests followed by each branch operation in the same
batch.
In `@crates/rustynes-frontend/src/debugger/event_panel.rs`:
- Around line 98-115: Update detachable_window to apply applicable WindowCfg
fields, including default size/position and resizable, when constructing the
detached native ViewportBuilder, while preserving docked-window behavior; verify
this on native desktop targets. The affected call sites require no direct
changes: crates/rustynes-frontend/src/debugger/event_panel.rs:98-115,
crates/rustynes-frontend/src/debugger/input_miniatures_panel.rs:130-147,
crates/rustynes-frontend/src/debugger/mapper_panel.rs:41-60,
crates/rustynes-frontend/src/debugger/perf_panel.rs:195-214,
crates/rustynes-frontend/src/debugger/ppu_panel.rs:122-141,
crates/rustynes-frontend/src/debugger/replay_panel.rs:81-99,
crates/rustynes-frontend/src/debugger/rom_info_panel.rs:71-90,
crates/rustynes-frontend/src/debugger/trace_panel.rs:60-78, and
crates/rustynes-frontend/src/debugger/watch_panel.rs:456-478.
In `@crates/rustynes-frontend/src/debugger/mod.rs`:
- Around line 303-320: Remove or gate the Detach action and its
show_viewport_immediate path until secondary viewport rendering is implemented.
Before re-enabling it, update DebuggerOverlay::render and
DebuggerOverlay::render_shell to own child winit windows, route their
input/output, and render secondary viewport output so the detached panel remains
interactive.
In `@docs/adr/0036-relicense-gplv3-derivative-work.md`:
- Around line 95-97: Update the accuracy statement in the ADR to describe
AccuracyCoin’s 141/141 and nestest’s 0-diff results as verified release-check
evidence rather than “by construction,” and link to the authoritative
docs/STATUS.md pass counts.
- Around line 78-82: Revise the historical-release language in
docs/adr/0036-relicense-gplv3-derivative-work.md (lines 78-82) and
docs/originality-and-provenance.md (lines 98-102) to preserve immutable
snapshot/tag history without stating or implying that prior unauthorized
permissive license grants remain valid. Use legally reviewed wording
consistently in both documents, while retaining the GPL-3.0-or-later
applicability from v2.2.9 onward.
In `@docs/originality-and-provenance.md`:
- Around line 43-62: Revise Section 1 so its introductory claims apply only to
rows derived from GPL-licensed emulators and do not include the BSD/Apache,
LGPL, or MIT components. Separate the ares, blip_buf, emu2413, and shader
reimplementation entries into appropriately labeled non-GPL or
visual-reimplementation provenance tables/sections, and update the licensing
rationale to match each table’s documented relationship and license.
In `@README.md`:
- Line 12: Update the version citation in the README’s Current Release section,
including the matching citation at the additional referenced location, from
2.2.8 to 2.2.9 so it aligns with the release badge and current release metadata;
alternatively, explicitly mark it as historical if it must remain unchanged.
In `@SUPPORT.md`:
- Line 109: Synchronize release metadata across SUPPORT.md:97 and
crates/rustynes-libretro/rustynes_libretro.info:6: update the current-release
description from v2.0.4 “Harbor” to v2.2.9 “Studio II”, and change the Libretro
display_version from v2.2.5 to v2.2.9 unless an independently documented
Libretro version contract is added instead.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb00d6a6-da48-4c8c-9089-fca3cd6173c7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (44)
.github/workflows/release.ymlAGENTS.mdCHANGELOG.mdCONTRIBUTING.mdCargo.tomlLICENSELICENSE-APACHELICENSE-MITNOTICEREADME.mdROADMAP.mdSUPPORT.mdandroid/app/src/main/res/values-es/strings.xmlandroid/app/src/main/res/values/strings.xmlcrates/rustynes-cheevos/Cargo.tomlcrates/rustynes-core/src/bk2_interop.rscrates/rustynes-frontend/src/app.rscrates/rustynes-frontend/src/cli.rscrates/rustynes-frontend/src/debugger/apu_panel.rscrates/rustynes-frontend/src/debugger/audio_mixer.rscrates/rustynes-frontend/src/debugger/cheat_panel.rscrates/rustynes-frontend/src/debugger/doc_panel.rscrates/rustynes-frontend/src/debugger/event_panel.rscrates/rustynes-frontend/src/debugger/game_db_panel.rscrates/rustynes-frontend/src/debugger/input_miniatures_panel.rscrates/rustynes-frontend/src/debugger/mapper_panel.rscrates/rustynes-frontend/src/debugger/memory_compare_panel.rscrates/rustynes-frontend/src/debugger/memory_panel.rscrates/rustynes-frontend/src/debugger/mod.rscrates/rustynes-frontend/src/debugger/nsf_panel.rscrates/rustynes-frontend/src/debugger/oam_panel.rscrates/rustynes-frontend/src/debugger/perf_panel.rscrates/rustynes-frontend/src/debugger/ppu_panel.rscrates/rustynes-frontend/src/debugger/replay_panel.rscrates/rustynes-frontend/src/debugger/rom_info_panel.rscrates/rustynes-frontend/src/debugger/trace_panel.rscrates/rustynes-frontend/src/debugger/watch_panel.rscrates/rustynes-frontend/src/ui_shell.rscrates/rustynes-libretro/rustynes_libretro.infodeny.tomldocs/STATUS.mddocs/adr/0036-relicense-gplv3-derivative-work.mddocs/frontend.mddocs/originality-and-provenance.md
💤 Files with no reviewable changes (2)
- LICENSE-APACHE
- LICENSE-MIT
…-mortem Two things: the maintainer-directed forensic analysis of the GPL-provenance failure, and the CodeRabbit review pass on the v2.2.9 PR. **docs/provenance-failure-postmortem.md (new).** A complete, evidence-cited reconstruction of how RustyNES came to incorporate lifted GPL emulator code despite a black-box-only instruction: the timeline across RustyNES_v2 (the private "engine stack" where the porting happened, 2026-05, Opus 4.7) and the 2026-06-13 transplant into this repo; the two distinct failures (the port itself, then the v2.2.5 scrubbing of the honest "port of" comments); the root causes (GPL source on disk + accuracy-bar goal + no firewall; the guardrail post-dating the violation; honest-at-build-time then laundered; multi-session framing propagation; trusted AI self-attestation); and an honest accounting of what is NOT recoverable (the RustyNES_v2 porting-era session logs are gone). The single hardest fact: the original "Faithful port of Mesen2's ..." comments still exist verbatim in RustyNES_v2 today — only this public repo scrubbed them. Linked from originality-and-provenance.md §8. **CodeRabbit #346 review (9 threads):** - **`.bk2` LogKey empty-field bug (Major, data integrity).** `parse_log_key` filtered out empty positional fields, shifting later columns/groups (an empty console group promoted P2's map into P1; an empty interior column misaligned buttons so `U.A` replayed as `Up` alone). Now strips only the syntax delimiters and keeps interior empties; +regression test for both cases. - **TAS branch/load ordering (correctness).** `CreateBranch` / `LoadBranch` cleared `input_dirty` without flushing pending edits, so a branch snapshot captured stale state; they now `ed.seek` to flush first. - **`WindowCfg` -> `ViewportBuilder` (Major).** The detached branch maps default size / position / resizability onto the viewport, not just the docked window. - **Multi-viewport honesty (Major).** RustyNES's frontend is a single-viewport `egui_winit` integration, so `show_viewport_immediate` renders the "detached" panel EMBEDDED in the main window rather than a separate OS window — it does not yet fully resolve the Windows-10 trapped-window report. Documented honestly in code, CHANGELOG, AGENTS.md, and docs/frontend.md; true OS-window detach is tracked follow-up. Corrects an overclaim. - Doc/metadata: panel count 17 -> 18; ADR 0036 "by construction" -> verified release-check evidence + STATUS link; README badge/BibTeX -> v2.2.9; SUPPORT current-release v2.0.4 -> v2.2.9; libretro display_version -> v2.2.9; Android about_body 139/139 -> 141/141 and 168 -> 172 mappers (EN + ES). Core-affecting fixes (bk2, TAS) are core/frontend only; cargo check + the bk2 tests pass. AccuracyCoin 141/141 unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d/followed Reconcile the root-cause framing with the maintainer's correction: the black-box / oracle-only instruction WAS given — the failure was that it was not mechanically enforced (no barrier at the tool boundary; no persisted written rule in the loaded guidance until 2026-06-13) and the porting model did not follow it. §4.1 and §4.2 reframed from "no guardrail / the guardrail post-dated the violation" to "instruction given, neither persisted early nor enforced"; §4.5 sharpened (an instruction the agent can silently disregard and then falsely certify is not a control). The evidentiary caveat is unchanged: the porting-era logs are gone, so the exact wording/timing of the spoken instruction cannot be quoted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/rustynes-core/src/bk2_interop.rs (1)
369-391: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not allocate every
LogKeygroup.Line 369 collects all
#-separated groups, but this parser reads only the console, P1, and P2 groups. A movie with many repeated#delimiters creates oneVec<&str>entry per empty group. This can exhaust memory during import.Consume the first three groups from the
split('#')iterator.Iterator::next()preserves empty groups.Proposed fix
- let groups: Vec<&str> = body.split('#').collect(); - let cols = |g: Option<&&str>| -> Vec<Option<Buttons>> { + let mut groups = body.split('#'); + let _console = groups.next(); + let cols = |g: Option<&str>| -> Vec<Option<Buttons>> { let mapped: Vec<Option<Buttons>> = g.map_or_else(Vec::new, |grp| { grp.strip_suffix('|') .unwrap_or(grp) .split('|') .map(button_for_column) .collect() }); @@ - (cols(groups.get(1)), cols(groups.get(2))) + (cols(groups.next()), cols(groups.next()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/rustynes-core/src/bk2_interop.rs` around lines 369 - 391, Replace the groups Vec allocation in the button-column parsing flow with a split('#') iterator, then consume exactly the console, P1, and P2 entries via successive next() calls while preserving empty groups. Update the existing cols calls to use those optional group references so only the required three groups are processed and trailing delimiters cannot cause unbounded allocation.CHANGELOG.md (2)
19-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove future v2.3.0 references from both release-facing documents.
The repository documents v2.2.9 as the current release and must not claim a later release.
As per coding guidelines: do not claim or document a release later than v2.2.9.
CHANGELOG.md#L19-L19: replace the v2.2.6 → v2.3.0 wording with neutral current-line wording.SUPPORT.md#L97-L97: remove the v2.3.0 endpoint from the current-release description.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 19, Update CHANGELOG.md line 19 to replace the v2.2.6 → v2.3.0 wording with neutral wording for the current release line, and update SUPPORT.md line 97 to remove the v2.3.0 endpoint from the current-release description. Ensure neither release-facing document claims a version later than v2.2.9.Source: Coding guidelines
28-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the detached-window description.
These lines say that detached tool windows use real OS windows. Lines 86-93 state that the current single-viewport integration embeds them in the main window. Keep the release overview consistent with the documented limitation.
As per path instructions: Markdown documentation must match the behavior it describes.
Proposed fix
- > **Windowing needs an on-device check.** Detached tool windows use egui - > multi-viewport (real OS windows); the mechanism compiles and clippy-passes on + > **Windowing needs an on-device check.** Detached tool windows remain embedded + > in the main viewport; the multi-viewport mechanism compiles and clippy-passes🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 28 - 31, Update the detached tool window description in the release overview to state that the current single-viewport integration embeds tool windows in the main window, matching the limitation documented later in CHANGELOG.md. Remove the claim that detached windows currently use real OS windows, while preserving the existing note about on-device validation.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 30: Remove future-release references from AGENTS.md: at AGENTS.md lines
30-30, replace the “v2.2.6 → v2.3.0” wording with version-neutral post-v2.2.9
future-work language; at lines 188-188, remove the detailed v2.3.0 release
narrative and retain only post-v2.2.9 planning language.
In `@docs/adr/0036-relicense-gplv3-derivative-work.md`:
- Around line 62-68: The per-file provenance requirement must match the Kaiser
header. In docs/adr/0036-relicense-gplv3-derivative-work.md lines 62-68 and
docs/originality-and-provenance.md lines 244-252, retain or revise the
requirement consistently so every derived file names an exact upstream source
location or function; in crates/rustynes-mappers/src/kaiser.rs lines 1-4, add
the specific Mesen2 source path or function to the provenance header.
In `@docs/provenance-failure-postmortem.md`:
- Around line 25-28: In docs/provenance-failure-postmortem.md lines 25-28,
qualify the claims about the model deciding to match Mesen2 and partially
porting it as inference rather than established fact. In lines 89-99, clearly
distinguish maintainer testimony from recovered evidence and state when the
reasoning or exact instruction is unavailable; keep the evidentiary standard
consistent across both sections.
- Line 52: Update the 2026-06-13 RustyNES provenance entry to identify the
transplanted stack as an internal “RustyNES_v2” engine-lineage snapshot,
explicitly stating that it is not a RustyNES release. Preserve the existing
event details while avoiding any wording that presents v2.8.0 as a RustyNES
release beyond the permitted v2.2.9 documentation limit.
---
Outside diff comments:
In `@CHANGELOG.md`:
- Line 19: Update CHANGELOG.md line 19 to replace the v2.2.6 → v2.3.0 wording
with neutral wording for the current release line, and update SUPPORT.md line 97
to remove the v2.3.0 endpoint from the current-release description. Ensure
neither release-facing document claims a version later than v2.2.9.
- Around line 28-31: Update the detached tool window description in the release
overview to state that the current single-viewport integration embeds tool
windows in the main window, matching the limitation documented later in
CHANGELOG.md. Remove the claim that detached windows currently use real OS
windows, while preserving the existing note about on-device validation.
In `@crates/rustynes-core/src/bk2_interop.rs`:
- Around line 369-391: Replace the groups Vec allocation in the button-column
parsing flow with a split('#') iterator, then consume exactly the console, P1,
and P2 entries via successive next() calls while preserving empty groups. Update
the existing cols calls to use those optional group references so only the
required three groups are processed and trailing delimiters cannot cause
unbounded allocation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd7a7a68-e916-42c5-8106-089b5b939e3a
📒 Files selected for processing (37)
AGENTS.mdCHANGELOG.mdREADME.mdSUPPORT.mdandroid/app/src/main/res/values-es/strings.xmlandroid/app/src/main/res/values/strings.xmlcrates/rustynes-apu/src/blip.rscrates/rustynes-apu/src/opll.rscrates/rustynes-core/src/bk2_interop.rscrates/rustynes-cpu/src/cpu.rscrates/rustynes-frontend/src/app.rscrates/rustynes-frontend/src/debugger/mod.rscrates/rustynes-frontend/src/debugger/source_map.rscrates/rustynes-frontend/src/ntsc_bisqwit.rscrates/rustynes-gfx-shaders/src/crt_stack.rscrates/rustynes-libretro/rustynes_libretro.infocrates/rustynes-mappers/src/fds.rscrates/rustynes-mappers/src/kaiser.rscrates/rustynes-mappers/src/lib.rscrates/rustynes-mappers/src/m016_bandai_fcg.rscrates/rustynes-mappers/src/m035_jy_asic.rscrates/rustynes-mappers/src/m069_sunsoft_fme7.rscrates/rustynes-mappers/src/m176_bmc_fk23c.rscrates/rustynes-mappers/src/m268_bmc_coolboy.rscrates/rustynes-mappers/src/m513_sachen_9602.rscrates/rustynes-mappers/src/mmc3_clones.rscrates/rustynes-mappers/src/multicart_discrete.rscrates/rustynes-mappers/src/ntdec.rscrates/rustynes-mappers/src/sachen_discrete.rscrates/rustynes-mappers/src/unif.rscrates/rustynes-ppu/src/palette_gen.rscrates/rustynes-ppu/src/ppu.rscrates/rustynes-test-harness/src/bin/pgo_trainer.rsdocs/adr/0036-relicense-gplv3-derivative-work.mddocs/frontend.mddocs/originality-and-provenance.mddocs/provenance-failure-postmortem.md
| RustyNES is a cycle-accurate Nintendo Entertainment System emulator written in pure Rust. The accuracy bar is Mesen2 / higan / ares: tight lockstep scheduling at PPU-dot resolution on a master-clock-precise timebase, sub-instruction PPU events visible to subsequent CPU code, and a lookup-table non-linear audio mixer with band-limited synthesis. The frontend is pure Rust (`winit` + `wgpu` + `cpal` + `egui`). | ||
|
|
||
| **Current release: v2.2.8 "Aperture II"** (2026-08-04, a presentation-fidelity release — gamma-correct scanlines in linear light + a WebGL2 gamma fix + a sharper Gaussian scanline profile in the base `CRT_WGSL`; presentation-only, so the pre-shader framebuffer + AccuracyCoin 141/141 are byte-identical and the shipped native default is unchanged [the native sRGB surface passes `aux = 0`, which selects the exact pre-v2.2.8 output; the new linear-light + sharpness path is keyed on a non-zero `aux`, set on the WebGL2 non-sRGB path and when the scanline knob is raised]; the shader/appearance changes await on-display + browser visual verification), on top of **v2.2.7 "Timbre II"** (2026-08-04) — an **expansion-audio fidelity** release (of the v2.2.6 → v2.3.0 NESdev-remediation line), driven by a measure-first cross-reference of VRC6 and Sunsoft 5B against 11 reference emulators + the NESdev wiki (Mesen2-only comparison hides where Mesen2 is the outlier). **VRC6 recalibrated to ~1.0× a 2A03 pulse** (`VRC6_MIX_SCALE` 979 → 650; the NESdev/field consensus — rustico/tetanes/BizHawk encode 1.0× exactly; Mesen2's louder ~1.506× mixer weighting was the outlier a reviewer flagged; `db_vrc6a/b` oracle 1.506 → 1.0), and the **Sunsoft 5B envelope moved to the exact 5-bit 1.5 dB/step DAC** (`SUNSOFT5B_LOG_VOL32`, matching nestopia/rustico, replacing the 4-bit 3 dB approximation). **Expansion-only — base 2A03 byte-identical**, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff; the base BLEP is a verified 81.6 dB-SFDR band-limited decimator. Built on **v2.2.6 "Almanac"** (2026-08-04) — a **de-monetization + provenance** release opening the **v2.2.6 → v2.3.0** NESdev-remediation line. **RustyNES is permanently open-source and income-free (ADR 0035)**: all planned monetization is removed (the `rustynes-monetization` crate, `docs/monetization/`, and the Android/iOS billing / ad / freemium / paywall layers deleted) and the native apps are kept as **free FOSS apps** (no ads, no tracking, no paid unlock; the free Google-Play services + `foss`/`play` split retained). It also discloses (ADR 0030) that the PPU hybrid-address *timing* was calibrated to TriCNES (reproducing the Rad Racer mis-render), flagged for a documentation-derived rework in v2.3.0. **Zero emulation-core behavior changes**, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction. On top of **v2.2.5 "Colophon"** (2026-08-03) — a **provenance, licensing, and documentation-integrity** release with **zero emulation-core behavior changes** (so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction). It reworded in-source comments that had mischaracterized publicly-documented hardware-behavior implementations as "ports of" copyleft emulators (Mesen2 GPLv3, puNES GPLv2) into the accurate oracle framing; rewrote `NOTICE` to disclose the behavioral-oracle use of GPL emulators (Mesen2/MesenCE, higan, GeraNES, ares, FCEUX, Nestopia, puNES — no code incorporated) and to attribute the genuinely-incorporated permissive components (emu2413, TriCNES, rcheevos — all MIT), the bundled fonts and test ROMs, and the CRT-shader/NTSC-filter visual influences as independent reimplementations; disclosed **GeraNES (GPL-3.0-only)** as an oracle; added `docs/originality-and-provenance.md`; and added an AI-assistance disclosure to the README (removing a misleading comparison graphic and fixing a mislabeled screenshot caption). On top of **v2.2.4 "Cartridge"** (2026-07-24) — a **libretro / RetroArch distribution** cut whose purpose is that the RustyNES core **builds and installs cleanly through the Libretro buildbot** (<https://git.libretro.com/libretro/RustyNES>) for in-RetroArch use. **Zero emulation-core changes** — the deterministic `#![no_std]` chip stack, save-state / TAS / netplay formats, and every golden vector are byte-identical to v2.2.3, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction. `crates/rustynes-libretro` wraps `rustynes-core`, so it inherits every v2.2.3 change automatically (the fast-dot-path default; the `PPU_SNAPSHOT_VERSION` 8 / APU v4 save-state schema, transparent because `get_serialize_size` / `on_serialize` size and emit the *current* snapshot via `Nes::snapshot_core_into` rather than a fixed layout; the `Mapper::mix_audio` i32 widening; the Zapper model; the `mNNN_` mapper rename), and both buildbot cross-ABIs the CI early-warning gate models — `x86_64-pc-windows-gnu` and `aarch64-linux-android` — `cargo check --release -p rustynes-libretro` clean. The concrete change is a **`rustynes_libretro.info` metadata correction**: **`disk_control` `false` → `true`** (the real fix — the FDS multi-side Disk Control interface has been wired since the buildbot recipe landed but was advertised as absent, hiding multi-disk FDS swapping from RetroArch's Quick Menu), `display_version` `v1.0.0` → `v2.2.4`, and the description mapper count `168` → `172`. Libretro **core options** (region / overscan / palette / accuracy toggles) remain unexposed — `core_options = "false"` is accurate, a documented future enhancement rather than a v2.2.4 gap. The Antigravity PR reviewer standardization onto the shared template rides along. On top of **v2.2.3 "Datum"** (2026-07-23) — a **performance and accuracy-closure patch**, the product of a measure-first appraisal that profiled the emulator and acted on what the profile showed rather than on intuition. **Performance:** the specialized PPU fast dot path is promoted to the **default** and exposed to users for the first time — `Nes::set_fast_dotloop` had **no caller outside the core**, so a **−11.3%** frame-time win (fresh clean-host Criterion, reproducing v2.1.8's +12.3% by a different method; differential-tested bit-identical every frame since v2.1.8) shipped switched off and unreachable; release builds now ship **PGO-optimized** Linux binaries when the existing >3%-and-byte-identical gate passes; and CI gained a same-runner **relative** frame-time regression gate, closing a hole where a 2.5x slowdown passed the deliberately-loose absolute ceiling. **Two optimizations were measured and REJECTED** and are documented with their numbers per `docs/performance.md`'s convention — P3 (`emit_pixel` bounds-check elision) made the shipped default *slower* (+4.32% / +3.35% on the `_fast` workloads, p ≤ 0.02), and P4 (`cpu_clock`) found both textbook optimizations already implemented with the one remaining lever capped at **≤1.9%**. **Accuracy:** the **last two Holy Mapperel residuals are closed**, so all 17 ROMs report `detail=0000` (was 15/17) — MMC1's two software WRAM write-protect layers (`$E000` bit 4 + SNROM's CHR-register layer, gated on `chr_is_ram`) and FME-7's open bus on the RAM-selected-but-disabled window, both routed through the trait's existing `cpu_read_unmapped` contract. MMC1 is the change Holy Mapperel's README calls a game-compatibility hazard (FCEUX / PowerPak omit it), so it was validated before landing: **60/60** commercial ROMs including seven battery-backed MMC1 saves, plus **138/138** extended. The **Sunsoft 5B absolute level** is calibrated against Mesen2, which required widening `Mapper::mix_audio` to `i32` (the correct full-scale 5B tone `1882 * 18.471 = 34,761` does not fit `i16`). A **save-state schema gap** is fixed — `PPU_SNAPSHOT_VERSION` **8** carrying the sprite-eval FSM + OAM data-bus state, plus an APU **v4** tail — which is what made AccuracyCoin report **141/141 through run-ahead** as well as without it; a new standing field-vs-schema audit found it and the two APU gaps mechanically. A **Zapper beam-relative light model** lands opt-in / default-off (no pass-fail light-gun ROM exists to adjudicate it). **AccuracyCoin holds 141/141 (100.00%)**, nestest 0-diff. Also: the eleven `sprintN.rs` mapper modules (27,631 lines, ~110 boards) are renamed for the boards they emulate with `mNNN_` mapper-number prefixes, proven content-preserving by a byte-for-byte item comparison (930 items, 0 altered) and an identical 172-ID dispatch table. | ||
| **Current release: v2.2.9 "Studio II"** (2026-08-04, a frontend quality-of-life release opening the second half of the v2.2.6 → v2.3.0 NESdev-remediation line — TAStudio piano-roll edits now drive the emulator [`handle_tas_requests` re-seeks the `Nes` after a `SetInput` batch, matching the scripting path], `.bk2` movies play back honoring their `LogKey` column order [`bk2_interop` parses the real column header instead of a fixed order, with parse errors surfaced on the status bar], and tool windows gain a **detach / pop-out** affordance (the shared `detachable_window` helper across 18 panels) [native-only, docked on wasm; **honest scope caveat:** the frontend is currently a single-viewport `egui_winit` integration, so `show_viewport_immediate` renders the panel *embedded in the main window*, NOT yet a separate OS window — so this does not yet fully resolve the Windows-10 trapped-window report; true OS-window detach needs multi-viewport render-loop wiring (`set_embed_viewports(false)` + per-viewport winit windows), tracked as follow-up]; frontend-only, so the deterministic core is untouched and **AccuracyCoin holds 141/141 (100.00%)** with nestest 0-diff), on top of **v2.2.8 "Aperture II"** (2026-08-04, a presentation-fidelity release — gamma-correct scanlines in linear light + a WebGL2 gamma fix + a sharper Gaussian scanline profile in the base `CRT_WGSL`; presentation-only, so the pre-shader framebuffer + AccuracyCoin 141/141 are byte-identical and the shipped native default is unchanged [the native sRGB surface passes `aux = 0`, which selects the exact pre-v2.2.8 output; the new linear-light + sharpness path is keyed on a non-zero `aux`, set on the WebGL2 non-sRGB path and when the scanline knob is raised]; the shader/appearance changes await on-display + browser visual verification), on top of **v2.2.7 "Timbre II"** (2026-08-04) — an **expansion-audio fidelity** release (of the v2.2.6 → v2.3.0 NESdev-remediation line), driven by a measure-first cross-reference of VRC6 and Sunsoft 5B against 11 reference emulators + the NESdev wiki (Mesen2-only comparison hides where Mesen2 is the outlier). **VRC6 recalibrated to ~1.0× a 2A03 pulse** (`VRC6_MIX_SCALE` 979 → 650; the NESdev/field consensus — rustico/tetanes/BizHawk encode 1.0× exactly; Mesen2's louder ~1.506× mixer weighting was the outlier a reviewer flagged; `db_vrc6a/b` oracle 1.506 → 1.0), and the **Sunsoft 5B envelope moved to the exact 5-bit 1.5 dB/step DAC** (`SUNSOFT5B_LOG_VOL32`, matching nestopia/rustico, replacing the 4-bit 3 dB approximation). **Expansion-only — base 2A03 byte-identical**, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff; the base BLEP is a verified 81.6 dB-SFDR band-limited decimator. Built on **v2.2.6 "Almanac"** (2026-08-04) — a **de-monetization + provenance** release opening the **v2.2.6 → v2.3.0** NESdev-remediation line. **RustyNES is permanently open-source and income-free (ADR 0035)**: all planned monetization is removed (the `rustynes-monetization` crate, `docs/monetization/`, and the Android/iOS billing / ad / freemium / paywall layers deleted) and the native apps are kept as **free FOSS apps** (no ads, no tracking, no paid unlock; the free Google-Play services + `foss`/`play` split retained). It also discloses (ADR 0030) that the PPU hybrid-address *timing* was calibrated to TriCNES (reproducing the Rad Racer mis-render), flagged for a documentation-derived rework in v2.3.0. **Zero emulation-core behavior changes**, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction. On top of **v2.2.5 "Colophon"** (2026-08-03) — a **provenance, licensing, and documentation-integrity** release with **zero emulation-core behavior changes** (so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction). It reworded in-source comments that had mischaracterized publicly-documented hardware-behavior implementations as "ports of" copyleft emulators (Mesen2 GPLv3, puNES GPLv2) into the accurate oracle framing; rewrote `NOTICE` to disclose the behavioral-oracle use of GPL emulators (Mesen2/MesenCE, higan, GeraNES, ares, FCEUX, Nestopia, puNES — no code incorporated) and to attribute the genuinely-incorporated permissive components (emu2413, TriCNES, rcheevos — all MIT), the bundled fonts and test ROMs, and the CRT-shader/NTSC-filter visual influences as independent reimplementations; disclosed **GeraNES (GPL-3.0-only)** as an oracle; added `docs/originality-and-provenance.md`; and added an AI-assistance disclosure to the README (removing a misleading comparison graphic and fixing a mislabeled screenshot caption). On top of **v2.2.4 "Cartridge"** (2026-07-24) — a **libretro / RetroArch distribution** cut whose purpose is that the RustyNES core **builds and installs cleanly through the Libretro buildbot** (<https://git.libretro.com/libretro/RustyNES>) for in-RetroArch use. **Zero emulation-core changes** — the deterministic `#![no_std]` chip stack, save-state / TAS / netplay formats, and every golden vector are byte-identical to v2.2.3, so **AccuracyCoin holds 141/141 (100.00%)** and nestest is 0-diff by construction. `crates/rustynes-libretro` wraps `rustynes-core`, so it inherits every v2.2.3 change automatically (the fast-dot-path default; the `PPU_SNAPSHOT_VERSION` 8 / APU v4 save-state schema, transparent because `get_serialize_size` / `on_serialize` size and emit the *current* snapshot via `Nes::snapshot_core_into` rather than a fixed layout; the `Mapper::mix_audio` i32 widening; the Zapper model; the `mNNN_` mapper rename), and both buildbot cross-ABIs the CI early-warning gate models — `x86_64-pc-windows-gnu` and `aarch64-linux-android` — `cargo check --release -p rustynes-libretro` clean. The concrete change is a **`rustynes_libretro.info` metadata correction**: **`disk_control` `false` → `true`** (the real fix — the FDS multi-side Disk Control interface has been wired since the buildbot recipe landed but was advertised as absent, hiding multi-disk FDS swapping from RetroArch's Quick Menu), `display_version` `v1.0.0` → `v2.2.4`, and the description mapper count `168` → `172`. Libretro **core options** (region / overscan / palette / accuracy toggles) remain unexposed — `core_options = "false"` is accurate, a documented future enhancement rather than a v2.2.4 gap. The Antigravity PR reviewer standardization onto the shared template rides along. On top of **v2.2.3 "Datum"** (2026-07-23) — a **performance and accuracy-closure patch**, the product of a measure-first appraisal that profiled the emulator and acted on what the profile showed rather than on intuition. **Performance:** the specialized PPU fast dot path is promoted to the **default** and exposed to users for the first time — `Nes::set_fast_dotloop` had **no caller outside the core**, so a **−11.3%** frame-time win (fresh clean-host Criterion, reproducing v2.1.8's +12.3% by a different method; differential-tested bit-identical every frame since v2.1.8) shipped switched off and unreachable; release builds now ship **PGO-optimized** Linux binaries when the existing >3%-and-byte-identical gate passes; and CI gained a same-runner **relative** frame-time regression gate, closing a hole where a 2.5x slowdown passed the deliberately-loose absolute ceiling. **Two optimizations were measured and REJECTED** and are documented with their numbers per `docs/performance.md`'s convention — P3 (`emit_pixel` bounds-check elision) made the shipped default *slower* (+4.32% / +3.35% on the `_fast` workloads, p ≤ 0.02), and P4 (`cpu_clock`) found both textbook optimizations already implemented with the one remaining lever capped at **≤1.9%**. **Accuracy:** the **last two Holy Mapperel residuals are closed**, so all 17 ROMs report `detail=0000` (was 15/17) — MMC1's two software WRAM write-protect layers (`$E000` bit 4 + SNROM's CHR-register layer, gated on `chr_is_ram`) and FME-7's open bus on the RAM-selected-but-disabled window, both routed through the trait's existing `cpu_read_unmapped` contract. MMC1 is the change Holy Mapperel's README calls a game-compatibility hazard (FCEUX / PowerPak omit it), so it was validated before landing: **60/60** commercial ROMs including seven battery-backed MMC1 saves, plus **138/138** extended. The **Sunsoft 5B absolute level** is calibrated against Mesen2, which required widening `Mapper::mix_audio` to `i32` (the correct full-scale 5B tone `1882 * 18.471 = 34,761` does not fit `i16`). A **save-state schema gap** is fixed — `PPU_SNAPSHOT_VERSION` **8** carrying the sprite-eval FSM + OAM data-bus state, plus an APU **v4** tail — which is what made AccuracyCoin report **141/141 through run-ahead** as well as without it; a new standing field-vs-schema audit found it and the two APU gaps mechanically. A **Zapper beam-relative light model** lands opt-in / default-off (no pass-fail light-gun ROM exists to adjudicate it). **AccuracyCoin holds 141/141 (100.00%)**, nestest 0-diff. Also: the eleven `sprintN.rs` mapper modules (27,631 lines, ~110 boards) are renamed for the boards they emulate with `mNNN_` mapper-number prefixes, proven content-preserving by a byte-for-byte item comparison (930 items, 0 altered) and an identical 172-ID dispatch table. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep future-release references out of AGENTS.md.
Both changed sections name v2.3.0, which is later than the permitted latest documented release, v2.2.9.
AGENTS.md#L30-L30: replace thev2.2.6 → v2.3.0release-line reference with version-neutral post-v2.2.9 future work.AGENTS.md#L188-L188: remove the detailedv2.3.0release narrative and retain only post-v2.2.9 planning language.
📍 Affects 1 file
AGENTS.md#L30-L30(this comment)AGENTS.md#L188-L188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 30, Remove future-release references from AGENTS.md: at
AGENTS.md lines 30-30, replace the “v2.2.6 → v2.3.0” wording with
version-neutral post-v2.2.9 future-work language; at lines 188-188, remove the
detailed v2.3.0 release narrative and retain only post-v2.2.9 planning language.
Source: Coding guidelines
| 3. **Mark the source, accurately.** Each derived source file carries an | ||
| `SPDX-License-Identifier: GPL-3.0-or-later` header and a specific provenance note | ||
| naming its upstream file/function (e.g. Mesen2 `NesPpu.cpp`, puNES `JV001.c`) and | ||
| pointing to the §1 table. The old scattered, imprecise per-line "port of" comments | ||
| are not restored verbatim — the SPDX + provenance headers plus the centralized | ||
| audited table in `docs/originality-and-provenance.md` + `NOTICE` are their | ||
| accurate, discoverable replacement. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the per-file provenance contract consistent.
The two documents require every derived source file to name an upstream file or function. The Kaiser header names only Mesen2. Add the exact source location, or narrow both document statements.
- docs/adr/0036-relicense-gplv3-derivative-work.md#L62-L68: keep the requirement only if the per-file headers satisfy it.
- docs/originality-and-provenance.md#L244-L252: mirror the corrected requirement.
- crates/rustynes-mappers/src/kaiser.rs#L1-L4: add the exact Mesen2 source path or function.
As per path instructions, docs are the specification, so the correction record must match the per-file provenance headers.
📍 Affects 3 files
docs/adr/0036-relicense-gplv3-derivative-work.md#L62-L68(this comment)docs/originality-and-provenance.md#L244-L252crates/rustynes-mappers/src/kaiser.rs#L1-L4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0036-relicense-gplv3-derivative-work.md` around lines 62 - 68, The
per-file provenance requirement must match the Kaiser header. In
docs/adr/0036-relicense-gplv3-derivative-work.md lines 62-68 and
docs/originality-and-provenance.md lines 244-252, retain or revise the
requirement consistently so every derived file names an exact upstream source
location or function; in crates/rustynes-mappers/src/kaiser.rs lines 1-4, add
the specific Mesen2 source path or function to the provenance header.
Source: Path instructions
| |---|---|---|---| | ||
| | **2026-05-10** | RustyNES_v2 | Project "bootstrapped **from a deep-research workflow**." The Mesen2/higan/ares "accuracy bar" framing and the reference-emulator source tree (`ref-proj/`) entered here. Phases 1–2 (6502, nestest pass, first mappers, PPU) landed the same day. | `3ec2230 chore: bootstrap RustyNES v2 from deep-research workflow`; `4d3cf47`, `b386595`, `69e9373` | | ||
| | **~2026-05-10 → 05-25** | RustyNES_v2 | The cycle-accurate chip core built in phases. With the GPL **source** on disk and an accuracy-matching goal, code was **ported** from it and labeled as such: CPU SH\*/unstable stores from Mesen2 `NesCpu.h`; PPU sprite-eval/OAM from Mesen2 `NesPpu.cpp:1015-1141`; mappers from Mesen2; JV001/FDS from puNES; UNIF from FCEUX. | `9e00032 fix(cpu): SH* unstable stores` (2026-05-23); `941d448 fix(ppu): Phase 3b — OAM-corruption row tracking` (2026-05-23) | | ||
| | **2026-06-13** | RustyNES → | The "**v2.8.0 engine stack**" was **transplanted** into the public repo as the `rustynes-*` crates. The honest "port of" comments came along verbatim. The "oracle / do NOT port" framing was written into the docs **for the first time** on this same day — *after* the porting was already done. | `dba2e75c feat(synthesis): Phase A — transplant v2.8.0 engine stack as rustynes-*`; `4e1844f7 docs(synthesis): Phase C` (first "do NOT port" text) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Disambiguate the v2.8.0 identifier.
The repository policy allows RustyNES release documentation only through v2.2.9 and requires a clear distinction from engine-lineage versions. The phrase “v2.8.0 engine stack” can be read as a later RustyNES release. Label it as an internal RustyNES_v2 engine-lineage snapshot and state that it is not a RustyNES release.
Based on coding guidelines, do not claim or document a release later than v2.2.9; distinguish RustyNES release versions from upstream engine-lineage version numbers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/provenance-failure-postmortem.md` at line 52, Update the 2026-06-13
RustyNES provenance entry to identify the transplanted stack as an internal
“RustyNES_v2” engine-lineage snapshot, explicitly stating that it is not a
RustyNES release. Preserve the existing event details while avoiding any wording
that presents v2.8.0 as a RustyNES release beyond the permitted v2.2.9
documentation limit.
Source: Coding guidelines
…r's edits
Non-substantive cleanup of the maintainer's review edits: stripped trailing
whitespace (§1, §2, and the closing NOTE), evened out the wrap widths the inline
edits left uneven, standardized hyphen-as-dash to em-dash in the NOTE, fixed one
phrase that didn't parse ("finally did baseline" -> "finally did become the
baseline"), and reconciled §5 with §4.2's "written instruction" framing (dropped
the now-inconsistent "or verbally"; the honest "cannot be quoted, logs gone"
point is unchanged). No substantive claims or the maintainer's wording/voice
were altered. markdownlint clean, no trailing whitespace.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR relicenses the repository to GPL-3.0-or-later with updated provenance notices, connects TAStudio frame edits to re-seek the emulator state, parses Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
v2.2.9 "Studio II" — TAS/movie wiring + detachable tool windows
Fourth step of the v2.2.6 → v2.3.0 NESdev-remediation line. Addresses three
forum items: TAStudio piano-roll edits that never reached the emulator,
.bk2movies that imported but didn't play back, and tool windows trapped inside the
main OS window on Windows 10.
Frontend-only — core untouched
Nothing here touches emulation, so the deterministic chip stack, save-states, and
every golden vector are byte-identical (AccuracyCoin 141/141, nestest 0-diff).
Fixed
handle_tas_requestsmutatedTasEditor::input_logonly and never re-seeked theNes, so a cell edit wasinvisible until an unrelated seek. It now re-derives through
TasEditor::seekafter the batch — the path the scripting bridge (
apply_tas_commands) already used..bk2playback honors the movie'sLogKeycolumn order.bk2_interopmappedcolumns by a fixed built-in order and ignored the
LogKey:header, so BizHawkmovies with a different column order drove the wrong buttons. It now parses the
real
LogKey:order (standard-order fallback), and import parse errors surface onthe on-screen status bar instead of only
eprintln!.Added
detachable_windowhelper gives each tool panel a "⧉ Detach" button that pops it into a real OS
window (
show_viewport_immediate) with a "⧉ Reattach" affordance; 17 panels arerouted through it. Native-only — egui multi-viewport needs winit multi-window, so
on wasm panels stay docked in an
egui::Window(unchanged).Checks
native
clippy -D warnings(sweeper + pre-commit), wasm32clippy -D warningsgreen on both the default and
wasm-canvasfeature sets (the detach path is#[cfg]-gated native-only; a wasm-scopedallow/discard keepsunused_variablesneedless_pass_by_ref_mutclean),cargo fmt, markdownlint,cargo check --workspace. Docs: STATUS/README/AGENTS/CHANGELOG +docs/frontend.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
.bk2playback honors declaredLogKeyordering and accepts extra columns.Bug Fixes
Documentation
Chores