Phase 2.7 review fixes : resetLoggedErrors wiring + IDLE self-heal + JSON cleanup + tests
This commit is contained in:
notevil
@@ -37,6 +37,7 @@ import com.tiedup.remake.rig.TiedUpRigConstants;
|
||||
import com.tiedup.remake.rig.event.PatchedRenderersEvent;
|
||||
import com.tiedup.remake.rig.patch.LivingEntityPatch;
|
||||
import com.tiedup.remake.rig.patch.TiedUpCapabilities;
|
||||
import com.tiedup.remake.rig.tick.RigAnimationTickHandler;
|
||||
|
||||
/**
|
||||
* Dispatch renderer RIG — Phase 2.6.
|
||||
@@ -192,6 +193,12 @@ public final class TiedUpRenderEngine {
|
||||
// nouveau crash post-reload. Borne aussi la taille en session longue.
|
||||
loggedRenderErrors.clear();
|
||||
|
||||
// Même raisonnement pour l'anti-spam du tick handler anim : un
|
||||
// UUID ayant crashé pré-reload doit pouvoir à nouveau logger
|
||||
// post-reload. F3+T = point naturel pour resetter tout l'état
|
||||
// log accumulé côté RIG client.
|
||||
RigAnimationTickHandler.resetLoggedErrors();
|
||||
|
||||
// Phase 2 : player seul. Le lambda capture la context finale.
|
||||
entityRendererProvider.put(EntityType.PLAYER, entityType -> new TiedUpPlayerRenderer(context, entityType));
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
|
||||
package com.tiedup.remake.rig.tick;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
@@ -22,6 +21,7 @@ import org.slf4j.Logger;
|
||||
|
||||
import com.tiedup.remake.core.TiedUpMod;
|
||||
import com.tiedup.remake.rig.TiedUpAnimationRegistry;
|
||||
import com.tiedup.remake.rig.TiedUpRigRegistry;
|
||||
import com.tiedup.remake.rig.anim.Animator;
|
||||
import com.tiedup.remake.rig.anim.LivingMotions;
|
||||
import com.tiedup.remake.rig.anim.client.ClientAnimator;
|
||||
@@ -119,13 +119,21 @@ public final class RigAnimationTickHandler {
|
||||
LOGGED_ERRORS.clear();
|
||||
}
|
||||
|
||||
// Copie défensive : le set level.players() peut muter pendant notre
|
||||
// itération si un joueur join/leave (MP). Le CME-safe serait itérer
|
||||
// sur une copie. On tolère le CME via try/catch, le coût copy n'en
|
||||
// vaut pas la peine pour une liste typiquement petite (~10 joueurs
|
||||
// visibles max).
|
||||
for (Player player : new HashSet<>(mc.level.players())) {
|
||||
tickPlayer(player);
|
||||
// On itère directement level.players() — pas de copie défensive :
|
||||
// le coût d'allocation par tick client (~60 fois/sec) n'est pas
|
||||
// justifié pour une liste typiquement petite (~10 joueurs visibles
|
||||
// max) alors qu'un CME join/leave pendant une frame est déjà attrapé
|
||||
// par le try/catch per-entity dans tickPlayer(). Tradeoff perf vs
|
||||
// correctness : correctness déjà assurée par le catch, le CME est
|
||||
// une anomalie transitoire qui sera resync à la frame suivante.
|
||||
try {
|
||||
for (Player player : mc.level.players()) {
|
||||
tickPlayer(player);
|
||||
}
|
||||
} catch (java.util.ConcurrentModificationException cme) {
|
||||
// Un join/leave MP ou un unload de dimension pendant l'itération.
|
||||
// On skip le reste du tick — la frame suivante repartira propre.
|
||||
// Pas de log : anomalie transitoire attendue (<1/session typique).
|
||||
}
|
||||
}
|
||||
|
||||
@@ -227,12 +235,45 @@ public final class RigAnimationTickHandler {
|
||||
}
|
||||
}
|
||||
|
||||
// Self-heal : si le bind IDLE d'un patch (PlayerPatch.addAnimations)
|
||||
// a été construit AVANT que TiedUpAnimationRegistry.initStaticAnimations()
|
||||
// ait tourné, le bind pointe encore sur EMPTY_ANIMATION au lieu de
|
||||
// CONTEXT_STAND_IDLE. On rebind ici — idempotent : les addLivingAnimation
|
||||
// suivantes écrasent simplement la précédente entry dans la map.
|
||||
//
|
||||
// Ce cas se produit si :
|
||||
// - Le patch construit la première fois sur le login (pré-setup)
|
||||
// - Un test runClient démarre avant FMLCommonSetupEvent async enqueueWork
|
||||
// - L'asset CONTEXT_STAND_IDLE est en fallback EMPTY après échec JSON
|
||||
// (registry throw swallowed → CONTEXT_STAND_IDLE reste null cf. le
|
||||
// isReady() guard au-dessus → on n'entre même pas dans ce bloc).
|
||||
//
|
||||
// On utilise getLivingAnimation(motion, defaultGetter) avec null comme
|
||||
// default pour distinguer "pas de bind" d'un bind explicite vers
|
||||
// EMPTY_ANIMATION. Dans les deux cas on rebind.
|
||||
AssetAccessor<? extends StaticAnimation> currentIdleBind =
|
||||
clientAnimator.getLivingAnimation(LivingMotions.IDLE, null);
|
||||
if (currentIdleBind == null || currentIdleBind == TiedUpRigRegistry.EMPTY_ANIMATION) {
|
||||
clientAnimator.addLivingAnimation(LivingMotions.IDLE, target.getAccessor());
|
||||
}
|
||||
|
||||
clientAnimator.playAnimation(target.getAccessor(), 0.2F);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reset des erreurs loggées — utile aux tests unitaires et au F3+T
|
||||
* reload (aligné avec {@code TiedUpRenderEngine.onAddLayers}).
|
||||
* Reset des erreurs loggées. Appelé :
|
||||
* <ul>
|
||||
* <li>Par les tests unitaires (isolation d'état entre cas)</li>
|
||||
* <li>Par {@code TiedUpRenderEngine.ModBusEvents#onAddLayers} sur chaque
|
||||
* F3+T reload / resource-pack swap. Un UUID ayant crashé pré-reload
|
||||
* reste sinon marqué et masquerait tout nouveau succès / crash
|
||||
* post-reload. Borne aussi la taille du set en session longue.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>Thread-safety : {@code LOGGED_ERRORS} est un
|
||||
* {@link ConcurrentHashMap#newKeySet()} — aucun besoin de synchronized
|
||||
* même si appelé depuis un thread loader pendant qu'un tick client est
|
||||
* en flight.</p>
|
||||
*/
|
||||
public static void resetLoggedErrors() {
|
||||
LOGGED_ERRORS.clear();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"_comment": "PLACEHOLDER Phase 2.7 — animation idle 2-keyframes identity (joueur statique, pas de balancement). Boucle 2 secondes. DOIT parser avec JsonAssetLoader.loadClipForAnimation(). À remplacer par un export Blender authored (respiration/balancement subtle) quand dispo. Voir docs/plans/rig/ASSETS_NEEDED.md section 2.",
|
||||
"_comment_armature": "Pas de champ 'armature' ici — le loader ne lit pas ce champ au runtime. L'armature est résolue par le call site (voir DirectStaticAnimation ctor dans TiedUpAnimationRegistry.initStaticAnimations, qui passe TiedUpArmatures.BIPED au super).",
|
||||
"format": "ATTRIBUTES",
|
||||
"armature": "tiedup:biped",
|
||||
"animation": [
|
||||
{
|
||||
"name": "Root",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
{
|
||||
"_comment": "PLACEHOLDER Phase 2.7 — identity transforms (joints empilés à l'origine). Doit parser avec JsonAssetLoader.loadArmature() mais n'est PAS encore utilisé au runtime (TiedUpArmatures.BIPED est construit proceduralement en Java). À remplacer par un export Blender addon Antikythera-Studios quand dispo. Voir docs/plans/rig/ASSETS_NEEDED.md section 1.",
|
||||
"_comment": "PLACEHOLDER FILE - NOT LOADED AT RUNTIME Phase 2.7. TiedUpArmatures.BIPED is built procedurally (voir TiedUpArmatures.buildBiped). This JSON is a reference for future Blender-authored biped.json (Phase 3+). Voir docs/plans/rig/ASSETS_NEEDED.md section 1.",
|
||||
"armature": {
|
||||
"armature_format": "ATTRIBUTES",
|
||||
"joints": [
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
/*
|
||||
* © 2026 TiedUp! Remake Contributors, distributed under GPLv3.
|
||||
*/
|
||||
|
||||
package com.tiedup.remake.rig;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
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.types.DirectStaticAnimation;
|
||||
|
||||
/**
|
||||
* Tests du registry {@link TiedUpAnimationRegistry}.
|
||||
*
|
||||
* <p>Ces tests verrouillent :</p>
|
||||
* <ul>
|
||||
* <li>L'idempotence de {@link TiedUpAnimationRegistry#initStaticAnimations()}
|
||||
* — un double-appel ne doit pas créer une nouvelle instance / doubler
|
||||
* l'enregistrement. Pattern important car {@code FMLCommonSetupEvent}
|
||||
* peut firer plusieurs fois sur hot-reload dev ou test runner.</li>
|
||||
* <li>Le {@code registryName()} de {@link TiedUpAnimationRegistry#CONTEXT_STAND_IDLE}
|
||||
* — alignement avec le path du JSON asset
|
||||
* ({@code assets/tiedup/animmodels/animations/context_stand_idle.json}).
|
||||
* Si quelqu'un renomme l'asset sans toucher le code (ou vice-versa), ce
|
||||
* test attrape immédiatement la désynchronisation.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>Les tests n'ont PAS besoin du runtime MC — {@code initStaticAnimations()}
|
||||
* construit juste les instances {@link DirectStaticAnimation} (pas de
|
||||
* {@code loadAnimation()} déclenché tant que {@code getAnimationClip()} n'est
|
||||
* pas lu). La {@code ResourceLocation} créée via le constructor n'a pas besoin
|
||||
* de Minecraft runtime non plus.</p>
|
||||
*/
|
||||
class TiedUpAnimationRegistryTest {
|
||||
|
||||
/**
|
||||
* Appeler {@link TiedUpAnimationRegistry#initStaticAnimations()} deux fois
|
||||
* de suite doit être un no-op sur le second appel — le champ
|
||||
* {@code CONTEXT_STAND_IDLE} garde la même référence.
|
||||
*
|
||||
* <p>Si l'init n'était pas idempotent, un double-call (scénario hot-reload
|
||||
* dev, test runner partageant la JVM, etc.) recréerait l'instance et
|
||||
* invaliderait tous les sites qui l'ont capturée en field final ou local.
|
||||
* Le guard {@code if (CONTEXT_STAND_IDLE != null) return;} protège de ça —
|
||||
* ce test s'assure qu'il n'est pas retiré en régression.</p>
|
||||
*/
|
||||
@Test
|
||||
void initStaticAnimationsIsIdempotent() {
|
||||
TiedUpAnimationRegistry.initStaticAnimations();
|
||||
|
||||
assertNotNull(TiedUpAnimationRegistry.CONTEXT_STAND_IDLE,
|
||||
"Après initStaticAnimations, CONTEXT_STAND_IDLE doit être non-null");
|
||||
|
||||
DirectStaticAnimation firstInstance = TiedUpAnimationRegistry.CONTEXT_STAND_IDLE;
|
||||
|
||||
// Deuxième call — doit être no-op, pas recréer l'instance.
|
||||
TiedUpAnimationRegistry.initStaticAnimations();
|
||||
|
||||
assertSame(firstInstance, TiedUpAnimationRegistry.CONTEXT_STAND_IDLE,
|
||||
"Un second call à initStaticAnimations ne doit pas remplacer l'instance — "
|
||||
+ "le guard `if (CONTEXT_STAND_IDLE != null) return;` serait cassé sinon");
|
||||
}
|
||||
|
||||
/**
|
||||
* Le {@code registryName()} de {@code CONTEXT_STAND_IDLE} doit matcher
|
||||
* {@code tiedup:context_stand_idle} — alignement avec le path du JSON
|
||||
* asset {@code assets/tiedup/animmodels/animations/context_stand_idle.json}.
|
||||
*
|
||||
* <p>Si le registry name dérive du path (via la résolution
|
||||
* {@code resourceLocation = animmodels/animations/${registry.path}.json}),
|
||||
* une désynchronisation code vs asset se traduit par un
|
||||
* {@code AssetLoadingException} runtime. Ce test est un smoke check rapide
|
||||
* au niveau de l'identité du registry name.</p>
|
||||
*/
|
||||
@Test
|
||||
void contextStandIdleHasRegistryName() {
|
||||
TiedUpAnimationRegistry.initStaticAnimations();
|
||||
|
||||
DirectStaticAnimation anim = TiedUpAnimationRegistry.CONTEXT_STAND_IDLE;
|
||||
assertNotNull(anim, "CONTEXT_STAND_IDLE doit être initialisé");
|
||||
|
||||
ResourceLocation expected =
|
||||
ResourceLocation.fromNamespaceAndPath(TiedUpRigConstants.MODID, "context_stand_idle");
|
||||
|
||||
assertEquals(expected, anim.registryName(),
|
||||
"CONTEXT_STAND_IDLE.registryName() doit être tiedup:context_stand_idle — "
|
||||
+ "désynchronisation avec assets/tiedup/animmodels/animations/context_stand_idle.json sinon");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user