From 13b0f8f5906ad105ae89d2e58dbc6ca2d98b2faa Mon Sep 17 00:00:00 2001 From: notevil Date: Thu, 23 Apr 2026 22:41:44 +0200 Subject: [PATCH] P3-06 : wire ClientRigEquipmentHandler from 2 event sources Trigger rebuildBondageAnimations from : 1. PacketSyncV2Equipment.handleOnClient -- after deserializeNBT capability 2. LivingEquipmentChangeEvent -- filtered to bondage items, client only Third source (LoggingIn / dimension change) scoped to P3-20 with TODO. body : idempotent si double-fire, dedup via isBondageRelevant filter. Caveat Forge doc LivingEquipmentChangeEvent = server-side only : le handler est neanmoins enregistre @Dist.CLIENT comme filet defensif pour les edge cases creative/admin. Le path canonique reste PacketSyncV2Equipment qui couvre 100%% des changements V2 via la capability dediee (V2 n'utilise pas les slots armor vanilla). isBondageItem pattern : DataDrivenItemRegistry.get(stack) != null (coherent avec ClientRigEquipmentHandler.extractSortedDefinitions). Refactor generic pour testabilite sans MC bootstrap (Mockito ItemStack crash la static init registries). --- .../network/PacketSyncV2Equipment.java | 16 ++ .../BondageEquipmentChangeListener.java | 214 ++++++++++++++++++ .../BondageEquipmentChangeListenerTest.java | 212 +++++++++++++++++ 3 files changed, 442 insertions(+) create mode 100644 src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java create mode 100644 src/test/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListenerTest.java diff --git a/src/main/java/com/tiedup/remake/v2/bondage/network/PacketSyncV2Equipment.java b/src/main/java/com/tiedup/remake/v2/bondage/network/PacketSyncV2Equipment.java index 116ff06..35e96f1 100644 --- a/src/main/java/com/tiedup/remake/v2/bondage/network/PacketSyncV2Equipment.java +++ b/src/main/java/com/tiedup/remake/v2/bondage/network/PacketSyncV2Equipment.java @@ -2,12 +2,14 @@ package com.tiedup.remake.v2.bondage.network; import com.tiedup.remake.v2.bondage.IV2EquipmentHolder; import com.tiedup.remake.v2.bondage.capability.V2BondageEquipmentProvider; +import com.tiedup.remake.v2.client.ClientRigEquipmentHandler; import java.util.function.Supplier; import net.minecraft.client.Minecraft; import net.minecraft.nbt.CompoundTag; import net.minecraft.network.FriendlyByteBuf; import net.minecraft.world.entity.Entity; import net.minecraft.world.entity.LivingEntity; +import net.minecraft.world.entity.player.Player; import net.minecraft.world.level.Level; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.api.distmarker.OnlyIn; @@ -86,6 +88,20 @@ public class PacketSyncV2Equipment { living .getCapability(V2BondageEquipmentProvider.V2_BONDAGE_EQUIPMENT) .ifPresent(equip -> equip.deserializeNBT(msg.data)); + + // P3-06 : l'equipment a changé côté client, on rebuild la map + // livingAnimations du ClientAnimator. Limité aux Player car + // ClientRigEquipmentHandler ne connaît que le PlayerPatch pour + // l'instant (les NPCs bondage sont traités via IV2EquipmentHolder + // branch ci-dessus + leur propre sync path — rebuild NPC est + // scopé à P3-07/P3-08). + // + // Double-fire safe : si LivingEquipmentChangeEvent fire en même + // temps pour le même tick, on rebuild 2× (idempotent, seul le + // filtre isBondageRelevant gaspille un appel — pas critique). + if (living instanceof Player player) { + ClientRigEquipmentHandler.rebuildBondageAnimations(player); + } } } } diff --git a/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java b/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java new file mode 100644 index 0000000..be2e45c --- /dev/null +++ b/src/main/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListener.java @@ -0,0 +1,214 @@ +/* + * © 2026 TiedUp! Remake Contributors, distributed under GPLv3. + */ + +package com.tiedup.remake.v2.client; + +import java.util.function.Predicate; + +import net.minecraft.world.entity.player.Player; +import net.minecraft.world.item.ItemStack; +import net.minecraftforge.api.distmarker.Dist; +import net.minecraftforge.api.distmarker.OnlyIn; +import net.minecraftforge.event.entity.living.LivingEquipmentChangeEvent; +import net.minecraftforge.eventbus.api.SubscribeEvent; +import net.minecraftforge.fml.common.Mod; + +import com.tiedup.remake.core.TiedUpMod; +import com.tiedup.remake.v2.bondage.datadriven.DataDrivenItemRegistry; + +/** + * Deuxième event source pour {@link ClientRigEquipmentHandler#rebuildBondageAnimations} + * (P3-06). Miroir de {@link com.tiedup.remake.v2.bondage.network.PacketSyncV2Equipment} + * pour couvrir les changements d'équipement qui passeraient par les slots + * armor vanilla (creative, commandes admin, mods tiers qui inject des items + * bondage via {@code setItemSlot}). + * + *

Caveat important — Forge sémantique "server-side only"

+ *

La javadoc Forge de {@link LivingEquipmentChangeEvent} indique explicitement + * "This event is fired on server-side only". L'appel + * {@code ForgeEventFactory.onLivingEquipmentChange} dans {@code LivingEntity#tick()} + * n'a pas de garde {@code level.isClientSide} explicite, mais en pratique + * Forge le documente comme authoritative server. On enregistre néanmoins ce + * handler client-only comme filet de sécurité :

+ * + * + *

Décision design : V2 bondage n'utilise PAS les slots armor vanilla + * — toute l'equipment passe par la capability {@code V2_BONDAGE_EQUIPMENT} et + * son packet {@code PacketSyncV2Equipment}. Ce listener est donc défensif : + * il attrape les cas edge où un item bondage atterrirait dans un slot armor + * (ex : creative mode {@code /item replace}, mods tiers qui manipulent + * directement {@code setItemSlot}, future migration partielle).

+ * + *

Workflow

+ *
    + *
  1. Filtre par {@link #isBondageRelevant} : au moins un des deux stacks + * (from/to) doit être un item bondage data-driven, sinon ignore.
    2. + *
  2. Filtre par {@code instanceof Player} — le handler ne gère que les + * {@link Player} pour le moment. NPCs bondage (Damsels, etc.) seront + * câblés dans P3-07/P3-08 séparément via leur propre pipeline.
    3. + *
  3. Guard {@code level().isClientSide()} pour éviter le rebuild + * côté serveur au cas où le tick fire sur les deux côtés.
    4. + *
  4. Appelle {@link ClientRigEquipmentHandler#rebuildBondageAnimations}.
    5. + *
+ * + *

Idempotence + double-fire

+ *

Si {@code PacketSyncV2Equipment} ET ce listener fire pour le même + * changement (cas hypothétique d'un item bondage V2 déplacé vers un slot + * armor et syncé via la capability en parallèle), on rebuild 2×. Le rebuild + * est idempotent côté {@link ClientRigEquipmentHandler} — aucune corruption + * d'état, juste du gaspillage CPU. Pas de dedup tick-level pour l'instant + * (MVP), à mesurer si l'overhead devient visible.

+ * + * @see ClientRigEquipmentHandler + * @see com.tiedup.remake.v2.bondage.network.PacketSyncV2Equipment + */ +@OnlyIn(Dist.CLIENT) +@Mod.EventBusSubscriber( + modid = TiedUpMod.MOD_ID, + bus = Mod.EventBusSubscriber.Bus.FORGE, + value = Dist.CLIENT +) +public final class BondageEquipmentChangeListener { + + private BondageEquipmentChangeListener() { + // utility subscriber class + } + + /** + * Subscriber principal — déclenche un rebuild si au moins un des items + * (from/to) est un bondage item data-driven. + * + *

Ce handler est un best-effort pour les changements d'armor slot ; + * le path canonique reste {@code PacketSyncV2Equipment.handleOnClient} + * qui cible la capability V2 dédiée. Cf. class javadoc pour la + * sémantique "server-side only" de l'event.

+ * + * @param event l'event Forge (from/to ItemStack non-null par contrat) + */ + @SubscribeEvent + public static void onEquipmentChange(LivingEquipmentChangeEvent event) { + if (!(event.getEntity() instanceof Player player)) return; + + // Double-guard : en cas de fire côté serveur (comportement Forge + // documenté), on skip. L'event peut fire côté client dans certains + // paths malgré la doc — on gate explicitement. + if (!player.level().isClientSide()) return; + + // En prod : emptiness check = ItemStack::isEmpty ; bondage check = + // DataDrivenItemRegistry::get non-null. Factorisés en deux method-refs + // pour permettre leur substitution en test par des lambdas génériques + // (les overloads acceptent n'importe quel type opaque — l'Object + // test-friendly évite le MC bootstrap qu'impose Mockito sur ItemStack). + if (!isBondageRelevant( + event.getFrom(), + event.getTo(), + ItemStack::isEmpty, + DEFAULT_BONDAGE_PREDICATE + )) { + return; + } + + ClientRigEquipmentHandler.rebuildBondageAnimations(player); + } + + /** + * Predicate production qui utilise {@link DataDrivenItemRegistry#get(ItemStack)}. + * Factoré en constante pour permettre l'injection en test (les overloads + * package-private {@code isBondageRelevant}/{@code isBondageItem} acceptent + * un predicate alternatif, évitant le besoin de MC bootstrap pour les + * ItemStack + registry init). + */ + static final Predicate DEFAULT_BONDAGE_PREDICATE = + stack -> DataDrivenItemRegistry.get(stack) != null; + + /** + * Détermine si un changement d'équipement (from → to) implique au moins + * un item bondage data-driven. Overload générique {@code } pour + * permettre les unit tests sans {@link ItemStack} réel (les tests passent + * des {@code Object} dummy + predicates contrôlés, évitant le crash + * {@code Mockito.mock(ItemStack.class)} qui déclenche la static init MC). + * + *

Sémantique : vrai ssi au moins un des deux stacks n'est ni + * null ni vide ET matche le {@code bondagePredicate}. On utilise le + * pattern registry (plutôt que {@code instanceof AbstractV2BondageItem}) + * car l'animation rebuild n'a d'effet que pour les items qui portent une + * {@code DataDrivenItemDefinition} — un item V2 legacy sans JSON + * definition ne contribuerait pas au rebuild et gaspillerait un appel. + * Même pattern que {@link ClientRigEquipmentHandler#extractSortedDefinitions} + * pour la cohérence.

+ * + *

Null-safety : les deux stacks sont contractuellement non-null + * selon {@link LivingEquipmentChangeEvent#getFrom()} et + * {@link LivingEquipmentChangeEvent#getTo()} ({@code @NotNull}), mais on + * double-check null + empty dans {@link #isBondageItem}.

+ * + * @param type du stack ({@link ItemStack} en prod, + * {@link Object} en test) + * @param from item précédemment équipé (tolère null) + * @param to item nouvellement équipé (tolère null) + * @param emptyPredicate predicate qui retourne {@code true} si le stack + * est "empty" (en prod {@code ItemStack::isEmpty}) + * @param bondagePredicate predicate qui retourne {@code true} si un stack + * non-null/non-empty est data-driven (en prod + * {@link #DEFAULT_BONDAGE_PREDICATE}) + * @return {@code true} si au moins un item est un bondage data-driven + */ + static boolean isBondageRelevant( + T from, + T to, + Predicate emptyPredicate, + Predicate bondagePredicate + ) { + return isBondageItem(from, emptyPredicate, bondagePredicate) + || isBondageItem(to, emptyPredicate, bondagePredicate); + } + + /** + * Vérifie si un stack est un item bondage data-driven (overload générique + * pour testabilité). + * + *

Gère null/empty : retourne {@code false} sans appeler + * {@code bondagePredicate}. Ne throw jamais. Le {@code bondagePredicate} + * n'est appelé que pour des stacks non-null et non-empty + * (short-circuit).

+ * + * @param type du stack + * @param stack item à inspecter (tolère null) + * @param emptyPredicate predicate "est-il empty ?" ({@code ItemStack::isEmpty} + * en prod) + * @param bondagePredicate predicate "est-il bondage data-driven ?" + * (appelé uniquement pour stack non-null/non-empty) + * @return {@code true} ssi stack ≠ null, ≠ empty ET bondagePredicate.test(stack) + */ + static boolean isBondageItem( + T stack, + Predicate emptyPredicate, + Predicate bondagePredicate + ) { + if (stack == null) return false; + if (emptyPredicate.test(stack)) return false; + 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. +} diff --git a/src/test/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListenerTest.java b/src/test/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListenerTest.java new file mode 100644 index 0000000..2c351f9 --- /dev/null +++ b/src/test/java/com/tiedup/remake/v2/client/BondageEquipmentChangeListenerTest.java @@ -0,0 +1,212 @@ +/* + * © 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.IdentityHashMap; +import java.util.function.Predicate; + +import org.junit.jupiter.api.Test; + +/** + * Tests unitaires pour {@link BondageEquipmentChangeListener} (P3-06). + * + *

Stratégie de test

+ *

L'entrée publique + * {@link BondageEquipmentChangeListener#onEquipmentChange} nécessite un + * {@code LivingEquipmentChangeEvent} + {@code Player} + MC runtime — non + * testable sans bootstrap. On se limite ici à la logique pure + * {@link BondageEquipmentChangeListener#isBondageItem} et + * {@link BondageEquipmentChangeListener#isBondageRelevant} via leurs overloads + * génériques {@code }.

+ * + *

Les {@link net.minecraft.world.item.ItemStack ItemStack} sont remplacés + * par des {@code Object} dummy (même pattern que + * {@link ClientRigEquipmentHandlerTest#extractSortedDefinitions_nullResolver_skipsItem}). + * Les fonctions {@code isEmpty} et {@code isBondage} sont des lambdas + * contrôlées qui inspectent via {@link IdentityHashMap} les objets dummy — + * évite le crash {@code Mockito.mock(ItemStack.class)} qui déclenche la + * static init MC.

+ */ +class BondageEquipmentChangeListenerTest { + + /** Predicate qui considère chaque stack distinct comme "non-empty". */ + private static final Predicate NEVER_EMPTY = stack -> false; + + /** Predicate qui considère chaque stack comme "empty". */ + private static final Predicate ALWAYS_EMPTY = stack -> true; + + /** Predicate qui considère chaque stack comme "bondage". */ + private static final Predicate ALWAYS_BONDAGE = stack -> true; + + /** Predicate qui ne considère aucun stack comme bondage. */ + private static final Predicate NEVER_BONDAGE = stack -> false; + + // ========== isBondageItem (generic overload) ========== + + /** null stack → false ; aucun predicate n'est appelé. */ + @Test + void isBondageItem_null_returnsFalse_noPredicateInvoked() { + Predicate emptyTripwire = stack -> { + throw new AssertionError("emptyPredicate ne doit pas etre appele pour null"); + }; + Predicate bondageTripwire = stack -> { + throw new AssertionError("bondagePredicate ne doit pas etre appele pour null"); + }; + assertFalse( + BondageEquipmentChangeListener.isBondageItem( + null, emptyTripwire, bondageTripwire), + "null stack doit retourner false sans invoquer les predicates"); + } + + /** Empty stack → false ; bondagePredicate n'est PAS appelé (short-circuit). */ + @Test + void isBondageItem_empty_returnsFalse_bondageNotInvoked() { + Object stack = new Object(); + Predicate bondageTripwire = s -> { + throw new AssertionError( + "bondagePredicate ne doit pas etre appele apres isEmpty==true"); + }; + assertFalse( + BondageEquipmentChangeListener.isBondageItem( + stack, ALWAYS_EMPTY, bondageTripwire), + "empty stack doit retourner false sans invoquer bondagePredicate"); + } + + /** Non-empty stack, bondage=false (vanilla item) → false. */ + @Test + void isBondageItem_nonEmptyNonBondage_returnsFalse() { + Object stack = new Object(); + assertFalse( + BondageEquipmentChangeListener.isBondageItem( + stack, NEVER_EMPTY, NEVER_BONDAGE), + "item vanilla (bondagePredicate false) doit retourner false"); + } + + /** Non-empty stack, bondage=true (data-driven) → true. */ + @Test + void isBondageItem_nonEmptyBondage_returnsTrue() { + Object stack = new Object(); + assertTrue( + BondageEquipmentChangeListener.isBondageItem( + stack, NEVER_EMPTY, ALWAYS_BONDAGE), + "item data-driven (bondagePredicate true) doit retourner true"); + } + + // ========== isBondageRelevant (OR sur from+to) ========== + + /** Les deux stacks empty → false, même avec ALWAYS_BONDAGE. */ + @Test + void isBondageRelevant_bothEmpty_returnsFalse() { + Object from = new Object(); + Object to = new Object(); + assertFalse( + BondageEquipmentChangeListener.isBondageRelevant( + from, to, ALWAYS_EMPTY, ALWAYS_BONDAGE), + "deux stacks empty doivent retourner false meme avec ALWAYS_BONDAGE"); + } + + /** From=bondage, to=empty → true (cas unequip bondage). */ + @Test + void isBondageRelevant_fromBondageToEmpty_returnsTrue() { + Object from = new Object(); + Object to = new Object(); + // Discriminant : seul 'from' est non-empty + IdentityHashMap emptyMap = new IdentityHashMap<>(); + emptyMap.put(from, Boolean.FALSE); + emptyMap.put(to, Boolean.TRUE); + + assertTrue( + BondageEquipmentChangeListener.isBondageRelevant( + from, to, emptyMap::get, ALWAYS_BONDAGE), + "unequip bondage (from=bondage non-empty, to=empty) doit retourner true"); + } + + /** From=empty, to=bondage → true (cas equip bondage). */ + @Test + void isBondageRelevant_fromEmptyToBondage_returnsTrue() { + Object from = new Object(); + Object to = new Object(); + IdentityHashMap emptyMap = new IdentityHashMap<>(); + emptyMap.put(from, Boolean.TRUE); + emptyMap.put(to, Boolean.FALSE); + + assertTrue( + BondageEquipmentChangeListener.isBondageRelevant( + from, to, emptyMap::get, ALWAYS_BONDAGE), + "equip bondage (from=empty, to=bondage) doit retourner true"); + } + + /** Both non-empty, neither bondage (swap vanilla armor) → false. */ + @Test + void isBondageRelevant_bothNonBondage_returnsFalse() { + Object from = new Object(); + Object to = new Object(); + assertFalse( + BondageEquipmentChangeListener.isBondageRelevant( + from, to, NEVER_EMPTY, NEVER_BONDAGE), + "swap vanilla armor (aucun bondage) ne doit pas declencher rebuild"); + } + + /** from=null, to=bondage → true (first equip après spawn). */ + @Test + void isBondageRelevant_nullFromBondageTo_returnsTrue() { + Object to = new Object(); + assertTrue( + BondageEquipmentChangeListener.isBondageRelevant( + null, to, NEVER_EMPTY, ALWAYS_BONDAGE), + "null from + bondage to doit retourner true (null-safe)"); + } + + /** both null → false, aucun predicate appelé. */ + @Test + void isBondageRelevant_bothNull_returnsFalse() { + Predicate emptyTripwire = s -> { + throw new AssertionError("ne doit pas etre appele"); + }; + Predicate bondageTripwire = s -> { + throw new AssertionError("ne doit pas etre appele"); + }; + assertFalse( + BondageEquipmentChangeListener.isBondageRelevant( + null, null, emptyTripwire, bondageTripwire), + "two null stacks doivent retourner false sans appel predicates"); + } + + /** + * Both non-empty, BOTH bondage → true — sanity : on ne short-circuit + * pas sur un state incohérent, et on n'appelle pas nécessairement les + * deux (l'OR court-circuite sur le premier true). + */ + @Test + void isBondageRelevant_bothBondage_returnsTrue() { + Object from = new Object(); + Object to = new Object(); + assertTrue( + BondageEquipmentChangeListener.isBondageRelevant( + from, to, NEVER_EMPTY, ALWAYS_BONDAGE), + "deux stacks bondage doit retourner true (sanity OR logic)"); + } + + // ========== DEFAULT_BONDAGE_PREDICATE sanity ========== + + /** + * Le {@link BondageEquipmentChangeListener#DEFAULT_BONDAGE_PREDICATE} + * existe et est non-null. On ne peut pas l'invoquer sans MC bootstrap + * (il appelle {@code DataDrivenItemRegistry.get(stack)} qui lit + * {@code stack.getTag()} et accède au registry volatile), mais on + * vérifie qu'il est bien câblé en référence — garde contre un + * refactor qui le nullifierait silencieusement. + */ + @Test + void defaultBondagePredicate_isWired() { + assertNotNull( + BondageEquipmentChangeListener.DEFAULT_BONDAGE_PREDICATE, + "DEFAULT_BONDAGE_PREDICATE doit etre non-null (wiring sanity)"); + } +}