AnimationTree: New "Phase" Node

[Eoin-ONeill-Yokai]

Phase node allows for the use of connected animation player nodes as a normalized seek (0.0->1.0) where you describe where in the animation you want to be. It will use the longest time of all connected animations to determine the phase scale.

This can be useful in circumstances where the user wants to treat an animation as it's own blend space, where the beginning and end of the animation represent a blend between states.

In the below video, I'm using hand tailored animation to represent a blend space between "full turn left" and "full turn right" of a snowboarder. When combined with other layers of animation, you could see that this would be quite powerful when programmatically animating a character. (The below image links to a youtube video.)

[Calinou]

@Eoin-ONeill-Yokai Consider opening a proposal to help get this merged more easily 🙂

In particular, the proposal should explain how the current nodes don't suffice for your use case.

[TokageItLab]

I think it would be better to implement a Normalize and KeepFrame option to SeekNode instead of implementing a new node. Having two nodes that do the same job makes it less maintainable. But, depending on the way of the KeepFrame function is implemented (Set delta to 0? or Always Seek? or Something), it might make sense to implement this as a separate Node. In that case, I think it should add a Normalize or not option to both Seek and Phase nodes to avoid confusion caused by different values in two Nodes that do similar work.

The function that uses Seek is not compatible with TimeScale and some other Nodes, so I am a little worried about that. I've thought about something similar before, but was wondering how to do it since my use case required looping the root motion.

Ref:
#23414 (comment), #23414 (comment) and #39734 (comment)

[Eoin-ONeill-Yokai]

I think it would be better to implement a Normalize and KeepFrame option to SeekNode instead of implementing a new node. Having two nodes that do the same job makes it less maintainable.

@ToakageItLab Believe it or not, I already have the ability to do this on my own Godot fork that I've held back on upstreaming for a few reasons -- one of which is just the lack wanting to propose it and face an uphill battle.

The second reason is a much more reasonable one -- having a freeze and normalize setting on the right-hand side inspector might technically work, but it won't be easily visible via the node tree itself. I think the concept of phase makes a lot of sense in the world of DSP (You're not thinking in "time" but rather an abstract unit) so I think it actually makes sense to split these concepts as separate nodes. I do think that freezing a seek node should also be possible though! (And already have the work done if people are interested.) However, I think the concepts are different and should be easily seen at a glance of the node tree which one you're currently working in!

@Eoin-ONeill-Yokai Consider opening a proposal to help get this merged more easily slightly_smiling_face

In particular, the proposal should explain how the current nodes don't suffice for your use case.

@Calinou Yeah I can get around to that eventually. Since I already had the work of it done on my own branch, I figured just proposing an upstream solution would take less time overall as it's been relatively useful in my use case of needing to drive a full (arbitrary length) animation as a pseudo-blendspace.

With that being said, I think a much more important proposal to be made here would be a better system for allowing animation node additions without the need of hopping into C++. I was relatively surprised that I wasn't able to make my own module / plugin animation node and have that node show up in the "Add Node" menu in the editor. There probably should be a way to provide that within the current editor!

Additionally, perhaps it's just me, but compared to most modular systems, the input / output flow of the Animation tree seems like it might be backwards. Time should be input on the left side, and mesh data should be output on the right side.

[fire]

I'm having trouble approving this phase node pull request.

1. There is no proposal. You have written more words explaining the proposal in your last comment, but you haven't filled in the proposal template.
2. One of the animation maintainers proposed a suggestion which did not get a design review.

Normalize and KeepFrame option to SeekNode instead of implementing a new node. Having two nodes that do the same job makes it less maintainable. But, depending on the way of the KeepFrame function is implemented (Set delta to 0? or Always Seek? or Something), it might make sense to implement this as a separate Node. In that case, I think it should add a Normalize or not option to both Seek and Phase nodes to avoid confusion caused by different values in two Nodes that do similar work.

3. Unknown interactions with TimeScale and other Nodes
4. Unknown interactions with root scale

[TokageItLab]

I rather agree with the implementation of this node, but I think it needs to at least match the behavior of NodeSeek. As I said above, it's confusing that this node is the only one that is normalized.

Honestly, I'm not too worried about TimeScale, NodeSeek's behavior ignores TimeScale and other Nodes in the first place. As long as this NodePhase behaves the same way, I don't see a problem. However, I thought that we should rather do some fix about the problem of NodeSeek ignoring them.

I think NodePhase allows the user to ignore the AnimationTree process and allow the animation to proceed as desired. This is also possible with Seek, but when you loop, i.e. when you go from 1 to 0, the root motion will play backwards. To solve this, for example, we need to allow a value greater than 1 and perform a mod calculation... or do something.

We also need to consider that always stopping means that there is always a possibility that the action of the stop frame key will be played. For example, the sound effect may continue to play every frame.

[Eoin-ONeill-Yokai]

@fire @Calinou @TakageItLab I've gone ahead and made a godot proposal to discuss this current implementation and provide more details on why this should be merged.

Truth be told, I would be perfectly happy to keep these types of modifications to my own C++ module (like I do with some other tasks and custom nodes) but this particular type of modification did not seem easily achievable via either C++ module or GDScript due to how Add Node is implemented in the AnimationNodeTreeEditor where it won't automatically register subclasses of AnimationNode as valid nodes for the tree. See godotengine/godot-proposals#4077 for more details and discussion.

[TokageItLab]

I know this is useful, but I think two modifications are needed.

1. Currently, this implementation achieves stopping the animation by always doing a Seek. However, Seek always is problematic because it fires events for the keyframes after Seek. So it needs to set Delta to 0 without Seek always to stop the animation, just like TimeScale does. In short, it is recommended that only one frame be Seek and nothing else after that.

2. Avoid time normalization. It will create an inconsistency with SeekNode, which has a similar role. Or, implement an option to normalize or not normalize, and then implement the same option for SeekNode.

The naming of the "Phase" may need more thought, but that can be changed at the final review.

[TokageItLab]

I was thinking about implementing this, but it seems to me that we need to allow optional animation lengths to be set in the NodePhase itself, since in blended animations they do not always have to be the same length.

Also, in NodeSeek, the optional animation length is needed identically to support loops. Therefore, I thought this feature should be integrated within NodeSeek.

[Eoin-ONeill-Yokai]

I was thinking about implementing this, but it seems to me that we need to allow optional animation lengths to be set in the NodePhase itself, since in blended animations they do not always have to be the same length.

I see. Currently I'm just recursively going through the tree and finding the longest animation currently attached as the basis for the phase. I suppose we could modify this so that the time is manually set by the user instead. My thought process when making it was to make it as convenient to switch in / out new animations as possible while also supporting smaller "noise" channels that allow for more procedurals (for example, a breathing layer, a shakiness layer for hand movement, etc.)

I think we could actually program that part of the logic in the GUI side though -- where you could click a button when the phase node is selected to get the longest animation length down the chain of connected animation nodes. That might be a nice middle ground here.

[eddieataberk]

Any update on this?