-
-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide a way to halt future navigation #286
Comments
Multiple navigations occurring roughly simultaneously should never happen in the first place. That's bad, and undefined, behavior as-is—i.e., it's broken even without an override. Maybe I'm not seeing the full picture here, but my gut reaction is that if end user code is doing bad things, then fix that code. |
There are other use-cases for cancelling transition. For example, you may wish to prevent all navigation if some condition is not met. Yes, every link could be modified to include that conditional check (perhaps as a widget), but it would be much easier to be able to globally prevent the transition without creating a new moment (as navigating back to the passage would do). |
Personally it sounds like a foot-gun solution to a situation that should be handled by a better designed custom interface, and I can see potential confusions when trying to debug a request for help. Why show the end-user what appears to be a selectable link that actually isn't. |
A practical example. At the end of one game I transitioned to a Death passage, but wanted to show the text from the previous passage to indicate how the user died. On that passage I would like to shut down all links. I think I did it by modifying the links themselves, but an override would have made it a lot easier. In the end, this is something more than one person has suggested (e.g. see Maliface's pull request: https://github.com/tmedwards/sugarcube-2/pull/302/files). I'd be interested in what harm you think having such a method in the Engine would cause? |
But this feature wouldn't of visibly disable those links, so from the end-user's point-of-view they would still be selectable options , unless some other method was being used to force those links to appear disabled, and if this is the case then why isn't that method also being used to make those links actually unselect-able? So using this purposed method of "disabling" a link results in a potentially confusing situation where nothing happens (1) if one of those selectable links are selected, thus a badly designed User Experience, and thus breaking the principle of least astonishment / surprise. (1) and I understand that JavaScript could be used in the handler to notify the end-user, but that would happen after a selection was made. Is there a potential need to be able to disable multiple links at a time, Maybe. But that functionality also needs to make it clear that those links have been disabled, and more importantly why! And this is where things become complex, because if there is one set of such links then there could potentially be multiple such sets, and it's likely that each set will have its own why. |
That is one of my primary concerns. You have no method of notifying the caller why or even how a link should be disabled. |
It is possible to override navigation using
Config.navigation.override
, but it not possible to prevent it. That means that in any situation where you interrupt navigation (e.g. when a player dies) using the override, multiple navigation instances (e.g. a<<goto>>
triggering inside a link, or a<<goto>>
followed by another<<goto>>
, will result in the same passage being navigated to more than once.It would be nice to have a way to prevent navigation entirely. This could be by returning a special constant from the override function, or by setting a flag/calling a function (similar to how
event.preventDefault()
allows an event handler to cancel the remaining effects of the original event. Alternatively the flag could be a globalConfig.navigation.prevented
which could be set to turn off all navigation until it is turned back on.The text was updated successfully, but these errors were encountered: