From 462081d7fcda6395773cb79a5c65c3dc121888bf Mon Sep 17 00:00:00 2001 From: Jerry Belich Date: Fri, 13 Nov 2026 18:04:25 -0800 Subject: [PATCH] fix(runtime): snapshot of a story with a sequence fails to load Loading a snapshot into the story it was taken from threw "Invalid container id reference." or "tracked containere are not allowed to move" for many stories: any with a sequence ({a|b}, {stopping:}, {cycle:}, {shuffle:}), and some with a labelled gather inside a choice. globals_impl::snap_load() re-derived every tracked container from the name hash stored with its visit count, even when not migrating. 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 resolves to nothing, and named containers below an indexed one are hashed without the index, so two containers can share a hash and resolve to the other one. When not migrating the counts are loaded in container order, so the lookup is not needed: compare the stored hash with the container's own instead, which still catches a snapshot from a different story. The migration path is unchanged. Found by snapshotting at every choice of 70 stories and restoring 1-4 choices back (32,031 restores): 3 stories threw before, none after, and every replay matched the original run. --- inkcpp/globals_impl.cpp | 23 ++++++++++------ inkcpp_test/Fixes.cpp | 35 +++++++++++++++++++++++++ inkcpp_test/ink/SnapshotVisitCounts.ink | 7 +++++ 3 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 inkcpp_test/ink/SnapshotVisitCounts.ink diff --git a/inkcpp/globals_impl.cpp b/inkcpp/globals_impl.cpp index 17be9185..d062be8d 100644 --- a/inkcpp/globals_impl.cpp +++ b/inkcpp/globals_impl.cpp @@ -285,20 +285,27 @@ const unsigned char* globals_impl::snap_load(const unsigned char* ptr, const loa ); for (size_t i = 0; i < old_capacity; ++i) { hash_t path; - ptr = snap_read(ptr, path); + ptr = snap_read(ptr, path); + if (! loader.migratable) { + // Not migrating, so this is the story the snapshot was taken from and the counts were loaded + // in container order. The stored hash only confirms that; it cannot be used to look the + // container up again. A tracked container without a name of its own (a sequence) stores + // hash 0, and named containers below an indexed one can share a name, so the lookup finds + // nothing or a different container. + inkAssert( + _owner->container_data(i)._hash == path, + "Snapshot does not match this story: tracked container %u differs.", ( unsigned ) i + ); + continue; + } container_t c_id = ~0U; ip_t container_ip = _owner->find_offset_for(path); bool found = container_ip != nullptr && _owner->find_container_id( static_cast(container_ip - _owner->instructions()), c_id ); - if (! loader.migratable) { - inkAssert(found, "Invalid container id reference."); - inkAssert(c_id == i, "tracked containere are not allowed to move, expect we migrate"); - } else { - if (found) { - _visit_counts.set(c_id, _visit_counts.get_old(i)); - } + if (found) { + _visit_counts.set(c_id, _visit_counts.get_old(i)); } } if (loader.migratable) { diff --git a/inkcpp_test/Fixes.cpp b/inkcpp_test/Fixes.cpp index 510e78ad..437408db 100644 --- a/inkcpp_test/Fixes.cpp +++ b/inkcpp_test/Fixes.cpp @@ -767,3 +767,38 @@ SCENARIO( } } } + +SCENARIO( + "snapshot restores visit counts of unnamed and same-named containers", + "[regression][snapshot][runtime]" +) +{ + // The sequence's container is tracked but has no name of its own, and the labelled gather sits + // below a choice's indexed container. Restoring a snapshot used to look each tracked container up + // again by name, which found nothing for the first and could find another container for the + // second, and threw. + GIVEN("a story with a sequence and a labelled gather inside a choice") + { + std::unique_ptr ink{story::from_file(INK_TEST_RESOURCE_DIR "SnapshotVisitCounts.bin")}; + runner thread = ink->new_runner(); + + WHEN("it is played to the first choice and a snapshot is taken") + { + std::string text = thread->getall(); + REQUIRE(text == "First visit.\n"); + REQUIRE(thread->num_choices() == 2); + std::unique_ptr snap{thread->create_snapshot()}; + + THEN("the snapshot loads into a new runner, which continues the sequence") + { + runner loaded; + REQUIRE_NOTHROW(loaded = ink->new_runner_from_snapshot(*snap)); + REQUIRE(loaded->num_choices() == 2); + loaded->choose(0); + REQUIRE(loaded->getall() == "Inside.\n"); + loaded->choose(0); + REQUIRE(loaded->getall() == "Later visit.\n"); + } + } + } +} diff --git a/inkcpp_test/ink/SnapshotVisitCounts.ink b/inkcpp_test/ink/SnapshotVisitCounts.ink new file mode 100644 index 00000000..56fd0e7a --- /dev/null +++ b/inkcpp_test/ink/SnapshotVisitCounts.ink @@ -0,0 +1,7 @@ +-> knot +=== knot === +{First visit.|Later visit.} +* [Again] + - - (inner) Inside. + ** [Back] -> knot +* [Leave] -> END