Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state #71418

TokageItLab · 2023-01-14T18:00:29Z

Closes godotengine/godot-proposals#5965
Fixed #69570
Fixed #69382
Fixed #71195

Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state. Also, allow NodeTransition to use String values.

Containing #71264. At first I was working with it as same PR #71264, but a few fundamental modifications caused break compatibility, so I separated this PR.

StateMachine

Make it restart when it tries to travel() to the same state. This can be resolved with one line change.

NodeTransition / NodeOneShot

For this, there was a problem with the original implementation, and 2 fundamental modifications are necessary.

1:
Currently they detect state changes by whether the property has changed from the previous frame, but this does not allow changes to the same state.

This means that they need an independent property as a request, like a StateMachine. So, implement the following properties for state changes.

NodeTransition:

# Property for the transition state requests. It will be cleared immediately in the next animation tree process.
anim_tree["parameters/TransitionNode/transition_request"] = "Input 2" # It is changed to String from int, to fix #69382.

# To avoid finding for an index every frame and to keep better performance, separate the following:
print(anim_tree["parameters/TransitionNode/current_state"]) # String value as state name for display. Read-only.
print(anim_tree["parameters/TransitionNode/current_index"]) # Int value as state index for internal use. Read-only.

NodeOneShot:

# Property for the transition state requests. It will be cleared immediately in the next animation tree process.
anim_tree["parameters/OneShotNode/request"] = AnimationNodeOneShot.ONE_SHOT_REQUEST_FIRE
anim_tree["parameters/OneShotNode/request"] = AnimationNodeOneShot.ONE_SHOT_REQUEST_ABORT

print(anim_tree["parameters/OneShotNode/active"]) # Read-only.

In addition, since there was only one active flag, there was no way to stop auto restart from the property, but now auto restart is stopped by an abort request.

2:
Some properties need to be changed only by request to prevent to make broken state, but the current status should be retrievable, so it should be PROPERTY_USAGE_READ_ONLY, not PROPERTY_USAGE_NONE.

However, the entity of the AnimationNode property is stored in the AnimationTree's map. During playback, the AnimationNode fetches the properties from the map, which means that read-only cannot be set using the USAGE flag.

Therefore, it is necessary to add a bool value to the AnimationTree map to implement read-only for the AnimationNode's parameter.

Video

Godot.2023.01.15.-.02.43.27.02.mp4

Godot.2023.01.15.-.02.43.27.03.mp4

restart_test.zip

If approved, I will update the document as soon as possible.

Works, but crashes whenever I select the AnimationPlayer node.

SaracenOne

I've had a chance to sit down with this properly now, and I think it's fine. The only thing I would still potentially recommend changing is the name of the 'interrupt' which I think might be too generic and could mean other things depending on the context. My current recommendation is changing it to the more literal 'skip_to_next_state'.

TokageItLab · 2023-01-18T02:09:05Z

@SaracenOne I get the feeling that "skip" is going to ignore xfade. So how about the following:

step()
forth()
ahead()
go_next()

TokageItLab · 2023-01-18T18:06:27Z

For now, I renamed interrupt() to next(). Possibly, it could be mixed up with the List iterator on cpp, but for gdscript and general users, I guess it is no problem.

reduz · 2023-01-19T10:13:12Z

This looks mostly good to me, would be good to agree a proper name with @SaracenOne before merging

reduz · 2023-01-19T10:14:25Z

editor/editor_properties.cpp

@@ -342,12 +342,17 @@ void EditorPropertyTextEnum::_notification(int p_what) {
 }

 EditorPropertyTextEnum::EditorPropertyTextEnum() {


Would be good to explain what this is for. Maybe some example screen of the change to show the problem?

Yeah, wait a second, there are screens that can be broken by it.

Here it is. Previous TextEnum had a problem with minimum_size not being obtained correctly since the containers existed in parallel. This PR fixes that by adding a wrapper container.

akien-mga · 2023-01-19T16:17:19Z

scene/animation/animation_blend_tree.cpp

-	r_list->push_back(PropertyInfo(Variant::BOOL, active));
-	r_list->push_back(PropertyInfo(Variant::BOOL, prev_active, PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NONE));
+	r_list->push_back(PropertyInfo(Variant::BOOL, active, PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_READ_ONLY));
+	r_list->push_back(PropertyInfo(Variant::INT, request, PROPERTY_HINT_ENUM, ",Fire,Abort"));


The first one has no name?

Yes. If there is a string here, the BlendTreeEditor pull-down will display something like "None". I think it's fine to leave it blank, or though it could be "--".

Alright, I'm not familiar enough with the API to say what would make sense but UX wise it doesn't seem immediately obvious why there's an empty drop down list, and what the empty option does compared to the other two.

But I trust your judgment on this. Can always be improved later on if users do report this as confusing :)

SaracenOne

Yeah, this looks good to me!

