[jerrytron]

The bug

Loading a snapshot back into the story it was taken from throws for many stories:

Invalid container id reference.
tracked containere are not allowed to move, expect we migrate

It happens for any story with a sequence ({a|b}, {stopping: ...}, {cycle: ...}, {shuffle: ...}), and for some with a labelled gather inside a choice. Seven lines are enough:

-> knot
=== knot ===
{First visit.|Later visit.}
* [Again]
  - - (inner) Inside.
  ** [Back] -> knot
* [Leave] -> END

Play to the first choice, create_snapshot(), new_runner_from_snapshot(): it throws.

Why

When not migrating, globals_impl::snap_load() looked every tracked container up again from the name hash stored with its visit count, and asserted that it found the same id. That lookup is not one-to-one:

• A sequence's container is tracked but has no name of its own, so its hash is 0 and find_offset_for() finds nothing.
• build_container_hash_map() names the named children of an indexed container without the index, so two containers can end up with the same name and hash, and the lookup returns the other one.

The fix

When not migrating, the visit counts are already loaded in container order and nothing needs looking up. The stored hash is now compared with the container's own hash at the same index, which still catches a snapshot from a different story. The lookup moves into the migration branch, which is otherwise unchanged.

Tests

• New regression scenario in inkcpp_test/Fixes.cpp with the story above (inkcpp_test/ink/SnapshotVisitCounts.ink): it throws without the fix, and with it the restored runner carries on the sequence ("Later visit.").
• ctest passes locally (70 cases). Formatted with clang-format 18.1.8, matching .pre-commit-config.yaml.
• Outside the suite: we snapshotted at every choice of 70 real stories (5 different playthroughs each) and restored 1-4 choices back, 32,031 restores in all, comparing every replayed line, tag and choice list with the original run. 3 stories threw before the fix; after it, every restore matched.

Not in this PR

From reading the code (not tested), the migration path still resolves containers through the same lookup, so migrating a story with sequences would drop their visit counts (hash 0 finds nothing) or put a count on the wrong container (shared hash). Fixing that means giving those containers unique names in build_container_hash_map(), which changes the file format, so it seemed better as a separate discussion. Happy to open an issue for it.

[github-actions]

Ink Proof Results

These results are obtained by running the Ink-Proof Testing Suite on the compiled binaries in this pull request.

System	Results
Linux x64	130/130 passed
MacOSX	DISABLED
Windows x64	130/130 passed