Thoughts on overall structure

[loopier]

I'm not sure about the current overall structure. Maybe the actor and assets-related methods should be in Main instead of CommandInterface, as actors are child nodes of Mainand the methods have to call the main instance to access them. The xxxReply() functions should probably also be in Main. This way we would completely decouple OscXxxx from CommandInterface. In CommandInterface we should maybe just keep the code strictly related to command parsing, like creating /defs, OSC variables, etc...; Main would pass to CommandInterface the command, that would return an array of callables that could be iterated calling each function on the child nodes (actors and assets).

This might sound very confusing, sorry.

[totalgee]

Yes, decoupling is good. ;-) For a start, I just made OSC sending work "as is", without changing anything of the architecture. I agree those Reply things should be in Main (decoupling CommandInterface from OSC), but in the case of commands that need to return something specific (not simply a status or error message), like the "list assets/anims/actors" commands, they need to communicate that back to Main. The CommandInterface is the (only) one implementing commands, so you wouldn't want to duplicate/couple the knowledge about those "list" commands in Main. So you need to be able return (probably better to signal) that you need to send the client a specific reply (like /list/actors/reply), along with a list of arguments to send, but generically. Also, if there is no client (or if the UI is the client), then it needs to do its equivalent "reply".

(I'm just talking about OSC-related stuff in my reply, for now not addressing your earlier comment about actors/assets.)

[loopier]

Right. I think I had something like this in mind when I created the command_finished and command_error messages in CommandInterface, so we could connect OSC sender to them when we had implemented it.

[totalgee]

So in that case, we can just remove/move the report{Error,Status}() methods (as you said) to Main. But still need to decide about how to return other kinds of things (like the "list" commands).

[totalgee]

I see your Status class actually supports returning a value (e.g. the Array of actors) as well as a string message. So we can potentially use that to return things. For this, the parseMessage() call should possibly return its value back to its caller (currently Main calls it), rather than doing everything with signals. To be discussed...

[loopier]

I see your Status class actually supports returning a value (e.g. the Array of actors) as well as a string message. So we can potentially use that to return things. For this, the parseMessage() call should possibly return its value back to its caller (currently Main calls it), rather than doing everything with signals. To be discussed...

I agree signals -- as currently implemented -- are not the best approach. Here's an idea:

1. Main receives a signal from OSC receiever.
2. Main calls a method in CommandInterface -- or equivalent parser -- which returns an array of commands to be executed. It would take care to convert any variables, for loops, defs, or whatever needs complicated parsing to an array of simple commands.
3. Main would then execute every command in the array, in order. Or the parser in CommandInterface could emit a signal for every command in the array (I think the former is more elegant).

I found it would be useful to have an execute_command() signal in Main so any other class can emit it (see issue https://github.com/loopier/animatron/issues/6 for an possible use case).

[loopier]

These are the types of parsing that we're currently using, that's why we needed different dictionaries:

coreCommands = commandValue.callv(args) // args is an array received by OSC
nodeCommands = commandValue.call(key, args) // uses key as Node method with an arbitrary number of args
arrayCommands = commandValue.call(args) // array is converted to arbitrary number of args
ocl.reservedWords: parseCommandsArray(commandValue.call(args)) // special dictionary for reserved words.

I'll try to unify.

[loopier]

I think I'm on the right track for a first approach to the change of structure Main <-> CommandInterface. But it would be useful to agree on how it would interact with the OSC parser.

I suggest that upon receiving the OSC message, Main sends it to the parser, and the parser returns an array of commands, so Main can iterate over it and evaluate each command. Would that work for you?

What would be the format of the array?

[
  [/cmdA, targetA, arg1, ... , argN], 
  [/cmdB, targetB, arg1, ... , argN]
]

[totalgee]

You mean that the parser would also be responsible to handle the defs, then? (Returning a list of basic/core commands)

On Wed, Oct 25, 2023, 17:15 Roger Pibernat @.***> wrote:

I think I'm on the right track for a first approach to the change of structure Main <-> CommandInterface. But it would be useful to agree on how it would interact with the OSC parser.

I suggest that upon receiving the OSC message, Main sends it to the parser, and the parser returns an array of commands, so Main can iterate it and evaluate each command. Would that work for you?

— Reply to this email directly, view it on GitHub https://github.com/loopier/animatron/issues/2#issuecomment-1779511848, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKXIUCKHOFPHZTBZU3EA3YBEUJRAVCNFSM6AAAAAA22FODASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZZGUYTCOBUHA . You are receiving this because you commented.Message ID: @.***>

[loopier]

Maybe we should stick to what we agreed on letting Main handle defs, and only parse the commands contained in the /def when a call to that /def arrives. This way we'd have all the dictionaries of commands in one place. So the workflow would be:

1. Main receives OSC
2. if it's a /def store it
3. if it's a call to a /def command, either: a. iterate the array of commands of the /def in Main and pass them one by one to Parser; or b. send the array of commands of the /def to the Parser and let it iterate over them.
4. if its another command, send it to the Parser.
5. Parser returns an array "pure" commands back to Main

We should agree on either 3a or 3b. What do you think it's best? Or what works better for you?

'Parser' always returning an array makes it easier to handle cases like /for loops, even if in most cases it will return just one command. I don't see any advantage of having it return single commands in some cases and arrays in others, we would need to check every time or use 2 methods depending on the case. Looks cumbersome to me.

Does it make sense?

[totalgee]

At the moment I don't really have a reason to prefer one over the other. I guess I would pick 3a (without any other information to help me decide). I think the way you've described it makes sense, and the parser always returns a list of commands.

On Thu, Oct 26, 2023, 12:45 Roger Pibernat @.***> wrote:

Maybe we should stick to what we agreed on letting Main handle defs, and only parse the commands contained in the /def when a call to that /def arrives. This way we'd have all the dictionaries of commands in one place. So the workflow would be:

1. Main receives OSC
2. if it's a /def store it
3. if it's a call to a /def command, either: a. iterate the array of commands of the /def in Main and pass them one by one to Parser; or b. send the array of commands of the /def to the Parser and let it iterate over them.
4. if its another command, send it to the Parser.
5. Parser returns an array "pure" commands back to Main

We should agree on either 3a or 3b. What do you think it's best? Or what works better for you?

'Parser' always returning an array makes it easier to handle cases like /for loops, even if in most cases it will return just one command. I don't see any advantage of having it return single commands in some cases and arrays in others, we would need to check every time or use 2 methods depending on the case. Looks cumbersome to me.

Does it make sense?

— Reply to this email directly, view it on GitHub https://github.com/loopier/animatron/issues/2#issuecomment-1780871116, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKXIVSBNMQYCPNBXD5SZLYBI5MHAVCNFSM6AAAAAA22FODASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBQHA3TCMJRGY . You are receiving this because you commented.Message ID: @.***>

[loopier]

When implementing tween (e27a011021a7aaad29a47e7a31732f7fe73e61dc), I realized that we can set node properties and call node methods with just one command each. Then we can substitute all commands that currently use the caller setActorProperty* for a /def that calls /property [/]<PROPERTY> <ACTOR> <VALUE_1> ... <VALUE_N>.

For example (something I already tested in another branch):

/def /position actor x y
     /property /position $actor $x $y # the property can optionally have a leading /

We should probably have different commands for actor, animation and text properties/methods, because they might have the same name (e.g: position). Or we could just call animation and text properties if the parent actor doesn't have them, exposing only the ones in the actor if they are duplicated.

Pros:

• significantly reduce the size of the core commands dictionary.
• documentation in core commands dictionary is not very readable (kind of a pain to maintain). We could move documentation to the comments of the /defs in a dedicated commands-extended.ocl file (that already exists), à-la Doxygen or GDScript.
• I think it's more elegant

Cons:

• we loose control over what is exposed
• it's a serious refactor, including documentation parsing, command parsing and calling, probably requiring to change the syntax and parsing of /defs to include types in the arguments declaration (/def /bla arg1:s arg2:f rgb:c)

What do you think?

[totalgee]

Interesting idea. How would you handle preparing the correct data type (OSC arguments) for the (built-in) property? For example, scalar (rotation) vs vector (position, scale, colour) types? And would you still support (optionally) calling with a single value but setting both components of the vector, like in the case of scale 2 vs scale 3 1?

To avoid exposing every possible thing, we could have a set of possible/valid properties, in case there are some we definitely want to disallow.

[loopier]

How would you handle preparing the correct data type (OSC arguments) for the (built-in) property? For example, scalar (rotation) vs vector (position, scale, colour) types?

to set the property we call node.get("property"). This returns the value in the correct type. Then we match against the type. If nothing matches it's probably an axis of a structure type like Vector or Color. See the implementation.

I think this could be done better, because now we specify the type in the /def and could use that as we do now with CommandDescritpion.

In the experimental branch everything is very messy, but it seem to work. I'll try to polish it.

And would you still support (optionally) calling with a single value but setting both components of the vector, like in the case of scale 2 vs scale 3 1?

I guess we would need different commands for this:

/def /scale/xy actor x y
    /property /scale $actor $x $y

/def /scale actor value
    /property /scale $actor $value $value