Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Proposal: ability to call a node and return to the next spot in execution #303

Closed
TheMartianLife opened this issue Sep 1, 2021 · 20 comments
Labels
enhancement language Issues that are about the design and syntax of the Yarn programming language. proposal A proposal to add or change a feature in Yarn Spinner in a way that might affect existing users. suggestion

Comments

@TheMartianLife
Copy link
Collaborator

TheMartianLife commented Sep 1, 2021

Issue #92 by @rcuddy proposed adding functionality to the jump command that told the runner to return to the callsite once that node completed. There were several proposals for syntax surrounding the jump command, primarily [[NodeName]] #return. @rcuddy made a fork with the functionality implemented, but this was not merged as it coincided with the Yarn migration to ANTLR.

Issue #264 by @McJones proposed changing the [[Text|NodeName]] syntax to -> Text <<jump NodeName>> and assumed the discussion of the return functionality when @desplesda proposed including syntax for <<jump NodeName return>>. When the ANTLR migration was complete and the jump syntax changed, Issue #264 was closed and the discussion of return died with it.

This issue opens new discussion about return.

Example with Updated Syntax

title: Start
---
<<set $hygiene = 0 >>
Day 1 Starts
<<jump MorningRoutine return>>
Show some more text. Do more things.
<<jump Day2>>
===
title: Day2
---
Day 2 Starts
<<jump MorningRoutine return>>
I hope today goes better.
<<jump End>>
===
title: MorningRoutine
---
I'm some content that can be shown repeatedly inline and called from different blocks.
-> Brush Teeth
    <<set $hygiene += 1 >>
-> Don't Brush Teeth
    <<set $hygiene -= 1 >>
===
title: End
---
<< if $hygiene > 0 >>
    You win!
<<else>>
    You have died of gingivitis.
<< endif >>
===

Discussion Thus Far

@McJones asked and @desplesda answered:

Does this count MorningRoutine as having been visited? Yes, I'd count the node you've just run as having been listed.

Do we allow nested embedding? Yes, I'd allow nesting this behaviour. All we'd need to do is maintain a stack of return addresses.

Do we report we are in a different node or are we still inside the calling node? I'd report that we're in a different node.

Do we do embedded node cycle detection? We don't currently detect or report on the cycles that are possible already (example: Node A can say <<jump NodeB>>, and Node B can say <<jump NodeA>>. If we add cycle detection, I don't think it would need to have any special considerations for this functionality.

@rcuddy responded to the same questions with very similar views.

@TheMartianLife TheMartianLife added the proposal A proposal to add or change a feature in Yarn Spinner in a way that might affect existing users. label Sep 1, 2021
@TheMartianLife TheMartianLife added enhancement language Issues that are about the design and syntax of the Yarn programming language. suggestion labels Sep 1, 2021
@McJones
Copy link
Collaborator

McJones commented Sep 1, 2021

Originally I was more in favour of this, however as the language has evolved and grown in that time and as we see more people use it I am less enamoured with this as an idea.
It feels like an idea that is just begging for bugs in both the narrative and the implementation.

Back at the time I said I think embedding is a better idea over returns and I still hold that view now, just more strongly.
It solves the desire of separating repeated-use nodes, or for refactoring big nodes into smaller more manageable nodes, in a far simpler and easier to manage form.
I suppose you could still do an embed and just call it a return and use the same syntax as proposed here although I think there is however an advantage to make it clear that you are embedding a node instead of returning from one

The main downside to embedding is an embedded node won’t really be visited as such as it will more or less just be dumped straight into the guts of another node, so lines like <<if visited(“MorningRoutine”)>> make no sense and will always fail.
One potential solution to this would be to inject visit tracking inside the node at compile time whenever an embed is reached.
Alternatively can just not allow the visited function inside embedded nodes or make it clear that they will always fail.

Additionally if the embedded node has a jump inside that will essentially close the embedding node as well as the embedded node.
Could get around this by:

  • Not allowing jumps inside embedded nodes
  • Tracking the jump and then unravelling it after that node ends (which just gets us back to being a return…)
  • Make all jumps inside embeds an embed (feels risky, potentially changes writers intent)
  • Accept that a jump inside an embedded node will also close the embedding node

tldr

I think supporting node embedding, that is to say dumping the contents of a node inside another one, is a better option than embedding that covers most (if not all) the situations where you want nodes to return.

