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

Implement LoopModeOverride to AnimationNodeAnimation to properly manage the loop state for each AnimationTree #79400

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/classes/Animation.xml
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,9 @@
<constant name="LOOP_PINGPONG" value="2" enum="LoopMode">
Repeats playback and reverse playback at both ends of the animation.
</constant>
<constant name="LOOP_NO_OVERRIDE" value="3" enum="LoopMode">
Disables overriding loop mode if the overriding can be caused in the other class which uses animation.
</constant>
<constant name="LOOPED_FLAG_NONE" value="0" enum="LoopedFlag">
This flag indicates that the animation proceeds without any looping.
</constant>
Expand Down
21 changes: 21 additions & 0 deletions doc/classes/AnimationNodeAnimation.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,27 @@
</brief_description>
<description>
A resource to add to an [AnimationNodeBlendTree]. Only has one output port using the [member animation] property. Used as an input for [AnimationNode]s that blend animations together.
The [AnimationNodeAnimation] has [code]loop_mode_override[/code] parameter to manage the looping state of each [AnimationNodeAnimation] for each [AnimationTree] as a node. See also [enum Animation.LoopMode].
[codeblocks]
[gdscript]
# Set loop mode override.
animation_tree.set("parameters/Animation/loop_mode_override", Animation.LOOP_LINEAR)
# Alternative syntax (same result as above).
animation_tree["parameters/Animation/loop_mode_override"] = Animation.LOOP_LINEAR

# Get loop mode override.
animation_tree.get("parameters/Animation/loop_mode_override")
# Alternative syntax (same result as above).
animation_tree["parameters/Animation/loop_mode_override"]
[/gdscript]
[csharp]
// Set loop mode override.
animationTree.Set("parameters/Animation/loop_mode_override", (int)Animation.LoopMode.Linear);

// Get loop mode override.
animationTree.Get("parameters/Animation/loop_mode_override");
[/csharp]
[/codeblocks]
</description>
<tutorials>
<link title="Using AnimationTree">$DOCS_URL/tutorials/animation/animation_tree.html</link>
Expand Down
18 changes: 16 additions & 2 deletions scene/animation/animation_blend_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ Vector<String> (*AnimationNodeAnimation::get_editable_animation_list)() = nullpt

void AnimationNodeAnimation::get_parameter_list(List<PropertyInfo> *r_list) const {
r_list->push_back(PropertyInfo(Variant::FLOAT, time, PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NONE));
r_list->push_back(PropertyInfo(Variant::INT, loop_mode_override, PROPERTY_HINT_ENUM, "None,Linear,Pingpong,No Override"));
}

Variant AnimationNodeAnimation::get_parameter_default_value(const StringName &p_parameter) const {
if (p_parameter == loop_mode_override) {
return Animation::LOOP_NO_OVERRIDE;
} else {
return 0.0;
}
}

void AnimationNodeAnimation::_validate_property(PropertyInfo &p_property) const {
Expand Down Expand Up @@ -100,7 +109,12 @@ double AnimationNodeAnimation::_process(double p_time, bool p_seek, bool p_is_ex
}

bool is_looping = false;
if (anim->get_loop_mode() == Animation::LOOP_PINGPONG) {
Animation::LoopMode loop_mode = anim->get_loop_mode();
Animation::LoopMode override = static_cast<Animation::LoopMode>((int)get_parameter(loop_mode_override));
if (override != Animation::LOOP_NO_OVERRIDE) {
loop_mode = override;
}
if (loop_mode == Animation::LOOP_PINGPONG) {
if (!Math::is_zero_approx(anim_size)) {
if (prev_time >= 0 && cur_time < 0) {
backward = !backward;
Expand All @@ -113,7 +127,7 @@ double AnimationNodeAnimation::_process(double p_time, bool p_seek, bool p_is_ex
cur_time = Math::pingpong(cur_time, anim_size);
}
is_looping = true;
} else if (anim->get_loop_mode() == Animation::LOOP_LINEAR) {
} else if (loop_mode == Animation::LOOP_LINEAR) {
if (!Math::is_zero_approx(anim_size)) {
if (prev_time >= 0 && cur_time < 0) {
looped_flag = node_backward ? Animation::LOOPED_FLAG_END : Animation::LOOPED_FLAG_START;
Expand Down
14 changes: 9 additions & 5 deletions scene/animation/animation_blend_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,23 @@
class AnimationNodeAnimation : public AnimationRootNode {
GDCLASS(AnimationNodeAnimation, AnimationRootNode);

public:
enum PlayMode {
PLAY_MODE_FORWARD,
PLAY_MODE_BACKWARD
};

private:
StringName animation;
StringName time = "time";
StringName loop_mode_override = "loop_mode_override";

uint64_t last_version = 0;
bool skip = false;

public:
enum PlayMode {
PLAY_MODE_FORWARD,
PLAY_MODE_BACKWARD
};

void get_parameter_list(List<PropertyInfo> *r_list) const override;
virtual Variant get_parameter_default_value(const StringName &p_parameter) const override;

static Vector<String> (*get_editable_animation_list)();

Expand Down
3 changes: 3 additions & 0 deletions scene/resources/animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3123,6 +3123,8 @@ void Animation::track_get_key_indices_in_range(int p_track, double p_time, doubl
from_time += CMP_EPSILON;
}
} break;
default: {
} break;
Comment on lines +3126 to +3127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so the challenge with combining LoopMode enum into Animation and AnimationNodeAnimation is that the extra enum value may be possible to set in loop_mode here, and the unused case statement may be needed.

Should the default case be the same as LOOP_NONE?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the default case be the same as LOOP_NONE?

It will break compatibility. Also, it should not be overridden by default. So No Override should be the default for AnimationNodeAnimation.

}
switch (t->type) {
case TYPE_POSITION_3D: {
Expand Down Expand Up @@ -3938,6 +3940,7 @@ void Animation::_bind_methods() {
BIND_ENUM_CONSTANT(LOOP_NONE);
BIND_ENUM_CONSTANT(LOOP_LINEAR);
BIND_ENUM_CONSTANT(LOOP_PINGPONG);
BIND_ENUM_CONSTANT(LOOP_NO_OVERRIDE);

BIND_ENUM_CONSTANT(LOOPED_FLAG_NONE);
BIND_ENUM_CONSTANT(LOOPED_FLAG_END);
Expand Down
1 change: 1 addition & 0 deletions scene/resources/animation.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class Animation : public Resource {
LOOP_NONE,
LOOP_LINEAR,
LOOP_PINGPONG,
LOOP_NO_OVERRIDE,
};

enum LoopedFlag {
Expand Down