P3 Wave α hardening : snapshot consistency + parse warn + reload hook

HIGH RISK-001 : snapshot update moved between reset+applyDefinitions
and equip oneshots. If an equip oneshot throws, animator state matches
snapshot (currentKeys already applied). Prevents next-tick desync.

MEDIUM Option B warn : parser now logs WARN if modder binds IDLE.
Points to BONDAGE_ANIMATION_DESIGN §5.1 convention. Binding still
accepted (functional), just non-idiomatic.

MEDIUM WARNED_MISSING_ANIMS reload reset : new
TiedUpRigRegistryReloadListener wires resetWarnedMissing() on
AddReloadListenerEvent + RegisterClientReloadListenersEvent. Prevents
unbounded set growth in long-running sessions with datapack swaps.

LOW naming : unified MOD_ID in Wave α files (BondageRehydrateListener
switched from TiedUpRigConstants.MODID to TiedUpMod.MOD_ID for
consistency with sibling BondageEquipmentChangeListener ; both
constants hold the literal "tiedup" so no behavioral change).
This commit is contained in:
notevil
2026-04-24 02:16:28 +02:00
parent 5428f13f98
commit 7281548a6a
6 changed files with 357 additions and 8 deletions

View File

@@ -0,0 +1,152 @@
/*
* © 2026 TiedUp! Remake Contributors, distributed under GPLv3.
*/
package com.tiedup.remake.rig;
import net.minecraft.server.packs.resources.ResourceManager;
import net.minecraft.server.packs.resources.SimplePreparableReloadListener;
import net.minecraft.util.profiling.ProfilerFiller;
import net.minecraftforge.api.distmarker.Dist;
import net.minecraftforge.client.event.RegisterClientReloadListenersEvent;
import net.minecraftforge.event.AddReloadListenerEvent;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.Mod;
import com.tiedup.remake.core.TiedUpMod;
/**
* Hook {@code /reload} (serveur) + {@code F3+T} (client) pour reset le set dedup
* {@link TiedUpAnimationRegistry#resetWarnedMissing()}.
*
* <h2>Pourquoi ?</h2>
* <p>Le set statique {@code WARNED_MISSING_ANIMS} de
* {@link TiedUpAnimationRegistry} accumule les IDs d'animations introuvables
* pour éviter le spam log (un miss reloggué tick après tick). Sans reset, le
* set grandit sans borne en prod (N items cassés × K sessions × long uptime)
* et empêche de re-signaler un miss qui redevient cassé après un reload.</p>
*
* <h2>Scénario typique</h2>
* <ol>
* <li>Session N : un item data-driven référence {@code tiedup:broken_anim} →
* WARN log, ID ajouté au set.</li>
* <li>Modder corrige l'asset, {@code /reload}.</li>
* <li>Session N+1 : resolveWithFallback dispose de l'anim → pas de fallback,
* pas de WARN. Le set contient toujours l'ID de la session N (résiduel).</li>
* <li>Modder casse à nouveau l'anim, {@code /reload}.</li>
* <li>Session N+2 : miss de nouveau, <b>mais l'ID est déjà dans le set</b> →
* silencieux, aucun feedback modder.</li>
* </ol>
*
* <p>Avec ce listener, chaque reload clear le set — un miss sur une anim
* précédemment warnée redéclenche le WARN (comportement attendu : le modder
* doit voir l'erreur s'il re-casse l'asset).</p>
*
* <h2>Les deux sides</h2>
* <ul>
* <li>{@link AddReloadListenerEvent} fire côté serveur à chaque
* {@code /reload} datapack.
* {@code resolveWithFallback} peut être appelé côté serveur (validation
* parse-time des items, checks administratifs). Les entrées set accumulées
* côté serveur sont cleared là.</li>
* <li>{@link RegisterClientReloadListenersEvent} fire une fois au setup client
* pour enregistrer un listener qui tourne à chaque resource reload client
* (F3+T ou resource pack swap).
* {@code resolveWithFallback} est majoritairement appelé côté client
* (pipeline rebuild animations, packet handlers). Les entrées set accumulées
* côté client sont cleared là.</li>
* </ul>
*
* <p>Le set statique dans {@link TiedUpAnimationRegistry} est le MÊME sur les
* deux sides (même JVM pour un serveur intégré / solo) — les deux hooks
* clearent le même set, pas de double comptabilité. Sur serveur dédié
* seul le hook server fire, côté client pur (rare — lobby / fast reconnect)
* seul le hook client.</p>
*
* <h2>Threading</h2>
* <p>{@link TiedUpAnimationRegistry#resetWarnedMissing()} utilise
* {@link java.util.concurrent.ConcurrentHashMap#newKeySet()} donc safe à
* l'appel concurrent. Les reload events peuvent fire depuis le main thread
* (server tick) ou le render thread (client reload) — aucune contrainte
* supplémentaire.</p>
*/
@Mod.EventBusSubscriber(modid = TiedUpMod.MOD_ID)
public final class TiedUpRigRegistryReloadListener
extends SimplePreparableReloadListener<Void> {
private TiedUpRigRegistryReloadListener() {
// Instancié par les event handlers ci-dessous — pas d'API publique
// à part comme reload listener.
}
@Override
protected Void prepare(ResourceManager mgr, ProfilerFiller profiler) {
// No-op — tout se fait dans apply(). Pas de I/O à faire off-thread,
// l'opération est juste un Set.clear() en O(1) amortisé.
return null;
}
@Override
protected void apply(Void nothing, ResourceManager mgr, ProfilerFiller profiler) {
TiedUpAnimationRegistry.resetWarnedMissing();
TiedUpRigConstants.LOGGER.debug(
"[TiedUpRigRegistryReloadListener] WARNED_MISSING_ANIMS reset on reload"
);
}
/**
* Hook {@link AddReloadListenerEvent} — fire à chaque {@code /reload}
* datapack côté serveur (et aussi au server start). Enregistre un
* listener qui clear le set dedup WARN.
*
* <p>Note : {@code AddReloadListenerEvent} est sur le FORGE bus (pas MOD bus),
* mais le {@code @Mod.EventBusSubscriber} par défaut cible le MOD bus. On
* spécifie {@link Mod.EventBusSubscriber.Bus#FORGE} sur le subscriber ci-dessus ?
* Non : en 1.20.1, {@code AddReloadListenerEvent extends Event} bus-inferred
* automatiquement — le subscriber static-registered via annotation
* fonctionne sur le FORGE bus par défaut tant qu'on n'indique pas
* {@code Bus.MOD}. Par souci de clarté, on s'aligne sur le pattern du mod
* ({@code TiedUpMod.ForgeEvents} utilise le FORGE bus).</p>
*/
@SubscribeEvent
public static void onAddReloadListeners(AddReloadListenerEvent event) {
event.addListener(new TiedUpRigRegistryReloadListener());
TiedUpRigConstants.LOGGER.debug(
"[TiedUpRigRegistryReloadListener] Registered server-side reload listener for WARN dedup reset"
);
}
/**
* Hook {@link RegisterClientReloadListenersEvent} — fire une fois côté client
* pendant le setup pour enregistrer des listeners qui tourneront à chaque
* resource reload client (F3+T, resource pack swap).
*
* <p>Gardé {@link Dist#CLIENT} : sur serveur dédié, l'event n'existe pas
* (event client), le subscriber static est skip sans class-load.</p>
*
* <p>Ce subscriber est un inner static class pour isoler la gate de Dist —
* {@code @Mod.EventBusSubscriber(value = Dist.CLIENT)} sur la classe entière
* exclurait aussi le hook {@link #onAddReloadListeners} server-side.</p>
*/
@Mod.EventBusSubscriber(
modid = TiedUpMod.MOD_ID,
bus = Mod.EventBusSubscriber.Bus.MOD,
value = Dist.CLIENT
)
public static final class ClientReloadHook {
private ClientReloadHook() {
// holder class for @SubscribeEvent static
}
@SubscribeEvent
public static void onRegisterClientReloadListeners(
RegisterClientReloadListenersEvent event
) {
event.registerReloadListener(new TiedUpRigRegistryReloadListener());
TiedUpRigConstants.LOGGER.debug(
"[TiedUpRigRegistryReloadListener] Registered client-side reload listener for WARN dedup reset"
);
}
}
}

