From 73264db3c69be24aa4770a8c22795cc50fbd3279 Mon Sep 17 00:00:00 2001 From: notevil Date: Wed, 22 Apr 2026 23:41:07 +0200 Subject: [PATCH] Phase 2.6 review fixes : level guard + thread-safety + priority revert + V3-REW-11 track MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RISK-001 : early-return si entity.level() == null dans onRenderLiving (NPE guard sur detach race despawn / dim teleport). Aligne sur EF. RISK-004 : clear loggedRenderErrors en tête de onAddLayers() — reset du set anti-spam sur reload resources (F3+T), borne aussi la taille en session longue. RISK-005 : remplace HashSet par ConcurrentHashMap.newKeySet() pour loggedRenderErrors — render thread + loader thread + test runner muteront le set. RISK-002 : retire priority = EventPriority.HIGH sur onRenderLiving, revert au défaut NORMAL aligné EF upstream. Aucun cas concret ne justifie HIGH aujourd'hui ; on réévaluera si conflit d'interception apparaît Phase 3+. Import EventPriority supprimé. RISK-003 : tracké V3-REW-11 dans V3_REWORK_BACKLOG.md (inventory GUI special-case EF pas porté — rotation/scale/pose preview inventaire). Docs locaux (gitignored) mis à jour : - PHASE0_DEGRADATIONS.md : nouvelle section Phase 2.6 findings - V3_REWORK_BACKLOG.md : V3-REW-11 ajouté section MAJEURE Tests : compileJava GREEN, 18/18 rig tests GREEN. --- .../remake/rig/render/TiedUpRenderEngine.java | 45 ++++++++++++++----- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/tiedup/remake/rig/render/TiedUpRenderEngine.java b/src/main/java/com/tiedup/remake/rig/render/TiedUpRenderEngine.java index 46d59d7..2904968 100644 --- a/src/main/java/com/tiedup/remake/rig/render/TiedUpRenderEngine.java +++ b/src/main/java/com/tiedup/remake/rig/render/TiedUpRenderEngine.java @@ -6,10 +6,10 @@ package com.tiedup.remake.rig.render; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import com.google.common.collect.Maps; @@ -27,7 +27,6 @@ import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.api.distmarker.OnlyIn; import net.minecraftforge.client.event.EntityRenderersEvent; import net.minecraftforge.client.event.RenderLivingEvent; -import net.minecraftforge.eventbus.api.EventPriority; import net.minecraftforge.eventbus.api.SubscribeEvent; import net.minecraftforge.fml.ModLoader; import net.minecraftforge.fml.common.Mod; @@ -51,7 +50,7 @@ import com.tiedup.remake.rig.patch.TiedUpCapabilities; * et poste {@link PatchedRenderersEvent.Add} sur le mod event bus pour * que des tiers (datapacks, modules TiedUp futurs) puissent enregistrer * leurs propres patched renderers. - *
  • FORGE bus — {@link RenderLivingEvent.Pre} (priority HIGH) : + *
  • FORGE bus — {@link RenderLivingEvent.Pre} (priority NORMAL) : * intercepte le render vanilla des entités whitelist, vérifie la * présence d'un patch avec {@code overrideRender() == true}, dispatche * vers le {@link PatchedEntityRenderer} approprié, puis annule le render @@ -70,9 +69,11 @@ import com.tiedup.remake.rig.patch.TiedUpCapabilities; * registre une clé PLAYER pour un VillagerEntity custom (très improbable mais * défensif).

    * - *

    Priority HIGH : firer avant les subscribers à priorité NORMAL (ex : - * mods cosmetics qui veulent canceler notre render). Si un subscriber HIGH+ - * nous a déjà canceled, on respecte et on skip.

    + *

    Priority NORMAL (default) : aligné sur Epic Fight upstream. Aucun cas + * concret actuel ne justifie de firer avant d'autres mods ; si on constate un + * conflit d'interception en Phase 3+ (ex : mod cosmetic qui cancel notre + * render), on réévaluera la priorité à ce moment-là. Si un autre subscriber + * (NORMAL ou higher) a déjà canceled l'event, on respecte et on skip.

    * *

    Robustesse : toute exception pendant le dispatch est logguée une * seule fois par UUID d'entity (évite le spam log si le rendu crash pour une @@ -107,8 +108,16 @@ public final class TiedUpRenderEngine { */ private static final Map, PatchedEntityRenderer> rendererCache = Maps.newHashMap(); - /** UUIDs pour lesquels on a déjà loggé une erreur render (anti-spam). */ - private static final Set loggedRenderErrors = new HashSet<>(); + /** + * UUIDs pour lesquels on a déjà loggé une erreur render (anti-spam). + * + *

    Thread-safety : le dispatch render se fait côté client render thread, + * mais le reset via {@link #resetForTest()} peut provenir d'un test runner thread + * et {@link ModBusEvents#onAddLayers} d'un loader thread lors du reload des + * resources. On passe par un set concurrent pour éviter toute lost-add / + * ConcurrentModificationException pendant un reload en plein milieu d'un tick.

    + */ + private static final Set loggedRenderErrors = ConcurrentHashMap.newKeySet(); private TiedUpRenderEngine() {} @@ -178,6 +187,10 @@ public final class TiedUpRenderEngine { entityRendererProvider.clear(); rendererCache.clear(); + // Reset anti-spam log set sur reload resources (F3+T) — sinon un + // UUID ayant crashé pré-reload resterait marqué et masquerait un + // nouveau crash post-reload. Borne aussi la taille en session longue. + loggedRenderErrors.clear(); // Phase 2 : player seul. Le lambda capture la context finale. entityRendererProvider.put(EntityType.PLAYER, entityType -> new TiedUpPlayerRenderer(context, entityType)); @@ -203,15 +216,15 @@ public final class TiedUpRenderEngine { private ForgeBusEvents() {} /** - * Dispatch principal — priority HIGH pour firer avant les mods cosmetics - * (NORMAL). Si un subscriber HIGH+ a déjà canceled, on respecte. + * Dispatch principal — priority NORMAL (défaut, aligné sur Epic Fight). + * Si un subscriber plus prioritaire a déjà canceled l'event, on respecte. * *

    Generic wildcard avec {@code extends LivingEntity} — Forge fire un * seul event instance générique, on ne peut pas filtrer par type param. * Le filtre se fait par {@code instanceof} sur l'entity, et par * {@link #getRendererFor}.

    */ - @SubscribeEvent(priority = EventPriority.HIGH) + @SubscribeEvent public static void onRenderLiving(RenderLivingEvent.Pre> event) { if (event.isCanceled()) { return; @@ -219,6 +232,16 @@ public final class TiedUpRenderEngine { LivingEntity entity = event.getEntity(); + // NPE guard — une entity peut se détacher de son Level (despawn race, + // dimension teleport) pendant qu'un render frame est encore en flight. + // Plusieurs call sites downstream (patch.getAnimator().tick(level), etc.) + // supposent un level non-null. Pas de log : fail silencieusement, MC + // re-render à la frame suivante. Aligne sur EF qui évite aussi de toucher + // aux entities sans level via RenderEngine guard. + if (entity.level() == null) { + return; + } + // STRICT FILTER — Player (Phase 2) + NPCs TiedUp (Phase 5). Protège // notamment contre une interception accidentelle des MCA villagers // (cf. V3-REW-10 dans V3_REWORK_BACKLOG.md). Bonus : si un tiers mod