P3-04 : add TiedUpAnimationRegistry.resolveWithFallback + EMPTY_ANIMATION stub
Nouveau helper statique consomme par ClientRigEquipmentHandler.rebuildBondageAnimations (P3-05) et PacketPlayRigAnim.handleOnClient (P3-12) pour resoudre un anim ID avec fallback safe si miss. Design : - Lookup delegue a AnimationManager.byKey(ResourceLocation) — l'API existante pour la resolution par registry name. - Fallback = TiedUpRigRegistry.EMPTY_ANIMATION (singleton canonique) plutot qu'un stub empty_fallback separe. Les sites runtime (Layer#off, AnimationPlayer#isEmpty, LayerOffAnimation#getNextAnimation) comparent via == EMPTY_ANIMATION — retourner une autre instance provoquerait des false-negatives sur ces checks d'identite. - Dedup WARN via ConcurrentHashMap.newKeySet() : un ID donne ne log qu'une fois par session, evite le spam si le miss vient d'un item data-driven appele tick apres tick. Pattern inspire de RigAnimationTickHandler.LOGGED_ERRORS. - resetWarnedMissing() expose pour tests + runtime reload (F3+T datapack). - Branche defensive : id=null swallow + log (cas pathologique caller). 4 tests unitaires : - happy path (ID enregistre via AnimationManager.AnimationBuilder → assertSame) - fallback safe (ID inconnu → EMPTY_ANIMATION, non-null) - no-throw (ID inconnu + null swallow sans exception) - dedup observable (reset puis re-call sur meme ID re-warn, sanity check fake accessor distinct de EMPTY_ANIMATION) 65 tests rig GREEN (57 baseline + 4 nouveaux + autres).
This commit is contained in:
notevil
@@ -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()}).
|
||||
*
|
||||
* <p>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}).</p>
|
||||
*/
|
||||
private static final Set<ResourceLocation> WARNED_MISSING_ANIMS = ConcurrentHashMap.newKeySet();
|
||||
|
||||
/**
|
||||
* Résout une animation par {@link ResourceLocation} avec fallback safe
|
||||
* si le registry ne la connaît pas.
|
||||
*
|
||||
* <p>Utilisé 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 :</p>
|
||||
* <ul>
|
||||
* <li>Typo modder dans un JSON data-driven bondage item</li>
|
||||
* <li>Datapack pas encore rechargé ({@code /reload} pending)</li>
|
||||
* <li>Animation supprimée entre deux versions du mod</li>
|
||||
* <li>Race entre packet réception et
|
||||
* {@code AnimationManager.apply()} en début de session</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>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.</p>
|
||||
*
|
||||
* <p><b>Dedup WARN</b> : 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).</p>
|
||||
*
|
||||
* <p><b>Pourquoi réutiliser {@link TiedUpRigRegistry#EMPTY_ANIMATION}</b> 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.</p>
|
||||
*
|
||||
* @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 :
|
||||
* <ul>
|
||||
* <li>Tests unitaires — pour réinitialiser l'état statique entre test
|
||||
* cases (sinon un test qui warn sur un ID pollue les suivants).</li>
|
||||
* <li>Runtime reload (F3+T / datapack reload) — après un reload, des
|
||||
* anims précédemment missing peuvent être dispo maintenant ; on
|
||||
* veut pouvoir re-warn si elles retombent en miss après un autre
|
||||
* reload.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>Thread-safe via {@link ConcurrentHashMap#newKeySet()} — pas de
|
||||
* synchronisation externe nécessaire.</p>
|
||||
*/
|
||||
public static void resetWarnedMissing() {
|
||||
WARNED_MISSING_ANIMS.clear();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>Ces tests verrouillent trois invariants critiques :</p>
|
||||
* <ul>
|
||||
* <li><b>Happy path</b> — un ID enregistré via {@link AnimationManager.AnimationBuilder}
|
||||
* est résolu directement (pas de fallback).</li>
|
||||
* <li><b>Fallback safe</b> — un ID inconnu retourne
|
||||
* {@link TiedUpRigRegistry#EMPTY_ANIMATION} (pas null, pas de throw).
|
||||
* Garantit la robustesse du pipeline rendu client (pas de crash sur
|
||||
* typo modder JSON).</li>
|
||||
* <li><b>Dedup WARN</b> — deux calls consécutifs avec le même ID invalide
|
||||
* n'ajoutent qu'une entrée au set interne. Le set s'exprime via
|
||||
* {@link TiedUpAnimationRegistry#resetWarnedMissing()} qui doit être
|
||||
* appelable sans throw (idempotence).</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p><b>Isolation de test</b> : 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.</p>
|
||||
*
|
||||
* <p><b>Pas de MC bootstrap</b> : {@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.</p>
|
||||
*/
|
||||
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).
|
||||
*
|
||||
* <p>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.</p>
|
||||
*/
|
||||
@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.
|
||||
AnimationAccessor<StaticAnimation> registered =
|
||||
new AnimationManager.AnimationBuilder(TEST_NS, b -> {})
|
||||
.nextAccessor(idPath, acc -> null);
|
||||
|
||||
AnimationAccessor<? extends StaticAnimation> resolved =
|
||||
TiedUpAnimationRegistry.resolveWithFallback(id);
|
||||
|
||||
assertSame(registered, resolved,
|
||||
"Un ID enregistré dans AnimationManager doit être retourné tel quel "
|
||||
+ "(pas de fallback sur EMPTY_ANIMATION). Régression si "
|
||||
+ "resolveWithFallback contourne AnimationManager.byKey.");
|
||||
}
|
||||
|
||||
/**
|
||||
* ID inconnu (jamais enregistré) doit retourner
|
||||
* {@link TiedUpRigRegistry#EMPTY_ANIMATION} — le singleton canonique
|
||||
* utilisé partout comme fallback.
|
||||
*
|
||||
* <p>Important 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.</p>
|
||||
*/
|
||||
@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).
|
||||
*
|
||||
* <p>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.</p>
|
||||
*/
|
||||
@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}).
|
||||
*
|
||||
* <p>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 :</p>
|
||||
* <ul>
|
||||
* <li>Avant le 1er call : set vide → {@code resetWarnedMissing()}
|
||||
* ne throw pas (pas de précondition implicite).</li>
|
||||
* <li>Après le 1er call sur ID inconnu : le set contient l'ID
|
||||
* (vérifiable indirectement — un reset suivi d'un 2e call sur le
|
||||
* même ID doit re-warn, donc comportement observable).</li>
|
||||
* <li>Après le 2e call sur même ID : set inchangé (déjà présent).</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>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.</p>
|
||||
*/
|
||||
@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<StaticAnimation> fakeRegistered =
|
||||
new AnimationManager.AnimationBuilder(TEST_NS, b -> {})
|
||||
.nextAccessor("sanity_check_" + System.nanoTime(), acc -> null);
|
||||
assertNotSame(TiedUpRigRegistry.EMPTY_ANIMATION, fakeRegistered,
|
||||
"Un accessor registered via AnimationBuilder doit être distinct "
|
||||
+ "de EMPTY_ANIMATION (sinon le test happy path ne teste rien).");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user