Divert targets allow knots to be called as functions

[joningold]

... and hell breaks loose if you do. (Well, the game crashes)

~ temp x = -> knot 

* { x() } The choice. 
    -> END 

=== knot 
    *   More choices 
    *   Even more choices!
    -   
    *   And then we slept. 
        -> DONE     

[clembu]

Additional info

In Inky ( v0.10.0 ; ink v0.8.2 ; inkjs v1.7.1 with hotfix ), choices appear.
When choosing any one, crash happens, with this stacktrace:

System.Exception: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index (Internal story path: 0.4.9) ---> System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at Ink.Runtime.StoryState.PopEvaluationStack()
   at Ink.Runtime.Story.ProcessChoice(ChoicePoint choicePoint)
   at Ink.Runtime.Story.Step()
   at Ink.Runtime.Story.ContinueSingleStep()
   at Ink.Runtime.Story.ContinueInternal(Single millisecsLimitAsync)
   at Ink.Runtime.Story.ContinueAsync(Single millisecsLimitAsync)
   at Ink.Runtime.Story.Continue()
   at Ink.CommandLinePlayer.EvaluateStory()
   at Ink.CommandLinePlayer.Begin()
   at Ink.CommandLineTool..ctor(String[] args)
   --- End of inner exception stack trace ---
   at Ink.CommandLineTool..ctor(String[] args)
   at Ink.CommandLineTool.Main(String[] args)

[joningold]

This crash trace looks like an incorrect number of parameters being passed somewhere?

On Thu, 30 May 2019 at 5:01 pm, Clément Busschaert ***@***.***> wrote: Additional info In Inky ( v0.10.0 ; ink v0.8.2 ; inkjs v1.7.1 with hotfix ), choices appear. When choosing any one, crash happens, with this stacktrace: System.Exception: Index was out of range. Must be non-negative and less than the size of the collection. Parameter name: index (Internal story path: 0.4.9) ---> System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. Parameter name: index at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource) at System.Collections.Generic.List`1.get_Item(Int32 index) at Ink.Runtime.StoryState.PopEvaluationStack() at Ink.Runtime.Story.ProcessChoice(ChoicePoint choicePoint) at Ink.Runtime.Story.Step() at Ink.Runtime.Story.ContinueSingleStep() at Ink.Runtime.Story.ContinueInternal(Single millisecsLimitAsync) at Ink.Runtime.Story.ContinueAsync(Single millisecsLimitAsync) at Ink.Runtime.Story.Continue() at Ink.CommandLinePlayer.EvaluateStory() at Ink.CommandLinePlayer.Begin() at Ink.CommandLineTool..ctor(String[] args) --- End of inner exception stack trace --- at Ink.CommandLineTool..ctor(String[] args) at Ink.CommandLineTool.Main(String[] args) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#296?email_source=notifications&email_token=AAHDJI6RKXA4DCENERBW4OLPX725NA5CNFSM4DF53GR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWSXOZQ#issuecomment-497383270>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHDJIYZLAU7UC7CYRLOR5DPX725NANCNFSM4DF53GRQ> .

-- Jazz, in Three-Four Time: A Novel http://www.amazon.co.uk/dp/B004K1EYIE

[clembu]

I'll have a look at the generated runtime json tree once I get home. Looks to me that the function call "pops" a value that is never pushed (no "void" is pushed on the stack in knot?)

Do you want to allow calling divert targets as functions? It'd be nice if you want dynamic delegate switching, but in that case maybe there should be an explicit check at runtime that the target points to a function and not any "gotoable" point.

[joningold]

Divert targets as functions was something we found the engine could handle almost "accidentally", but it's extremely useful. A runtime check that a function really is a function would be a good idea, except I think the "function" information is removed entirely when the ink is compiled.

[clembu]

indeed there is no static way to know that a container corresponds to a function, right now. The runtime assumes that the compiler did its thing and that a function call means the target is a function.

The least effort fix doable right now would be to check if the stack is poppable before popping, to avoid an exception and throw a custom runtime error with a message that would like "tried to get a non-existing value, have you called a function that was actually not a function?" or something like that, but it would be better anyway to be able to make sure that a function call points to an actual function, directly or indirectly. Adding a flag in the #f field of the container metadata object wouldn't add that much of an overhead in the json, and would correspond to a bool in the c# object that could also be used in other dynamic checks.

[clembu]

I looked at the JSON, and indeed, this choice

* { x() } The choice.

Generates the following json object (minus all the text evaluation)

                {
                    "*": "0.c-0",
                    "flg": 19
                }

which sets it to check for a condition before doing anything (flag 1 is set),
so it pops from the eval stack, but the function call x() never pushed on the stack, so as I had assumed, the runtime tries to pop from an empty stack.
This is a decently error to catch in StoryState.PopEvaluationStack (which currently does a straight count - 1 access, which fails when count is 0. Same thing for PeekEvaluationStack. Checking for empty stacks there would already help prevent crashes, and instead generate nice errors.)

[joningold]

If this is a pull request you could do we would gratefully accept it, I think! (Caveat: @joethephish manages the ink codebase not me)

…

-- Jazz, in Three-Four Time: A Novel http://www.amazon.co.uk/dp/B004K1EYIE