diff --git a/src/main/java/com/tiedup/remake/rig/TiedUpArmatures.java b/src/main/java/com/tiedup/remake/rig/TiedUpArmatures.java index c0f7d34..e004a66 100644 --- a/src/main/java/com/tiedup/remake/rig/TiedUpArmatures.java +++ b/src/main/java/com/tiedup/remake/rig/TiedUpArmatures.java @@ -41,16 +41,20 @@ import com.tiedup.remake.rig.math.OpenMatrix4f; *
- * Root - * ├─ Thigh_R ── Leg_R ── Knee_R - * ├─ Thigh_L ── Leg_L ── Knee_L - * └─ Torso - * └─ Chest - * ├─ Head - * ├─ Shoulder_R ── Arm_R ── Elbow_R ── Hand_R ── Tool_R - * └─ Shoulder_L ── Arm_L ── Elbow_L ── Hand_L ── Tool_L + * Root id=0 + * ├─ Thigh_R ── Leg_R ── Knee_R id=1,2,3 + * ├─ Thigh_L ── Leg_L ── Knee_L id=4,5,6 + * └─ Torso id=7 + * └─ Chest id=8 + * ├─ Head id=9 + * ├─ Shoulder_R ── Arm_R ── Elbow_R ── Hand_R ── Tool_R ids=10,11,14,12,13 + * └─ Shoulder_L ── Arm_L ── Elbow_L ── Hand_L ── Tool_L ids=15,16,19,17,18 ** + *
Les IDs des bras ne suivent pas l'ordre hiérarchique parent→enfant : c'est + * voulu pour rester aligné avec le layout attendu par {@code VanillaModelTransformer} + * (upperJoint=Arm, lowerJoint=Hand, middleJoint=Elbow). Voir {@link #buildBiped()}.
+ * *Noms conservés verbatim EF (pas renommés en TiedUp style) car :
*Si construit plus tôt (ex. static init combat-lourd avant FMLCommonSetup) - * risquerait de toucher des classes client — garder cet accès lazy si les - * call sites évoluent.
+ *Le class loader JVM garantit qu'une classe est initialisée au plus une + * fois, et que l'init est visible à tous les threads (JLS §12.4.1 — the class + * initialization lock est acquis automatiquement). Deux threads qui touchent + * {@code Holder.INSTANCE} simultanément ne peuvent pas observer l'instance + * non-initialisée ni en créer deux exemplaires. Intégré SP (client + server + * threads concurrents sur la même JVM) safe.
+ * + *Raison du fix (review Phase 2.4, P0-BUG-002) : le pattern précédent + * {@code if (BIPED_INSTANCE == null) BIPED_INSTANCE = buildBiped();} est + * un double-init race — deux threads entrent tous les deux dans le if, + * les deux créent un HumanoidArmature distinct, dernier gagne et pollue + * le cache.
*/ - private static HumanoidArmature BIPED_INSTANCE; + private static final class Holder { + static final HumanoidArmature INSTANCE = buildBiped(); + private Holder() {} + } /** - * AssetAccessor biped biped TiedUp. Construit à la première lecture de - * {@link AssetAccessor#get()}. + * AssetAccessor biped TiedUp. L'instance est construite lazy à la première + * référence {@code Holder.INSTANCE} (thread-safe via class-init lock JVM). * *Utilisé par {@link com.tiedup.remake.rig.patch.PlayerPatch#getArmature()} * et par les futurs {@code StaticAnimation(… , BIPED)} Phase 2.7+.
@@ -89,10 +103,7 @@ public final class TiedUpArmatures { public static final AssetAccessorOrdre d'insertion LinkedHashMap ≠ ordre des IDs pour les bras (hand + * vient AVANT elbow dans les IDs, pour aligner sur la sémantique EF où + * {@code middleJoint = elbow}). Voir commentaires inline.
* *Toutes les transforms sont identity — Phase 2.7 remplacera par les * offsets Blender mesurés (cf. doc header).
*/ private static HumanoidArmature buildBiped() { - // Tous les joints (ID == ordre d'insertion dans la map). - // On utilise LinkedHashMap pour garantir l'ordre d'itération stable - // (OpenMatrix4f.allocateMatrixArray dimensionne sur jointCount). + // ID Joint assigné explicitement au constructeur (pas par position dans + // la map) — Armature.jointById est construit depuis joint.getId(). + // On utilise LinkedHashMap malgré tout pour garantir un ordre d'itération + // stable (utile pour le debug et pour OpenMatrix4f.allocateMatrixArray + // qui dimensionne sur jointCount). MapCes tests verrouillent en particulier le mapping {@code id → joint name} + * qui doit rester aligné sur les constantes {@code RIGHT_ARM} / {@code LEFT_ARM} + * de {@code VanillaModelTransformer} — sinon les bras rendent tordus (ex : + * {@code lowerJoint=17} pointerait vers Elbow_L au lieu de Hand_L).
+ * + *La review Phase 2.4 (P0-BUG-001) a montré que l'ordre précédent + * Arm_R=11, Elbow_R=12, Hand_R=13, Tool_R=14 cassait silencieusement le + * transformer. {@link #jointIdsMatchEfLayout()} aurait attrapé ce bug.
+ */ +class TiedUpArmaturesTest { + + @Test + void bipedHas20Joints() { + assertEquals(20, TiedUpArmatures.BIPED.get().getJointNumber(), + "Le biped EF standard doit avoir exactement 20 joints"); + } + + @Test + void searchBipedJointByNameReturnsNonNull() { + HumanoidArmature biped = TiedUpArmatures.BIPED.get(); + + String[] expectedNames = { + "Root", "Head", + "Torso", "Chest", + "Shoulder_L", "Shoulder_R", + "Arm_L", "Arm_R", + "Hand_L", "Hand_R", + "Elbow_L", "Elbow_R", + "Tool_L", "Tool_R", + "Thigh_L", "Thigh_R", + "Leg_L", "Leg_R", + "Knee_L", "Knee_R" + }; + + for (String name : expectedNames) { + assertNotNull(biped.searchJointByName(name), + "Joint '" + name + "' introuvable dans le biped — alignement EF cassé"); + } + } + + /** + * Verrou explicite contre P0-BUG-001. Les constantes EF + * {@code VanillaModelTransformer.RIGHT_ARM} encodent + * {@code upperJoint=11, lowerJoint=12, middleJoint=14}, i.e. + * Arm_R=11, Hand_R=12, Elbow_R=14 (symétrique gauche : 16/17/19). + * + *Ce test DOIT échouer si quelqu'un réordonne les IDs dans + * {@link TiedUpArmatures#buildBiped()} sans mettre à jour + * {@code VanillaModelTransformer} en miroir.
+ */ + @Test + void jointIdsMatchEfLayout() { + HumanoidArmature biped = TiedUpArmatures.BIPED.get(); + + // Right arm layout (VanillaModelTransformer.RIGHT_ARM : 11, 12, _, 14) + assertEquals("Arm_R", biped.searchJointById(11).getName(), + "id=11 doit être Arm_R (VanillaModelTransformer.RIGHT_ARM.upperJoint)"); + assertEquals("Hand_R", biped.searchJointById(12).getName(), + "id=12 doit être Hand_R (VanillaModelTransformer.RIGHT_ARM.lowerJoint)"); + assertEquals("Elbow_R", biped.searchJointById(14).getName(), + "id=14 doit être Elbow_R (VanillaModelTransformer.RIGHT_ARM.middleJoint)"); + + // Left arm layout (VanillaModelTransformer.LEFT_ARM : 16, 17, _, 19) + assertEquals("Arm_L", biped.searchJointById(16).getName(), + "id=16 doit être Arm_L (VanillaModelTransformer.LEFT_ARM.upperJoint)"); + assertEquals("Hand_L", biped.searchJointById(17).getName(), + "id=17 doit être Hand_L (VanillaModelTransformer.LEFT_ARM.lowerJoint)"); + assertEquals("Elbow_L", biped.searchJointById(19).getName(), + "id=19 doit être Elbow_L (VanillaModelTransformer.LEFT_ARM.middleJoint)"); + + // Tool_R=13, Tool_L=18 (pas dans les LimbPartTransformer mais utilisé + // par StaticAnimation itemInHand — on verrouille l'emplacement aussi). + assertEquals("Tool_R", biped.searchJointById(13).getName(), + "id=13 doit être Tool_R (attaché après Hand_R dans la chaîne)"); + assertEquals("Tool_L", biped.searchJointById(18).getName(), + "id=18 doit être Tool_L (attaché après Hand_L dans la chaîne)"); + } + + /** + * Verrou contre P0-BUG-002. La première implémentation utilisait un + * double-null-check non thread-safe — SP intégré (client + server threads + * concurrent) pouvait créer deux instances distinctes. + * + *Le Holder idiome (static inner class) garantit via le class-init lock + * JVM qu'il n'y a qu'une seule init. Ce test vérifie que deux appels + * successifs de {@code BIPED.get()} renvoient bien la même référence — + * ne prouve pas l'atomicité mais couvre le cas régression "retour à un + * nouveau build à chaque get".
+ */ + @Test + void bipedSingleton() { + HumanoidArmature first = TiedUpArmatures.BIPED.get(); + HumanoidArmature second = TiedUpArmatures.BIPED.get(); + assertSame(first, second, + "BIPED.get() doit retourner la même instance (singleton Holder)"); + } +}