-
Notifications
You must be signed in to change notification settings - Fork 43
chore: CI and code-quality improvements #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
4c321e6
Update CLAUDE.md
JoeMatt 10de094
ci: remove broken Claude Code Review workflow
JoeMatt 5e6ca34
ci: add EditorConfig + checker job
JoeMatt 738e449
ci: bump deprecated GitHub Action versions
JoeMatt 3ba3ad6
build: generate version.h from a single Makefile source
JoeMatt c9122c6
build: gate Mach-O symbol exports on macOS / iOS / tvOS
JoeMatt 9137389
ci: add cppcheck static analysis job
JoeMatt 47821a7
ci: cross-compile macos-x86_64 from arm64 runner
JoeMatt e9b6058
ci: fix CI failures introduced by version.h refactor + Copilot review
JoeMatt 01400b4
ci: pre-generate version.h for ndk-build
JoeMatt fe10eec
ci: adopt GitFlow with develop integration branch
JoeMatt 2b3c4d2
ci: dependabot + concurrency cancel + .info version check
JoeMatt 50954e8
docs: CONTRIBUTING.md + harden install-hooks.sh
JoeMatt 9f8e831
ci: AddressSanitizer + UndefinedBehaviorSanitizer job
JoeMatt 454e3b4
ci: clang-format check on changed lines (advisory)
JoeMatt 9447692
ci: clang-tidy on PR-changed files (advisory)
JoeMatt 8e05b86
ci: code coverage (gcov + codecov)
JoeMatt ee93fe5
perf: make benchmark target + docs/profiling.md
JoeMatt 3a506d5
docs: issue templates (bug / perf / feature)
JoeMatt 91721e9
ci: PR auto-labeler by file path
JoeMatt 0debf47
ci: clang-format job is informational only
JoeMatt 1f5a5fe
ci: address Copilot review comments
JoeMatt c8affcd
ci: clang-format -- drop -e so diff-returns-1 doesn't kill the script
JoeMatt c4bf8b9
ci: sanitizers job -- pass -fsanitize via LD too
JoeMatt f884f68
ci: tighten .clang-tidy check list + fix two real findings
JoeMatt f141f86
ci: sanitizers job -- use -shared-libasan for .so target
JoeMatt cb385d6
ci: UBSAN ignorelist for src/m68000 + fix profiling.md
JoeMatt ba042e4
ci: address Copilot round 3 review
JoeMatt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,125 +1,82 @@ | ||
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
| Guidance for Claude Code working in this repository. | ||
|
|
||
| ## Project Overview | ||
| ## Project | ||
|
|
||
| Virtual Jaguar libretro core — an Atari Jaguar emulator ported to the libretro API. Written in C, licensed under GPLv3. Upstream: `http://shamusworld.gotdns.org/git/virtualjaguar`. | ||
| Virtual Jaguar libretro core — Atari Jaguar emulator on the libretro API. C, GPLv3. Upstream: `http://shamusworld.gotdns.org/git/virtualjaguar`. | ||
|
|
||
| ## Build Commands | ||
| ## Build | ||
|
|
||
| ```bash | ||
| make -j$(getconf _NPROCESSORS_ONLN) # Build (auto-detects platform) | ||
| make -j$(getconf _NPROCESSORS_ONLN) DEBUG=1 # Debug build (-O0 -g) | ||
| make clean # Clean build artifacts | ||
| make platform=ios-arm64 # Cross-compile for specific platform | ||
| make -j$(getconf _NPROCESSORS_ONLN) DEBUG=1 # Debug (-O0 -g) | ||
| make clean | ||
| make platform=ios-arm64 # Cross-compile target | ||
| ``` | ||
|
|
||
| Output binary name varies by platform: | ||
| - macOS: `virtualjaguar_libretro.dylib` | ||
| - Linux: `virtualjaguar_libretro.so` | ||
| - Windows: `virtualjaguar_libretro.dll` | ||
| Output: `virtualjaguar_libretro.{dylib,so,dll}`. CI: `make -j4` on Ubuntu (GCC) and macOS (Clang) plus `test/regression_test.sh` screenshots. | ||
|
|
||
| CI runs `make -j4` on Ubuntu (GCC) and macOS (Clang), plus screenshot regression tests via `test/regression_test.sh`. See `docs/test-infrastructure.md` for the full test harness inventory. | ||
| ## C89 / GNU89 — strict | ||
|
|
||
| ## Architecture | ||
| The libretro buildbot uses MSVC on Windows. CI has a `c89-lint` job. Run `bash scripts/c89-lint.sh src/YOURFILE.c` before pushing. | ||
|
|
||
| ### C Language Standard — C89/GNU89 | ||
| - **No mid-block declarations.** All vars at top of block, before any statement. Most common violation. | ||
| - `//` comments allowed (GNU89), but prefer `/* */` for new code. | ||
| - No C99: no `for (int i…)`, no compound literals, no designated initializers, no VLAs. | ||
| - Exempt: `src/tom/blitter_simd_{sse2,neon}.c` (need platform headers), `src/m68000/*` (machine-generated). | ||
|
|
||
| This codebase **must** compile as C89 (GNU89 dialect). The libretro buildbot uses MSVC on Windows, which enforces C89 strictly. CI includes a `c89-lint` job that catches violations. | ||
| ## Hardware model | ||
|
|
||
| **Rules:** | ||
| - **No mid-block variable declarations.** All variables must be declared at the top of their enclosing block (function or `{}`), before any statements. This is the most common violation. | ||
| - `//` comments are allowed (GNU89 extension), but `/* */` is preferred for new code. | ||
| - No C99 features: no `for (int i = ...)`, no compound literals, no designated initializers, no VLAs. | ||
| - SIMD files (`src/tom/blitter_simd_sse2.c`, `src/tom/blitter_simd_neon.c`) are exempt from the lint check since they require platform-specific headers. | ||
| - Machine-generated files (`src/m68000/*`) are also exempt. | ||
| Four processors, unified memory map, big-endian. `GET16/GET32/SET16/SET32` macros byte-swap on LE hosts. Map in `src/core/vjag_memory.h`: RAM 0x000000 (2 MB), cart 0x800000, TOM regs 0xF00000, JERRY regs 0xF10000. | ||
|
JoeMatt marked this conversation as resolved.
Outdated
|
||
|
|
||
| **Local check before pushing:** | ||
| ```bash | ||
| gcc -fsyntax-only -std=gnu89 -Werror=declaration-after-statement \ | ||
| -I. -Isrc -Isrc/core -Isrc/tom -Isrc/jerry -Isrc/cd -Isrc/bios -Isrc/m68000 -Ilibretro-common/include \ | ||
| -D__LIBRETRO__ -DINLINE="inline" src/YOURFILE.c | ||
| ``` | ||
|
|
||
| ### Atari Jaguar Hardware Emulation | ||
|
|
||
| The Jaguar has four processors sharing a unified memory-mapped address space: | ||
|
|
||
| - **Motorola 68000** (13.3 MHz) — main CPU for game logic. Emulated via UAE-derived core in `src/m68000/`. The `cpuemu.c` file is machine-generated and very large (~1.8 MB). | ||
| - **GPU** (26.6 MHz RISC) — graphics coprocessor in `src/tom/gpu.c` | ||
| - **DSP** (26.6 MHz RISC) — audio coprocessor in `src/jerry/dsp.c`, same instruction set as GPU | ||
| - **Object Processor** — sprite/bitmap rendering in `src/tom/op.c` | ||
|
|
||
| Two custom chips contain these processors: | ||
| - **TOM** (`src/tom/tom.c`) — video output, GPU, Object Processor, Blitter (`src/tom/blitter.c`) | ||
| - **JERRY** (`src/jerry/jerry.c`) — audio DAC (`src/jerry/dac.c`), DSP, timers, EEPROM (`src/jerry/eeprom.c`) | ||
|
|
||
| ### Execution Model | ||
|
|
||
| Frame execution is event-driven, not cycle-accurate. `JaguarExecuteNew()` in `src/core/jaguar.c` runs the main loop: the 68K executes until the next timed event, then GPU runs for the same timeslice, then event callbacks fire (half-line rendering, timer interrupts, etc.). | ||
|
|
||
| ### Memory | ||
|
|
||
| Memory map defined in `src/core/vjag_memory.h`. The Jaguar is big-endian; `GET16/GET32/SET16/SET32` macros handle byte-swapping on little-endian hosts. Main RAM is 2 MB at 0x000000, cart ROM at 0x800000, TOM registers at 0xF00000, JERRY registers at 0xF10000. | ||
|
|
||
| ### Libretro Integration | ||
|
|
||
| `libretro.c` (top-level) implements the libretro API — initialization, per-frame execution, input polling, video/audio output. Video is XRGB8888 at dynamic resolution (typically 320x240 NTSC / 320x256 PAL). Audio is 48 kHz 16-bit stereo. | ||
|
|
||
| Core options defined in `libretro_core_options.h` control blitter mode, BIOS usage, NTSC/PAL, DSP execution, and input mapping. | ||
|
|
||
| ### Key Directories | ||
|
|
||
| - `src/core/` — top-level emulator orchestration, memory map, events, settings, files, cheats | ||
| - `src/tom/` — TOM-side video, GPU, Object Processor, blitter, and blitter SIMD | ||
| - `src/jerry/` — JERRY-side audio, DSP, DAC, EEPROM, input, wavetable | ||
| - `src/cd/` — Jaguar CD/BUTCH and disc-interface layer | ||
| - `src/bios/` — embedded BIOS and boot stub arrays | ||
| - `src/m68000/` — UAE-derived 68K CPU emulation | ||
| - `libretro-common/` — shared libretro utility library (string, file, VFS) | ||
| - `docs/` — documentation: changelog, known issues, BUTCH register map, CD data flow, test infrastructure | ||
| - `test/tools` — test scripts and headless front-ends | ||
| - `test/roms` — test ROMs; `private/` subdirectory has commercial ROMs and BIOSes | ||
|
|
||
| ### Build System | ||
| - **68000** (13.3 MHz, `src/m68000/`) — main CPU. UAE-derived. `cpuemu.c` is **machine-generated, ~1.8 MB** — never read whole; grep first, then `Read` with offset/limit only on matched ranges. | ||
| - **GPU** (26.6 MHz RISC, `src/tom/gpu.c`) — graphics coprocessor. | ||
| - **DSP** (`src/jerry/dsp.c`) — same ISA as GPU; audio. | ||
| - **Object Processor** (`src/tom/op.c`) — sprite/bitmap rendering. | ||
| - **TOM** (`src/tom/tom.c`) — video, GPU, OP, Blitter (`src/tom/blitter.c`). | ||
| - **JERRY** (`src/jerry/jerry.c`) — audio DAC, DSP, timers, EEPROM. | ||
|
|
||
| `Makefile` handles 30+ platform targets with auto-detection. `Makefile.common` lists all source files. Platform is selected via `platform=` variable or auto-detected from `uname`. Key flags: `-D__LIBRETRO__`, `-DMSB_FIRST` for big-endian platforms. | ||
| Frame loop is event-driven (not cycle-accurate): `JaguarExecuteNew()` in `src/core/jaguar.c` runs 68K to next event, then GPU, then fires callbacks (half-line render, timers). | ||
|
|
||
| ### Jaguar CD Emulation | ||
| ## Libretro layer | ||
|
|
||
| CD support is implemented across `src/cd/cdrom.c` (BUTCH chip / FIFO / DSA commands), `src/cd/cdintf.c` (disc image loading: CUE/BIN, CHD, CDI), and hooks in `src/core/jaguar.c` (BIOS auth bypass, boot stub injection). | ||
| `libretro.c` (top-level) implements the API. Video XRGB8888 dynamic res (320×240 NTSC / 320×256 PAL). Audio 48 kHz 16-bit stereo. Core options in `libretro_core_options.h` (blitter mode, BIOS, NTSC/PAL, DSP, input). | ||
|
|
||
| Key docs: | ||
| - `docs/butch-registers.md` — full BUTCH register map ($DFFF00-$DFFF2F) with bit definitions | ||
| - `docs/cd-data-flow.md` — how CD data moves from disc to RAM (I2S -> FIFO -> GPU ISR -> RAM), BIOS code map, boot stub layout | ||
| ## Layout | ||
|
|
||
| ### Testing | ||
| - `src/core/` — orchestration, memory map, events, settings, files, cheats | ||
| - `src/tom/` — video, GPU, OP, blitter (+ SIMD) | ||
| - `src/jerry/` — audio, DSP, DAC, EEPROM, input, wavetable | ||
| - `src/cd/` — Jaguar CD: BUTCH/FIFO/DSA in `cdrom.c`, image loading (CUE/BIN, CHD, CDI) in `cdintf.c`; BIOS auth bypass + boot stub in `src/core/jaguar.c` | ||
| - `src/bios/` — embedded BIOS / boot stubs | ||
| - `src/m68000/` — UAE 68K (machine-generated; treat as opaque) | ||
| - `libretro-common/` — shared utility lib | ||
| - `test/tools/` — test harnesses; `test/roms/private/` — commercial ROMs/BIOSes (gitignored) | ||
|
|
||
| RetroAchievements-related — **no RetroAchievements account, API, or gameplay server**; local validation only. The E2E harness still **fetches the pinned rcheevos source tarball from GitHub** when `build/rcheevos-static` is missing (CI may cache that directory); that is unrelated to contacting RetroAchievements services. | ||
| ## Build system | ||
|
|
||
| - `test/tools/test_memory_map.c` — asserts `SET_MEMORY_MAPS`, `SET_SUPPORT_ACHIEVEMENTS` with **`true`**, and descriptor layout vs `retro_get_memory_data(SYSTEM_RAM)`. | ||
| - `test/tools/test_rcheevos_e2e.sh` — downloads pinned **rcheevos** (`RCHEEVOS_REF`) when needed, builds `librcheevos.a`, then runs `test_rcheevos_e2e` to verify **rc_libretro** memory resolution (`RC_CONSOLE_ATARI_JAGUAR`) matches host RAM — the same mapping stack RetroArch uses before any RA cloud call. | ||
| `Makefile` covers 30+ targets, auto-detected via `uname` or `platform=`. `Makefile.common` lists sources. Flags: `-D__LIBRETRO__`, `-DMSB_FIRST` for big-endian. | ||
|
|
||
| See `docs/test-infrastructure.md` for all test harnesses: | ||
| - `test/test_dsp_mac40.c` — Jaguar DSP **40-bit MAC** accumulator semantics (`dsp_acc40.h`), run in CI with SIMD tests; relevant for long IIR chains (e.g. pink-noise generators on DSP). | ||
| - `test/headless.py` — Python headless runner via libretro.py (screenshots, frame control) | ||
| - `test/regression_test.sh` — screenshot regression suite with baseline comparison | ||
| - `test/test_cd_boot.c` — low-level C harness with dlsym access to 68K registers and RAM | ||
| - `test/sram_test.sh` — SRAM interface round-trip testing | ||
| ## Testing | ||
|
|
||
| #### Headless framebuffer / 240p suite — how to report issues | ||
| Local-only RetroAchievements validation — no RA account/API/server. `test/tools/test_rcheevos_e2e.sh` downloads pinned `RCHEEVOS_REF` and verifies `rc_libretro` mapping (`RC_CONSOLE_ATARI_JAGUAR`) matches host RAM. | ||
|
|
||
| Profiling symbols like **`__muldi3`** (64-bit multiply helpers on some 32-bit ABIs) are a **compiler/performance** concern, **not** evidence that the Jaguar 240p test ROM’s DSP pink-noise path or NTSC timing is wrong. Do **not** frame “240p fails” primarily as a **`__muldi3`** bug unless you are optimizing a 32-bit build for speed. | ||
| Key harnesses: | ||
| - `test/regression_test.sh` — screenshot regression vs `test/baselines/` | ||
| - `test/headless.py` — libretro.py runner (frames, screenshots) | ||
| - `test/tools/test_memory_map.c` — asserts `SET_MEMORY_MAPS`, `SET_SUPPORT_ACHIEVEMENTS=true`, descriptor layout | ||
|
JoeMatt marked this conversation as resolved.
Outdated
|
||
| - `test/tools/test_blitter_compare` — fast vs accurate blitter diff | ||
| - `test/test_dsp_mac40.c` — DSP 40-bit MAC accumulator (`dsp_acc40.h`) | ||
| - `test/test_cd_boot.c` — dlsym harness for 68K regs/RAM | ||
| - `test/sram_test.sh` — SRAM round-trip | ||
|
|
||
| The **useful** regression story for automated screenshot / libretro.py / SessionBuilder runs is: | ||
| ### Headless framebuffer caveat | ||
|
|
||
| - **Symptom:** On some cores or builds, a **non-RetroArch headless session** does not expose the **same composited framebuffer** via the libretro API (`video_screenshot`, etc.) as **RetroArch with the same core binary** — e.g. main menu of **jag_240p_test_suite v1.0.0** shows only a **thin band** (~order of **1k** non-black RGB pixels) vs **tens of thousands** on a known-good path. | ||
| - **Interpretation:** Suspect **presentation / pixel source** — Object Processor and blitter output vs **what headless clients actually read** — until disproven with hardware or reference captures. This is **not** “prove 240p timing is wrong first.” | ||
| - **Checks:** Use in-repo gates (e.g. **screenshots-preflight** / main-menu sanity, non-black pixel floor on the **~2000+** scale). Passing preflight ⇒ the **headless read-path** issue is resolved for that artifact; failing preflight ⇒ file a **framebuffer/compositing** bug for **headless libretro** consumption (logs, **two artifacts**: broken vs good), not a long **`__muldi3`** narrative. | ||
| Some non-RetroArch headless harnesses (libretro.py, miniretro) don't expose the same composited framebuffer that RetroArch reads. Symptom: `jag_240p_test_suite` main menu shows ~1k non-black pixels via headless vs tens of thousands via RetroArch. Treat that as a **headless read-path / presentation bug** (OP+blitter output vs what the host reads), not a 240p timing or `__muldi3` performance bug. Gate via `make screenshots-preflight`. | ||
|
JoeMatt marked this conversation as resolved.
Outdated
|
||
|
|
||
| ### Known Limitations | ||
| ## Known limitations | ||
|
|
||
| - Blitter not fully cycle-accurate (some games need fast blitter mode) | ||
| - Bus contention between processors not emulated | ||
| - Vertical count (VC) register behavior not fully accurate | ||
| - Blitter not fully cycle-accurate (some games need fast mode). | ||
| - No bus contention modeling. | ||
| - VC register behavior not fully accurate. | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.