Add AnimationNodeObservers for signaling animation events

[a-johnston]

This implements godotengine/godot-proposals#13386 For AnimationNodeOneShot, AnimationNodeTransition, AnimationNodeBlendSpace1D, and AnimationNodeBlendSpace2D.

The new parameter matches how it looks in the proposed mockup:

And here's how it appears in the tree itself:

I think this is OK although the view within the AnimationTree tab is a bit confusing without labels or tooltips.

Usage is to manually set the observer in the animation tree's parameter list and then in code like:

var observer := tree.get(observer_path) as AnimationNodeObserverOneShot
if observer:
    observer.started.connect(callback)

The following test project has scenes testing each of the new observer types. Arrow keys to drive the animation node(s) and escape to return to the main menu.

animation_tree_test.zip

[TokageItLab]

The direction should be right, since we agreed on it during the animation meeting.

I suggest creating a base class called AnimationNodeObserver and extending it. After that, it would be useful to adopt this approach in Transition and BlendSpace as well, so that signals can be triggered when the highest-priority port changes; See also #111893 (comment).

At last, it would be better to include the following in the description of the NodeOneShot documentation: If you want to use signals, you can use [AnimationNodeObserverOneShot]. The same applies to each AnimationNode.

[a-johnston]

I suggest creating a base class called AnimationNodeObserver and extending it. After that, it would be useful to adopt this approach in Transition and BlendSpace as well, so that signals can be triggered when the highest-priority port changes

Would anything live in that base class or would it stay empty for now? I omitted the shared base class in my first pass since the signal signatures would be different although the usage pattern was very similar.

[TokageItLab]

Would anything live in that base class or would it stay empty for now? I omitted the shared base class in my first pass since the signal signatures would be different although the usage pattern was very similar.

I think empty class is fine, and you can register it as abstract class.

GDREGISTER_ABSTRACT_CLASS(AnimationNodeObserver);

FYI, the base class of Texture is empty.

class Texture : public Resource {
	GDCLASS(Texture, Resource);
};

[a-johnston]

The base class turned out to be a good spot for the local to scene override. For the observer classes is the following signal set acceptable:

AnimationNodeOneShotObserver
- started()
- fadein_finished()  # Only fires if fade-in time > 0
- fadeout_started()  # Only fires if fade-out time > 0
- finished()

AnimationNodeBlendSpaceObserver
- closest_changed()

AnimationNodeTransitionObserver
- transition()  # fires immediately before requested animation becomes current, so both are queryable through the tree as `current_state` and `transition_request`
- fadein_finished()  # only fires if xfade_time > 0

In particular, should we pass the closest index through that signal? It would only be useful for fetching the position or associated animation root node, and isn't currently exposed anywhere else. I think it makes sense to just leave that empty and rely on users to track whatever state they need.

[TokageItLab]

For the naming consistency:

• It should be AnimationNodeObserverXXX
• fadein should be separated like fade_in
  • We weren’t able to complete the unification of "fadexxx" into "fade_xxx" in time for the 4.0 launch, but it should be unified into it in 5.0; The names of C++'s functions should serve as the standard

Basically, signals should only be emitted in cases where they cannot be tracked state by the user (implicitly occurred by the core), and that is the case I was referring to. I have provided further details below:

AnimationNodeObserverOneShot

• started()
  • It is useful for knowing that a transition has occurred when Auto Restart is enabled
• fade_in_finished()
• fade_out_started()
• finished()

AnimationNodeObserverBlendSpace

• state_changed(StringName p_point) or closest_point_changed(StringName p_point)
  • Especially in Discrete Blend mode, only one animation is played, so this is useful after crossing that boundary
  • The user inputs coordinates. Since the state is not updated until _process() is called once after setting the coordinates, simple closest-tracking results in a one-frame delay
  • BlendSpaces now have points with StringNames, see also Display and allow setting name/index of BlendSpace points #110369

AnimationNodeObserverTransition

• state_changed(StringName p_state) or port_changed(StringName p_state)
  • It is useful for knowing that a transition has occurred when Auto Advance is enabled
• fade_in_finished()

It's debatable whether we should standardize the naming of "changed" signals to “state_changed“ or whether they should indicate the arguments like "closest_point_changed / port_changed". I'd like to hear others' opinions as well.

[a-johnston]

This is more or less ready. I opted to use closest_point_changed for the blend space observer and crossfade_finished for the transition node but these can be renamed to whatever is desired. I also manually tested one shots with Auto Restart enabled and it behaves as expected, although I don't have it turned on in the sample project. The docs are pretty sparse right now, but I think for a somewhat self-explanatory experimental api it's ok. The only other thing I'd call out is that the signal names aren't being cached anywhere currently so it does create a StringName when emitting. I can add the StringNames as fields like the parameter names are on each instance although it felt a bit weird since they're instance members.

[a-johnston]

CI failed with ERROR: Cannot open resource pack '/tmp/test_project.pck'. at: _setup (core/config/project_settings.cpp:691) which seems unrelated but I don't see a way for me to rerun just that step.

[TokageItLab]

That should be fine for the most part. It should be documented that Observers are available (such as link and sample code) in the extension classes of AnimationNodes that support Observers.

Since we are currently in feature freeze for 4.7, we will review this later at the AnimationMeeting as scope for 4.8.

[a-johnston]

Ah right I forgot to update those other docs. I'll do that and separately I can make a docs pr for adding it to the animation tree tutorial. Although that would also need to wait for after 4.7 ofc