Phase 2.6 review fixes : level guard + thread-safety + priority revert + V3-REW-11 track

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.
This commit is contained in:
notevil
2026-04-22 23:41:07 +02:00
parent 987efde86b
commit 73264db3c6

View File

@@ -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.</li>
* <li><b>FORGE bus — {@link RenderLivingEvent.Pre}</b> (priority HIGH) :
* <li><b>FORGE bus — {@link RenderLivingEvent.Pre}</b> (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).</p>
*
* <p><b>Priority HIGH</b> : 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.</p>
* <p><b>Priority NORMAL (default)</b> : 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.</p>
*
* <p><b>Robustesse</b> : 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<EntityType<?>, PatchedEntityRenderer> rendererCache = Maps.newHashMap();
/** UUIDs pour lesquels on a déjà loggé une erreur render (anti-spam). */
private static final Set<UUID> loggedRenderErrors = new HashSet<>();
/**
* UUIDs pour lesquels on a déjà loggé une erreur render (anti-spam).
*
* <p><b>Thread-safety</b> : 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.</p>
*/
private static final Set<UUID> 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.
*
* <p>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}.</p>
*/
@SubscribeEvent(priority = EventPriority.HIGH)
@SubscribeEvent
public static void onRenderLiving(RenderLivingEvent.Pre<? extends LivingEntity, ? extends EntityModel<? extends LivingEntity>> 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