Notation for expression and variable evaluation in commands

[totalgee]

Proposal for how to handle expression evaluation, and variables (getting/setting)...

Various options considered at our last meeting (I don't have my notes with me right now), a few that I recall are:

/set x 3.5
/position/x bob /expr "sin($x * 4 + 1.5)"

or just something like

/position/x bob {sin($x * 4 + 1.5)}

or bash-style

/position/x bob $((sin($x * 4 + 1.5)))

and for variables (get), we could just use $ notation (no need for /expr)?

/position/x bob $x

[totalgee]

We had a discussion about this on 2023-11-04, and decided to use {expr} notation (we can always change it if needed). This stands out (is not found in a simple GDScript expression) and is minimal in terms of keystrokes.

[loopier]

In https://github.com/loopier/animatron/commit/ec5bb0fbd524ef6081bfe474b0c48bf62e1c2c96 I've implemented a variables manager. It contains a "system" variables dictionary and a user variables dictionary. I join them both to pass them to expr.parse() and expr.execute().

It works fine, but when a variable's name partially matches with any other, it is also replaced by it's value:

/set a 40
/def /bla animation:s
    /load $animation

/bla mama # results in /load 40nimation

[totalgee]

I can fix this one. It's probably just a question of searching with a Regex that uses word boundaries.

[loopier]

We should also fix expression concatenation with strings:

/post {time} # -> 325.224 - OK
/post x-{time} # -> x-{time} - NOT RIGHT should output something like x-324.224

[totalgee]

[...] when a variable's name partially matches with any other, it is also replaced by it's value:

/set a 40
/def /bla animation:s
    /load $animation

/bla mama # results in /load 40nimation

I don't get this result on the current experimental branch... For me it works properly (it loads the animation).

(also, the set command seems to have changed, now it needs to be something like: /set a:f 40)

[loopier]

ah, right, yes. It needs to specify the type.

It does seem to work. You fixed it! :)

[totalgee]

It does seem to work. You fixed it! :)

I didn't even try to fix it yet...but it seems to work... (-;

[totalgee]

In the original proposal, I suggested that we use the $ prefix notation to refer to our own variables. So even in an expression, you'd have to do that, like /post {time + $a}, where time is a built-in variable we provide, and a is an Animatron user-defined variable (e.g. /set a:f 3.14). I don't mind not requiring the $ in expressions, although this may cause some confusion, for example if people define variables with names like exp or cos. Also, users might wonder: "why do I have to use the $ in some places, but not in others?"

[totalgee]

Another issue @loopier mentioned to me was that something like this wasn't working (on the experimental branch):

/post {sin(time)}

In fact, any expressions that did something with time (besides just returning it) failed. That problem (and another issue about setting vars giving an error message) was fixed in 41ae50d.

[loopier]

In the original proposal, I suggested that we use the $ prefix notation to refer to our own variables. So even in an expression, you'd have to do that, like /post {time + $a}, where time is a built-in variable we provide, and a is an Animatron user-defined variable (e.g. /set a:f 3.14). I don't mind not requiring the $ in expressions, although this may cause some confusion, for example if people define variables with names like exp or cos. Also, users might wonder: "why do I have to use the $ in some places, but not in others?"

Yes, let's do that. But how do we implement it? Should the variables dictionary keys be $*?

[totalgee]

Should the variables dictionary keys be $*?

No, that part shouldn't change. Just I'll add something to find the relevant $var tokens in expressions (converting them to var before evaluating). The dictionary could just send the required vars to the evaluate function, or we could send the whole dictionary (as now).

[loopier]

In the original proposal, I suggested that we use the $ prefix notation to refer to our own variables. So even in an expression, you'd have to do that, like /post {time + $a}, where time is a built-in variable we provide, and a is an Animatron user-defined variable (e.g. /set a:f 3.14). I don't mind not requiring the $ in expressions, although this may cause some confusion, for example if people define variables with names like exp or cos. Also, users might wonder: "why do I have to use the $ in some places, but not in others?"

You mentioned this in https://github.com/loopier/animatron/issues/23#issue-1994704402

[totalgee]

I've fixed a bunch more issues in 9d5c74a

This improves the $var stuff (it's not very efficient still, but better than it was...needs more work). The reason lots of expressions weren't working is asArray(true) was set in several OSC command definitions where they should be asArray(false). That bool argument should be true if the command should have expressions (and vars) deferred, which is the case for /for loops and /def and /routine, but not for most built-in "primitive" commands, such as all the property setters. I saw several other command definitions where it's still wrong, but I haven't changed the ones I wasn't testing, for now.

With these changes, the following cases now work:

/set a:i 40
/post stuff$a-here
/for i 10 /post {$a+$i}

/load default
/for i 10 /create ln$i default
/routine r 50 0.1 /angle ln0 { sin(time+$a) * 360 }

Routine with inf repeats still doesn't work (I didn't touch it), but I do see why: it's trying to convert the string "inf" into a number (so it gets zero)...

[loopier]

Great news!

Routine with inf repeats still doesn't work (I didn't touch it), but I do see why: it's trying to convert the string "inf" into a number (so it gets zero)...

this is strange, as it did work before, maybe I broke it while mangling the $var stuff. I'll fix it

[totalgee]

Another commit (1d7b61d) starts to add unit tests for variable resolution, and also fixes var/expressions for /set.