Embedding has some concerns to be worked out but these are not hugely difficult and either require clear messaging in documentation around embedded usage or some compile time variable tracking.
I consider these better problems to solve than handling the edge cases of supporting return.

The syntax for this would be either <<jump nodename embed>> or <<embed nodename>>.

@TheMartianLife
Copy link
Collaborator Author

So like you would inline a function in C?

@McJones
Copy link
Collaborator

McJones commented Sep 1, 2021

Pretty much yes, albeit one without the weirdness of the C linker if possible…

@Haapavuo
Copy link

Haapavuo commented Sep 1, 2021

<<embed nodename>> feels like the most reasonable syntax.

@TheMartianLife
Copy link
Collaborator Author

I don't know, I still really like the original plan. Making a special type of node that looks normal but doesn't ever get marked as visited or other behaviours you expect of nodes I think is strange, and you can see how representations of this in things like the Yarn editor would be confusing.

I know we did away with goto in modern programming languages and so it's not a very typical pattern anymore, but I still think "jump here and then, when you're done, come back" is a more broadly understandable concept than "this fancy syntax is basically a big string interpolation that can hold a whole node".

And the times I have wished for such a functionality have been things like:

title: TalkToJoe
—
Hey there!
<<if not visited("TalkToJoe")>>
    <<jump JoeIntro return>>
<<endif>>
// rest of dialogue
===

@TheMartianLife
Copy link
Collaborator Author

TheMartianLife commented Sep 1, 2021

And the power I foresee of return over embed is that, in my mind, return means "whenever you hit the end of that node" so it doesn't necessarily have to return if you jump from that node or whatever. OR, alternatively, you make it so it returns whenever it hits OnDialogueComplete() which would mean it doesn't need to even end in that node.

Honestly, these arguments all lead to the same logical conclusion though: arbitrary jumps. Like in languages where you can mark break statements so you always know explicitly which loop you are breaking out of, if you could

title: TalkToJoe
—
Hey there!
<<if not visited("TalkToJoe")>>
    <<jump JoeIntro>>
<<endif>>
<<mark JoeIntroReturn>>
// rest of dialogue
===
title: JoeIntro
---
// intro lines
<<jump JoeIntroReturn>>
===

that would be even better.

@heygleeson
Copy link

heygleeson commented Sep 1, 2021

I agree that suggesting visited() should not work on embedded nodes creates a lot of senseless ambiguity on how nodes are meant to be treated. If you had a chain of conditional embeds than that's a significant chunk of story where YarnSpinner would not be keeping track of where you have been, and someone may just want to write their story that way.

Ink's documentation on Tunnels seems to solve this issue pretty well.

Their solution (if translated to Yarn), would be to allow <<jumps>> to contain parameters that define what to do/ where to go when a node hits an endpoint (or something like <<finish>> or <<return>>) - Stop, return to the point in the previous node, or carry-on into a new node entirely.

@heygleeson
Copy link

heygleeson commented Sep 1, 2021

However it's probably worth mentioning here that Ink also has a pretty good grasp on hierarchical structures (e.g. "stitches" which are basically sub-nodes), which arguably goes hand-in-hand with <<embed>> when giving writers ways to organise their story structure without having to repeat themselves.

To me, the use of <<mark JoeIntroReturn>> in @TheMartianLife's previous example already kinda looks like a "sub-node", or an alternate "entry-point" into a node.

@McJones
Copy link
Collaborator

McJones commented Sep 2, 2021

Yeah sorry I think I spoke a bit too negatively about the edges cases of embeds, in fact almost every edge case of the embed also applies to the returns as well, I was just trying to list them for discussion.
I wasn't advocating for not tracking visiting of embedded nodes, more it will be something the compiler will have to special case because it won't by default get the "entered a new node" message because it will be inlining a node technically.
This, and a few other quirks, are all mostly trivially handled at compile time when the <<embed nodename>> is reached.

Likewise you could allow nested embeds and jumps from embeds I just think it invites spaghetti dialogue, not that they couldn't be done, which is why I was initially against allowing this.
It is just simpler if an embed has a clear in and out which is only realistically possible if you don't allow jumps inside them.
As an aside I was curious how tunnels in ink handle this and in their docs they say "So you will need to write carefully to ensure that all the flows into a tunnel really do come out again."
If the answer to all my concerns are "we trust the writer" then they pretty much all evaporate, if you write horrible spaghetti yarn then it is all on you to handle.

