diff --git a/architecture.md b/architecture.md index 469c11d97..4e1ba6786 100644 --- a/architecture.md +++ b/architecture.md @@ -133,7 +133,7 @@ The `src/bin/roms.rs` file is a library binary (accessed via `cargo run --bin ro | `src/platform/ram_init.rs` | `initialize_ram` helper applying a `RamInitMode` (`Zero`, `Random`, `SeededRandom`) to a byte buffer. Shared by the NES and SNES cores; re-exported as `nes::console::initialize_ram` for the historical call sites. | | `src/platform/rom_extensions.rs` | The one table of which ROM file extension means which console (`ROM_EXTENSIONS`, over the catalog's `Platform`, which lives here and converts to `SystemType`). `rom_loader`, the ROM catalog and its disk scan read it, and the web page receives it through the wasm binding `rom_extension_table()` at start-up (nr-di6), so the copies cannot drift. | | `src/platform/key_bindings.rs` | The one table of keyboard bindings (`KEY_BINDINGS`): which key drives which console input (joypad, Power Pad, SNES pad on the NES, Vs. coin/service, Super Scope) on which console, rows for one key tried in order. The desktop keyboard (`frontends/native/keyboard/controller_mapping.rs`) looks every key up here, and the web page reads the same rows through the wasm binding `key_binding_table()` (nr-tlf). A binding only one shell has is declared `DesktopOnly`/`WebOnly` in its row, and a test pins the full list of those. | -| `src/platform/rom_loader.rs` | Shared "ROM path to ready-to-run `Console`" loading — `detect_system_type()` (by file extension) and `load_console()`. Used by both the native frontend and headless capture so the two cannot drift. The console comes back powered on and ready to run, as the web frontend's `load_rom` leaves it; no frontend resets a console it has just loaded (nr-sc7). | +| `src/platform/rom_loader.rs` | Shared "ROM path to ready-to-run `Console`" loading — `detect_system_type()` (by file extension) and `load_console()`. An NES load prints the pending release-build CPU trace warning held in `AppContext`, once per run, before the console is built (nr-alq). Used by both the native frontend and headless capture so the two cannot drift. The console comes back powered on and ready to run, as the web frontend's `load_rom` leaves it; no frontend resets a console it has just loaded (nr-sc7). | | `src/platform/png_utils.rs` | `write_rgb_png()` — writes an RGB888 buffer as an 8-bit PNG, creating missing parent directories. Fallible: every failure, including a buffer that does not match the given dimensions, is returned as an `io::Error` rather than panicking. | | `src/platform/test_roms.rs` | Minimal synthetic ROMs (`minimal_nes_rom`, `minimal_gb_rom`, `minimal_gba_rom`, `minimal_snes_rom`) for platform-level tests that only need a console to construct. Note they render an identical screen every frame, so tests that must distinguish frame counts use a real ROM from `roms/` instead. | | `src/platform/app_context.rs` | `AppContext` — shared application state including configuration, ROM database, and toast notification manager. Wrapped in `Rc>` for interior mutability. A core may raise a toast on any path: the desktop draws them with `visible_toasts`, and every web console's `drain_toasts` forwards them with `take_toasts`. | @@ -493,10 +493,10 @@ The SNES (Super Nintendo Entertainment System) module now includes active 65816 | Directory/File | Description | | ---------------- | ------------- | -| `src/debugging/` | Generic debugging and diagnostic tools. | -| `src/debugging/breakpoints.rs` | Breakpoint system — supports address breakpoints and conditional breaks. | -| `src/debugging/tracing.rs` | CPU/PPU/APU/Mapper trace output at configurable verbosity levels, plus a `--trace-from=`/`--trace-to=` master-clock window (`trace_clock_in_window`) that narrows the clock-stamped SNES trace lines to a range. A full bus trace of a real game runs to millions of lines, so gating both NESER and a reference emulator to the same window is what makes an ordinal-aligned cross-emulator diff practical (#3050). The SNES integration runner reads the same settings from `NESER_TRACE_CPU`/`_PPU`/`_APU`/`_FROM`/`_TO` so any ROM suite can be traced headless. | -| `src/debugging/logging.rs` | Debug logging infrastructure. | +| `src/platform/debugging/` | Generic debugging and diagnostic tools. | +| `src/platform/debugging/breakpoints.rs` | Breakpoint system — supports address breakpoints and conditional breaks. | +| `src/platform/debugging/tracing.rs` | CPU/PPU/APU/Mapper trace output at configurable verbosity levels, plus a `--trace-from=`/`--trace-to=` master-clock window (`trace_clock_in_window`) that narrows the clock-stamped SNES trace lines to a range. A full bus trace of a real game runs to millions of lines, so gating both NESER and a reference emulator to the same window is what makes an ordinal-aligned cross-emulator diff practical (#3050). The SNES integration runner reads the same settings from `NESER_TRACE_CPU`/`_PPU`/`_APU`/`_FROM`/`_TO` so any ROM suite can be traced headless. The NES instruction trace is compiled into debug builds only (the SNES and Game Boy ones are in every build), so `release_nes_trace_warning` builds the stderr warning a release run prints, once, as its first NES game starts (nr-alq). | +| `src/platform/debugging/logging.rs` | Debug logging infrastructure. | | `src/nes/debugging/` | NES-specific debugging tools. | | `src/nes/debugging/ui.rs` | egui-based debugger UI with CPU state, memory viewer, and disassembly. | | `src/nes/debugging/disasm.rs` | 6502 disassembler for real-time instruction display. | diff --git a/src/main.rs b/src/main.rs index a6308a24e..2e8601bfe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -167,6 +167,10 @@ fn main() -> Result<(), Box> { }; let app_context = Rc::new(RefCell::new(AppContext::new_with_config(parsed_config))); + // A release build compiles the NES instruction trace out; the first NES game says so. + app_context.borrow_mut().set_nes_trace_warning( + neser::platform::debugging::release_nes_trace_warning(&args, cfg!(debug_assertions)), + ); // Handle --tui: launch the interactive TUI ROM browser and exit. // Must be checked before refresh_startup_cartridge_catalog so the catalog diff --git a/src/platform/app_context.rs b/src/platform/app_context.rs index d009b3966..ca5388bc5 100644 --- a/src/platform/app_context.rs +++ b/src/platform/app_context.rs @@ -17,6 +17,8 @@ pub trait IntoSharedAppContext { pub struct AppContext { toast_manager: ToastManager, config: Config, + /// The release-build CPU trace warning, printed as the first NES game of the run starts. + nes_trace_warning: Option, } impl Default for AppContext { @@ -24,6 +26,7 @@ impl Default for AppContext { Self { toast_manager: ToastManager::new(), config: Config::default(), + nes_trace_warning: None, } } } @@ -67,6 +70,22 @@ impl AppContext { &mut self.config } + /// Holds `warning` until the first NES game of the run starts; `None` clears it. + pub fn set_nes_trace_warning(&mut self, warning: Option) { + self.nes_trace_warning = warning; + } + + /// The pending NES trace warning, which is then gone for the rest of the run. + pub fn take_nes_trace_warning(&mut self) -> Option { + self.nes_trace_warning.take() + } + + /// Whether a NES trace warning is still waiting for the first NES game. + #[cfg(test)] + pub(crate) fn nes_trace_warning_pending(&self) -> bool { + self.nes_trace_warning.is_some() + } + /// Queues a toast. It reads no clock: `Instant::now()` panics on wasm32-unknown-unknown, /// and this is reached from load paths the browser build takes (nr-6sm). The toast's /// lifetime starts at the first [`Self::visible_toasts`] call after it is added. @@ -142,6 +161,18 @@ impl ToastManager { mod tests { use super::*; + #[test] + fn nes_trace_warning_is_taken_once() { + let mut context = AppContext::new(); + assert_eq!(context.take_nes_trace_warning(), None); + context.set_nes_trace_warning(Some("warning: x".to_string())); + assert_eq!( + context.take_nes_trace_warning().as_deref(), + Some("warning: x") + ); + assert_eq!(context.take_nes_trace_warning(), None); + } + #[test] fn take_toasts_returns_queued_toasts_in_order_and_empties_the_queue() { let mut context = AppContext::new(); diff --git a/src/platform/config/cli.rs b/src/platform/config/cli.rs index ed2beb32c..d15bdf62f 100644 --- a/src/platform/config/cli.rs +++ b/src/platform/config/cli.rs @@ -77,12 +77,12 @@ pub(crate) const PLATFORM_CLI_FLAGS: &[CliFlag] = &[ }, CliFlag { flag: "--trace", - help: Some("Enable CPU trace output"), + help: Some("Enable CPU trace output (NES instructions only in debug builds)"), has_value: false, }, CliFlag { flag: "--trace-cpu", - help: Some("Enable CPU trace output"), + help: Some("Enable CPU trace output (NES instructions only in debug builds)"), has_value: false, }, CliFlag { @@ -868,6 +868,21 @@ mod tests { assert!(sound_section < sound_flag); } + #[test] + fn trace_flags_help_says_nes_instructions_need_a_debug_build() { + let help = help_text(); + for flag in ["--trace ", "--trace-cpu "] { + let line = help + .lines() + .find(|line| line.trim_start().starts_with(flag)) + .unwrap_or_else(|| panic!("{flag} missing from help")); + assert!( + line.ends_with("Enable CPU trace output (NES instructions only in debug builds)"), + "{line}" + ); + } + } + #[test] fn test_help_text_lists_snes_filter_under_video_and_display() { assert_eq!( diff --git a/src/platform/debugging/tracing.rs b/src/platform/debugging/tracing.rs index aa1a8bdb4..1bbbe9c29 100644 --- a/src/platform/debugging/tracing.rs +++ b/src/platform/debugging/tracing.rs @@ -305,9 +305,9 @@ impl Tracing { /// Only overrides values that are explicitly specified in args. pub fn apply_args(&mut self, args: &[String]) { for arg in args { - if arg == "--trace" { + if let Some((_, level)) = Self::cpu_level_from_arg(arg) { self.enabled = true; - self.cpu = 1; + self.cpu = level; continue; } @@ -316,8 +316,6 @@ impl Tracing { if arg == "--trace-nestest" { self.nestest = true; - } else if let Some(rest) = arg.strip_prefix("--trace-cpu") { - self.cpu = Self::parse_level(rest); } else if let Some(rest) = arg.strip_prefix("--trace-ppu") { self.ppu = Self::clamp_ppu_level(Self::parse_level(rest)); } else if let Some(rest) = arg.strip_prefix("--trace-apu") { @@ -339,6 +337,18 @@ impl Tracing { && self.clock_to.is_none_or(|to| master_clock <= to) } + /// The CPU trace spelling `arg` is (without any `=N`) and the level it sets, or `None` + /// when it does not set the CPU trace level. The one place both [`Self::apply_args`] and + /// [`release_nes_trace_warning`] read it, so they cannot drift apart. + fn cpu_level_from_arg(arg: &str) -> Option<(&'static str, u8)> { + if arg == "--trace" { + Some(("--trace", 1)) + } else { + let rest = arg.strip_prefix("--trace-cpu")?; + Some(("--trace-cpu", Self::parse_level(rest))) + } + } + /// Parse a level from "" or "=N" suffix. Returns 1 if empty, N if "=N". fn parse_level(suffix: &str) -> u8 { if suffix.is_empty() { @@ -366,10 +376,121 @@ impl Tracing { } } +/// The warning a release build owes a CPU trace asked for on the command line, or `None`. +/// +/// A release build compiles the NES instruction trace out, so `--trace`/`--trace-cpu` print no +/// NES instruction lines there. The warning names the spelling that turned the CPU trace on +/// (the last one to move its level from 0), without any `=N` level. Only the command line +/// counts; tracing enabled any other way never warns. +pub fn release_nes_trace_warning(args: &[String], debug_build: bool) -> Option { + if debug_build { + return None; + } + let mut level = 0; + let mut turned_on_by = None; + for arg in args { + let Some((flag, new_level)) = Tracing::cpu_level_from_arg(arg) else { + continue; + }; + if new_level == 0 { + turned_on_by = None; + } else if level == 0 { + turned_on_by = Some(flag); + } + level = new_level; + } + turned_on_by.map(|flag| { + format!( + "warning: {flag}: this release build prints no NES instruction lines; build without --release for them" + ) + }) +} + #[cfg(test)] mod tests { use super::*; + fn args(list: &[&str]) -> Vec { + std::iter::once("neser") + .chain(list.iter().copied()) + .map(String::from) + .collect() + } + + fn warning_for(flag: &str) -> String { + format!( + "warning: {flag}: this release build prints no NES instruction lines; build without --release for them" + ) + } + + fn release_warning(list: &[&str]) -> Option { + release_nes_trace_warning(&args(list), false) + } + + #[test] + fn release_warning_names_trace_cpu() { + assert_eq!( + release_warning(&["--headless", "--trace-cpu", "game.nes"]), + Some(warning_for("--trace-cpu")) + ); + } + + #[test] + fn release_warning_names_trace() { + assert_eq!( + release_warning(&["--trace", "game.nes"]), + Some(warning_for("--trace")) + ); + } + + #[test] + fn release_warning_names_spelling_that_turned_trace_on() { + let cases: [(&[&str], &str); 4] = [ + (&["--trace", "--trace-cpu"], "--trace"), + (&["--trace-cpu", "--trace"], "--trace-cpu"), + (&["--trace-cpu=0", "--trace"], "--trace"), + ( + &["--trace", "--trace-cpu=0", "--trace-cpu=2"], + "--trace-cpu", + ), + ]; + for (list, flag) in cases { + let warning = release_warning(list).unwrap_or_else(|| panic!("{list:?}")); + assert_eq!(warning, warning_for(flag), "{list:?}"); + assert!(!warning.contains('\n')); + } + } + + #[test] + fn release_warning_strips_level_value() { + assert_eq!( + release_warning(&["--trace-cpu=2"]), + Some(warning_for("--trace-cpu")) + ); + } + + #[test] + fn no_warning_in_debug_build() { + for flag in ["--trace", "--trace-cpu", "--trace-cpu=2"] { + assert_eq!(release_nes_trace_warning(&args(&[flag]), true), None); + } + } + + #[test] + fn no_warning_without_cpu_trace_flag() { + assert_eq!(release_warning(&["game.nes"]), None); + assert_eq!( + release_warning(&["--trace-ppu", "--trace-nestest", "--gba-trace-cpu=1"]), + None + ); + } + + #[test] + fn no_warning_when_cpu_level_is_zero() { + assert_eq!(release_warning(&["--trace-cpu=0"]), None); + assert_eq!(release_warning(&["--trace-cpu", "--trace-cpu=0"]), None); + } + fn parse_tracing(args: &[String]) -> Tracing { let mut tracing = Tracing::default(); tracing.apply_args(args); diff --git a/src/platform/rom_loader.rs b/src/platform/rom_loader.rs index 49a0c5019..28af617eb 100644 --- a/src/platform/rom_loader.rs +++ b/src/platform/rom_loader.rs @@ -139,6 +139,12 @@ fn build_nes_console( .config_mut() .apply_rom_timing_mode(cartridge.rom_timing_mode()); + // The game will start: a release build's CPU trace warning goes out now, before any of + // its trace lines, and only for the run's first NES game (nr-alq). + if let Some(warning) = app_context.borrow_mut().take_nes_trace_warning() { + eprintln!("{warning}"); + } + let mut console = Console::new_nes(app_context.clone()); console .as_nes_mut() @@ -377,6 +383,60 @@ mod tests { assert_eq!(detect_system_type("noext"), SystemType::Nes); } + // --- the release-build NES trace warning (nr-alq) --- + + fn context_with_trace_warning() -> SharedAppContext { + let context = make_app_context(); + context + .borrow_mut() + .set_nes_trace_warning(Some("warning: pending".to_string())); + context + } + + #[test] + fn nes_load_consumes_pending_trace_warning() { + let dir = TempDir::new().expect("create temp dir"); + let rom_path = write_rom(&dir, "nes", &minimal_nes_rom(false)); + let context = context_with_trace_warning(); + + load_console(&context, &rom_path).expect("NES ROM should load"); + + assert_eq!(context.borrow_mut().take_nes_trace_warning(), None); + } + + #[test] + fn snes_gb_gba_loads_leave_trace_warning_pending() { + let dir = TempDir::new().expect("create temp dir"); + let context = context_with_trace_warning(); + for (extension, rom) in [ + ("sfc", minimal_snes_rom()), + ("gb", minimal_gb_rom()), + ("gba", minimal_gba_rom()), + ] { + let rom_path = write_rom(&dir, extension, &rom); + load_console(&context, &rom_path).expect("ROM should load"); + assert!( + context.borrow().nes_trace_warning_pending(), + "{extension} consumed the NES warning" + ); + } + + let rom_path = write_rom(&dir, "nes", &minimal_nes_rom(false)); + load_console(&context, &rom_path).expect("NES ROM should load"); + assert!(!context.borrow().nes_trace_warning_pending()); + } + + #[test] + fn failed_nes_load_leaves_trace_warning_pending() { + let dir = TempDir::new().expect("create temp dir"); + let rom_path = write_rom(&dir, "nes", b"not an iNES image"); + let context = context_with_trace_warning(); + + assert!(load_console(&context, &rom_path).is_err()); + + assert!(context.borrow().nes_trace_warning_pending()); + } + // --- load_console --- #[test]