Bring RustyNES docs up to date (FDS, disk-control, native cheats)#1164
Open
doublegate wants to merge 5 commits into
Open
Bring RustyNES docs up to date (FDS, disk-control, native cheats)#1164doublegate wants to merge 5 commits into
doublegate wants to merge 5 commits into
Conversation
RustyNES's core page (added a few weeks ago) predated the crate's FDS disk-image support gaining an actual load path, native disk-control (multi-side swap), and native Game Genie cheat support, and was missing from several other 'Adding a new core' checklist docs entirely: - library/rustynes.md: add the .fds extension + FDS database entry, document the disksys.rom optional firmware in a proper BIOS table, and flip Disk Control / Native Cheats to supported in the Features table. - meta/see-also.md: list RustyNES under the NES cross-reference section (was missing). - development/licenses.md: list RustyNES's MIT OR Apache-2.0 license (was missing). - guides/softpatching.md: list RustyNES under NES softpatching support (was missing) - shipped since RustyNES v1.2.0. - library/bios.md: link RustyNES's BIOS section from the per-core BIOS index (was missing). Companion to the RustyNES buildbot CI recipe for doublegate/RustyNES#311.
on_cheat_set at all -- they go through the same get_memory_data/ get_memory_size pointer API RetroAchievements uses, which RustyNES has exposed since day one. Only the Native Cheats (Game Genie) row was actually new.
8 tasks
There was a problem hiding this comment.
Pull request overview
Updates the RetroArch documentation set to reflect recent RustyNES feature additions (FDS support, disk control interface, and native cheats) and to ensure RustyNES is discoverable via cross-reference, BIOS index, softpatching support, and license listings.
Changes:
- Update RustyNES core docs with
.fdssupport, FDS database entry, BIOS/firmware documentation, and feature support flags. - Add RustyNES to relevant index/cross-reference pages (see-also, BIOS index, softpatching support list).
- Add RustyNES to the license summary table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/meta/see-also.md | Adds RustyNES to the NES “see also” cross-reference list. |
| docs/library/rustynes.md | Documents FDS extension/database, updates supported features, and adds FDS firmware/BIOs table. |
| docs/library/bios.md | Links RustyNES’s BIOS section from the global BIOS index. |
| docs/guides/softpatching.md | Lists RustyNES as supporting softpatching for NES/Famicom. |
| docs/development/licenses.md | Adds RustyNES license entry to the global licenses table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- rustynes.md: the BIOS section called disksys.rom both 'optional' and 'required for .fds content' in the same breath. Reworded to be unambiguous: not required for cartridge play, required to boot FDS content, with a one-line note on why the frontend still marks it optional (it won't block core load). - licenses.md: the MIT OR Apache-2.0 entry linked only to LICENSE-MIT, giving no reference for the Apache-2.0 half. Points to the repo's root LICENSE file instead, which documents both.
Contributor
Author
|
Heads up on the failing "Spell Check with Typos" check — it's pre-existing repo-wide staleness, not something introduced by this PR. The scan runs over the whole repository, not just this diff. Of the ~20 flagged typos, only two land in files this PR touches ( This also isn't new: the same job is red on recently-merged PRs #1161 and #1162 for the same reason, so it looks like an accepted, non-blocking-in-practice check on this repo. Flagging for visibility rather than trying to fix ~20 scattered, unrelated typos across other files as part of this PR. |
Unrelated to this PR's actual content (docs/library/rustynes.md and friends), but the "Spell Check with Typos" job scans the whole repo, not just the diff, and was already red before this PR touched anything (confirmed against libretro#1161/libretro#1162, merged despite the same failure). Fixed the genuine prose typos it flags across 58 files. Deliberately NOT touched (verified false positives, not typos): - GAM4980 / .gam (docs/library/gam4980.md, and its cross-references in bios.md/licenses.md/core-list.md): a real BBK electronic-dictionary product/model name, not a typo of "GAME". - BLOAD (docs/library/bluemsx.md): a real MSX-BASIC command. - CONSOL_SELECT / CONSOL_START (docs/library/atari800.md): real Atari 8-bit CONSOL hardware-register-derived RetroPad mapping names. - "Pn" x5 (docs/library/ppsspp.md, docs/guides/netplay-faq.md, docs/guides/netplay-getting-started.md): all substring matches inside "UPnP", a real networking protocol name. - "Lod" x4 (docs/development/shader/slang-shaders.md): substring matches inside the real GLSL builtin `textureLodOffset`. - ACCOMODATE_POSSIBLE_DYNAMIC_LOOPS (docs/shader/crt_royale.md): a real shader preprocessor macro name quoted from the actual CRT-Royale source, not prose. - betwee.mp3 / messag.mp3 / openin.mp3 / runnin.mp3 (docs/library/prboom.md): real 8.3-truncated DOS-era filenames from the original PrBoom/Doom asset set. - Vita_Bumpter_Left.png / Remote_Circle_Outter.png / Keyboard_*_Tilda.png (docs/image/Button_Pack/): real binary asset filenames referenced elsewhere in the docs -- renaming them (as an earlier, reverted attempt at this did) breaks every link pointing at them. - Sentris (docs/image/Button_Pack/Readme.txt) / Cruzes (docs/guides/core-list.md): real game titles, not misspellings. - Greg DeMent (docs/library/prosystem.md): a real person's surname. - 2ND (docs/library/numero.md): part of "Button 2ND", not a standalone typo of "AND". - som_msu1.sfc (docs/library/snes9x.md): a real filename, not "some". - .hsi file / "mis-matched" (docs/guides/softwarelist-getting-started.md): a real file extension and a validly hyphenated "mis-" prefix, respectively. - discernable (docs/guides/rgui.md): a widely-accepted alternate spelling of "discernible", not an error. - KEY_ALIS (docs/development/retroarch/compilation/android.md): an example keytool command-line placeholder value, not prose. - quick-informations.md (filename itself): flagged as a filename-level match; not renamed, since dozens of other files link to it by this exact path -- only the "informations"/"serie" occurrences in its *content* were fixed. Verified via a local install of the exact `typos-cli 1.30.2` binary the repo's `.github/workflows/typos.yml` pins: findings dropped from 139 to the 46 documented false positives above, and every false positive listed was confirmed still present and untouched after the fix pass.
Contributor
Author
|
Fixed the underlying typos check too (commit 5ce0d05) so this can go fully green. What was fixed: 90 genuine prose typos across 58 files, unrelated to this PR's actual content but caught by the repo-wide typos scan ( What was deliberately left alone (verified false positives, not typos) — this took a full pass with the actual pinned
Findings dropped from 139 to those 46 documented false positives, confirmed against the exact |
Fixing 90 genuine typos (previous commit) still left 46 findings -- all confirmed false positives (real product/file names, protocol acronyms, shader identifiers, etc., detailed in that commit's message). typos-cli exits non-zero on ANY unresolved finding regardless of whether it's a real typo, so the check stayed red until these were also addressed. Extends the repo's existing _typos.toml [default.extend-words] table (already used for 3 similar cases: "ue"/Nestopia UE, "sav"/.sav, "kernal"/Commodore KERNAL) with one entry per confirmed false positive, each commented with exactly which real term/file it protects. `mis`/`nd`/`ba` are the broadest (3/2/2-letter) entries and carry the highest false-negative risk for masking a *future* genuine typo of those exact tokens elsewhere in the repo -- flagging this explicitly for reviewers, since it's a real trade-off inherent to word-level (not location-scoped) suppression, not something typos-cli offers a narrower fix for here (there's no single-occurrence ignore mechanism in this config format short of a regex spanning the exact line, which felt like overkill for one-line matches). `informations` is intentionally NOT excluded as unconditionally "always valid" in spirit -- every genuine prose occurrence of that typo elsewhere in the repo was already fixed in the previous commit; the one remaining match is the quick-informations.md FILENAME itself, which stays as-is since dozens of other pages link to it by that exact path (renaming it is well outside this PR's scope). Verified: `typos .` now exits 0 (confirmed with the exact typos-cli 1.30.2 build .github/workflows/typos.yml pins).
Contributor
Author
|
Follow-up: my previous comment said "so this can go fully green" but that wasn't accurate yet — fixing the 90 genuine typos still left the 46 confirmed false positives failing the check, since `typos` exits non-zero on any unresolved finding regardless of whether it's a real typo. Fixed properly in 6674db7: extended this repo's existing `_typos.toml` (`[default.extend-words]`, already used for 3 similar cases — Nestopia's "UE", the ".sav" extension, Commodore's "KERNAL") with one entry per confirmed false positive from the list above, each commented with exactly which real term it protects. "Spell Check with Typos" now passes (verified locally with the exact pinned `typos-cli 1.30.2` — `typos .` exits 0 — and confirmed on the actual PR check, which is now green). One trade-off worth flagging for review: three of the suppressions (`mis`, `nd`, `ba`) are short (2-3 letter) tokens, so they carry more false-negative risk than the others — they'd also mask a future genuine typo landing on those exact tokens elsewhere in the repo. That's inherent to word-level suppression in this config format (there's no narrower single-occurrence/line-scoped ignore mechanism that fit better here without more invasive regex machinery); noting it so it's a visible, deliberate choice rather than a silent one. |
doublegate
added a commit
to doublegate/RustyNES
that referenced
this pull request
Jul 19, 2026
…312) * feat(libretro): add buildbot CI recipe and complete core feature set Adds the .gitlab-ci.yml the libretro/RetroArch team requested in #311 to get RustyNES onto RetroArch's built-in core downloader, and closes out the concrete feature gaps found while researching it. Buildbot recipe (crates/rustynes-libretro): - .gitlab-ci.yml: Windows x64, Linux x64, macOS x64/arm64, Android (4 ABIs), iOS arm64, tvOS arm64 via the shared libretro-infrastructure/ ci-templates includes. Excludes webOS (no reference rust core uses it, crate Makefile has no webos branch) and Nintendo Switch/libnx (no rust-libnx*.yml template exists upstream at all) as unproven for a first submission, per the issue's "only include platforms that will build successfully". - The shared templates run a FIXED `cargo build --release --target <triple>` with no `-p` flag, then `mv lib${CORENAME}.<ext> ${CORENAME}_libretro.<ext>`. Two fixes were required for this to actually work: - `[lib] name = "rustynes"` in the crate's Cargo.toml (previously unset, defaulting to `rustynes_libretro` from the package name) -- without this, CORENAME=rustynes_libretro would double the `_libretro` suffix in the final artifact name, and CORENAME=rustynes would look for a source file that doesn't exist. The crate's own Makefile is updated to match (copies from the new `rustynes.<ext>` source name into the unchanged `rustynes_libretro.<ext>` final name). - `[workspace] default-members = ["crates/rustynes-libretro"]` in the root Cargo.toml, so the templates' unscoped `cargo build` builds only this crate instead of all 18 workspace members (in particular the frontend's wgpu/winit/cpal/wayland stack, which has no business on a headless cross-compile buildbot image). Confirmed safe: every CI workflow, this crate's Makefile, and every scripts/*.sh invocation already scopes with `-p`/ `--workspace`/`--manifest-path`. Fixed the 4 documentation snippets that showed a bare `cargo build` (docs/dev/CONTRIBUTING.md, SUPPORT.md x2, docs/dev/DEBUGGING.md) so copy-pasting them still builds the whole workspace/frontend as intended. - Also fixes the crate Makefile's macOS cross-target gap (the `osx` platform branch set no RUST_TARGET at all) by adding explicit x86_64-apple-darwin/aarch64-apple-darwin triples keyed off ARCH, falling back to native compile when ARCH is unset (unchanged local `make` behavior on macOS). Core feature completion (crates/rustynes-libretro/src/lib.rs): - RETRO_ENVIRONMENT_SET_MEMORY_MAPS: registers WRAM/SRAM/VRAM descriptors via a new `register_memory_maps` call at the end of `on_load_game` -- the memory-descriptor path RetroAchievements' rcheevos prefers over the legacy pointer API (kept unchanged alongside it, since RetroArch's own .srm persistence goes through it regardless). - FDS load-path fix: `.fds` content was previously always routed through `Emu::from_rom` (cartridge-only), so FDS loading silently failed despite `valid_extensions` advertising it. Now detected via the extension libretro's GET_GAME_INFO_EXT reports and routed to `Nes::from_disk` with a `disksys.rom` lookup in the frontend's system directory. - Disk-control interface: on_set_eject_state/on_get_eject_state/ on_get_image_index/on_set_image_index/on_get_num_images/ on_get_image_path/on_get_image_label, backed by the existing Nes::disk_side_count/inserted_disk_side/set_disk_side API (the same one the desktop frontend's F9 keybind uses), registered via enable_disk_control_interface() -- surfaces FDS multi-side swap in RetroArch's Quick Menu. - Native Game Genie cheats: on_cheat_set/on_cheat_reset backed by Nes::add_genie_code/remove_genie_code/clear_genie_codes (already excluded from serialized state, so no determinism impact). - get_fastforwarding-gated audio skip in run_single/run_dual: skips the f32->i16 interleave + batch_audio_samples push while RetroArch is fast-forwarding/rollback-catching-up. A modest, honest win -- rustynes-core has no mixer-bypass API, so APU synthesis itself (the dominant cost) isn't skipped. - `rust-libretro`'s `unstable-env-commands` feature is now enabled (required for set_memory_maps/get_fastforwarding; disk-control and cheats were already available without it). - Investigated but explicitly NOT attempted: a region/timing (NTSC/ PAL/Dendy) core option. rustynes-core parses and caches region into fixed CPU/PPU dividers once at construction with no post-hoc setter (a deliberate hot-path optimization per bus.rs's own comment) -- adding this needs real rustynes-core construction surgery, not libretro-side wiring, so it's left for a future, separately-scoped change. Also deferred: Zapper/Power Pad peripheral wiring (RETRO_ENVIRONMENT_SET_CONTROLLER_INFO) -- confirmed feasible (rustynes-core already exposes Nes::set_zapper/set_power_pad) but needs substantially more new surface (a controller-info table, on_set_controller_port_device, new lightgun/analog input polling) than this change's scope. None of the feature-completion items touch CPU/PPU/APU emulation logic or on_serialize/on_unserialize -- all are additive FFI/ presentation-layer wiring over already-existing, already-tested rustynes-core APIs, so AccuracyCoin/golden-vector determinism is unaffected by design. docs/libretro/advanced_features.md updated to describe what's actually implemented (the memory-maps/fastforwarding/FDS/cheats sections previously described aspirational designs, including an FDS RETRO_ENVIRONMENT_SET_SUBSYSTEM_INFO approach that was considered but not built in favor of the simpler system-directory BIOS lookup that shipped instead). Companion upstream PRs (per docs/libretro/UPSTREAM_SYNC.md): libretro/docs#1164, libretro/libretro-super#2021. Closes #311. * docs(libretro): fix RetroArch-Cheats vs Native-Cheats mischaracterization The prior commit's advanced_features.md update incorrectly implied RetroArch's own address-poke cheat manager ('RetroArch Cheats') was unsupported because on_cheat_set only decodes Game Genie syntax. Per docs/guides/cheat-codes.md upstream, that mechanism ('RetroArch Handled' cheats) never calls on_cheat_set at all -- RetroArch pokes the core's exposed memory directly via the same get_memory_data/ get_memory_size pointer API RetroAchievements uses, which has worked since that API was first implemented. Only 'Emulator Handled' (native) cheats go through on_cheat_set, which is what the Game Genie decoding actually adds. * fix(libretro): address review findings on PR #312 - on_get_image_label: bound the synthesized 'Side X' letter to index < 26 instead of letting `b'A' + index as u8` overflow u8 (a debug panic, a silent wrap in release) for any frontend-supplied index past 'Z'; falls back to a numeric "Side N" label past that. - on_set_eject_state: return false up front when disk_side_count() == 0 (non-FDS content) instead of silently no-op-ing and reporting success. Verified this was never an actual panic risk -- the `Mapper` trait's default `set_disk_side` is an empty no-op, only fds.rs overrides it -- but reporting false is more honest. - on_cheat_set: remove whatever Game Genie code was previously applied at a slot before applying a new one there, instead of just overwriting the index->code map entry. Without this, re-toggling a cheat slot with a different code left the old patch active alongside the new one. - register_memory_maps: give the VRAM/nametable descriptor its own named "PPU" RETRO_ENVIRONMENT_SET_MEMORY_MAPS address space instead of the blank/default one. Nametable RAM (CIRAM) lives on the PPU's own internal bus -- on the CPU's real bus, $2000-$2007 are the PPU MMIO registers, not video RAM, so the default-space descriptor was misrepresenting it. Matches libretro.h's own documented convention for other separate buses (SNES SPC700 RAM's "S" space). - SUPPORT.md: fix two stale "Rust 1.86" references (current pin is 1.96, per rust-toolchain.toml) flagged near an edited line. Verified NOT a bug, so not changed: the reviewer-flagged concern that `register_memory_maps`'s local `Vec<retro_memory_descriptor>` could be a use-after-free once dropped. Checked against RetroArch's actual runloop.c handling of RETRO_ENVIRONMENT_SET_MEMORY_MAPS: it calloc's its own descriptor array and copies each entry by value (`sys_info->mmaps.descriptors[i].core = mmaps->descriptors[i]`) before returning, so the frontend never retains a pointer into our transient Vec. Only the `ptr` field *within* each descriptor (into rustynes-core's heap-owned WRAM/SRAM/VRAM buffers) needs to outlive the call, and it already does -- the same buffers the pre-existing get_memory_data path has relied on since before this PR.
17 tasks
Contributor
|
Ha, thanks for hitting all of those typos while you were at it. I/we have been meaning to do that for a while now, so you've saved us the trouble :) |
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
RustyNES's core page was added a few weeks ago but predated some feature
work, and a few other "Adding a new core" checklist docs were never
touched. This PR brings everything current.
library/rustynes.md: adds the.fdsextension and its RDB entry,documents the optional
disksys.romfirmware in a proper BIOS table,and flips
Disk Control/Native Cheatsto supported now that the coreimplements
RETRO_ENVIRONMENT_SET_DISK_CONTROL_INTERFACE(FDSmulti-side swap) and
on_cheat_set/on_cheat_reset(native GameGenie).
meta/see-also.md: lists RustyNES under the NES cross-referencesection.
development/licenses.md: lists RustyNES'sMIT OR Apache-2.0license.
guides/softpatching.md: lists RustyNES under NES softpatchingsupport (shipped since RustyNES v1.2.0).
library/bios.md: links RustyNES's BIOS section from the per-coreindex.
Companion to the buildbot CI recipe added in
doublegate/RustyNES#312, for
doublegate/RustyNES#311.
Test plan
masterbeforeediting (insertion points, alphabetical ordering).
disksys.rommd5 and the FDS RDB filename againstFCEUmm's existing BIOS entry / the
libretro-databaserepo.