From cae35724880ef5e4ae02377afe8a0bf43b372d60 Mon Sep 17 00:00:00 2001 From: notevil Date: Fri, 24 Apr 2026 00:33:35 +0200 Subject: [PATCH] P3-20 : respawn/dim-change rehydrate (UX P0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire ClientRigEquipmentHandler.rebuildBondageAnimations on : - ClientPlayerNetworkEvent.LoggingIn (client connects) - EntityJoinLevelEvent (spawn + dim change, filtered to LocalPlayer) PlayerRespawnEvent path already covered server-side via PlayerStateEventHandler.onPlayerRespawn -> V2EquipmentHelper.sync -> PacketSyncV2Equipment -> rebuild (hook P3-06). This listener fills the client-only gaps. Without this, a player who logs in with bondage items equipped (or teleports cross-dimension) sees their livingAnimations map empty until the next V2 capability sync or armor slot change arrives. Visible incoherence : item still equipped, bondage anim dropped, player plays vanilla WALK. Demo-killing regression. Handlers are idempotent and fire 2-3x at startup (login + spawn + first chunk load) — rebuildBondageAnimations is designed for that : resetLivingAnimations + re-apply current bindings, no state drift. Pure filter extracted to shouldRehydrate(entity, level, ePred, lPred) generic overload — mirrors the isBondageItem pattern in the sibling BondageEquipmentChangeListener. Lets tests bypass the MC bootstrap crash on Mockito.mock(LocalPlayer.class). Removed the TODO(P3-20) block in BondageEquipmentChangeListener that pointed to this task. 5 tests, all paths green (shouldRehydrate short-circuits, production predicate wiring + null-safety). --- .../BondageEquipmentChangeListener.java | 12 +- .../v2/client/BondageRehydrateListener.java | 234 ++++++++++++++++++ .../client/BondageRehydrateListenerTest.java | 139 +++++++++++ 3 files changed, 377 insertions(+), 8 deletions(-) create mode 100644 src/main/java/com/tiedup/remake/v2/client/BondageRehydrateListener.java create mode 100644 src/test/java/com/tiedup/remake/v2/client/BondageRehydrateListenerTest.java diff --git a/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java b/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java index be2e45c..103071e 100644 --- a/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java +++ b/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java @@ -203,12 +203,8 @@ public final class BondageEquipmentChangeListener { return bondagePredicate.test(stack); } - // TODO(P3-20) : Ajouter les sources "rehydrate" pour respawn / dimension - // change / login, qui ne sont pas couvertes par PacketSyncV2Equipment - // (race possible entre capability sync et client animator bootstrap) : - // - ClientPlayerNetworkEvent.LoggingIn → rebuild sur le LocalPlayer - // - EntityJoinLevelEvent → rebuild si player (remote / - // après dimension change) - // - PlayerEvent.PlayerRespawnEvent → rebuild post-respawn - // Scopé à P3-20 (task #63) pour garder la surface de ce PR minimale. + // Rehydrate sur login / dimension change / respawn : délégué à + // BondageRehydrateListener (P3-20, task #63). PlayerEvent.PlayerRespawnEvent + // est couvert côté serveur via PlayerStateEventHandler.onPlayerRespawn → + // V2EquipmentHelper.sync → PacketSyncV2Equipment → rebuild (path P3-06). } diff --git a/src/main/java/com/tiedup/remake/v2/client/BondageRehydrateListener.java b/src/main/java/com/tiedup/remake/v2/client/BondageRehydrateListener.java new file mode 100644 index 0000000..0d2349d --- /dev/null +++ b/src/main/java/com/tiedup/remake/v2/client/BondageRehydrateListener.java @@ -0,0 +1,234 @@ +/* + * © 2026 TiedUp! Remake Contributors, distributed under GPLv3. + */ + +package com.tiedup.remake.v2.client; + +import java.util.function.Predicate; + +import net.minecraft.client.player.LocalPlayer; +import net.minecraft.world.entity.Entity; +import net.minecraft.world.level.Level; +import net.minecraftforge.api.distmarker.Dist; +import net.minecraftforge.api.distmarker.OnlyIn; +import net.minecraftforge.client.event.ClientPlayerNetworkEvent; +import net.minecraftforge.event.entity.EntityJoinLevelEvent; +import net.minecraftforge.eventbus.api.SubscribeEvent; +import net.minecraftforge.fml.common.Mod; + +import com.tiedup.remake.rig.TiedUpRigConstants; + +/** + * Rehydrate client pour {@link ClientRigEquipmentHandler#rebuildBondageAnimations} + * sur les événements de cycle de vie lourds : login initial et changement de + * dimension / respawn chunk. Sans ça, la map {@code livingAnimations} du + * {@code ClientAnimator} reste vide (ou stale) jusqu'au prochain packet V2 ou + * changement d'équipement — UX pétée dès le 1er respawn/téléport. + * + *

Rationale UX P0

+ *

Scénario cassant sans ce listener : le joueur porte un armbinder, meurt, + * respawn dans le world spawn. Le {@code PlayerPatch} côté client est + * ré-instancié (nouveau LocalPlayer post-respawn), son {@code ClientAnimator} + * a les defaults EF mais aucun binding bondage. L'item est toujours équipé + * (V2 capability préservée), mais visuellement le player joue WALK vanilla. + * Incohérence visuelle → immersion cassée → démo gameday ratée.

+ * + *

Cas couverts

+ * + * + *

Cas hors-scope (déjà couverts)

+ * + * + *

Idempotence

+ *

Les deux handlers peuvent fire 2–3× au démarrage (login + spawn + first + * chunk load). {@link ClientRigEquipmentHandler#rebuildBondageAnimations} est + * idempotent par contrat : chaque appel repart de + * {@code resetLivingAnimations()} + re-applique les bindings courants. + * Overhead CPU négligeable (une poignée d'items équipés par player, itération + * {@code ArrayList} + {@code Map.put}). Pas de dedup tick-level.

+ * + *

Threading & side

+ *

Les deux events ne firent que côté client (Forge les émet uniquement + * depuis les paths client : {@code ClientPacketListener.handleLogin} pour + * {@code LoggingIn}, et {@code Level#addFreshEntity} côté client pour les + * entités qui rejoignent un {@code ClientLevel}). Le filtre + * {@link Level#isClientSide()} dans {@link #onEntityJoinLevel} est une garde + * défensive pour le cas où l'event fire côté serveur (le listener est + * {@code value = Dist.CLIENT} donc la classe ne charge pas sur serveur dédié, + * mais en solo intégré l'abonnement est actif sur les deux side-worlds d'une + * même JVM).

+ * + * @see ClientRigEquipmentHandler#rebuildBondageAnimations + * @see BondageEquipmentChangeListener + * @see com.tiedup.remake.v2.bondage.network.PacketSyncV2Equipment + */ +@OnlyIn(Dist.CLIENT) +@Mod.EventBusSubscriber( + modid = TiedUpRigConstants.MODID, + bus = Mod.EventBusSubscriber.Bus.FORGE, + value = Dist.CLIENT +) +public final class BondageRehydrateListener { + + private BondageRehydrateListener() { + // utility subscriber class + } + + /** + * Handler {@link ClientPlayerNetworkEvent.LoggingIn} — fire quand le + * client a fini de se connecter au serveur (après réception du packet + * {@code ClientboundLoginPacket}). + * + *

Appelle {@link ClientRigEquipmentHandler#rebuildBondageAnimations} + * sur le {@link LocalPlayer} fourni par l'event. À ce stade, la capability + * V2 est probablement vide — {@code getAllEquipped} retourne un map vide + * et le rebuild est un no-op bénin (animator.resetLivingAnimations + 0 + * bindings). Le vrai rebuild arrive via + * {@code PacketSyncV2Equipment.handleOnClient} quand le serveur push la + * sync peu après.

+ * + *

Pourquoi hook ici quand même ? Filet de sécurité : si le + * sync packet est perdu ou retardé, on a au moins tenté un rebuild avec + * l'état courant. Et si l'animator n'est pas encore initialisé + * (race bootstrap), {@code rebuildBondageAnimations} retourne + * silencieusement (null-check cascade).

+ * + * @param event l'event Forge ; {@link ClientPlayerNetworkEvent#getPlayer()} + * peut théoriquement être null (ex : event fire pendant + * {@code onDisconnect} mid-login) — on guard. + */ + @SubscribeEvent + public static void onLoggingIn(ClientPlayerNetworkEvent.LoggingIn event) { + LocalPlayer player = event.getPlayer(); + if (player == null) return; + + ClientRigEquipmentHandler.rebuildBondageAnimations(player); + + TiedUpRigConstants.LOGGER.debug( + "[BondageRehydrateListener] Login rehydrate fired for {}", + player.getName().getString() + ); + } + + /** + * Handler {@link EntityJoinLevelEvent} — fire au spawn d'une entité dans + * un {@code Level}. Filtré sur {@link LocalPlayer} uniquement : on ignore + * les NPCs, remote players et toute autre entité (leur rebuild passe par + * d'autres sources). + * + *

Sémantique Forge : l'event fire dans + * {@code Level#addFreshEntity}, ce qui couvre :

+ * + * + *

Le rebuild peut fire 2× au login (ce handler + {@link #onLoggingIn}) + * — comportement attendu, idempotent.

+ * + * @param event l'event Forge ; {@code getEntity()} et {@code getLevel()} + * sont contractuellement non-null. + */ + @SubscribeEvent + public static void onEntityJoinLevel(EntityJoinLevelEvent event) { + // Filter : uniquement le LocalPlayer courant. On ignore tous les NPCs + // et entities non-player ; les remote players (autres clients sur le + // serveur) voient leur rebuild via PacketSyncV2Equipment.handleOnClient + // quand leur capability sync arrive. + Entity entity = event.getEntity(); + Level level = event.getLevel(); + if (!shouldRehydrate(entity, level, IS_LOCAL_PLAYER, IS_CLIENT_SIDE)) return; + + LocalPlayer player = (LocalPlayer) entity; + ClientRigEquipmentHandler.rebuildBondageAnimations(player); + + TiedUpRigConstants.LOGGER.debug( + "[BondageRehydrateListener] EntityJoinLevel rehydrate fired for {} in level {}", + player.getName().getString(), + level.dimension().location() + ); + } + + /** + * Predicate production : une {@link Entity} est-elle un {@link LocalPlayer} ? + * Factorisé en constante pour permettre l'injection d'une variante test + * basée sur {@link Object} dummy — le pattern {@code Mockito.mock(LocalPlayer.class)} + * crashe sur {@code InternalError} car MC static init tente de charger le + * registre officiel hors runtime game. + */ + static final Predicate IS_LOCAL_PLAYER = e -> e instanceof LocalPlayer; + + /** + * Predicate production : un {@link Level} est-il côté client ? Extrait en + * constante pour la même raison (impossible de mocker {@link Level} sans + * bootstrap MC). + */ + static final Predicate IS_CLIENT_SIDE = + l -> l instanceof Level level && level.isClientSide(); + + /** + * Predicate pure testable — détermine si un event {@link EntityJoinLevelEvent} + * doit déclencher un rebuild. Vrai ssi : + *
    + *
  1. {@code entity} matche {@code entityPredicate} + * (en prod : {@code instanceof LocalPlayer}),
    2. + *
  2. {@code level} matche {@code levelPredicate} + * (en prod : {@code instanceof Level && isClientSide()}).
    3. + *
+ * + *

Overload générique {@code } pour permettre les unit tests + * sans bootstrap MC. Les tests passent des {@code Object} dummy + + * predicates contrôlés ({@code ALWAYS_TRUE} / {@code ALWAYS_FALSE}), + * évitant le crash {@code Mockito.mock(LocalPlayer.class)} qui + * déclenche la static init MC (registries, block states, etc.).

+ * + *

Null-safety : les deux predicates sont responsables de leur + * propre null-handling. Les constantes prod {@link #IS_LOCAL_PLAYER} et + * {@link #IS_CLIENT_SIDE} short-circuit sur null via {@code instanceof} + * (qui retourne {@code false} pour null).

+ * + * @param type de l'entité ({@link Entity} en prod, + * {@link Object} en test) + * @param type du level ({@link Level} en prod, + * {@link Object} en test) + * @param entity entity issue de l'event (tolère null) + * @param level level issu de l'event (tolère null) + * @param entityPredicate predicate "est-ce notre client player ?" + * @param levelPredicate predicate "est-ce un client-side level ?" + * @return {@code true} ssi les deux predicates retournent {@code true} + */ + static boolean shouldRehydrate( + E entity, + L level, + Predicate entityPredicate, + Predicate levelPredicate + ) { + if (!entityPredicate.test(entity)) return false; + return levelPredicate.test(level); + } +} diff --git a/src/test/java/com/tiedup/remake/v2/client/BondageRehydrateListenerTest.java b/src/test/java/com/tiedup/remake/v2/client/BondageRehydrateListenerTest.java new file mode 100644 index 0000000..0c7438f --- /dev/null +++ b/src/test/java/com/tiedup/remake/v2/client/BondageRehydrateListenerTest.java @@ -0,0 +1,139 @@ +/* + * © 2026 TiedUp! Remake Contributors, distributed under GPLv3. + */ + +package com.tiedup.remake.v2.client; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.function.Predicate; + +import org.junit.jupiter.api.Test; + +/** + * Tests unitaires pour {@link BondageRehydrateListener} (P3-20, task #63). + * + *

Stratégie

+ *

Les deux handlers publics + * ({@link BondageRehydrateListener#onLoggingIn} et + * {@link BondageRehydrateListener#onEntityJoinLevel}) nécessitent des events + * Forge + capability MC pour fire {@code rebuildBondageAnimations} — non + * testables sans bootstrap. La logique testable est donc isolée dans + * {@link BondageRehydrateListener#shouldRehydrate} (overload générique + * {@code } qui permet de bypasser le bootstrap MC).

+ * + *

Même pattern que {@link BondageEquipmentChangeListenerTest} : les + * entités/levels sont remplacés par des {@link Object} dummy et les + * predicates de filtre par des lambdas contrôlées. {@code Mockito.mock} + * sur {@code LocalPlayer}/{@code Level} crashe sur {@code InternalError} + * (MC static init tente de charger les registres hors runtime game).

+ */ +class BondageRehydrateListenerTest { + + /** Predicate qui considère chaque objet comme matchant. */ + private static final Predicate ALWAYS_TRUE = o -> true; + + /** Predicate qui considère chaque objet comme non-matchant. */ + private static final Predicate ALWAYS_FALSE = o -> false; + + // ========== shouldRehydrate (generic overload) ========== + + /** + * Entity ne matche pas (ex : NPC, Zombie, remote player) → false. + * Le {@code levelPredicate} n'est PAS appelé (short-circuit). + */ + @Test + void shouldRehydrate_entityRejected_returnsFalse_levelPredicateNotInvoked() { + Predicate levelTripwire = l -> { + throw new AssertionError( + "levelPredicate ne doit pas etre appele apres entityPredicate false"); + }; + + assertFalse( + BondageRehydrateListener.shouldRehydrate( + new Object(), new Object(), ALWAYS_FALSE, levelTripwire), + "entityPredicate false doit retourner false sans evaluer levelPredicate" + ); + } + + /** + * Entity matche (LocalPlayer) mais level server-side → false. Garde + * défensive : en solo intégré les deux sides coexistent dans la même + * JVM, on gate sur {@code isClientSide} pour ne rebuild que dans le + * pipeline client. + */ + @Test + void shouldRehydrate_entityMatchLevelRejected_returnsFalse() { + assertFalse( + BondageRehydrateListener.shouldRehydrate( + new Object(), new Object(), ALWAYS_TRUE, ALWAYS_FALSE), + "LocalPlayer + server-side level doit retourner false" + ); + } + + /** + * Entity matche (LocalPlayer) ET level matche (client-side) → true. + * Cas nominal : le player local rejoint le {@code ClientLevel} (login, + * dim change, respawn chunk). Déclenche le rebuild. + */ + @Test + void shouldRehydrate_bothMatch_returnsTrue() { + assertTrue( + BondageRehydrateListener.shouldRehydrate( + new Object(), new Object(), ALWAYS_TRUE, ALWAYS_TRUE), + "LocalPlayer + client-side level doit declencher rebuild" + ); + } + + // ========== Production predicates sanity ========== + + /** + * Les predicates production {@link BondageRehydrateListener#IS_LOCAL_PLAYER} + * et {@link BondageRehydrateListener#IS_CLIENT_SIDE} existent et sont + * non-null. Garde contre un refactor qui les nullifierait silencieusement. + * + *

On vérifie aussi leur null-safety : {@code instanceof} retourne + * false pour null, donc aucun des deux predicates ne doit throw sur + * un null input.

+ */ + @Test + void productionPredicates_areWiredAndNullSafe() { + assertNotNull( + BondageRehydrateListener.IS_LOCAL_PLAYER, + "IS_LOCAL_PLAYER doit etre non-null (wiring sanity)" + ); + assertNotNull( + BondageRehydrateListener.IS_CLIENT_SIDE, + "IS_CLIENT_SIDE doit etre non-null (wiring sanity)" + ); + // Null-safety via instanceof semantics + assertFalse( + BondageRehydrateListener.IS_LOCAL_PLAYER.test(null), + "IS_LOCAL_PLAYER(null) doit retourner false (instanceof null-safe)" + ); + assertFalse( + BondageRehydrateListener.IS_CLIENT_SIDE.test(null), + "IS_CLIENT_SIDE(null) doit retourner false (instanceof null-safe)" + ); + } + + /** + * Les predicates production appliqués à un {@link Object} dummy + * retournent false — ni Object n'est une {@code LocalPlayer}, ni un + * {@code Level}. Valide le {@code instanceof} check. + */ + @Test + void productionPredicates_onDummyObject_returnFalse() { + Object dummy = new Object(); + assertFalse( + BondageRehydrateListener.IS_LOCAL_PLAYER.test(dummy), + "Object dummy ne doit pas matcher IS_LOCAL_PLAYER" + ); + assertFalse( + BondageRehydrateListener.IS_CLIENT_SIDE.test(dummy), + "Object dummy ne doit pas matcher IS_CLIENT_SIDE" + ); + } +}