Skip to content

Reset to the first animation in the stack#42

Open
lyuma wants to merge 1 commit intogodotengine:masterfrom
lyuma:use_default_anim_stack
Open

Reset to the first animation in the stack#42
lyuma wants to merge 1 commit intogodotengine:masterfrom
lyuma:use_default_anim_stack

Conversation

@lyuma
Copy link
Copy Markdown
Contributor

@lyuma lyuma commented Jun 9, 2023

FBX files may contain multiple AnimationStack objects, in which the first is expected to be used as a rest pose. This is the case in kenney's assets at https://kenney.nl/assets/series:Animated%20Characters

I have verified that other FBX implementations take the ordering of AnimationStack into account and use the first item to determine the rest pose. However, the way the code here is written, it may leave it on the last animation rather than the first.

@lyuma lyuma force-pushed the use_default_anim_stack branch from a90c22d to 45a55f4 Compare June 9, 2023 04:13
@lyuma
Copy link
Copy Markdown
Contributor Author

lyuma commented Jun 9, 2023

This does not solve the issue: This change would have to be coupled with passing the first frame timestamp into EvaluateLocalTransform(), rather than the default (infinity) which ignores the animation tracks.

If we go forward with this change, it would then require changing the animation code to use this computed base transform as the baseTransform/Translation/Rotation/Scaling, rather than the result of EvaluateLocalTransform(infinity)

Somehow Unity seems to have this behavior of using the pose of the first frame of the first animation, but it's not clear to me why this is the correct approach.

I'll mark this as a draft for now. If anyone can test Kenney's assets in other FBX software (such as Unreal, Maya, etc.) and see which put the model into a T-pose rest pose and which don't.

In terms of a Godot workflow, another option is to make this a manual step of the Godot advanced importer, and choose an animation frame to be copied from there.

@fire
Copy link
Copy Markdown
Member

fire commented Jun 13, 2023

I don’t think think we have a plan for resolving this. So I’m trending on closing the pr in a few weeks.

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.

2 participants