search

tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
177 stars 41 forks source link

Running Engine.restart() inside a link with a destination breaks StoryInit #201

Closed david-donachie closed 1 year ago

david-donachie commented 1 year ago

Describe the bug.

Running Engine.restart() inside a link with a destination, does not correctly run StoryInit. StoryInit is run, but the state is overwritten. Any variables set inside StoryInit are undefined.

To Reproduce:

Set some variable in StoryInit

<<set $test = { started: true }>>

And reference it in your Start passage

:: StartPassage
<<if $test.started>> ...

Make a link like the following:

<<link "Restart" "StartPassage">><<run Engine.restart()>><</link>>

If you click the link the game restarts, but $test is undefined, and the reference to $test.started throws an error.

Expected behaviour.

The game should restart as normal, either ignoring the destination given, or navigating to it after the StoryInit is run. In either case, variables set in StoryInit should be defined as normal.

Additional Info

Courtesy of @fg109 on the Twine Discord it looks like this may be a race condition. I'll quote his notes in full.

When you call Engine.restart(), the State is reset and the browser is refreshed. When the browser refreshes, Engine.start() runs "StoryInit" and later attempts to restore the active State. If there is no active State, then the Engine plays Config.passages.start instead. I think what's happening with your link is some sort of race condition. During the Engine.restart(), after State.reset() is called and before the browser gets refreshed, Engine.play('Start') gets called (from the link macro). Therefore, when the browser is refreshed, there is an "active" State. It's pretty much empty but it still exists, so it overwrites the current State that has all the initialized variables from "StoryInit". https://github.com/tmedwards/sugarcube-2/blob/master/src/engine.js#L249 I edited my HTML and replaced if (State.restore()) with if (false) which means it can't restore the active State. After doing so, <<link "» Back" "Start">><<if $ended>><<run Engine.restart()>><</if>><</link>> worked like you expect it to.

I'll note that the example above may seem contrived (i.e. why have a destination and a restart()). My actual use case was:

<<link "» Back" "Start">><<if $ended>><<run Engine.restart()>><</if>><</link>>

Project details.

tmedwards commented 1 year ago

A <<link>>'s passage forwarding is always executed after the contents of the link.

In other words, the following:

<<link "» Back" "Start">>
    <<if $ended>>
        <<run Engine.restart()>>
    <</if>>
<</link>>

Is functionally equivalent to:

<<link "» Back">>
    <<if $ended>>
        <<run Engine.restart()>>
    <</if>>
    <<goto "Start">>
<</link>>

The critical part being:

<<run Engine.restart()>>
<<goto "Start">>

That's a race condition within SugarCube itself. Worse, Engine.restart() calls for the reload of the window/tab as its final act. Thus, the race condition becomes some bizarre combination of SugarCube and browser engine race.

In essence, play stupid games with the engine, win stupid prizes.