-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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 per-bone meta to Skeleton3D #87150
Conversation
There's no such system AFAIK, just make it clear in the PR description that it depends on that PR and the production team and reviewers will keep that in mind |
0a8c475
to
437d183
Compare
24d5aed
to
a9153b9
Compare
675783f
to
d1f18eb
Compare
f66f248
to
8db5072
Compare
I like this idea because it enables round-tripping of GLTF extras. Because we have heavily modified the skeleton class late in the release cycle already, I'm a bit hesitant to add another big change to the skeleton (adding another property per-bone). I think @TokageItLab would need to review the skeleton changes to make sure both the serialization and the API changes ok, but I don't want to put more on the plate for 4.3 this late in the cycle. I'd feel more comfortable trying the original PR #86183 in for this release cycle, understanding that we won't have full round tripping of skeleton bone extras for now... and depending on feedback and when we have time, we can also add the skeleton bone support in the future. That said, the code looks reasonably simple if I read only the second commit (since it's stacked on the other PR)... if Tokage approves it and we think the timing works, I think it's ok. |
7387b86
to
19d5c94
Compare
Actually playing with this it will need some more work, I'll switch it to draft. |
d95fdfe
to
7e9b40a
Compare
2169c9b
to
7a27247
Compare
This comment was marked as resolved.
This comment was marked as resolved.
6867c87
to
6ca053f
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.
LGTM
Individual bones are not represented as `Node`s in Godot, in order to support meta functionality for them the skeleton has to carry the information similarly to how other per-bone properties are handled. - Also adds support for GLTF import/export
Thanks! |
Node
s and don't inherit get/set meta functionality fromObject
, so the skeleton has to carry the meta information similarly to how other per-bone properties are handled.This was split off of #86183 which only deals with import/export of metadata for nodes, meshes and materials - all of which do inherit from Object and therefore support metadata already.
Supporting the same for bones requires changes to
Skeleton3D
datastructure, so it's better to have the discussion separate.