feat: validate wearable armature bone names against dcl avatar skeleton#3429
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report for CI Build 28429301540Coverage increased (+0.2%) to 52.401%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
decentraland-bot
left a comment
There was a problem hiding this comment.
✅ Approved — Solid new validators with good test coverage
Overview
Adds two wearable validators: validateArmatureBoneNames (checks skinned-mesh joints match the DCL avatar skeleton) and validateNoUnriggedVertices (flags zero-weight vertices). Both emit WARNINGs (non-blocking), consistent with the existing validator pattern. i18n added for en/es/zh.
Findings
[P2] Bone count comment says 62 — verify it's accurate
src/lib/glbValidation/constants.ts — The JSDoc claims "62 bones." By my count: 6 (spine/head) + 8 (arm chains) + 40 (fingers: 5×4×2) + 8 (leg chains) = 62. ✅ Checks out, but the count is non-obvious because of the programmatic handFingerBones() expansion — consider adding a brief breakdown in the comment so future maintainers don't have to re-derive it.
[P2] Avatar_Spine1/Avatar_Spine2 and shoulders omitted from AVATAR_CORE_BONE_NAMES
src/lib/glbValidation/constants.ts — The core subset (16 bones) omits Spine1, Spine2, LeftShoulder, and RightShoulder. If this is intentional (allowing simpler partial skeletons for accessories), a brief comment explaining the rationale would help. If not, they should be added.
[P2] No multi-mesh aggregation test
src/lib/glbValidation/wearableValidators.spec.ts — validateArmatureBoneNames collects joints across all SkinnedMesh nodes into one Set, but every test uses a single mesh. Consider adding a test with two meshes whose combined bone sets pass/fail the core check.
[P2] No test for the (+N more) truncation branch
The formatting code truncates bone lists to 5 items. A test with a fully foreign skeleton (>5 unknown bones) would exercise this path.
[P2] Armature root node could be flagged as "unknown"
src/lib/glbValidation/wearableValidators.ts — If a GLB exporter includes the armature root in skeleton.bones, it would be reported as unknown since AVATAR_BONE_NAMES doesn't include 'Armature'. Consider adding it to the allowlist alongside the spring bone exception.
[P2] Case-sensitive matching with no near-miss hint
Bone matching is case-sensitive (correct), but spring bone filtering is case-insensitive. A bone named avatar_hips (wrong case) gets reported as "unknown" with no hint about casing. A case-insensitive near-miss suggestion would help creators using non-standard DCC tools.
CI Status
All checks passing (audit ✅, test ✅, Vercel ✅).
Verdict
No P0 or P1 issues. The validators are well-structured, follow existing patterns, and have solid test coverage. The P2 items are minor improvements that can be addressed in follow-ups.
Reviewed by Jarvis 🤖 · Requested by Juan Manuel Hidalgo via Slack
Why
Analysis of 4,340 real wearable submissions shows rigging issues are a top curation-feedback reason (12.7% of collections). The Builder already validates bone influences (>4/vertex), leaf bones, and non-deform bones for wearables — but it did not validate that a wearable's armature bone names match the Decentraland avatar skeleton. A wearable skinned to a non-DCL or misnamed skeleton passes import and then breaks in-world / fails curation. (
validateArmatureNamingexisted but was wired only into emote validation.)Changes
validateArmatureBoneNames(insrc/lib/glbValidation/wearableValidators.ts): collects skinned-mesh joint names and reports a WARNING when the skeleton looks wrong — required core avatar bones are missing, or joints reference unknown/misnamed bones. Spring bones (names containingspringbone) are allowed extras and ignored. Wearables with no skinned meshes (rigid accessories) are skipped.validateNoUnriggedVertices: flags skinned-mesh vertices whose total skin weight is effectively zero (not bound to any bone), which won't follow the avatar's movement.validateWearableGLTFinsrc/lib/glbValidation/index.ts.AVATAR_BONE_NAMES/AVATAR_BONE_NAME_SET) and a required core-bone subset (AVATAR_CORE_BONE_NAMES) tosrc/lib/glbValidation/constants.ts. No existing source of truth for this list was found in the repo or@dcl/schemas.armature_bone_namesandunrigged_verticesmessage keys toen.json,es.json, andzh.json.Severity is WARNING (non-blocking), consistent with the other non-fatal wearable checks, since valid edge cases exist.
Test plan
prettiercheck passeslint(eslint) passesbuildpassestest—glbValidationsuite green (126 tests)