akien-mga · 2023-01-19T21:47:07Z

Thanks!

macryc · 2023-01-21T10:02:46Z

How do I trigger a Fire or Abort request on a OneShot node in the code??
I've tried AnimatrionTree.set("parameters/oneshotanim/request", "Fire") but it doesn't do anything...

TokageItLab · 2023-01-21T10:22:30Z

It has been described in the description. The document(tutorial) for AnimationTree is overall out of date and needs to be updated later.

# Property for the transition state requests. It will be cleared immediately in the next animation tree process.
anim_tree["parameters/OneShotNode/request"] = AnimationNodeOneShot.ONE_SHOT_REQUEST_FIRE
anim_tree["parameters/OneShotNode/request"] = AnimationNodeOneShot.ONE_SHOT_REQUEST_ABORT

anim_tree.set("parameters/OneShotNode/request", AnimationNodeOneShot.ONE_SHOT_REQUEST_FIRE)
anim_tree.set("parameters/OneShotNode/request", AnimationNodeOneShot.ONE_SHOT_REQUEST_ABORT)

print(anim_tree["parameters/OneShotNode/active"]) # Read-only.

macryc · 2023-01-21T10:43:12Z

Why can't we just say "Fire" instead of AnimationNodeOneShot.ONE_SHOT_REQUEST_FIRE? This is making things more complicated. It's also inconsistent with Transitions where you can set the state parameter via a string...

TokageItLab · 2023-01-21T10:58:46Z

Transition has a clear list of strings, but OneShot uses just an Enum as an Int for the request.
I don't know of any other method in Godot that allows a specific static const string to be input for a property instead of an Enum. If there is already such a method and it is allowed, I would consider some kind of binding.

jcarlosrc · 2023-01-22T06:00:23Z

Any way of checking if StateMachine is already in a given state so not to restart the animation when using travel() to the same state? Maybe the travel() function could have a second parameter (restart = true, false) to avoid restarting so to resemble the previous behavior. Or maybe is about documentation.

TokageItLab · 2023-01-22T06:18:26Z

if state_machine_playback.get_current_node() != "A":
  state_machine_playback.travel("A")

TokageItLab · 2023-01-22T06:42:36Z

If there is a long crossfade, it may have already moved to the next state, and we may be better to expose the fading_from. It seems that cpp has a function get_blend_from_node(), but it is not bound and not exposed... I will send a PR.

TokageItLab · 2023-01-22T07:00:30Z

There is also a second argument, reset_on_teleport, but if it is false, it would certainly make more sense not to restart with travel. I thought I had implemented it that way, but there seems to be some problem and it is not working. I will fix it later.

TokageItLab · 2023-01-22T07:24:45Z

@jcarlosrc I sent #71840. With that, you can write:

state_machine_playback.travel("A", state_machine_playback.get_current_node() != "A")

It does not allow its own restart. But it allows teleports and resets from other states. Note just that the existing travel path is to be discarded. In order to keep the existing travel path, if statement must be used.

TokageItLab added bug enhancement usability breaks compat topic:animation labels Jan 14, 2023

TokageItLab added this to the 4.0 milestone Jan 14, 2023

TokageItLab requested review from a team as code owners January 14, 2023 18:00

TokageItLab force-pushed the restart-anim-tree branch 4 times, most recently from e605d12 to 9685429 Compare January 15, 2023 12:28

TokageItLab mentioned this pull request Jan 17, 2023

AnimationNodeTransition gives no error message when you give it a String instead of an int #69570

Closed

eh-jogos added a commit to eh-jogos/godot4_method_track_bugs that referenced this pull request Jan 17, 2023

Testing with PR godotengine/godot#71418

1243667

Works, but crashes whenever I select the AnimationPlayer node.

SaracenOne approved these changes Jan 17, 2023

View reviewed changes

TokageItLab force-pushed the restart-anim-tree branch from 9685429 to cc1f489 Compare January 18, 2023 18:00

TokageItLab added 2 commits January 19, 2023 03:10

Add next/reset function to AnimationStateMachine

8bfaf09

Allow AnimationNodes to restart when transitioning to the same state

e480262

TokageItLab force-pushed the restart-anim-tree branch from cc1f489 to e480262 Compare January 18, 2023 18:11

TokageItLab requested a review from reduz January 18, 2023 18:14

TokageItLab mentioned this pull request Jan 18, 2023

AnimationTree ignores first frame of first child state when traveling from root to SubStateMachine #71562

Closed

reduz reviewed Jan 19, 2023

View reviewed changes

akien-mga requested a review from SaracenOne January 19, 2023 11:22

akien-mga reviewed Jan 19, 2023

View reviewed changes

TokageItLab requested a review from akien-mga January 19, 2023 18:34

SaracenOne approved these changes Jan 19, 2023

View reviewed changes

