Difference in Skeleton3D.get_bone_global_pose_no_override behavior for 4.1

[uguu-org]

Godot version

4.1.stable

System information

Windows 10

Issue description

In Godot 4.0.3 and earlier, Skeleton3D.get_global_bone_pose_no_override appears to take into account of all transformations and overrides of the parent bone, while in Godot 4.1, parent overrides are ignored.

Godot 4.0.3 behavior is more convenient for my use case, but I can't tell if it just has been wrong all this time, hence this bug.

Steps to reproduce

1. Load the attached project in Godot 4.0.3.
2. Run the project, and observe how the transformations for the 3 cones are stacked:

3. Load and convert the same project in Godot 4.1
4. Run the project again, observe that all three cones have the same transformation:

See comments in lines 13-20 of skeleton_test/main.gd for where the behavior differs.

Minimal reproduction project

skeleton_test.zip

[wagnerfs]

Was just about to file the same bug report, all code that relied on get_bone_global_pose_no_override are now broken.
It seems like it just does get_bone_global_pose_no_override recursively until the requested bone, instead of taking into account all overrides till there

[EDIT]

Alright, the problem seems to lie here:

godot/scene/3d/skeleton_3d.cpp

Line 933 in b7c2fd2

b.pose_global_no_override = bonesptr[b.parent].pose_global_no_override * pose;

Changing the requested parent's transform from a chain of pose_global_no_override to a chain of pose_global seems to fix it

Just not quite sure if it's supposed to be a chain of pose no_override or not, maybe have an additional method to be able to get the requested bone with all past pose updates and another for a chain without overrides?

[lyuma]

In Godot 4.0.x, get_bone_global_pose_no_override mistakenly returned the same value as get_bone_global_pose. This bug has been corrected in 4.1 but it means that scripts which depended on this incorrect behavior will appear to have broken.
See the PR which was introduced for 4.1 here:

• Fix get_bone_global_pose_no_override() returning incorrect values #77194

The good news is this: if your code worked properly in Godot 4.0, that means your code is designed to work with get_bone_global_pose instead, so you should replace all calls to get_bone_global_pose_no_override with get_bone_global_pose and it should then work the same between Godot 4.0 and 4.1

In short, this is not a bug, but it is something we should have indicated more clearly in the release notes.

[wagnerfs]

@lyuma not quite, the issue there is that the last bone, the one being requested, needs to be bone_global_pose_no_override, get_bone_global_pose will return the chain global_pose INCLUDING the requested bone + the previous frame's override.
The way it worked on 4.0 is that we could get a "rest" pose for the requested bone but transformed by the chain behind that had all changes to them.

I was curious if that behavior was intended, so in that case we probably need a method that can return the above, the chain having all changes applied and the last bone on rest transform, otherwise we'd have to calculate the entire chain by hand.

godot.windows.editor.x86_64_2023-07-07_09-47-53.mp4

As you can see here, changing get_bone_global_pose_no_override to get_bone_global_pose will completely unlink the bone from the current animation being played.

[akien-mga]

In short, this is not a bug, but it is something we should have indicated more clearly in the release notes.

We actually have it documented in the release notes in the "Fixed > Animation" section:

Fix get_bone_pose_global_no_override() returning incorrect values (GH-77194).

I'm not sure where to put it aside from there to make it more prominent. That PR was also cherry-picked for 4.0.4 so it will be relevant there too.

[wagnerfs]

@akien-mga alright, I checked and the change does completely ruin IK, although all previous calls to bone_global_pose_no_override will now have to be replaced by:

skeleton3d.get_bone_global_pose(parent_bone) * skeleton3d.get_bone_pose(bone), so just leaving this here in case other people might come across this

[lyuma]

The explanation is way simpler. Read what I wrote.

This is how the code looked in 4.0:

$ cgrep -r pose_global_no_override scene
scene/3d/skeleton_3d.cpp:       return bones[p_bone].pose_global_no_override;
scene/3d/skeleton_3d.cpp:                               b.pose_global_no_override = b.pose_global;
scene/3d/skeleton_3d.cpp:                               b.pose_global_no_override = b.pose_global;
scene/3d/skeleton_3d.cpp:                               b.pose_global_no_override = b.pose_global;
scene/3d/skeleton_3d.cpp:                               b.pose_global_no_override = b.pose_global;
scene/3d/skeleton_3d.h:         Transform3D pose_global_no_override;

That is to say, in 4.0, get_bone_pose_global_no_override() quite literally returns the same thing as what get_bone_pose_global() returns both in 4.0 and 4.1

Therefore, to upgrade your project from 4.0 to 4.1, replace your get_bone_pose_global_no_override() calls with get_bone_pose_global(). That's it.

To demonstrate, I took your MRP project and did this change, and it solves the issue:

Can you try this instead of the skeleton3d.get_bone_global_pose(parent_bone) * skeleton3d.get_bone_pose(bone) operation you are attempting?

[wagnerfs]

@lyuma it's because you're not taking into consideration animations, in all my examples I'm doing changes to bones while the skeleton is being animated, that's why you can't see the problem here, both get_bone_pose_global() and get_bone_pose_global_no_override() will certainly return the same if the skeleton has no other influences.

Also like I mentioned here: #79136 (comment)
That's after swapping get_bone_pose_global_no_override to get_bone_pose_global

[lyuma]

Ok, I see what you mean. My apologies for being wrong about this.
I missed that this logic was done after assigning pose_global_no_override in the old code.

if (b.global_pose_override_amount >= CMP_EPSILON) {
	b.pose_global = b.pose_global.interpolate_with(b.global_pose_override, b.global_pose_override_amount);
}

So you are correct. To emulate what the old get_bone_global_pose_no_override returned, it's probably something like this:

var parent_bone = skeleton3d.get_bone_parent(bone)
var parent_transform: Transform3D
if parent_bone != -1:
	parent_transform = skeleton3d.get_bone_global_pose(parent_bone)
if skeleton3d.is_bone_enabled(bone):
	return parent_transform * skeleton3d.get_bone_pose(bone)
else:
	return parent_transform * skeleton3d.get_bone_rest(bone);

[uguu-org]

Applying the skeleton.get_bone_global_pose(skeleton.get_bone_parent(bone)) * skeleton.get_bone_pose(bone) change above fixed it for me, thanks!

[elXill]

I could finaly be able to control multiple bones in the same hierarchy with this skeleton.get_bone_global_pose(skeleton.get_bone_parent(bone)) * skeleton.get_bone_pose(bone) while it works perfectly fine now finding this info was really hard and took me a few days..
I wish this could be done with a more straight forward way, even now I'm not sure what does multiplying Transform3D's do but I'm glad it works.