ephread / inkgd

Implementation of inkle's Ink in pure GDScript for Godot, with editor support.
MIT License
317 stars 33 forks source link

Combined lists and functions don't work as intended #12

Closed 2shady4u closed 4 years ago

2shady4u commented 4 years ago

Describe the bug Passing list elements to a switch function should result in this list element being matched. This is not the case... the function doesn't recognize the element and simply chooses the "else" path.

To Reproduce

OUTPUT in log:

Variable 'currentActor' changed to: Bobby
Variable 'currentActor' changed to: Bobby
Variable 'currentActor' changed to: Bobby

OUTPUT in game:

Hey, my name is Bobby. What about yours? 
I am Bobby and I need my rheumatism pills!
Would you like me, Bobby, to get some more for you?

This is the wrong behavior...

Expected behavior Go to Quill (http://jeejah.xyz/quill/) or download Inky, copy-paste the Ink content (see below) and press play. This gives following story (which is correct):

Hey, my name is Philippe. What about yours?

I am Andre and I need my rheumatism pills!

Would you like me, Philippe, to get some more for you?

(story complete)

Ink files Here's a minimum version of the *.ink-script that I'm using:

VAR currentActor = "Bobby"

LIST listOfActors = P, A, S, C
VAR s = -> set_actor
-> start

===function set_actor(x)
{ x:
- P: ~ currentActor = "Philippe"
- A: ~ currentActor = "Andre"
- else: ~ currentActor = "Bobby"
}

=== start ===
{s(P)} Hey, my name is {currentActor}. What about yours? 
{s(A)} I am {currentActor} and I need my rheumatism pills!
{s(P)} Would you like me, {currentActor}, to get some more for you?
-> END

Environment:

Additional context A current work-around is to not use LISTs and just use string (meaning: {s("P")} instead of {s(P)}), but I would very much like not to write the double quotes at all.

ephread commented 4 years ago

I won’t have access to a computer before the 18th of December, but I’ll investigate as soon as I’m back from my trip. This is a significant issue, thanks for taking the time to provide an example!

2shady4u commented 4 years ago

@ephread Any update on this? If you don't have time I can also start diving into the code to fix this, but this would probably take a lot longer since I'm not well-versed in the specifics of this repository's code.

ephread commented 4 years ago

@2shady4u sorry, I haven't had time yet, will look into it in the next few days. I noticed that you compiled the story with inklecate 9.0.0 though, which is not supported by the current runtime. Just to be certain, do you encounter the same behaviour with 0.8.3?

ephread commented 4 years ago

Update: I checked with two different versions of inklecate. Although the container hierarchy is slightly different between the two versions, it behaves in the same way in the runtime. I will investigate further.

2shady4u commented 4 years ago

@ephread Any update on this? If possible we could discuss this on Discord or through mail if that would be better for you?

ephread commented 4 years ago

@2shady4u sorry, I'm moving overseas at the moment, so I'm a bit overwhelmed. We can get in touch through inkle's Discord (same handle as Github) if you want, but I may take forever to respond.

ephread commented 4 years ago

OH. MY. GOODNESS.

I found the issue, it's so utterly silly that I can't help but be super mad at myself. The equality method between two lists was not implemented, so the switch statement was using the equality method of the parent object (which always returns false).

Over the past few months I spent so much time here and there investigating this when I got the chance. But today, I guess I went about the right way. I still need to add a proper test case and I should be able to commit and push a fix. (It's also fixing #13)

Sorry it took so long to fix.

2shady4u commented 4 years ago

Hello :) That's awesome! Thank you for fixing this! 😄 No problem that it took so long.

I would've tried fixing this myself, but I was swamped with work myself :(