View File

@@ -717,6 +717,24 @@ public final class DataDrivenItemParser {
);
continue;
}
// P3 Wave α — Option B convention warn : binding IDLE fonctionne
// techniquement mais écrase silencieusement le default EF re-injecté
// par ClientAnimator.resetLivingAnimations() post-rebuild. Design
// BONDAGE_ANIMATION_DESIGN.md §5.1 recommande d'utiliser des motions
// custom (POSE_KNEEL_BOUND / STRUGGLE_BOUND / WALK_BOUND) plutôt que
// d'écraser IDLE. On accepte le binding quand même (rétrocompat +
// cas légitimes rares), juste on pointe le modder vers la convention.
if (motion == LivingMotions.IDLE) {
LOGGER.warn(
"[DataDrivenItemParser] Item {} binds IDLE motion. Convention design Option B "
+ "(BONDAGE_ANIMATION_DESIGN.md §5.1) recommends not binding IDLE — "
+ "ClientAnimator.resetLivingAnimations() restores default EF IDLE after "
+ "rebuild, so binding IDLE overwrites the default silently. Prefer custom "
+ "motions like POSE_KNEEL_BOUND / STRUGGLE_BOUND / WALK_BOUND. The binding "
+ "will still work but may cause surprises.",
itemId
);
}
ResourceLocation animId = tryParseAnimationRL(
entry.getValue(),
"living_motions['" + motionName + "']",

View File

@@ -16,6 +16,7 @@ import net.minecraftforge.event.entity.EntityJoinLevelEvent;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.Mod;
import com.tiedup.remake.core.TiedUpMod;
import com.tiedup.remake.rig.TiedUpRigConstants;
/**
@@ -82,7 +83,13 @@ import com.tiedup.remake.rig.TiedUpRigConstants;
*/
@OnlyIn(Dist.CLIENT)
@Mod.EventBusSubscriber(
modid = TiedUpRigConstants.MODID,
// P3 Wave α naming : use TiedUpMod.MOD_ID (main mod constant) in v2.client
// files rather than TiedUpRigConstants.MODID (rig sub-module alias).
// Both hold the literal "tiedup" (TiedUpRigConstants.MODID = TiedUpMod.MOD_ID),
// so no behavioral change — just source consistency with sibling
// BondageEquipmentChangeListener. TiedUpRigConstants.LOGGER is kept below
// because the logger identity is rig-scoped for grep-friendly log filtering.
modid = TiedUpMod.MOD_ID,
bus = Mod.EventBusSubscriber.Bus.FORGE,
value = Dist.CLIENT
)

View File

@@ -335,6 +335,13 @@ public final class ClientRigEquipmentHandler {
// rig pendant que la map livingAnimations est encore celle de l'ancien
// équipement. Pragmatique : à ce stade le rebuild n'a pas encore tourné,
// donc les defaults EF sont toujours en place de la passe précédente.
//
// Snapshot RISK-001 (P3 Wave α review) : si un unequip oneshot throw
// ici, le rebuild (reset + applyDefinitions) n'a pas encore tourné,
// donc l'animator est toujours dans l'état "previousKeys" — on ne
// touche PAS à LAST_EQUIPPED_KEYS. Au prochain tick, le diff
// retentera avec previousKeys inchangé → même unequip redéclenché
// (au pire double son, mais état cohérent garanti).
for (Map.Entry<BodyRegionV2, ResourceLocation> entry : newlyUnequipped.entrySet()) {
ResourceLocation itemId = entry.getValue();
triggerOneshotById(
@@ -348,11 +355,6 @@ public final class ClientRigEquipmentHandler {
}
}
// Update snapshot AVANT le rebuild — si rebuild throw, on a quand même
// consigné l'état courant pour le tick suivant (évite un double-fire
// au prochain rebuild après exception).
LAST_EQUIPPED_KEYS.put(player, Map.copyOf(currentKeys));
// Reset + restore des defaults EF (IDLE, WALK, RUN, etc.). Les bindings
// custom qu'on ajoute après prennent le pas sur ces defaults via la
// map livingAnimations (dernière put gagne pour une même LivingMotion).
@@ -371,6 +373,26 @@ public final class ClientRigEquipmentHandler {
TiedUpAnimationRegistry::resolveWithFallback
);
// Update snapshot APRÈS reset+applyDefinitions, AVANT les equip oneshots
// (RISK-001 fix, P3 Wave α) : à ce stade l'animator est exactement dans
// l'état décrit par currentKeys (reset + bindings des items courants
// appliqués). Snapshot et animator state sont cohérents.
//
// Pourquoi PAS avant applyDefinitions : si applyDefinitions throw, on
// aurait un snapshot qui prétend que l'état est currentKeys alors que
// l'animator est à moitié construit (reset done, bindings partiels).
// Au tick suivant, diff(currentKeys, currentKeys) == vide → aucune
// tentative de réparer → animator reste en état cassé jusqu'à un vrai
// equipment change.
//
// Pourquoi PAS après les equip oneshots : le snapshot représente l'état
// _équipement_ (ce qui EST dans les slots), pas les oneshots joués. Si
// un equip oneshot throw, l'équipement est déjà correct côté animator —
// on garde le snapshot pour ne pas re-fire les equip oneshots au tick
// suivant (ce serait un double son + double animation-push).
LAST_EQUIPPED_KEYS.put(player, Map.copyOf(currentKeys));
FIRST_REBUILD_DONE.put(player, Boolean.TRUE);
if (!isFirstRebuild) {
// Equip APRÈS rebuild — l'anim one-shot joue sur "le nouvel état" du
// rig, après que les bindings de l'item nouvellement équipé ont été
@@ -389,8 +411,6 @@ public final class ClientRigEquipmentHandler {
}
}
FIRST_REBUILD_DONE.put(player, Boolean.TRUE);
TiedUpRigConstants.LOGGER.debug(
"[ClientRigEquipmentHandler] Rebuilt livingAnimations for player {} "
+ "({} data-driven items processed, {} equip / {} unequip one-shots,"

View File

@@ -0,0 +1,112 @@
/*
* © 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.assertSame;
import net.minecraft.resources.ResourceLocation;
import org.junit.jupiter.api.Test;
import com.tiedup.remake.rig.anim.AnimationManager.AnimationAccessor;
import com.tiedup.remake.rig.anim.types.StaticAnimation;
/**
* Smoke tests pour {@link TiedUpRigRegistryReloadListener} (P3 Wave α hardening).
*
* <p>Le listener lui-même ne peut être testé bout-en-bout sans runtime Forge
* (registration sur event bus + fire de l'event). On se limite à deux garanties
* observables :</p>
* <ul>
* <li>La classe se class-load sans throw (pas de static init piège — la
* signature {@code @Mod.EventBusSubscriber} + les events référencés
* doivent être résolvables dans le classpath test).</li>
* <li>L'API cible {@link TiedUpAnimationRegistry#resetWarnedMissing()} peut
* être appelée répétitivement sans throw et re-arme le state dedup
* (déjà couvert dans {@link TiedUpAnimationRegistryResolveTest}, on
* valide ici l'invariant "reset + new miss → WARN redéclenché
* observable via set.add=true").</li>
* </ul>
*
* <p>Le fire de l'event réel ({@code AddReloadListenerEvent},
* {@code RegisterClientReloadListenersEvent}) doit être validé en game-runtime
* (QA manual : run client, /reload, vérifier debug log émis). Impossible à
* mocker sans bootstrap Forge complet.</p>
*/
class TiedUpRigRegistryReloadListenerTest {
/** Simple classLoad-ability sanity — si la classe contient un static init
* qui throw (ex. reference à une API manquante du classpath test), le
* test-runner échoue immédiatement avec ExceptionInInitializerError. */
@Test
void classLoads_noStaticInitError() {
assertDoesNotThrow(() -> {
Class<?> cls = Class.forName(
"com.tiedup.remake.rig.TiedUpRigRegistryReloadListener"
);
assertNotNull(cls);
}, "TiedUpRigRegistryReloadListener doit pouvoir class-loader sans throw "
+ "(Forge annotations + event classes resolvables en test classpath)");
}
/** L'inner holder class {@code ClientReloadHook} class-load aussi (gated
* {@code Dist.CLIENT} mais la classe Java existe tout le temps — seul le
* subscriber event se no-op côté serveur). */
@Test
void clientReloadHook_classLoads() {
assertDoesNotThrow(() -> {
Class<?> cls = Class.forName(
"com.tiedup.remake.rig.TiedUpRigRegistryReloadListener$ClientReloadHook"
);
assertNotNull(cls);
}, "TiedUpRigRegistryReloadListener.ClientReloadHook doit class-loader "
+ "meme sans runtime Forge — juste une classe Java standard");
}
/**
* Sanity : le listener est censé appeler
* {@link TiedUpAnimationRegistry#resetWarnedMissing()} dans son apply().
* On ne peut pas mocker {@code ResourceManager} sans bootstrap MC, donc on
* valide juste le contrat : resetWarnedMissing est callable + repeatable +
* remet effectivement le set en état "fresh".
*
* <p>Pattern : warn sur un ID unknown → reset → warn rewarned (vérification
* indirecte du reset via le comportement observable du {@code Set.add}).</p>
*/
@Test
void resetWarnedMissing_idempotentAndReenablesWarn() {
ResourceLocation unknown = ResourceLocation.fromNamespaceAndPath(
"tiedup_test_reload", "never_registered_" + System.nanoTime()
);
// Clear initial — pas d'état hérité d'autres tests.
assertDoesNotThrow(TiedUpAnimationRegistry::resetWarnedMissing);
// Premier call : ID manquant → fallback + (premier) WARN silent-add.
AnimationAccessor<? extends StaticAnimation> first =
TiedUpAnimationRegistry.resolveWithFallback(unknown);
assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, first,
"Unknown ID → fallback EMPTY_ANIMATION");
// Reset simule un /reload — le set interne est cleared.
assertDoesNotThrow(TiedUpAnimationRegistry::resetWarnedMissing,
"resetWarnedMissing doit etre appelable sans throw "
+ "(contrat cible du TiedUpRigRegistryReloadListener.apply())");
// Deuxième call post-reset : fallback à nouveau, et le set repart de
// zero — le comportement de WARN est re-enabled. On ne peut pas
// observer le log directement mais le contrat set.add(id) == true
// est rétabli (sinon le reset serait une no-op dangereuse).
AnimationAccessor<? extends StaticAnimation> second =
TiedUpAnimationRegistry.resolveWithFallback(unknown);
assertSame(TiedUpRigRegistry.EMPTY_ANIMATION, second,
"Unknown ID apres reset → toujours fallback EMPTY_ANIMATION");
// Cleanup : ne pas polluer les suivants
TiedUpAnimationRegistry.resetWarnedMissing();
}
}

View File

@@ -169,6 +169,46 @@ class DataDrivenItemParserAnimationsTest {
assertNotNull(result.livingMotions().get(TiedUpLivingMotions.STRUGGLE_BOUND));
}
// ========== P3 Wave α : Option B convention warn on IDLE ==========
/**
* Option B convention (design BONDAGE_ANIMATION_DESIGN.md §5.1) : les modders
* ne devraient PAS binder {@link LivingMotions#IDLE} car
* {@code ClientAnimator.resetLivingAnimations()} re-injecte le default EF
* IDLE post-rebuild, et le binding custom l'écrase silencieusement — comportement
* fonctionnel mais non-idiomatique. Le parser log un WARN explicite pour pointer
* le modder vers la convention, mais accepte le binding quand même (rétrocompat).
*
* <p>Ce test verrouille le comportement "accepte + ne throw pas" — la présence
* du WARN log n'est pas directement vérifiable sans log capture, mais le
* binding doit être bel et bien présent dans le résultat.</p>
*/
@Test
void parseAnimations_IDLEBinding_logsWarnButAccepts() {
String jsonStr = """
{
"animations": {
"living_motions": {
"IDLE": "tiedup:arms_cuffed_idle"
}
}
}
""";
AnimationBindings result =
DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID);
assertNotNull(result,
"IDLE binding est tolere (non-idiomatique mais fonctionnel) => result non-null");
assertEquals(1, result.livingMotions().size(),
"Le binding IDLE est accepte apres le WARN de convention");
assertEquals(
ResourceLocation.fromNamespaceAndPath("tiedup", "arms_cuffed_idle"),
result.livingMotions().get(LivingMotions.IDLE),
"Le binding IDLE est bien present dans le resultat malgre le WARN"
);
}
// ========== living_motions : tolerance erreurs ==========
@Test