As to Mars' suggestion of arbitrary jumps into nodes I am very much against this.
Currently jumps in the stack machine are to a node, that is to say that essentially the VM can fully flush its stack of instructions and load a new set.
To support arbitrary jumps we have to keep the stack around and then have jumps be an offset of instructions relative to the current one, this can be calculated at compile time but is something that would require a lot of additional work to make happen.
Also I do slightly worry if the only real working example of this as a feature are for goto, which while everyone is far too dogmatic against it does also have a bad rep for a reason.
Doubly so when the example of labeled breaks is only really useful in nested loops, a feature Yarn does not have.

@Subliminalman
Copy link

If the answer to all my concerns are "we trust the writer" then they pretty much all evaporate, if you write horrible spaghetti yarn then it is all on you to handle.

Maybe there could be a compiler configuration to allow for advanced modes, one to allow for embeds, another to allow for embeds that allow for additional embeds and jumps. spaghetti mode and spaghetti with meatballs mode (probably could use better names haha)? This could keep the default to what is most straight forward for writers and for those who would like to have more control then it can be set in the Yarn Project file.

I effectively used embeds for Button City for world set up and while I'm going to be changing how I architect our next game I still think having these types of options would be really useful. I should also note that my implementation also had it track that the node was visited.

@CarstenGermer
Copy link

Developing an Adventure/VN type game atm I would very much like to simply be able to embed nodes to keep the "logic" nodes concise and to the point.

-> Ask about the shed <<if $stateShedDoor == 0>>
    <<embed HarbourMasterShed>>
    <<set $stateShedDoor = 1>
-> Where might the key to the shed be? <<if $stateShedDoor == 1>>
    <<embed HarbourMasterKey>>
    <<set $stateShedDoor = 2>
-> Enter the shed <<if $stateShedDoor == 2>>
    <<jump RoomShed>>

It would decouple developing the logic from the writing process more clearly by both not having to scroll up and down in the other file/text.
Plus during writing and fleshing out the nodes there'd be a clear place to look up all Vars with their states to see if there's a combination that would be appropriate for the dialog to comment on or such.

It's easy enough for simple logic as is but having multiple and more complex gamestates it gets very unwieldy and confusing to jump around nodes, keep the scheme in mind and find everything...

@CarstenGermer
Copy link

And embedded nodes should be placed at the indention level where the command is found, if that's not already on the radar ;)

@Haapavuo
Copy link

Haapavuo commented Oct 2, 2021

Embedding sounds dangerous in case there are circular references. I.e. "Node A embeds Node B which embeds Node C which embeds Node A". If the embedding is done lazily in runtime, then this may not be an issue but important to keep in mind...

@CarstenGermer
Copy link

CarstenGermer commented Oct 2, 2021

@Haapavuo Unless embedded nodes forbid further embeds that is very much true. As it would be for gosub-return.

What would be the consensus on how to ease developers pain in the scenario I described? We'd very much take embedding that prohibits further embedding for usability/writeability reasons.

Even better would be embedding that crawls all further embed found and throws an error if it detects circularity. Not simpel but doable and a good solution, no?

Add "It is just simpler if an embed has a clear in and out which is only realistically possible if you don't allow jumps inside them." and it sounds golden to me.
Embedding forbids jumps and crawls further embeds for circularity.

@McJones
Copy link
Collaborator

McJones commented Oct 4, 2021

Yeah the fear of cycles was exactly why I initially said the best option would be to just ban embeds inside of an embed node. There was a decentish amount of pushback against no allowing embeds so that would mean performing cycle detection.

This isn’t impossible and I reckon the easiest way (at least initially) is to just do a small limit on embedding depth, somewhere in the region of 3 to 5 I reckon.
Can do this at compile time or runtime, although the runtime one would have to throw an error which I’m not sure the best way you’d handle that.
Just getting a “can’t story any more, sorry” message would be a weird thing to encounter.

A jump would still be allowed because a jump removes all previous context and starts fresh on the jumped node.

@CarstenGermer
Copy link

CarstenGermer commented Oct 4, 2021

Was thinking about this some more yesterday and I would solve this with a recursion that crawls through all embeds in a project and checks the branches for back-embeds via lists of nodes.

I'd do this check at compile time but then again I'm not very familiar about Yarns structure and flow.

