Crash when saving and loading snapshots

[harryr0se]

Hey, thanks for the library it's great!

Unfortunately I'm getting a reproducible error when trying to use the snapshot feature. I've managed to create a minimal reproduction
I'm unsure if this is a bug in inkcpp or my code, any guidance would be great

#include <filesystem>
#include <inkcpp/include/story.h>
#include <inkcpp/include/globals.h>
#include <inkcpp/include/runner.h>
#include <inkcpp/include/snapshot.h>

int main()
{
    try
    {
        auto story = ink::runtime::story::from_file("path/to/game.ink.bin");
        printf("Loaded Story\n");

        ink::runtime::runner runner;
        auto savePath = "suitable/path/for/save.bin";
        if (std::filesystem::exists(savePath))
        {
            auto snap = ink::runtime::snapshot::from_file(savePath);
            runner = story->new_runner_from_snapshot(*snap);
        }
        else
            runner = story->new_runner();

       if (runner->can_continue())
        {
            auto basic_string = runner->getline();
            printf("Stepping story once: %s\n", basic_string.c_str());
        }
        else if (runner->has_choices())
        {
            for (const auto& choice : *runner)
                printf("* %s\n", choice.text());
        }

        auto snap = runner->create_snapshot();
        snap->write_to_file(savePath);
    }
    catch (std::runtime_error exception)
    {
        printf("Ink exception: %s\n", exception.what());
    }

    printf("Exiting\n");
    return 0;
}

With the following ink file

VAR test = false

-> start

=== start ===
First line of text
Second line of test

* Choice
~ test = true
* Choice

- end

-> END

Steps to reproduce:

1. Point the story path to the compiled version of the ink above
2. Update the path to the save.bin
3. Compile and run the program once, it will create a snapshot of the story one step in and save it to disk
4. Run the program again
5. Observe it either crashing or throwing an exception

The crash seems to occur in story_impl::container_flag

It appears that offset is invalid memory

CommandFlag story_impl::container_flag(ip_t offset) const
{
	inkAssert(
	    (static_cast<Command>(offset[0]) == Command::START_CONTAINER_MARKER
	     || static_cast<Command>(offset[0]) == Command::END_CONTAINER_MARKER),
	    "Tried to fetch container flag from non container command!"
	);
	return static_cast<CommandFlag>(offset[1]);
}

[harryr0se]

I've found that if I remove the knot from the repro ink so it's just the following:

VAR test = false

First line of text
Second line of test

* Choice
~ test = true
* Choice

-

-> END

Then I no longer get the crash

[harryr0se]

Hello :)

After doing a bit more digging today I've found that the issue seems to occur when creating a snapshot at a point where runner->can_continue() is true

As a test I changed the code in my project to the following and now no longer get the crash when I load the snapshot

m_StoryRunner->getall();
auto* snapShot = m_StoryRunner->create_snapshot();

Is that intended behaviour? I assumed that I'd be able to call create_snapshot() at any point when the story is executing

[JBenda]

Thanks for your deep dive.

The snapshot feature is intended to be used when "can_continue" is false, we were hoping it works but the states between choices can get quite complex.

I will check which causes this, because my demand is that it works everywhere

[JBenda]

Could you test it with #110, I was able to reproduce the error with the current main, but unable to reproduce it with #110.
PS: sorry did something wrong, can now reproduce it

It will merge soon to main.

[JBenda]

Found it, one stored state was not normalized. We were storing raw pointers of the currently entered knots in the .snap file. With this fixed #110 it should work now (at least on my machine TM).

Thank you so much for your nice minimal example, it was very helpful in reproducing the error and researching it.

[harryr0se]

@JBenda That’s great news thanks, happy to help! 😊

One other issue I have on main when I do get a loadable snapshot at a choice point is that loading a previous snapshot is very sensitive to changes of the story binary.

It seems like just adding a new choice or knot will trigger an exception about mismatching visitor counts when loading the snapshot

Do you know if that is part of this bug?
Does the library have a snapshot migration mechanism like the official C# library does?

[JBenda]

Sensitive sounds about right
There is no logic in place for migration.
It was not on our radar untill know.

The error you experienced is because the array for the visit count is not dynamic resizeable.

I will take a look into the C# migration algo maybe I can implement it