-
-
Notifications
You must be signed in to change notification settings - Fork 20.8k
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
Add 3D skeleton preview to Advanced Importer #96094
Add 3D skeleton preview to Advanced Importer #96094
Conversation
8ceaf0e
to
6de60e0
Compare
When an imported model Skeleton3D type node is selected, the bones are drawn using lines or octahedrons to provide a clearer reference to their position. Refactored Skeleton3DGizmoPlugin::redraw now uses a static function to generate bone meshes
6de60e0
to
f0f9a57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok to me.
- Did an online review
- Did not compile godot engine
- Did not run a test project
@jeronimo-schreyer Feel free to test if you want, since this is an updated version of your old pr. |
Sure, I can test on Windows 10 and Linux Manjaro 24. Also test changes proposed by @AThousandShips |
Better size calculation in advanced importer preview. Uses the skeleton mesh to calculate the scene's bounding box. This improves some situations where a mesh instances' scale does not match its visual representation when a skeleton is applied. Advanced importer skeletal preview UX improvement. Make the visibility of the skeletal preview in the advanced importer when selecting an animation dependent on a new dedicated toggle button rather than carrying over whether a skeletal node was or was not previously selected before selecting the animation. Advanced importer skeletal preview fix. Fixes the preview on scaled skeletons in the advanced importer by applying the node's scale to the preview and generating a skin for it.
f0f9a57
to
6532eff
Compare
I've implemented @AThousandShips' changes, so this should be good to merge now unless anyone has any additional changes to request. |
I reran the failed integration test job |
Thanks! |
This PR rebases and extends the earlier PR by @jeronimo-schreyer with the following additions: