tdw46 vrma fixes + non humanoid support#733
Conversation
…sforms. This seems to work for humnaoid bones, but non-humanoid bones need a different space transform.
| continue | ||
| bone_name_to_quaternions[bone_name] = [ | ||
| # ミュートされている項目とかあるとクオータニオンの値がノーマライズされて | ||
| # いないのでノーマライズしておく |
There was a problem hiding this comment.
Please don't remove this comment. It's necessary. If you want to avoid Japanese, please send a translation pull request instead.
| ) | ||
|
|
||
| # TODO: 3要素の場合はオイラー角になるか? | ||
| # TODO: Matrixだったら分解する |
There was a problem hiding this comment.
Please don't remove this comment. It's may necessary. If you want to avoid Japanese, please send a translation pull request instead.
| return | ||
|
|
||
| # Compute the armature world inverse for use with root bones | ||
| armature_world_inv = armature.matrix_world.inverted_safe() |
There was a problem hiding this comment.
This variable is not used, so please delete it.
| node_index = node_dict.get("node") | ||
| if not isinstance(node_index, int): | ||
| for name, node_dict_ in preset_dict.items(): | ||
| if not isinstance(node_dict_, dict): |
There was a problem hiding this comment.
Please use a name with an added modifier, such as preset_node_dict.
I think you added an underscore to avoid name duplication with other variables. However, that would mean that both the node_dict_ variable and the node_dict variable would exist, which would invite mistakes.
| translation = node_dict_.get("translation") | ||
| if isinstance(translation, list) and translation: | ||
| default_preview_value = translation[0] # TODO: Matrixだった場合 | ||
| default_preview_value = translation[0] |
There was a problem hiding this comment.
This Japanese means that glTF may also express the position of nodes using a matrix, so we will support that in the future. Please do not delete it, but leave it as it is.
Or, please send us a pull request to translate it into English.
There was a problem hiding this comment.
There are some points raised by the automatic coding style checker, so could you please check them?
https://github.com/saturday06/VRM-Addon-for-Blender/actions/runs/13339576683/job/37268118160?pr=733
|
Thank you! I'm going to merge it, but there are a few things I need to point out. Is it possible for you to check these? |
|
@tdw46 I have made some comments on the parts I would like to see changed. What do you think about these? |
|
@saturday06 I would like to try to make these changes! I think there is something wrong with the ruff auto formatting settings on my end then. Can you help? I followed your developer guidelines and setup the docker container and run things from my WSL distro. When I hit save ruff auto-formats things, but it doesn't seem to respect the maximum line width/character count required. |
|
Looks like my latest commit addresses most of these issues! |
|
Thank you! I'll merge and release it within two weeks! (I'm busy this weekend)
It seems that the automatic formatting sometimes fails. This is also a problem for me. I am currently correcting it manually. |
… tracks needed for non-humanoid animation features.
Please let me know when it is completed. I will work on the merge. |
…nd regular vrm importer)
2 participants
This implementation fixes both the issue reported here and here. Humanoid VRMA export now matches expected results reliably across all platforms, and imports match as well. Support for non-humanoid bones does not break existing humanoid functionality in any way.