As I'm also currently not that fluent with c# I better not volunteer to dev that.
But I'd gladly whip up some pseudo-code to illustrate the idea if you don't already know what I'm talking about and if it would help in some way. Ping me, if.

As my lecturer at uni said in the 90s "recursion to the rescue!"...

@st-pasha
Copy link

st-pasha commented Dec 9, 2022

It seems the discussion mostly centers around whether we should do embeddings or jump-and-returns. It would be beneficial then to lay down what exactly each option mean, and how it would solve existing (and future) use cases.

  1. Embedding, signified by the <<embed Target>> command (alternatively, we could consider <<include Target>>). Embedding a node X should behave as-if the content of that node was inserted at the location of the <<embed>> command. This naturally leads to the following design:

    • The content of node X must be valid, for example it cannot contain an orphaned <<else>> block.
    • Embedding should be resolvable statically, we will not support <<embed {$target}>>.
    • For the same reason recursive embeds or cyclical embeds will produce a compile error.
    • Embedding does not count as a node visit.
    • Executing embedded code does not trigger onNodeStart/onNodeComplete events.
    • The embedded code has access to the same local variables as defined inside the parent node (see Proposal: local variables #266).
    • It might make sense to distinguish embeddable nodes from regular nodes -- for example, instead of title: X such node may be introduced as embed: X.
      • an "embed" node cannot be run in a DialogueRunner;
      • an "embed" node cannot be a target of a <<jump>>.
    • If embedded code contains options, they will be merged with the parent's options into a single choice set. That is, the following code
      title: Start
      ---
      -> Option 1
      -> Option 2
      <<embed X>>
      ===
      embed: X
      ---
      -> Option 3
      -> Option 4
      ===
      
      would produce a single dialogue with 4 options present.
    • <<stop>> command inside an embed applies to the parent node.
  2. Jump-and-return, aka gosub, aka function call. I feel like this deserves a keyword of its own, so how about <<visit Target>> or <<hop Target>>. The functionality of this command is that the dialogue runner will temporarily suspend execution of the current node and start executing Target, but then once that finishes it will resume executing the original node. The details are as follows:

    • The target can be dynamic, i.e. we can allow <<visit {$target}>>.
    • <<visit X>> counts as a normal visit for the purpose of visitation tracking, and it also triggers onNodeStart/onNodeComplete events.
    • Visits can be recursive, though a runtime error will be thrown if the recursion depth becomes too big.
    • The visited node will not have access to its parent's local variables.
    • In the future we may want to allow passing parameters into the visited node, for example:
      What do you want as a reward?
      -> A mighty sword
         <<visit Reward weapon>>
      -> A deadly bow
         <<visit Reward bow>>
      -> A powerful potion
         <<visit Reward potion>>
      
    • <<stop>> command inside a visited node terminates that node, but not the parent node.
    • It might make sense to distinguish these "jump-and-return target" nodes from regular nodes, especially when we add parametrized calls.

Thus, there appears to be quite a significant number of differences between these two options, especially in how they could evolve in the future. Which makes choosing between the two difficult. Luckily, we don't actually have to choose -- we can implement both.

@CarstenGermer
Copy link

Thus, there appears to be quite a significant number of differences between these two options, especially in how they could evolve in the future. Which makes choosing between the two difficult. Luckily, we don't actually have to choose -- we can implement both.

Seems it calls for a classic pro/con comparison of the two. Effort to implement and maintain, amount of advantage for devs and so on.
I'd go with the one that takes less effort and see what devs do with it and how much it is used.
Honestly, each one would make structuring the yarns much easier tidy in my case.

@zed9h
Copy link

zed9h commented Jun 27, 2023

Jump and return seems like a natural choice to me, with much more alternatives for flow control. However, it does appear to be a more complex implementation compared to embedding. Considering the trade-offs, if the implementation cost of jump and return proves to be prohibitive, embedding can be a sufficient solution for many cases.

@desplesda
Copy link
Collaborator

This feature has been announced as part of Yarn Spinner 3.0. Thanks for all of the discussion, everyone - this thread was extremely useful in guiding the final implementation of the feature.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement language Issues that are about the design and syntax of the Yarn programming language. proposal A proposal to add or change a feature in Yarn Spinner in a way that might affect existing users. suggestion
Projects
None yet
Development

No branches or pull requests

9 participants