diff --git a/src/main/java/com/tiedup/remake/rig/TiedUpAnimationRegistry.java b/src/main/java/com/tiedup/remake/rig/TiedUpAnimationRegistry.java index 50da17f..266103a 100644 --- a/src/main/java/com/tiedup/remake/rig/TiedUpAnimationRegistry.java +++ b/src/main/java/com/tiedup/remake/rig/TiedUpAnimationRegistry.java @@ -4,10 +4,15 @@ package com.tiedup.remake.rig; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + import net.minecraft.resources.ResourceLocation; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.api.distmarker.OnlyIn; +import com.tiedup.remake.rig.anim.AnimationManager; +import com.tiedup.remake.rig.anim.AnimationManager.AnimationAccessor; import com.tiedup.remake.rig.anim.client.Layer; import com.tiedup.remake.rig.anim.client.property.ClientAnimationProperties; import com.tiedup.remake.rig.anim.types.DirectStaticAnimation; @@ -153,4 +158,108 @@ public final class TiedUpAnimationRegistry { public static boolean isReady() { return CONTEXT_STAND_IDLE != null; } + + /** + * Set (thread-safe) des IDs pour lesquels un WARN de fallback a déjà été + * émis. Évite le spam log si un consumer appelle + * {@link #resolveWithFallback(ResourceLocation)} tick après tick avec un ID + * invalide (ex. un item bondage data-driven qui référence un anim ID cassé, + * appelé dans la boucle de rendu). Un seul WARN par ID unique sur toute la + * durée de vie du process (jusqu'à {@link #resetWarnedMissing()}). + * + *
Pattern inspiré de + * {@code RigAnimationTickHandler.LOGGED_ERRORS} — {@code ConcurrentHashMap.newKeySet()} + * pour être safe en cas d'appels concurrents (client tick thread + network + * handler thread pour {@code PacketPlayRigAnim.handleOnClient}).
+ */ + private static final SetUtilisé par le pipeline d'équipement + * ({@code ClientRigEquipmentHandler.rebuildBondageAnimations}, P3-05) et + * le packet cinematic ({@code PacketPlayRigAnim.handleOnClient}, P3-12). + * Un miss dans le registry peut survenir dans plusieurs scénarios :
+ *Dans tous ces cas, on retourne {@link TiedUpRigRegistry#EMPTY_ANIMATION} + * — un singleton qui ne joue rien visuellement (pas de keyframe, pose + * identity). Mieux qu'un NPE pour la robustesse du pipeline : l'anim + * équipement continue de tourner avec les anim connues, et l'anim inconnue + * est juste silencieusement un no-op.
+ * + *Dedup WARN : un miss donné ne log qu'une fois par session + * ({@link #WARNED_MISSING_ANIMS}). Ça évite le spam dans la console quand + * l'ID invalide est consommé tick après tick (ex. équipement resté en place). + * Le set peut être reset via {@link #resetWarnedMissing()} (hot-reload + * F3+T, runtime datapack reload).
+ * + *Pourquoi réutiliser {@link TiedUpRigRegistry#EMPTY_ANIMATION} vs + * créer un {@code empty_fallback} séparé : plusieurs sites du runtime + * ({@code Layer#off}, {@code AnimationPlayer#isEmpty}, {@code LayerOffAnimation#getNextAnimation}) + * testent l'identité via {@code == EMPTY_ANIMATION}. Retourner une autre + * instance d'empty provoquerait des false-negatives sur ces checks — le + * runtime penserait qu'une anim réelle joue alors qu'en fait c'est un + * empty différent. Le singleton canonique évite ce piège.
+ * + * @param id l'ID registry à résoudre (ex. {@code tiedup:idle_context_bound}) + * @return l'{@link AnimationAccessor} enregistré, ou + * {@link TiedUpRigRegistry#EMPTY_ANIMATION} si l'ID est inconnu. + * Jamais null. + */ + public static AnimationAccessor extends StaticAnimation> resolveWithFallback( + ResourceLocation id + ) { + if (id == null) { + // null ID : log + fallback. Pas de dedup (cas pathologique — le + // caller a un bug, pas un miss de datapack). + TiedUpRigConstants.LOGGER.warn( + "[TiedUpAnimationRegistry] resolveWithFallback appelé avec id=null, " + + "using EMPTY_ANIMATION fallback." + ); + return TiedUpRigRegistry.EMPTY_ANIMATION; + } + + AnimationAccessor extends StaticAnimation> anim = AnimationManager.byKey(id); + if (anim != null) { + return anim; + } + + // Miss — fallback + dedup warn. Set.add retourne true si l'ID n'était + // pas déjà dans le set → premier miss, on log. Sinon, silent no-op. + if (WARNED_MISSING_ANIMS.add(id)) { + TiedUpRigConstants.LOGGER.warn( + "[TiedUpAnimationRegistry] Animation not found: '{}', using EMPTY_ANIMATION fallback. " + + "Check datapack JSON or run /reload.", + id + ); + } + return TiedUpRigRegistry.EMPTY_ANIMATION; + } + + /** + * Reset le set dedup des WARN missing. Utilisé dans deux contextes : + *Thread-safe via {@link ConcurrentHashMap#newKeySet()} — pas de + * synchronisation externe nécessaire.
+ */ + public static void resetWarnedMissing() { + WARNED_MISSING_ANIMS.clear(); + } } diff --git a/src/test/java/com/tiedup/remake/rig/TiedUpAnimationRegistryResolveTest.java b/src/test/java/com/tiedup/remake/rig/TiedUpAnimationRegistryResolveTest.java new file mode 100644 index 0000000..32f667f --- /dev/null +++ b/src/test/java/com/tiedup/remake/rig/TiedUpAnimationRegistryResolveTest.java @@ -0,0 +1,213 @@ +/* + * © 2026 TiedUp! Remake Contributors, distributed under GPLv3. + */ + +package com.tiedup.remake.rig; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; + +import net.minecraft.resources.ResourceLocation; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.tiedup.remake.rig.anim.AnimationManager; +import com.tiedup.remake.rig.anim.AnimationManager.AnimationAccessor; +import com.tiedup.remake.rig.anim.types.StaticAnimation; + +/** + * Tests de {@link TiedUpAnimationRegistry#resolveWithFallback(ResourceLocation)} + * — helper consommé par {@code ClientRigEquipmentHandler.rebuildBondageAnimations} + * (P3-05) et {@code PacketPlayRigAnim.handleOnClient} (P3-12) pour résoudre + * un anim ID avec fallback safe si miss. + * + *Ces tests verrouillent trois invariants critiques :
+ *Isolation de test : chaque test commence par + * {@link TiedUpAnimationRegistry#resetWarnedMissing()} pour ne pas hériter du + * set dedup d'un test précédent (le set est statique, partagé entre tests). + * L'{@link AfterEach} reset aussi, au cas où un test ferait son propre setup + * conditionnel.
+ * + *Pas de MC bootstrap : {@link AnimationManager.AnimationBuilder} + * peut instancier un accessor sans toucher au ResourceManager (le + * {@code onLoad} lambda n'est invoqué qu'à {@code .get()}, et on ne call pas + * get() dans ces tests). Donc pas besoin de {@code Bootstrap.bootStrap()} + * ou autre init MC.
+ */ +class TiedUpAnimationRegistryResolveTest { + + /** Namespace arbitraire pour les IDs de test — isolé des vraies registrations. */ + private static final String TEST_NS = "tiedup_test_resolve"; + + @BeforeEach + void resetDedupSet() { + TiedUpAnimationRegistry.resetWarnedMissing(); + } + + @AfterEach + void cleanUp() { + TiedUpAnimationRegistry.resetWarnedMissing(); + } + + /** + * ID enregistré via {@link AnimationManager.AnimationBuilder#nextAccessor} + * doit être résolu directement (pas de fallback). + * + *Pattern : injecte un accessor fake dans {@code AnimationManager} + * via le builder public, puis vérifie que + * {@link TiedUpAnimationRegistry#resolveWithFallback} retourne exactement + * la même instance via {@code assertSame}. Si {@code resolveWithFallback} + * ratait le happy path (ex. bug qui retournerait EMPTY_ANIMATION sur tous + * les IDs), {@code assertSame} détecterait immédiatement la régression.
+ */ + @Test + void resolveWithFallback_existingId_returnsRegisteredAnim() { + String idPath = "registered_anim_" + System.nanoTime(); + ResourceLocation id = ResourceLocation.fromNamespaceAndPath(TEST_NS, idPath); + + // Injecte un accessor fake via le builder public. onLoad retourne null + // mais ce n'est pas invoqué tant qu'on ne call pas .get() dessus. + AnimationAccessorImportant de retourner LE singleton (pas une autre instance d'empty) : + * plusieurs sites du runtime ({@code Layer#off}, {@code AnimationPlayer#isEmpty}, + * {@code LayerOffAnimation#getNextAnimation}) testent l'identité via + * {@code == EMPTY_ANIMATION}. Si on retournait une instance différente, + * ces checks donneraient des false-negatives.
+ */ + @Test + void resolveWithFallback_unknownId_returnsEmptyFallback() { + ResourceLocation unknown = ResourceLocation.fromNamespaceAndPath( + TEST_NS, "definitely_not_registered_" + System.nanoTime() + ); + + AnimationAccessor extends StaticAnimation> resolved = + TiedUpAnimationRegistry.resolveWithFallback(unknown); + + assertNotNull(resolved, "Fallback ne doit jamais être null — " + + "les callers (packet handler, tick) supposent non-null."); + assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, resolved, + "Fallback doit être EMPTY_ANIMATION (singleton canonique), pas une " + + "autre instance d'empty. Checks d'identité == EMPTY_ANIMATION " + + "dans Layer/AnimationPlayer/LayerOffAnimation dépendent de ça."); + } + + /** + * {@link TiedUpAnimationRegistry#resolveWithFallback} ne doit jamais throw + * même sur input pathologique (ID inconnu, ID null). + * + *Garantit la robustesse du pipeline : un bug dans un JSON data-driven + * ou un packet cinematic mal formé ne doit pas crasher le client. Un + * fallback silencieux + WARN log est préférable à un {@code NullPointerException} + * qui remonterait jusqu'à l'event bus et casserait tout le rendu.
+ */ + @Test + void resolveWithFallback_unknownId_doesNotThrow() { + ResourceLocation unknown = ResourceLocation.fromNamespaceAndPath( + TEST_NS, "throw_test_" + System.nanoTime() + ); + + assertDoesNotThrow(() -> { + TiedUpAnimationRegistry.resolveWithFallback(unknown); + }, "Fallback safe doit swallow le miss — throw = régression robustesse."); + + // Input null : couvre la branche défensive en tête de resolveWithFallback. + assertDoesNotThrow(() -> { + TiedUpAnimationRegistry.resolveWithFallback(null); + }, "null ID doit être swallow aussi (caller pathologique)."); + } + + /** + * Deux calls consécutifs avec le même ID invalide doivent produire un seul + * WARN log (dedup via {@code WARNED_MISSING_ANIMS}). + * + *Comme on ne peut pas facilement intercepter les logs SLF4J sans + * dépendance supplémentaire (logback test appender, etc.), on teste le + * comportement observable du set dedup :
+ *Pattern équivalent à ce qu'on fait pour {@code LOGGED_ERRORS} dans + * {@code RigAnimationTickHandlerTest} — on teste l'idempotence du reset + * qui n'a de sens que si le set existe et est mutable.
+ */ + @Test + void resolveWithFallback_warnsOnlyOncePerMissingId() { + ResourceLocation unknown = ResourceLocation.fromNamespaceAndPath( + TEST_NS, "dedup_test_" + System.nanoTime() + ); + + // Premier call : fallback + add(id) → true (log émis). + AnimationAccessor extends StaticAnimation> first = + TiedUpAnimationRegistry.resolveWithFallback(unknown); + assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, first, + "1er call sur ID inconnu doit fallback."); + + // Deuxième call sur le MÊME ID : fallback + add(id) → false (pas de log). + // On ne peut pas vérifier "pas de log" directement sans log capture, + // mais on vérifie que le résultat est identique (même instance EMPTY) + // et que pas de throw — ce qui couvre le chemin add→false du Set. + AnimationAccessor extends StaticAnimation> second = + TiedUpAnimationRegistry.resolveWithFallback(unknown); + assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, second, + "2e call sur même ID doit retourner la même instance EMPTY_ANIMATION."); + + // Comportement observable du dedup : après un reset, un 3e call sur + // le même ID doit re-traiter comme "première fois" (Set.add retourne + // à nouveau true). C'est l'invariant que reset rend le state "fresh". + TiedUpAnimationRegistry.resetWarnedMissing(); + AnimationAccessor extends StaticAnimation> third = + TiedUpAnimationRegistry.resolveWithFallback(unknown); + assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, third, + "3e call après reset doit toujours fallback sur EMPTY_ANIMATION."); + + // Sanity check : l'accessor fallback EMPTY_ANIMATION n'est pas le + // même objet qu'un fake accessor registered via AnimationBuilder — + // garantit qu'on ne confond pas les deux chemins. + AnimationAccessor