akien-mga merged commit d919d77 into godotengine:master Jan 19, 2023

TokageItLab mentioned this pull request Jan 22, 2023

Fix weird behavior of teleporting to self-state when reset_on_teleport is false in StateMachine #71840

Merged

eviltrout mentioned this pull request Jan 25, 2023

AnimationTree AnimationNodeStateMachinePlayback does not play at runtime #72070

Closed

This was referenced Jan 26, 2023

AnimationTree doesn't work right anymore godot 4 beta 14,15 #72122

Closed

Allow the Reset option of NodeTransition to be set for each Input #72388

Merged

geowarin mentioned this pull request Jan 30, 2023

Update project to Godot 4 beta 17 (animations still not fully updated) gdquest-demos/godot-4-3d-third-person-controller#13

Merged

This was referenced Jan 31, 2023

Animations in Animation Trees state machine are not looping #72446

Closed

Make restart in NodeStateMachine / NodeTransition optional #72450

Merged

Animationtree set function no longer working #72526

Closed

TokageItLab mentioned this pull request Feb 10, 2023

Add document for AnimationNodes which have setter as request property #73014

Merged

TokageItLab mentioned this pull request Feb 17, 2023

Cannot repeat a one-shot animation in BlendSpace2D #53858

Closed

TokageItLab deleted the restart-anim-tree branch February 14, 2024 05:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state #71418

Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state #71418

TokageItLab commented Jan 14, 2023 •

edited

Loading

SaracenOne left a comment •

edited

Loading

TokageItLab commented Jan 18, 2023

TokageItLab commented Jan 18, 2023 •

edited

Loading

reduz commented Jan 19, 2023 •

edited

Loading

reduz Jan 19, 2023

TokageItLab Jan 19, 2023

TokageItLab Jan 19, 2023

akien-mga Jan 19, 2023

TokageItLab Jan 19, 2023 •

edited

Loading

akien-mga Jan 19, 2023

SaracenOne left a comment

akien-mga commented Jan 19, 2023

macryc commented Jan 21, 2023 •

edited

Loading

TokageItLab commented Jan 21, 2023 •

edited

Loading

macryc commented Jan 21, 2023

TokageItLab commented Jan 21, 2023 •

edited

Loading

jcarlosrc commented Jan 22, 2023

TokageItLab commented Jan 22, 2023 •

edited

Loading

TokageItLab commented Jan 22, 2023 •

edited

Loading

TokageItLab commented Jan 22, 2023

TokageItLab commented Jan 22, 2023 •

edited

Loading

		@@ -342,12 +342,17 @@ void EditorPropertyTextEnum::_notification(int p_what) {
		}

		EditorPropertyTextEnum::EditorPropertyTextEnum() {

Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state #71418

Allow AnimationStateMachine / AnimationNode to restart when transitioning to the same state #71418

Conversation

TokageItLab commented Jan 14, 2023 • edited Loading

StateMachine

NodeTransition / NodeOneShot

Video

SaracenOne left a comment • edited Loading

Choose a reason for hiding this comment

TokageItLab commented Jan 18, 2023

TokageItLab commented Jan 18, 2023 • edited Loading

reduz commented Jan 19, 2023 • edited Loading

reduz Jan 19, 2023

Choose a reason for hiding this comment

TokageItLab Jan 19, 2023

Choose a reason for hiding this comment

TokageItLab Jan 19, 2023

Choose a reason for hiding this comment

akien-mga Jan 19, 2023

Choose a reason for hiding this comment

TokageItLab Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

akien-mga Jan 19, 2023

Choose a reason for hiding this comment

SaracenOne left a comment

Choose a reason for hiding this comment

akien-mga commented Jan 19, 2023

macryc commented Jan 21, 2023 • edited Loading

TokageItLab commented Jan 21, 2023 • edited Loading

macryc commented Jan 21, 2023

TokageItLab commented Jan 21, 2023 • edited Loading

jcarlosrc commented Jan 22, 2023

TokageItLab commented Jan 22, 2023 • edited Loading

TokageItLab commented Jan 22, 2023 • edited Loading

TokageItLab commented Jan 22, 2023

TokageItLab commented Jan 22, 2023 • edited Loading

TokageItLab commented Jan 14, 2023 •

edited

Loading

SaracenOne left a comment •

edited

Loading

TokageItLab commented Jan 18, 2023 •

edited

Loading

reduz commented Jan 19, 2023 •

edited

Loading

TokageItLab Jan 19, 2023 •

edited

Loading

macryc commented Jan 21, 2023 •

edited

Loading

TokageItLab commented Jan 21, 2023 •

edited

Loading

TokageItLab commented Jan 21, 2023 •

edited

Loading

TokageItLab commented Jan 22, 2023 •

edited

Loading

TokageItLab commented Jan 22, 2023 •

edited

Loading

TokageItLab commented Jan 22, 2023 •

edited

Loading