feat: auto-suggest wearable category from model geometry at import#3430
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report for CI Build 28383377116Coverage increased (+0.2%) to 52.184%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats💛 - Coveralls |
decentraland-bot
left a comment
There was a problem hiding this comment.
🔄 Changes Requested — One defensive fix needed before merge
Overview
Auto-suggests the most likely wearable category from model geometry by tallying skin weight per avatar region. Clean heuristic with a 60% dominance threshold, conservative region→category mapping, and proper guards for ambiguous/multi-region geometry. i18n added for en/es/zh.
P1 — Should fix before merge
[P1] Missing try/catch around suggestWearableCategory in loadAndValidateModel
src/lib/getModelData.ts, ~line 369 — suggestWearableCategory is a non-critical enhancement, but if it throws (e.g., unexpected geometry attribute format on a malformed skeleton), it will propagate up and fail the entire model import. Since the suggestion is purely a UX convenience, it should never break the critical path:
try {
suggestedCategory = suggestWearableCategory(Three, gltf.scene)
} catch {
suggestedCategory = null
}P2 — Minor improvements
[P2] forearm bones classified as spine (upper_body) instead of hands
src/lib/glbValidation/suggestWearableCategory.ts, classifyBone() — After normalization, Avatar_LeftForeArm → forearm. The hands patterns (hand, finger, thumb) don't match, but spine patterns catch it via "forearm".includes("arm"). For most wearables (shirts, jackets) this is arguably correct since the forearm is part of the upper body. But for bracers/gauntlets, it would dilute the hands signal. Consider adding 'forearm' explicitly to spine patterns with a comment explaining the design choice, or to hands if bracers are the more common case.
[P2] Suggested category hint stays visible after user manually changes category
src/components/Modals/CreateSingleItemModal/CommonFields.tsx — The "Suggested from your model: …" hint renders whenever suggestedCategory is set, even after the user picks a different category. Consider hiding it when category !== suggestedCategory.
[P2] No test for Avatar_LeftForeArm / Avatar_RightForeArm classification
src/lib/glbValidation/suggestWearableCategory.spec.ts — Adding a test with forearm bones would document the current behavior and catch regressions if the classification changes.
[P2] No test for multiple SkinnedMesh nodes
The accumulation logic sums weight across all meshes, but all tests use a single mesh.
[P2] Redundant await import('three') in loadAndValidateModel
src/lib/getModelData.ts, line ~353 — loadGltf already imports Three.js internally. The module cache makes this free, but a comment explaining why it's needed (for suggestWearableCategory) would help readability.
CI Status
All checks passing (audit ✅, test ✅, Vercel ✅).
Verdict
The heuristic is well-designed and the implementation follows existing patterns cleanly. The try/catch fix is a one-liner that prevents a non-critical feature from breaking the import flow.
Reviewed by Jarvis 🤖 · Requested by Juan Manuel Hidalgo via Slack
decentraland-bot
left a comment
There was a problem hiding this comment.
✅ Approved — All findings addressed
Re-reviewed after changes. Every flagged issue has been fixed:
P1 resolved:
- try/catch around
suggestWearableCategory(src/lib/getModelData.ts) — ✅ Now wrapped; any throw falls back tonullwithout breaking the import flow.
P2s resolved:
forearmclassification — ✅'forearm'added explicitly to thespinepatterns inLANDMARK_BONES, with a comment explaining the design choice (forearm = upper body for sleeved garments). New test confirmsAvatar_LeftForeArm/Avatar_RightForeArm→upper_body.- Stale suggestion hint — ✅ Hint now only renders when
category === suggestedCategory; disappears as soon as the user picks a different category. - Multi-mesh test — ✅ Added test with two
SkinnedMeshnodes whose accumulated spine weight crosses the threshold. - Redundant
import('three')comment — ✅ Comment explains the import is cached and needed forsuggestWearableCategory.
CI: all checks passing (audit ✅, test ✅, Vercel ✅).
Reviewed by Jarvis 🤖 · Requested by Juan Manuel Hidalgo via Slack
…ory-auto-suggest # Conflicts: # src/components/Modals/CreateSingleItemModal/CreateSingleItemModal.reducer.ts # src/components/Modals/CreateSingleItemModal/CreateSingleItemModal.types.ts
Why
Analysis of 4,340 real wearable submissions shows wrong-category is a top curation-feedback reason (11.5% of collections). Curators routinely ask creators to fix the category (e.g. "Making this an upper body means it replaces the torso. Please change the category to Top head."). Today the category is a manual dropdown with no guidance, so a wrong pick means a full curation round-trip and re-upload.
This change auto-suggests the most likely wearable category from the model geometry at import time and pre-selects it (still fully overridable), removing a whole class of "wrong category → re-upload" loops.
Changes
suggestWearableCategory(Three, scene)insrc/lib/glbValidation/suggestWearableCategory.ts. It walks the GLTF scene's skinned meshes, tallies skin weight per avatar region using the skeleton bone names (Avatar_Head/Avatar_Neck→ head,Avatar_Spine*/arms → spine,Avatar_*UpLeg/Avatar_*Leg→ legs,Avatar_*Foot/Avatar_*ToeBase→ feet, hands → hands), and returns aWearableCategoryonly when one region owns ≥60% of the classified weight. Returnsnullwhen ambiguous (e.g. full-body skins) so the creator picks manually.hat(default head category, not over-fit to helmet/top_head), spine →upper_body, legs →lower_body, feet →feet, hands →hands_wear.loadAndValidateModel→processModel→AcceptedFileProps→ modal state (mirrors the existingvalidationIssuesflow). No GLTF re-load — reuses the one already loaded for validation.SET_ACCEPTED_PROPSonly when no category was already declared (bare.glbwith nowearable.json), and only when it's part of the body-shape-filtered selectable list. Explicit categories (zipwearable.json, change-item-file, representation flows) are never overridden.CommonFields.create_single_item_modal.suggested_categorytoen/es/zh.null).Submission behavior is unchanged beyond the default selection + hint.
Test plan
suggestWearableCategoryunit tests pass (9/9)lintandbuildpass.glbskinned to the head → category defaults tohat, hint shown.glbskinned to legs → defaults tolower_bodywearable.jsondeclaring a category → declared category wins, no override