Godot: Modifying a bone's transform will freeze the pose

[SlothDevLlama]

Godot version:
3.2 Beta 1

OS/device including version:
Windows 10

Issue description:
Afaik there was a breaking change in Skeleton where the function set_bone_global_pose has been renamed to set_bone_global_pose_overridewith two additional arguments (amount, persistent)

My understanding is, that get_bone_global_pose and set_bone_global_pose complemented each other. So that you could actually get a bone's transform by using get_bone_global_pose, modify for example the rotation and put it back to the skeleton's transforms by using set_bone_global_pose, like stated here in thise reddit topic: https://www.reddit.com/r/godot/comments/b1aqdq/how_to_set_global_y_rotation_of_a_3d_bone/

@fire recently pointed out, that for deprecation purposes, the old method will internally call the new set_bone_global_pose_override with arguments amount = 1.0 and persistent = true.
https://github.com/godotengine/godot/issues/32637#issuecomment-539591516

So here starts the problem / bug I'm facing:
When I'm using the easiest setup, getting a bone's global pose, do no changes on it and then set it by using the new method, the bone will then stand still. (I'm using AnimationTrees)
If you want to "unfreeze" bones, you explicitly must call set_bone_global_pose_override with persistent = false then it will start moving again. If that is the purpose of the persistent argument, it kinda makes sense to me, but actually no (when I consider that set_bone_global_override should replace the old function by 1:1.

What I would expect here is, that at least the "normal" animation should be shown. This happens whether running in _process or _physics_process

Steps to reproduce:
Create a simple scene and add an animated character to it. Use one bone for transformation tests.

These lines of code should be enough to show the issue.

var bone_index = 0
var bone_transform:Transform = skeleton.get_bone_global_pose(bone_index)
skeleton.set_bone_global_pose_override(bone_index, bone_transform, 1.0, true)

My first guess is, that get_bone_global_pose should always get the global pose like it is without the overridden pose.

Minimal reproduction project:
bone_bug.zip
Here a video showing the issue:
https://youtu.be/VfGeCb2KK6g

btw: Is this really a bug, or is the usage of the override different? Maybe I first need to substract any other transforms? But then fire's comment would be falsly? I'm really unsecure about this.

[akien-mga]

CC @fire @marstaik if you can shed some light on the new API and whether this is a bug.

[marstaik]

I have tested this out before the skinning and post skinning.

I am going to say that it was incorrect behavior before the change, and that behavior really should never have worked.

You see, when you apply a global_pose override, it gets taken into account into the final global_pose.
Thus, get_bone_global_pose current implementation is correct, because the final pose should incorporate that override.

This results in simply calling

T = get_bone_global_pose(bone_id);
set_bone_global_pose_override(bone_id, T, 1.0, true)

just stacking the global_pose onto itself over and over, and causing it to explode.

Now, I guess that does add a bit of unexpected behavior to users in that if they want to modify the same bone over and over again, they would have to on each iteration:

set_bone_global_pose_override(bone_id, Transform(), 0.0, false)
T = get_bone_global_pose(bone_id);
# do some stuff to T
set_bone_global_pose_override(bone_id, T, 1.0, true)

which is extremely expensive, you are causing the entire skeleton tree to recompute all of its global_poses.

With PR https://github.com/godotengine/godot/pull/33958 , users could instead:

T = get_bone_global_pose_without_override(bone_id);
# do some stuff to T
set_bone_global_pose_override(bone_id, T, 1.0, true)

Note though, that you still need to set the bone_global_pose_override back to 0.0 for bones you are done overriding.

Your GDScript in your sample file could instead look something like this to mirror the behavior pre-skin changes:

tool
extends Spatial

var skeleton:Skeleton
export(int, 0, 3) var bone_index:int = 0
export(bool) var apply_persistence:bool = false

var last_bone_index = -1

func _process(delta: float) -> void:
    if skeleton == null: skeleton = get_node("simple_model/Armature/Skeleton")
    if last_bone_index > -1 && last_bone_index != bone_index:
        skeleton.set_bone_global_pose_override(last_bone_index, Transform(), 0.0, false)

    var bone_transform:Transform = skeleton.get_bone_global_pose_without_override(bone_index)
    skeleton.set_bone_global_pose_override(bone_index, bone_transform, 1.0, apply_persistence)
    last_bone_index = bone_index;

[jitspoe]

I'm also struggling with this. Here's the functionality I would expect:

set_bone_pose sets the pose relative to the parent bone.
set_bone_global_pose sets the pose in world space (converting it to the local space of the bone)

I'd like effectively the same behavior we have for set_transform vs set_global_transform.

What we had before was sort of like that, except the "global" pose was not actually global, so I had to convert to the transform of the skeleton before setting it. Now, it's even more confusing.

Now, setting the "global" pose (still not actually global) requires more parameters and overrides the computed pose (relative to the skeleton, not the parent bone) directly instead of computing the local pose.

There are some parameters here that seem useful, like the ability to keep the transform overridden, but the behavior is different from what I expect. What I would EXPECT is that I could pass in false for persistent, and this would behave the same as the old set_bone_global_pose. It would apply it for this frame, and the next frame, it would use the animation-driven position. In practice, setting persistent to false makes it not function at all, meaning you have to set persistent to true, then do the unoptimal hack of adding this before getting the pose, if that's what you wanted to get next frrame:
set_bone_global_pose_override(bone_id, Transform(), 0.0, false)

The other thing that's weird is that if you set persistent to true, it's not going to keep that bone in that pose relative to the parent pose, it's going to be relative to the whole skeleton node, so if you were to pose, say, a body part, that body part would just be left floating there while the rest of the skeleton. I've set the hair here globally and just left it persistent:

Now, if my character crouch slides, the hair is floating above, not moving with the head:

I guess there are some cases where this would be useful, but that seems like it should be a new function without removing the old functionality. I really just want a simple way to convert from global space to the local bone transform (in an optimized engine manner) so I don't have to do all those calculations in script.

In my case, I'm procedurally animating hair, so I compute points in world space using some chain-like physics, then I want to make each hair segment bone point toward the world space position. In the current state, I can't really find a good way to do that. I guess get_bone_global_pose_without_override() could work, but that's not integrated, yet. I'm just having trouble understanding how one would use these functions in their current state and why they changed. Also, it seems to have broken IK behavior, but that might be a different bug.

[AndreaCatania]

With this PR https://github.com/godotengine/godot/pull/37272 I've added the function clear_bones_global_pose_override that allows to remove the override from the global pose.

This approach may be not optimal depending the situation because it forces transform resolution; this aspect may deserve some time so to understand if add a new API would be better.