[VRMC_springBone_limit] specVersion and validation - #2833
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates UniVRM’s VRM10 spring bone limit extension (VRMC_springBone_limit) handling to better align with the extension schema/spec intent by adding exporter specVersion output and adding importer-side validation for invalid/unsupported limit data.
Changes:
- Importer: adds a validation gate before applying
VRMC_springBone_limitto a spring joint. - Exporter: introduces
VRMC_springBone_limit_SPEC_VERSIONand suppresses limit export on leaf joints. - Exporter: emits
specVersionfor the extension (but currently only in one of the limit-type branches).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Packages/VRM10/Runtime/IO/Vrm10Importer.cs | Adds validation before applying VRMC_springBone_limit during import. |
| Packages/VRM10/Runtime/IO/Vrm10Exporter.cs | Adds specVersion constant, avoids exporting leaf-joint limits, and serializes the limit extension (with incomplete specVersion coverage). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var validate = true; | ||
| if (extensionSpringBoneLimit.SpecVersion != Vrm10Exporter.VRMC_springBone_limit_SPEC_VERSION) | ||
| { | ||
| UniGLTFLogger.Warning($"Unknown VRMC_springBone_limit specVersion: {extensionSpringBoneLimit.SpecVersion}"); | ||
| } | ||
|
|
||
| var count = 0; | ||
| if (extensionSpringBoneLimit.Limit.Cone is UniGLTF.Extensions.VRMC_springBone_limit.ConeLimit cone) | ||
| { | ||
| ++count; | ||
| } | ||
| else if (extensionSpringBoneLimit.Limit.Hinge is UniGLTF.Extensions.VRMC_springBone_limit.HingeLimit hinge) | ||
| { | ||
| ++count; | ||
| } | ||
| else if (extensionSpringBoneLimit.Limit.Spherical is UniGLTF.Extensions.VRMC_springBone_limit.SphericalLimit spherical) | ||
| { | ||
| ++count; | ||
| } | ||
|
|
There was a problem hiding this comment.
Copilotの指摘通り、 specVersion が未知の場合は、当該limitの読み込みを中断し無視するのが望ましい挙動だと思います。
There was a problem hiding this comment.
また、limitの数が2つ以上のときのロジックエラーもCopilotの指摘どおりです。else ifでつなげると、どれか1つ存在を確認できた時点でこのif文全体を解決してしまいます。
| if (node.childCount == 0) | ||
| { | ||
| // leaf | ||
| UniGLTFLogger.Warning($"VRMC_springBone_limit: Leaf node cannot. skip"); |
There was a problem hiding this comment.
Copilotによる英文の指摘はこの方向性で良いと思います。
| if (y.transform.childCount > 0) | ||
| { | ||
| case UniGLTF.SpringBoneJobs.AnglelimitTypes.Cone: | ||
| { | ||
| var limit = new UniGLTF.Extensions.VRMC_springBone_limit.VRMC_springBone_limit | ||
| // childが存在する => 末端でない | ||
| switch (y.m_anglelimitType) | ||
| { |
There was a problem hiding this comment.
もしすでにroundtrip等があれば、お手軽にできる範囲でやっていただければで良いと思います。
| var limit = new UniGLTF.Extensions.VRMC_springBone_limit.VRMC_springBone_limit | ||
| { | ||
| Hinge = new UniGLTF.Extensions.VRMC_springBone_limit.HingeLimit | ||
| Limit = new UniGLTF.Extensions.VRMC_springBone_limit.Limit | ||
| { | ||
| Rotation = ReverseXToFloat4(y.m_limitSpaceOffset), | ||
| Angle = y.m_pitch, | ||
| Hinge = new UniGLTF.Extensions.VRMC_springBone_limit.HingeLimit |
There was a problem hiding this comment.
確かに、 SpecVersion = VRMC_springBone_limit_SPEC_VERSION, の変更がConeのみに適用されており、HingeとSphericalへの変更が漏れていそうです。
0b5vr
left a comment
0b5vr
left a comment
There was a problem hiding this comment.
兼ね、Copilotによるレビューに同意します。詳細は各コメントを参照してください。
fixed #2828
fixed #2829