loopier / animatron

Animatron for Godot 4.x <
15 stars 1 forks source link

Use `$var` to access variables (inside or outside of expressions) #23

Open totalgee opened 7 months ago

totalgee commented 7 months ago

This notation would be used for variables explicitly defined using /set myVar 5 and also "implicit" local variables like the ($i) iterator variable in /for i 5 /post $i (and possibly the $iter iteration number inside a routine).

Inside expressions, these variables would be substituted with the current value at the time of execution of the command, for example: /position/x bla {$myVar * 10}.

We would not need (or have) a separate /get command, only /set... to get the value you simply refer to the variable, in an expression or outside of it.

It also needs to work as a string, to concatenate with other text, such as: /for i 3 /post hello$i (would post "hello0, hello1 and hello2").

totalgee commented 5 months ago

What do you want to happen when someone references an undefined variable?

/post $unknown

For now, I post ??? (if you haven't defined unknown), but I guess it should throw an error and stop processing the command...?

loopier commented 5 months ago

Both sound good. Giving some feedback and stopping the command process, no?

totalgee commented 5 months ago

Yes, I set it up to return an error and give a message. About to commit the first version of this (on main), which works for basic commands, like /post.

totalgee commented 5 months ago

Initial wok for this here: 12a439d

loopier commented 5 months ago

What do you want to happen when someone references an undefined variable?

/post $unknown

For now, I post ??? (if you haven't defined unknown), but I guess it should throw an error and stop processing the command...?

I just realized it might not always be a good idea to interrupt the command process when an error occurs. One such case /new, a shortcut to /load and /create in one command:

/def /new actor:s animation:s
    /load $animation
    /create $actor $animation

/create changes the animation if the $actor already exists, and this works well when called directly. /load, on the other hand, interrupts the process. So calling /create and /new on existing actors have different behaviours which is kind of confusing.

If we decide to interrupt the process when an undefined variable is found, then we might have more inconsistencies of this type. I'm not sure which way is better, but we should either always interrupt the process or throw a warning.

Which one do you think is best?

totalgee commented 4 months ago

I'm not sure I understand your example about interrupting for errors. In your example, the variables are definitely defined, since they come from the calling of the def. I think I'm missing something.

loopier commented 4 months ago

In the example, when /load fails it interrupts the process, so /create is never called in such a case.

Imagine calling /new ma mama: it will load the animation and create the actor ma. Then, when calling /new mo mama nothing will happen because the animation mama is already loaded and /load will fail, interrupting the process, so actor mo is never created.

Calling /create ma mama then calling /create ma square will change the animation of ma without interrupting the process, which is what I would expect to happen. When preloading the animations (which I usually do to prevent freezing the app in the middle of a performance) /new will not work to change the animation as /create does. This feels inconsistent. To me, how /create behaves is more intuitive, but we should decide which one to use because the do almost the same but behave very differentlly and require extra thinking while at use, which is no desirable.

totalgee commented 4 months ago

Okay, but this concern has nothing to do with variables... It's a more general issue about what to do when some command in a /def fails... If I'm understanding that correctly, then you should maybe open a different issue to track it.

For me, BTW, trying to /load an animation that's already loaded should not be an error, maybe a warning...? Maybe your issue is that /load shouldn't return an error in that case?

loopier commented 4 months ago

ah, right, wrong thread...

For me, BTW, trying to /load an animation that's already loaded should not be an error, maybe a warning...? Maybe your issue is that /load shouldn't return an error in that case?

exactly