Add playback position mapping curve to "Sync" animation transition

[low-moon]

What problem(s) does this PR solve?

• closes Add "Sync from End" switch mode to animation transitions godot-proposals#12195

Additional information

This PR adds a curve for the "Sync" animation transition switch mode that specifies the mapping from the old state playback position to the new state playback position. This is useful as it allows far more control over how the sync switch mode behaves, allowing for different length states to sync as the artist intends. For my use case, it allows two states that perform the opposite motion to seamlessly transition to each other by using a negative linear mapping.

For example, for my game I have an animation that goes from the idle stance to the crouched stance, which plays between the idle and crouch poses. To allow for standing up I use the same animation but reversed. However, before this PR there is not a way to define the transitions such that the crouch can be interrupted half-way through to stand back up smoothly. With this new parameter, the sync mode can now be used to connect inverse states:

The attached proposal also states an example for the use-case of this inverse behaviour and would be solved by this PR.

While the proposal suggests a new switch mode for "From End" syncing, I realised that a general mapping of next position from previous would also account for animations that are different in length or if the artist has a specific sync in mind for the transition. If a new switch mode were added specifically for inverting the sync, then in the future it was requested that more control over sync is given to the artist, the "Sync From End" switch mode could be deprecated and pollute the switch mode namespace.

This PR adds a new parameter to the AnimationNodeStateMachineTransition class that is a Curve reference, and modifies the sync transition behaviour to use the curve to remap the playback position if it is valid. It also implements _get_property_list() to dynamically display the sync_mapping_curve parameter when "Sync" is selected as the switch_mode. At Line 191 in animation_node_state_machine.cpp, I had to define the sync_mapping_curve parameter in _bind_methods() as PROPERTY_USAGE_NONE despite it being added through _get_property_list(), so that the documentation tooling respected the parameter. I would appreciate if someone could let me know if this is a good practice. It stores and loads the variable so it seems to behave correctly. I also tested normal sync transitions without a sync mapping curve and these behave as usual.

Note: Apologies about the previous PR. I have fixed my branches and can see that this will contribute one commit with my changes.

[TokageItLab]

I assume the essence of this request is to synchronize animations of different durations. Then, is there a reason or use case that justifies the need for a curve here? Why wouldn’t a single boolean option like adjust_length_by_speed_scale with xfade suffice? We should not implement options that overlap with the functionality of xfade.

Also, as pointed out in similar implementation in #117275, since we cannot guarantee that the duration of animations other than NodeAnimation won’t change midway through, even if we were to implement adjust_length_by_speed_scale as an option, we would need to restrict it to connections between NodeAnimations only, by the similar way with #117275.

[low-moon]

It is intended to give more control over how the current position is transferred during a sync transition. The main use case that inspired this change is the need to invert the sync position so that the next state can perform the inverse of the current state after the sync, but I chose to implement it as a curve to allow for more control over other situations where the usual sync is restricting, such as if the two animations are different lengths.

It essentially acts as a remap of the current playback position to the new playback position at the instant of the sync, so I don't think it overlaps with xfade or custom_timeline, unless I am misunderstanding that functionality. I imagine most uses of this curve would be linear, as the intended use of the curve is to set the min_from, max_from, min_to and max_to of the hypothetical remap.

I understand the duration can change for non-animation states, but the sync position is only calculated at the instant the transition is triggered, so the length changing as a result shouldn't affect the position that is chosen.

[TokageItLab]

I see, now I understand that it doesn’t conflict with xfade. However, I’m still skeptical about whether it really needs to be a curve.

I imagine most uses of this curve would be linea

Yes, I think so too.

I assume the lack of documentation is one reason, curve is not intuitive. So if your intention is to allow remapping of from_start/from_end and to_start/to_end, I think it would be more intuitive to simply use those values as property names.

In this case, I think using "seconds" as the unit would be more intuitive, but in cases other than NodeAnimation, this would create a problem where settings cannot be shared across animations of different lengths. Therefore, the implementation would need to either restrict this option to NodeAnimation only or allow users to switch between "seconds/cycles" units.

[low-moon]

As an argument for using the curve, if one of the animations has a different rate to the other or goes through stages of recovery, the curve allows precise mapping of if the first animation has only played slightly it can be recovered quickly, and past a point it then jumps to a position in the second animation that takes longer to recover. This can probably be configured without the curve using code, but the curve allows easier configuration of more complex transitions.

I am unsure about using seconds as like you say it restricts the configuration to be specifically about two animations, but the alpha based units are not as easy to convert from the animation. Maybe the documentation could explicitly state that it is operating in units relative to the animation lengths, and an example on how to convert from a time in the animation to the correct alpha by dividing it by the length?

If this general remapping is too unintuitive or niche to be added, it could just be replaced by a boolean to invert the sync position? As this is the more in-demand use-case and is something that I have found is missing for certain uses of the animation system and is difficult to work around.