Skeleton3D::get_bone_global_pose(bone_idx: int) return value is misleading.

[sigrudds1]

Note - This post name and content has been changed from information provide in a comment.

Godot version

4.0.stable

System information

Windows10, Vulkan, NVIDIA GeForce RTX 2060

Issue description

What doesn't work:
The word global in most cases mean, for the Godot Engine, global to the project not a specific space, the description does say otherwise but it still does not take away the general meaning that is presented in most other uses of the word , so it is easy to skip over the true meaning.

How do I expect it to work:
Return the global transform of the bone pose, to the world.

Proposed fix:
Rename it to get_bone_skeleton_pose() or get_bone_skeletal_pose which is more local to the skeletal space, as suggested and agreed upon in #19551 and added to #16863 but are both closed and last edited on Oct 21, 2021, so I assumed dropped.

Additionally:
I can see getting the global pose relative to the world as a use when trying to get the looking_at to another object from the bones perspective, this can be achieved in GDScript but I think adding it to the source would be more performant.

[bitsawer]

I think the global in the name indeed refers to the skeleton space: https://docs.godotengine.org/en/latest/classes/class_skeleton3d.html#class-skeleton3d-method-get-bone-global-pose:

Returns the overall transform of the specified bone, with respect to the skeleton. Being relative to the skeleton frame, this is not the actual "global" transform of the bone.

And yes, I think many people have found it to be a bit misleading. Also possible duplicate of #19551

[sigrudds1]

OK, I limited my search to around 2020, definitely an old issue and in my frustration I missed the description seeing a lot of the descriptions are

There is currently no description for this method. Please help us by contributing one!

my fault for assuming, but since #19551 and #16863 are closed will it get overlooked?
Or has it been decided to forget the changes? If so, then I will rename the issue to the same as #19551 referencing Skeleton3D and close it and I will do a pull request on the docs to add an example work around

var skel_transform: Transform3D = skel.get_global_transform()
var bone_transform: Transform3D =  skel.get_bone_global_pose(p_bone_idx)
var bone_global_transform: Transform3D = skel_transform * bone_transform

Or maybe suggesting a

Transform3D Skeleton3D.get_bone_global_transform(bone_idx: int)

would be better?

[fire]

I don't think we're able to change the names of apis, because of Godot Engine 4.0 compatibility and this is an often used function for game code but documentation pull requests are welcome.

[sigrudds1]

I don't think we're able to change the names of apis, because of Godot Engine 4.0 compatibility and this is an often used function for game code but documentation pull requests are welcome.

I find this to be unlikely, considering that in the latest, API names are changed and deprecated.

[AThousandShips]

Between 3.x and 4.0 yes, but unlikely to happen within 4.x

[sigrudds1]

Between 3.x and 4.0 yes, but unlikely to happen within 4.x

That kind of pigeon holes you into limiting the development of an admittedly incomplete engine version doesn't it? Especially considering it was suggested and agreed upon in 3.x before 4.x was started? I find it wishful thinking and will be utterly amazed that names will remain unchanged for the entire life cycle of 4.x, but I get that is to be determined, when 5.x comes along.
None the less the addition function of get_bone_skeleton_pose(), get_bone_skeletal_pose or even get_bone_global_transform, would still be applicable and help with confusion and performance.

[AThousandShips]

Semantic versioning is important, how do you suggest we handle everyone who can't make it work because compatibility breaking changes? Now its possible this can be done in 4.1 but that requires decisions, but yes adding a new function in the meantime would absolutely be possible

This isn't a blocking issue, it's annoying, and should be fixed eventually, but it's not like it makes the engine unusable, compatibility is priority, changing it will cause way more problems

[AThousandShips]

Also to be clear no actual decision seems to have been made to actually change the name and no action was taken, not that this would automatically make this change ignore compatibility concerns, several other proposed changes didn't happen

[AThousandShips]

My suggestion would be, if anything at all should be done:

• Document this more
• Add a properly named function and make the old function point to it
• Deprecate the old function at an appropriate time (probably a minor release)
• Remove the old function when appropriate

Since this can be solved with a single line of code, and is "only" a matter of confusing names, any more drastic change is not really warranted imo

[fire]

The reason for a pause is because get_bone_global_pose is the main api that people use to access the skeleton.

I know it is confusing but we distinguish between "transform" and "pose".

Global and local transform can be relative to the PackedScene root or the MainLoop root. Global pose is relative to the skeleton and local pose is relative to the bone's parent.

Your proposal is to rename global pose to bone_global_transform and pose to bone_transform (local) which affects these apis:

• clear_bones_global_pose_override
• get_bone_global_pose
• get_bone_global_pose_no_override ( int bone_idx ) const
• get_bone_global_pose_override ( int bone_idx ) const
• get_bone_pose ( int bone_idx ) const
• get_bone_pose_position ( int bone_idx ) const
• get_bone_pose_rotation ( int bone_idx ) const
• get_bone_pose_scale ( int bone_idx ) const
• reset_bone_pose ( int bone_idx )
• reset_bone_poses ( )
• set_bone_global_pose_override ( int bone_idx, Transform3D pose, float amount, bool persistent=false )
• set_bone_pose_position ( int bone_idx, Vector3 position )
• set_bone_pose_rotation ( int bone_idx, Quaternion rotation )
• set_bone_pose_scale ( int bone_idx, Vector3 scale )

My conclusion is this is a lot of change to make for Godot Engine 4 and it is unclear the rename is worth the change.

Adding a duplicate version of the apis makes the animation team difficult because it needs to cover all the apis to provide animation retargeting and blending.

[fire]

Please write a proposal https://github.com/godotengine/godot-proposals/

There's an evaluation guide here https://github.com/godotengine/godot-proposals/#how-core-developers-evaluate-proposals

[Calinou]

Closing, as feature proposals should be discussed on the Godot proposals repository. This repository is now used for bug reports only.

Make sure to follow the proposal template when opening a new proposal – don't just copy and paste the text you wrote in this issue.