Setting bone rotation in _process gets overridden by running animation in 4.1-dev3

[KMouratidis]

Godot version

4.1 dev3

System information

Windows 11, Forward+, Intel 13700k, 4070ti

Issue description

In 4.0.x and in 4.1 dev2, if you overwrite the rotation of a bone in _process it has preferred over the currently running animation. In 4.1 dev3, the running animation takes precedence and overwrites any changes in _process. This happens both with a simple animation and with an animation tree, and happens in-editor and in-game.

Steps to reproduce

1. Open the attached project.
2. Click on AnimationPlayer and pick an animation, e.g. Armature|Standing. Set it to looping if it's not already. (@RandomShaper's note: and play it).
3. Click on the PointOfInterest and move it along the X axis.
4. If you're on:
   • 4.0.x or 4.1-dev2: Notice the head rotating
   • 4.1-dev3: Notice that nothing happens

Minimal reproduction project

BoneRotationIssue-4.0.3.zip
BoneRotationIssue-4.1-dev.zip

Should include all licensing stuff for the model, but let me just add the link for good measure.

[purin-dev]

It seems like changing the AnimationPlayer's Thread Group to anything other than "Inherit" causes this to behave as intended. This may be related to the multi-threading work being done as part of Godot Proposal #6424.

[KMouratidis]

Ah, you're right. And it seems changing Process Mode on the AnimationPlayer also affects it: Manual and Physics both rotate the bone, Idle doesn't.

[akien-mga]

Wasn't actually fixed by #78713.

[RandomShaper]

Hmmm... I can't reproduce it on current master, regardless #78713.

[akien-mga]

Weird, it reproduced for me on Linux with and without that PR. And I confirmed that with 4.1-dev2 the head properly follows the PointOfInterest node.

[bitsawer]

Just tested on latest master at 1f9e540 with #78713 in it unfortunately the issue remains.

The issue can be pretty subtle to notice, the small head tilt can easy to miss. I had to look around to understand what was wrong when I initially tested this. Also remember to set the AnimationPlayer to play and loop in the editor. Workaround mentioned in #77548 (comment) also still works, although I don't quite understand why.

Not working image (animation is playing and looping):

Setting AnimationPlayer thread group to Main Thread and enabling process checkbox (which you can't disable for some reason once enabled?) makes it work:

[RandomShaper]

I was silly enough not to realize I should plat the animation. Let's throw a heavy veil over that...

Investigating now.

[RandomShaper]

First of all, as long as both the animation player and the node controlling the head have the same process priority, there are no guarantees about the order in which they will be processed. Moreover, if they were, they would be tree-order, which would lead to the current behavior anyway.

So, to me, the change of behavior between 4.0 and 4.1 is due to observed side effects of implementation details that users shouldn't rely on. The way a user would have this under control and get the wanted outcome is to set the priority of the animation player to -1 or that of the root node to 1.

Aside, I've spotted a few little bugs in the code handling the Thread group properties, that explain the unable-to-reset, etc. issues. I'll PR to fix them.

Aside again, I'd still have to reason about the fix of putting the animation player in its own processing group, because, with the default priority of 0, we may be in the same situation as with the processing order. I mean, the root node would be in the implicit default process group, which is priority 0, too, so there would not be order guarantees in that case either.

[lyuma]

I don't see any indication that this project or scene enabled the new node multithreading. If everything is set to inherit, all nodes should be running on the main thread, and hence the order should be deterministic.

This quote from @RandomShaper 's comment does not make sense to me in the context of a scene running with the default settings.

First of all, as long as both the animation player and the node controlling the head have the same process priority, there are no guarantees about the order in which they will be processed.

This does not match what is written in the documentation.
https://docs.godotengine.org/en/stable/tutorials/scripting/scene_tree.html#tree-order

Generally speaking, the order in which nodes are handled in the tree is in top-down fashion, starting at the root and going down each branch in turn. Scene tree order is something that can cause a great deal of confusion for Godot beginners.

Many of my scripts relied on the deterministic processing order defined in Godot. Having a well-defined process order by default is a welcome feature compared to Unity's generally undefined update order.

[RandomShaper]

As I understand it, we have groups (processed in group process order) and each has nodes (processed in node process priority). So, what I say (right or wrong) applies to both threaded and non-threaded situation, as long as the involved nodes belong to the same group.

Regarding if the order of nodes is deterministic or not, I may indeed be wrong. I think I was told it wasn't anymore, but judging by the code, it still is.

[RandomShaper]

In any case, the update of the skeleton is made deferredly, which may not be relevant, provided there are many flushes during the processing of the frame that may make the analysis of this as simple as if those updates were just sequential.

On current 4.1 this happens on each iteration in the MRP:

1. GDScript code updates the bone transform. The skeleton is marketd dirty and NOTIFICATION_UPDATE_SKELETON is queued in the default group's call queue.
2. The AnimationPlayer updates the bone transform. Since the skeleton is marketd dirty, no extra notification is queued anywhere.
3. The default group's call queue is processed, which updates the skeleton to the last transform set (so the AnimationPlayer wins) and clears the flag.

I'll check 4.1-dev2.

[RandomShaper]

I think I've spotted something serious regarding internal/non-internal process. @lyuma, I think you were on the right track. Sorry for the many updates. I'll come back with specifics in the next one.

[KMouratidis]

@RandomShaper Thanks for the explainers both here and in the PR! I compiled the engine with your changes and can confirm it works.

[KMouratidis]

The second batch of changes caused the script to not work again. Having read the discussion and fixes, this is how it should be. Lower nodes in the tree (i.e. the AnimationPlayer / AnimationTree) are processed after higher nodes (root node). I'll just leave what I did here for future reference.

One workaround could be to attach a second script on the animation player and overwrite the animations there. The engine will run the internal stuff first and then run your script. I didn't want to do that (too troublesome for multiple scenes) so I ended up changing the process priority in the top-level script and it works fine for my purposes:

func _ready():
    process_priority = 1

[nagasawamaki]

I am sorry. I see it was fixed but I can still reproduce this in v4.2.1.stable.official [b09f793]

Am I missing something?

Or I do need to change the thread group to "Main Thread" in AnimationTree node as stated in #77548 (comment)

Thanks

[KMouratidis]

@nagasawamaki attach the override logic on the animation node or a node lower down the tree, or change the process_priority as I mentioned in my comment above. You can look at this docs page for a better explanation.

[nagasawamaki]

@nagasawamaki attach the override logic on the animation node or a node lower down the tree, or change the process_priority as I mentioned in my comment above. You can look at this docs page for a better explanation.

Hi, thank you for your comment. I have tried your method but it doesnt work for me

[nagasawamaki]

@KMouratidis for the "top-level script" do you mean the direct parent of the animationTree node?

I have tried to set process_priority = 1 in grandparent node, not the direct parent but not work.

Thanks.

[nagasawamaki]

I also made another issue #87428 but it seems it is intended behavior in Godot that bone rotation cannot be overridden when running animation by design...