Skip to content

Preflight guest snapshot layouts while preserving legacy title chunks - #198

Merged
mstan merged 5 commits into
RetroPortingToolKit:mainfrom
sergiomanzur:codex/transactional-snapshot-loads
Oct 24, 2026
Merged

mstan merged 5 commits into
RetroPortingToolKit:mainfrom
sergiomanzur:codex/transactional-snapshot-loads

Conversation

@sergiomanzur

@sergiomanzur sergiomanzur commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Truncated guest snapshots could mutate the running machine before load failure was reported. File and memory loads now validate the complete guest prefix with the production serializer and private CPU/DMA/Snes containers before restoring live guest state.

Title extensions keep their existing opaque loader contract. The framework does not call a save callback to infer accepted extension sizes. Optional snapshot_preflight runs outside the APU lock for titles that need extension rejection before live writes. Arbitrary late state_load_extra effects are not transactional.

Rebased onto current main 68896b6, preserving the DSP-1 implementation identity check. The extracted regression fixture now explicitly includes <limits.h> and snapshot_identity.h, repairing the failing compile on Linux and after the rebase.

Validation of head 64de342:

All three GitHub workflows (native analyzer, self-contained CLI and Lua) are green on this head, including Linux/Windows/macOS analyzer jobs.

Input remains capped at 16 MiB and must remain stable during the synchronous call. Current-build save/load and invalid-input safety are the contract; backward save-state compatibility is not a release requirement. No new cross-title gameplay or arbitrary title-callback transaction claim is made.

mstan
mstan previously requested changes Oct 21, 2026

@mstan mstan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: the legacy extension fallback breaks supported title save-state compatibility.

In snapshot_check_guest, counting state_save_extra and requiring its current size to equal the incoming extension rejects older layouts that existing state_load_extra callbacks deliberately accept. The local released Mega Man X implementation has version-dependent v1-v16+ extension sizes, and Super Mario World accepts legacy SMWS/foreign chunks; neither registers state_validate_extra or snapshot_preflight. Their existing snapshots would start failing after an engine update. MMX's save callback also calls MmxCoopCapture, so invoking it while checking a rejected input does not satisfy the new purity assumption.

ROM-free reproduction: in this PR's transaction fixture, leave the already-supported one-byte load_variable path installed but omit the newly added validator, as existing consumers do. A valid RtlLoadSnapshotFromMemory(snapshot, core+1) is rejected (reproduced locally). Documentation of a new required callback does not migrate those consumers.

Please preserve the existing loader contract or include coordinated validators and compatibility tests for supported older title chunks before merging. Do not merely relax the size check and claim transactional protection: arbitrary legacy callbacks still need a real validation strategy.

The rest of the five-PR bundle passed 826 Python tests (2 skips), 109 Rust tests, and production C interpreter/bridge, PPU, audio-delivery, DMA snapshot, APU-port and DSP trace-gate checks. This is a consumer compatibility blocker despite those green generic tests. No new cross-title gameplay qualification is claimed.

@sergiomanzur sergiomanzur changed the title Validate snapshot streams before mutating live state Preflight guest snapshot layouts while preserving legacy title chunks Oct 21, 2026
@mstan
mstan dismissed their stale review October 21, 2026 21:35

Withdrawing the backward-compatibility merge requirement. The owner explicitly confirmed that older save-state compatibility is never required, across engine/game/serializer/backend revisions. Breaking or rejecting old snapshots is acceptable and requires no migration work. Same-build save/load, rewind/rollback and safe rejection still matter; native game saves are a separate contract. This supersedes that part of my prior review. The updated PR also removes the save-callback purity problem. This dismissal does not constitute approval of the newly revised head; current CI/code review remain separate.

@mstan
mstan force-pushed the codex/transactional-snapshot-loads branch from 5027379 to 64de342 Compare October 24, 2026 15:35
@mstan
mstan merged commit 4099a58 into RetroPortingToolKit:main Oct 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants