Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make each AnimationNode have time information to retrieve time semantically and make it can be passed bidirectional #8083

Closed
TokageItLab opened this issue Oct 12, 2023 · 1 comment · Fixed by godotengine/godot#87171
Assignees
Milestone

Comments

@TokageItLab
Copy link
Member

TokageItLab commented Oct 12, 2023

Describe the project you are working on

Stabilize Godot animation system

Describe the problem or limitation you are having in your project

Current AnimationTree/Node

AnimationTree/Node uses only the remaining time of the AnimationNode to process the downstream AnimationNode's time.

If multiple remaining times are blended, the time with the highest blend weight (or longest time in some case) is selected. This inconsistency in the time estimation method depending on the AnimationNode type should be eliminated.

Also, if looping AnimationNode, the remaining time is HUGE_LENGTH. This works to some extent for simple and proprietary implementations, but it is less customizable.

Necessity of HUGE_LENGTH

For example, when a 2s animation and a 3s animation are blended together, the user has no choice as to which one to use as the basis for the remaining time.

Before godotengine/godot#75759, there was no HUGE_LENGTH and the downstream AnimationNode would return the remaining time.

This is bad because it means that if a 2s "looping" animation and a 3s "looping" animation are blended together with an unstable weight around 0.5, the remaining time will always increase or decrease.

Moreover, the current AnimationNode does not return information about whether it is looping or not. Therefore, if a upstream node needs to detect a loop, it must estimate whether or not it is looping based on the remaining time.

For example, the old AnimationStateMachine predicted a loop when the time decreased, but wrong detection occurred when the remaining time increased or decreased due to reverse playback animation or blending of different periodic animations as mentions above.

After godotengine/godot#75759, HUGE_LENGTH was returned, so it could be used correctly as the detection of the loop, but in exchange, the remaining time information was lost, resulting in the problem shown in godotengine/godot#79208 and godotengine/godot#82581.

To solve these problems, we need to add more informations managed and returned by the downstream AnimationNode.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Semantic time retrieval will be more semantic and allow for more customizability of AniamtionTree. At least for time retrieval, there have been requests like godotengine/godot#23629 by some people as comment in that.

Also, being able to retrieve animation length semantically would fill in one of the missing pieces for the implementation of cyclic sync. See also godotengine/godot#23414, godotengine/godot#34179 and godotengine/godot#62424.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Architectural design of AnimationNode

AnimationNode is a resource. Resources are referential and may be reused.

In the past, there was a bug godotengine/godot#22887 that caused multiple internal data to be processed when multiple AnimationTrees with the same AnimationBlendTree existed in a scene.

Therefore, the current AnimationNode must follow the following design when designing data. (Probably it will never change in the future)

AnimationNode properties

  • Read-only and reusable values (that can be the same value in multiple AnimationTrees)

AnimationNode parameters

  • this will be transcribed on the AnimationTree's map, and within the process the AnimationNode reads and writes the AnimationTree's map
  • Values that are allowed to be overwritten in the process (those that must have different values for each AnimationTree)
  • In GDScript, AnimationTree[parameters/XXX]

Assuming these assumptions, time informations are designed the following:


AnimationNode

  • Parameter
    • double current_time
  • Property
    • bool use_custom_length
      • double custom_length
      • Animation::LoopMode loop_mode
        • LOOP_NONE
        • LOOP_LINEAR
        • LOOP_PINGPONG
      • real_t start_offset
      • real_t end_offset end is determined by start + length, so it is duplicated prop

If use_custom_length = true, the timeline is treated as if it had an internal timeline with the downstream animation at the beginning, similar to AudioTrack or AnimationPlaybackTreck. Then, the downstream animation will loop within custom_length, but will stop at the end of custom_length unless loop_mode loops.

This means that in a case where a 2s animation and a 3s animation are blended upstream with synchronization, setting custom_length = 6s and loop_mode = LOOP_LINEAR will reproduce the blended looped animation with the correct period.

AnimationNodeAnimation

  • Property
    • AnimationNodeAnimation::LoopModeOverride loop_mode
      • LOOP_NONE
      • LOOP_LINEAR
      • LOOP_PINGPONG
      • LOOP_NO_OVERRIDE

See also godotengine/godot#79400.

This can be superseded by the custom_length option above.

AnimationNodeSync/BlendSpace1d/BlendSpace2d

  • Property
    • TimeEstimationMode time_estimation_mode (enable if Animation::use_custom_length = false)
      • TIME_ESTIMATION_MODE_LARGER_AMOUNT
      • TIME_ESTIMATION_MODE_LESSER_AMOUNT
      • TIME_ESTIMATION_MODE_LONGER_TIME
      • TIME_ESTIMATION_MODE_SHORTER_TIME
      • TIME_ESTIMATION_MODE_SPECIFIC_PORT
        • String input_port
          • int input_port_index (internal/private variant to follow NodeTransition method)

Probably a more detailed design for StateMachine is needed in the future to estimate the remaining time.

NodeTransition/NodeOneShot/AnimationNodeStateMachine

  • Property
    • bool break_loop

The break_loop option makes it possible to select whether or not transitions (such as AtEnd) will be enabled despite looping.

struct/function

struct NodeTimeInfo {
    double length;
    double position;
    bool is_looping;
}
NodeTimeInfo _process(const AnimationMixer::PlaybackInfo p_playback_info, bool p_test_only = false) override;

or

double _process(const AnimationMixer::PlaybackInfo p_playback_info, NodeTimeInfo &r_node_time_info bool p_test_only = false) override;

If this enhancement will not be used often, can it be worked around with a few lines of script?

NEVER

Is there a reason why this should be core and not an add-on in the asset library?

This is a reworking of the core feature

@SaracenOne
Copy link
Member

Reflecting on this proposal, I think it's important we make sure this is expressed in a way which is intuitive to the user, but foundationally, yeah, this needs to happen and this seems like a good proposal on how to do it.

@TokageItLab TokageItLab changed the title Make each AnimationNode have time information to retrieve time semantically and make it can be passed from upstream Make each AnimationNode have time information to retrieve time semantically and make it can be passed bidirectional Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants