fix(dm2-saves): Mac byte-order fidelity + robustness batch (#168–#176) #209

Merged

jqueuniet merged 0 commits from refs/pull/209/head into main

2026-07-06 08:42:53 +02:00

jqueuniet commented

2026-07-06 08:03:41 +02:00

(Migrated from codeberg.org)

All CI gates pass locally: cargo fmt --check, cargo clippy --workspace -D warnings, cargo test --workspace (997 passed), RUSTDOCFLAGS=-D warnings cargo doc --workspace.

Fixed

#171 — SaveFile::tile_at/set_tile_type_attr indexed map_data row-major while the grid is column-major (x*height+y); reads/edits were transposed for x≠y. Added an asymmetric-coordinate regression test.
#168 / #170 — Multi-byte fields inside the suppression bitstream were decoded little-endian on every platform; on Mac the 68k engine stores them big-endian. The mask is identical on both platforms, so only the byte interpretation differs. Made byte-order-aware: SGB scalars, the 9-bit timer count (one shared helper for decode_full, GameplaySections::decode, and dm2 info; also fixes the &0xFF extended-mode truncation), GlobalWords, Timer w_00/wvalueA/wvalueB, and the 10-bit cloud indices. Verified against the Mac fixtures (early_game's last_creature_tick now reads the 0xffffff38 new-game constant; timer counts 1/22/24).
#172 — convert_save now transcodes the SGB scalars, timer records, GlobalWords, 10-bit cloud indices, and phase-6 link records to the target byte order, so e.g. a Mac→DOS conversion preserves game_tick instead of leaving it ~256× off. Pinned by a fixture test.
#173 — The phase-6 timer-actuator link model was lossy (empty chain re-encoded as a full record; record type hardcoded to 7). TimerActuatorLink now preserves the decoded CheckcodeChain verbatim, so re-encode replays exactly; corrected the wvalueB doc claim.
#175 — Robustness batch (6 items): preserve parsed n_records for zero-size types; propagate map-table parse errors instead of silently dropping entries; recursion depth cap on read_chain/write_chain; bounds-check direct map.tiles[…] indexing; validate-before-unlink in the removal paths; debug-only cross-check for the phase-7 cloud-postfix count. None change behavior on valid input.
#176 — Corrected disproven save-format doc claims: the table1d6356 note (verified byte-identical to CHAMPION_MASK, popcount 1045 — earlier "8 extra bytes" was a comment-parsing artifact); the "always little-endian" claim; and the mac-post-sgb-reads claims (timer-count formula, the mask is identical/interpretation is BE, the CHECKCODE index-offset is 2 bits on both platforms).

Investigated → documented, no code change (deliberately)

#169 — The proposed single-record-mode + flag-clear change for the standalone-missile sub-chain was implemented and reverted: it regresses the DOS Beta SKSAVE0 fixture (engine-written ground truth), which contains a standalone missile that round-trips faithfully only under the current persistent-flag / sibling-termination model. The retail sksvgame.cpp hypothesis and the Beta binary disagree; resolving it needs binary-level tracing, not a fixture-blind edit. Recorded as an NB comment.
#174 — Confirmed the retail CREATURE_GIDX_ALT_TABLE extraction (re-derives exactly from skull.exe 0x72568). Extracting the Mac/Beta equivalents to prove identity-or-difference is blocked on binary work out of scope here (Beta Fires.exe is packed; Mac keeps its descriptors in the 68k resource fork, big-endian). It stays a risk flag, not a proven divergence — both the Mac and Beta fixtures round-trip byte-exact with the retail table, so re-selecting per variant would be a guess that could regress them. Documented precisely on the constant.

Addresses the dm2-saves fidelity/robustness batch (#168–#176) from a full-crate review — the same shape as the dm2-graphics batch (#186–#194 / PR #208). (#158, the Sega CD BRAM format, is a separate large reverse-engineering feature and is out of scope here.) All CI gates pass locally: `cargo fmt --check`, `cargo clippy --workspace -D warnings`, `cargo test --workspace` (997 passed), `RUSTDOCFLAGS=-D warnings cargo doc --workspace`. ## Fixed - **#171** — `SaveFile::tile_at`/`set_tile_type_attr` indexed `map_data` row-major while the grid is column-major (`x*height+y`); reads/edits were transposed for `x≠y`. Added an asymmetric-coordinate regression test. - **#168 / #170** — Multi-byte fields inside the suppression bitstream were decoded little-endian on every platform; on Mac the 68k engine stores them big-endian. The mask is identical on both platforms, so only the byte interpretation differs. Made byte-order-aware: SGB scalars, the 9-bit timer count (one shared helper for `decode_full`, `GameplaySections::decode`, and `dm2 info`; also fixes the `&0xFF` extended-mode truncation), GlobalWords, Timer `w_00`/`wvalueA`/`wvalueB`, and the 10-bit cloud indices. Verified against the Mac fixtures (`early_game`'s `last_creature_tick` now reads the `0xffffff38` new-game constant; timer counts 1/22/24). - **#172** — `convert_save` now transcodes the SGB scalars, timer records, GlobalWords, 10-bit cloud indices, and phase-6 link records to the target byte order, so e.g. a Mac→DOS conversion preserves `game_tick` instead of leaving it ~256× off. Pinned by a fixture test. - **#173** — The phase-6 timer-actuator link model was lossy (empty chain re-encoded as a full record; record type hardcoded to 7). `TimerActuatorLink` now preserves the decoded `CheckcodeChain` verbatim, so re-encode replays exactly; corrected the `wvalueB` doc claim. - **#175** — Robustness batch (6 items): preserve parsed `n_records` for zero-size types; propagate map-table parse errors instead of silently dropping entries; recursion depth cap on `read_chain`/`write_chain`; bounds-check direct `map.tiles[…]` indexing; validate-before-unlink in the removal paths; debug-only cross-check for the phase-7 cloud-postfix count. None change behavior on valid input. - **#176** — Corrected disproven save-format doc claims: the `table1d6356` note (verified byte-identical to `CHAMPION_MASK`, popcount 1045 — earlier "8 extra bytes" was a comment-parsing artifact); the "always little-endian" claim; and the mac-post-sgb-reads claims (timer-count formula, the mask is identical/interpretation is BE, the CHECKCODE index-offset is 2 bits on both platforms). ## Investigated → documented, no code change (deliberately) - **#169** — The proposed single-record-mode + flag-clear change for the standalone-missile sub-chain was implemented and **reverted**: it regresses the DOS Beta `SKSAVE0` fixture (engine-written ground truth), which contains a standalone missile that round-trips faithfully only under the current persistent-flag / sibling-termination model. The retail `sksvgame.cpp` hypothesis and the Beta binary disagree; resolving it needs binary-level tracing, not a fixture-blind edit. Recorded as an NB comment. - **#174** — Confirmed the retail `CREATURE_GIDX_ALT_TABLE` extraction (re-derives exactly from `skull.exe` `0x72568`). Extracting the Mac/Beta equivalents to prove identity-or-difference is blocked on binary work out of scope here (Beta `Fires.exe` is packed; Mac keeps its descriptors in the 68k resource fork, big-endian). It stays a risk flag, not a proven divergence — both the Mac and Beta fixtures round-trip byte-exact with the retail table, so re-selecting per variant would be a guess that could regress them. Documented precisely on the constant.

jqueuniet commented

2026-07-06 08:23:23 +02:00

(Migrated from codeberg.org)

Code review

Found 1 issue:

Stale timer-count formula survives in the "Decoded sections" table of the save-format doc. The Timer row still reads T = SaveGameBuffer.w_14 & 0xFF timers — the pre-#170, 8-bit, always-little-endian formula this PR retires. It now contradicts the byte-order section rewritten earlier in the same file (which names the 9-bit timer count as byte-order-dependent), the adjacent GlobalWords row updated in the same table, and the shipped code, which decodes the count via the 9-bit byte-order-correct decode_timer_count_raw / SaveGameBuffer::timer_count().

codeberg.org/jqueuniet/dm2-tools@dca12e2ca0/docs/formats/saves.md (L324-L329)

codeberg.org/jqueuniet/dm2-tools@dca12e2ca0/crates/dm2-saves/src/savegamebuffer.rs (L53-L60)

Checked for bugs (byte-order inverses, 9/10-bit field codecs, column-major indexing, recursion caps, transcode paths), historical regressions, doc/code consistency, and prior review feedback; also ran the fixture-backed byte-exact round-trip and Mac→DOS→Mac conversion tests locally — all pass. No functional issues found.

### Code review Found 1 issue: 1. Stale timer-count formula survives in the "Decoded sections" table of the save-format doc. The Timer row still reads `T = SaveGameBuffer.w_14 & 0xFF timers` — the pre-#170, 8-bit, always-little-endian formula this PR retires. It now contradicts the byte-order section rewritten earlier in the same file (which names the 9-bit timer count as byte-order-dependent), the adjacent GlobalWords row updated in the same table, and the shipped code, which decodes the count via the 9-bit byte-order-correct `decode_timer_count_raw` / `SaveGameBuffer::timer_count()`. https://codeberg.org/jqueuniet/dm2-tools/src/commit/dca12e2ca043c65c00ce04dd9288e204407a2fd9/docs/formats/saves.md#L324-L329 https://codeberg.org/jqueuniet/dm2-tools/src/commit/dca12e2ca043c65c00ce04dd9288e204407a2fd9/crates/dm2-saves/src/savegamebuffer.rs#L53-L60 Checked for bugs (byte-order inverses, 9/10-bit field codecs, column-major indexing, recursion caps, transcode paths), historical regressions, doc/code consistency, and prior review feedback; also ran the fixture-backed byte-exact round-trip and Mac→DOS→Mac conversion tests locally — all pass. No functional issues found.

jqueuniet commented

2026-07-06 08:33:20 +02:00

(Migrated from codeberg.org)

Fixed in da48cd9f. The Timer row of the "Decoded sections" table now points at SaveGameBuffer::timer_count() and the 9-bit byte-order-dependent decode (DOS w_14 & 0x1FF, Mac (b20<<1)|(b21&1)), matching the byte-order section, the adjacent GlobalWords row, and the shipped code. (The remaining & 0xFF occurrences are only in the dated docs/superpowers/plans/*.md history, left as-is.)

Fixed in da48cd9f. The `Timer` row of the "Decoded sections" table now points at `SaveGameBuffer::timer_count()` and the 9-bit byte-order-dependent decode (DOS `w_14 & 0x1FF`, Mac `(b20<<1)|(b21&1)`), matching the byte-order section, the adjacent GlobalWords row, and the shipped code. (The remaining `& 0xFF` occurrences are only in the dated `docs/superpowers/plans/*.md` history, left as-is.)

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jqueuniet/dm2-tools!209

No description provided.

Rows
Columns