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
185 stars 42 forks source link

History back with ephemeral passages #106

Closed Kassy2048 closed 3 years ago

Kassy2048 commented 3 years ago

Is your feature request related to a problem? Some passages call the goto macro while the passage is being loaded in order to redirect the player automatically to another passage without any user interaction. I call them "ephemeral" passages (the passages content never shows to the player because of the automatic redirect). When this happens, the init state from the ephemeral passage is stored in history, and then, the next passage is loaded (and its init state is also stored in history). If a player clicks the history back button, then the engine will load the ephemeral passage again, leading to the redirect again, leaving the player at the same passage he tried to go back from. The only way to go back in this case is to spam the back button hoping that the engine goes 2 states back in history to prevent the automatic redirect. Doing that is nearly impossible on slow devices (phones) and sometimes corrupts the state.

Describe the solution you'd like. It would be better if clicking on the history back button would never lead the game to an ephemeral passage. One solution would be to visit the history back in Engine.engineBackward() and select the most recent moment that did not produced an ephemeral passage.

Ephemeral passages can be detected by checking the Engine.isPlaying() function in the goto macro code: if the engine is playing during the call to goto, then this is an ephemeral passage. Marking the current history moment to skip it later could be a solution. There is no such issue when going forward in history.

I can work on it and make a PR for that if the concept is OK for you.

On a side note, why most of the objects are frozen/sealed? There seem to be no performance boost doing that (quite the opposite actually) and it complicates quick testing/extension through monkey patching.

tmedwards commented 3 years ago

It would be better if clicking on the history back button would never lead the game to an ephemeral passage.

Then solution is simple, don't create junk moments in the first place.

There are various ways to forward players programmatically without using "goto passages". For example, the two easiest:

  1. Put the logic at the borders—i.e., in the links. Example below.
  2. Use the Config.navigation.override setting. Example at link.

An example of the former:

<<link "Do the thing.">>
    …logic with <<goto>> here…
<</link>>

Or if you wanted keep the logic in its own passage or widget:

<<link "Do the thing.">>
    <<include "logic passage">>
<</link>>

<<link "Do the thing.">>
    <<logic_widget>>
<</link>>

I can work on it and make a PR for that if the concept is OK for you.

No offense, but I'm uninterested in Rube Goldberg solutions to non-problems.

On a side note, why most of the objects are frozen/sealed?

To prevent people from breaking their games in interesting and extremely hard to diagnose ways by monkey patching code that they have little or, usually, no understanding of. Been there, never again.

Kassy2048 commented 3 years ago

Thanks for your answers.

From a maintenance point of view, it probably makes more sense to put the redirect logic into a single related place (the passage) instead of duplicating it into different multiple places (the links, even when it "only" requires to include the same file in every links). It is like expecting that checks and redirections happen in every webpages that link to another webpage instead of in the target webpage.

BTW, the passages I was talking about are not necessarily empty: they can show valid content most of the time, but from time to time, a redirection to another passage happens because some random events take place. The passage content could be changed in that case to show a link instead of doing a redirection (like "Hey something happened, click here to continue"), but that would probably degrade the user experience.

Anyway, if you still believe that the current behavior should not change, it might be worth it to add a note in the goto macro description about how it can break the history back button. TBH, after reading the current note in the goto macro description, it was still unclear why it was "better" to use the link macro, until I've read your comment.

ChapelR commented 3 years ago

"Duplicating it into multiple passages" is exactly what you should do via <<include>> or similar. There is no real good reason to nav to a passage just to automatically nav to a different one. Remember, navigating to a passage is a big deal, you're creating a moment in the story history, taking a snapshot of the state, and firing up the rendering process. What you're doing is like loading a a whole level in a platformer because you want to load the effects of a power up.

HiEv commented 3 years ago

From a maintenance point of view, it probably makes more sense to put the redirect logic into a single related place (the passage)

While I agree it's better to put your redirect logic in a single place, I disagree that it should be a passage since, as you've seen, it breaks history navigation. It also adds unnecessary bloat to the history and slows down passage navigation.

Instead, I'd recommend creating a Config.navigation.override function to handle your redirects. That function is triggered on every passage navigation, and it also allows you redirect to a different passage if need be. In short, it does what you need without any of the downsides of using a "pass-through" passage like you're talking about.

Enjoy! :-)

tmedwards commented 3 years ago

From a maintenance point of view, it probably makes more sense to put the redirect logic into a single related place (the passage) instead of duplicating it into different multiple places (the links, even when it "only" requires to include the same file in every links). It is like expecting that checks and redirections happen in every webpages that link to another webpage instead of in the target webpage.

You're already referencing your goto passages with various links anyway. In other words, doing anything like the following, which is what I'm assuming that you're doing now:

[[text|a goto passage]]
<<link [[text|a goto passage]]>> … <</link>>
<<link "text" "a goto passage">> … <</link>>

Is really no different than the following, from a basic writing standpoint:

<<link "text">><<include "a goto passage">><</link>>

You'd still be referencing the goto passages in the same places you are now, just in a slightly different way.

The only thing that changes is from a technical standpoint. Namely, that the latter doesn't create junk moments in the history, unlike the former.

BTW, the passages I was talking about are not necessarily empty: they can show valid content most of the time, but from time to time, a redirection to another passage happens because some random events take place. The passage content could be changed in that case to show a link instead of doing a redirection (like "Hey something happened, click here to continue"), but that would probably degrade the user experience.

🤷‍♂️ I can only respond to what I know about and you didn't mention this wrinkle before.

Bearing this mind, my suggestion remains largely the same:

  1. For goto passages that do nothing else, include the logic at the borders—as I just went over again above.
  2. For those that can include content as well, leave the content as-is but move the event logic to Config.navigation.override.

To the latter, I realize that rewriting your logic in JavaScript may not be palatable, but it really is the best way currently to handle interrupt-style events.

Anyway, if you still believe that the current behavior should not change, it might be worth it to add a note in the goto macro description about how it can break the history back button.

I can do that.

Kassy2048 commented 3 years ago

Thanks all for your answers and sorry for the lack of details in the orignal post.

Indeed, moving the redirect logic to Config.navigation.override is not very convenient as it implies lots of changes. As I've found this issue in multiple stories from different authors, I thought it would be easier to fix it in SugarCube rather than convincing the authors to rewrite their code. Well, I guess I'm left with that anyway or offer them a workaround :roll_eyes:

Thanks also for adding the warning.