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

Preserve skeletons which do not contain a mesh #41

Merged
merged 1 commit into from
Jun 10, 2023

Conversation

lyuma
Copy link
Contributor

@lyuma lyuma commented Jun 8, 2023

Generates an extra unused glTF skin for each node hierarchy with a FBXSkeleton, regardless of whether that node is connected to an existing skin.

Fixes #19

Should solve import for some Mixamo animations.
See also https://twitter.com/KenneyNL/status/1666528263890522126

@lyuma lyuma force-pushed the preserve_skeletons branch 2 times, most recently from bbb9763 to 5cd8409 Compare June 8, 2023 05:52
@fire
Copy link
Member

fire commented Jun 8, 2023

How should I test this is working?

@lyuma
Copy link
Contributor Author

lyuma commented Jun 8, 2023

Download the characters from https://kenney.nl/assets/series:Animated%20Characters and confirm that they import correctly with a skeleton.

And then we need to make sure this doesn't cause a regression, since "sometimes the fbx skeleton is for advisory notice only" - similar to how the unused gltf joints are advisory only.

This change technically can't break anything in terms of glTF, since glTF is agnostic to bones vs nodes. But it could affect the generated Godot scene hierarchies which could lead to unexpected changes for some Godot users. We will only know through testing.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

I saw this work on discord.

We had concerned this will cause unforeseen consequences, but we are unable to test.

The blender gltf io addon was ok with gltf animations hacked with a skeleton skin.

We are unsure about the skeleton property @RevoluPowered, but they're advisory. So I think it's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FbxSkeleton should create empty skins with all of the bones.
2 participants