Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix converted 3.x Skeleton3D missing pose #88009

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

nikitalita
Copy link
Contributor

This PR fixes an issue that I missed in my previous PR (#87050), which introduced converting 3.x Skeletons.

For 3.x Skeletons, poses are relative to the rest position. If a pose is set to the rest position value, the pose will be set to default Transform() value. Since this is the default value for pose, this means that the pose property is not saved to the scene.

Thus, when a deprecated Skeleton is loaded, the pose value is not set, and the skeleton position, rotation, and scale will be set to their default values and you'll get funky looking skeletons like this:

image

Fortunately, there is an easy fix. bound_children is a deprecated property on 3.x skeletons that we weren't doing anything with because it's not present in the current Skeleton3D format (bound children find their parent index automatically in 4.x). However, this property just so happens to be always set on all Skeleton nodes from 3.0 to 3.x branch current regardless of value, and it's always set AFTER both the pose and the rest properties. Thus, when that property is set on the Skeleton3D node, we just check to see if the position, rotation, and scale are their default values, and if so, set the pose to the rest position.

@nikitalita nikitalita requested a review from a team as a code owner February 6, 2024 08:01
@akien-mga akien-mga requested a review from TokageItLab February 6, 2024 08:22
@akien-mga akien-mga added this to the 4.3 milestone Feb 6, 2024
Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable

@akien-mga akien-mga merged commit 7d4e24d into godotengine:master Feb 19, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants