Implement save state support (retro_serialize/retro_unserialize)#104
Implement save state support (retro_serialize/retro_unserialize)#104JoeMatt wants to merge 2 commits intolibretro:masterfrom
Conversation
Add full save state serialization for all Jaguar hardware modules. Each module provides StateSave/StateLoad functions that serialize their internal static state into a flat byte buffer (~2.4 MB fixed). Modules serialized: 68K CPU (with pointer-to-offset fixup for pc_p), GPU, DSP, Blitter, Event system (with callback registry for function pointer serialization), EEPROM, JERRY timers, TOM timers, CD-ROM, Joystick, Memory Track, and DAC. Closes libretro#93 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements libretro save state support by adding per-module state serialization/deserialization functions and wiring them into retro_serialize() / retro_unserialize() with a versioned header.
Changes:
- Added
src/state.hwith save-state constants and buffer read/write helper macros. - Implemented
StateSave/StateLoadroutines across hardware modules (CPU, GPU/DSP, blitter, timers, input, etc.). - Implemented
retro_serialize_size(),retro_serialize(), andretro_unserialize()inlibretro.c.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libretro.c | Adds libretro save/load entrypoints and orchestrates module serialization. |
| src/state.h | Defines state header constants and helper macros; declares module save/load APIs. |
| src/m68000/m68kinterface.c | Serializes 68K core state, including pointer reconstruction via offsets. |
| src/gpu.c | Serializes GPU RAM/register state and active register bank. |
| src/dsp.c | Serializes DSP RAM/register state plus pipeline/scoreboard. |
| src/blitter.c | Serializes blitter register file and control variables. |
| src/event.c | Serializes event queues using callback-ID registry. |
| src/eeprom.c | Serializes EEPROM state-machine variables. |
| src/jerry.c | Serializes JERRY timer/interrupt-related state. |
| src/tom.c | Serializes TOM timer/interrupt-related state and video dimensions. |
| src/cdrom.c | Serializes CD-ROM emulation buffers and control state. |
| src/memtrack.c | Serializes Memory Track contents and state variables. |
| src/joystick.c | Serializes joystick RAM and button state arrays. |
| src/dac.c | Serializes DAC buffer counters/flags. |
| CLAUDE.md | Updates documentation to remove “No save state support” limitation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Module state */ | ||
| buf += M68KStateSave(buf); | ||
| buf += GPUStateSave(buf); | ||
| buf += DSPStateSave(buf); | ||
| buf += BlitterStateSave(buf); | ||
| buf += EventStateSave(buf); | ||
| buf += EepromStateSave(buf); | ||
| buf += JERRYStateSave(buf); | ||
| buf += TOMStateSave(buf); | ||
| buf += CDROMStateSave(buf); | ||
| buf += JoystickStateSave(buf); | ||
| buf += MTStateSave(buf); | ||
| buf += DACStateSave(buf); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
retro_serialize() always returns true without verifying that the combined header + memory blocks + module StateSave writes exactly STATE_SIZE bytes. If STATE_SIZE is miscomputed (or changes), this can leave trailing bytes uninitialized (non-deterministic savestates / potential data leak) or overrun the provided buffer. Track bytes written and (a) return false if it exceeds size, and (b) zero-fill any remaining bytes so the entire STATE_SIZE is deterministic.
| size_t EepromStateSave(uint8_t *buf) | ||
| { | ||
| uint8_t *start = buf; | ||
|
|
||
| STATE_SAVE_VAR(buf, jerry_ee_state); | ||
| STATE_SAVE_VAR(buf, jerry_ee_op); | ||
| STATE_SAVE_VAR(buf, jerry_ee_rstate); | ||
| STATE_SAVE_VAR(buf, jerry_ee_address_data); | ||
| STATE_SAVE_VAR(buf, jerry_ee_address_cnt); | ||
| STATE_SAVE_VAR(buf, jerry_ee_data); | ||
| STATE_SAVE_VAR(buf, jerry_ee_data_cnt); | ||
| STATE_SAVE_VAR(buf, jerry_writes_enabled); | ||
| STATE_SAVE_VAR(buf, jerry_ee_direct_jump); | ||
|
|
There was a problem hiding this comment.
EepromStateSave/Load currently serializes only the EEPROM state-machine variables, but not the actual EEPROM contents (eeprom_ram / cdromEEPROM) or presence flags. This means loading a savestate will not restore EEPROM data to the saved point in time. Include the EEPROM data arrays (and any related flags needed to interpret them) in the serialized state.
| size_t JoystickStateSave(uint8_t *buf) | ||
| { | ||
| uint8_t *start = buf; | ||
|
|
||
| STATE_SAVE_BUF(buf, joystick_ram, sizeof(joystick_ram)); | ||
| STATE_SAVE_BUF(buf, joypad0Buttons, sizeof(joypad0Buttons)); | ||
| STATE_SAVE_BUF(buf, joypad1Buttons, sizeof(joypad1Buttons)); | ||
|
|
||
| return (size_t)(buf - start); | ||
| } | ||
|
|
||
| size_t JoystickStateLoad(const uint8_t *buf) | ||
| { | ||
| const uint8_t *start = buf; | ||
|
|
||
| STATE_LOAD_BUF(buf, joystick_ram, sizeof(joystick_ram)); | ||
| STATE_LOAD_BUF(buf, joypad0Buttons, sizeof(joypad0Buttons)); | ||
| STATE_LOAD_BUF(buf, joypad1Buttons, sizeof(joypad1Buttons)); | ||
|
|
There was a problem hiding this comment.
JoystickStateSave/Load restores joystick_ram and button arrays, but does not restore the derived enable flags (audioEnabled/joysticksEnabled). Since those flags are set from writes to joystick_ram, a loaded state can end up with joystick_ram saying enabled while the booleans remain stale. Either serialize these booleans too, or recompute them from joystick_ram after STATE_LOAD_BUF.
- Zero-fill remaining bytes in retro_serialize() for deterministic save states and add bounds check against STATE_SIZE overflow - Include eeprom_ram[64] and cdromEEPROM[64] in EepromStateSave/Load so EEPROM contents are preserved across save/load cycles - Include audioEnabled and joysticksEnabled flags in JoystickStateSave/Load so derived state from joystick_ram writes is not stale after load Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Addressed all three Copilot review comments in ea50415:
|
Summary
retro_serialize()/retro_unserialize()supportStateSave/StateLoadfunctions that serialize internal static state into a flat byte bufferVJSS, version 1) for forward compatibilitypc_p/pc_oldpraw pointers saved as offsets intojagMemSpaceand reconstructed on loadCloses #93
Architecture
Files Changed
src/state.h— header with magic/version constants, helper macros, extern declarationsStateSave/StateLoadfunctions added at endlibretro.c—retro_serialize_size,retro_serialize,retro_unserializeimplementedTest plan
🤖 Generated with Claude Code