diff --git a/src/main/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandler.java b/src/main/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandler.java index 21846b0..ca4b2cb 100644 --- a/src/main/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandler.java +++ b/src/main/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandler.java @@ -11,9 +11,14 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; +import java.util.function.Consumer; import java.util.function.Function; import net.minecraft.resources.ResourceLocation; +import net.minecraft.sounds.SoundEvent; +import net.minecraft.sounds.SoundEvents; +import net.minecraft.sounds.SoundSource; import net.minecraft.world.entity.player.Player; import net.minecraft.world.item.ItemStack; import net.minecraftforge.api.distmarker.Dist; @@ -118,6 +123,62 @@ public final class ClientRigEquipmentHandler { // utility class } + /** + * Transition time (secondes) pour jouer les one-shots on_equip / on_unequip. + * 0.15s = 3 ticks à 20 TPS, même valeur que + * {@link TiedUpRigConstants#GENERAL_ANIMATION_TRANSITION_TIME}. Rapide + * mais non-instantané — évite le pop sur les transitions. + */ + private static final float ONESHOT_TRANSITION_TIME = 0.15F; + + /** + * Son vanilla par défaut pour on_equip / on_unequip. Placeholder MVP : + * future evolution = field {@code equip_sound} dans + * {@link DataDrivenItemDefinition} pour custom per-item sound event. + * + *

{@link SoundEvents#ARMOR_EQUIP_LEATHER} a été choisi pour sa + * sonorité souple (cuir) — adapté aux cuffs / harnesses / gags majoritaires. + * Alternatives vanilla : {@code ARMOR_EQUIP_CHAIN} (métal), {@code ARMOR_EQUIP_IRON} + * (lourd), {@code ARMOR_EQUIP_NETHERITE} (très lourd).

+ * + *

Pourquoi resolver lazy : {@code SoundEvents.} exige + * {@code Bootstrap.bootStrap()} (BuiltInRegistries). En unit tests sans + * MC bootstrap, un field {@code static final SoundEvent = SoundEvents.X} + * throw un {@code ExceptionInInitializerError} dès la première référence + * à cette classe (même via un test qui n'exerce pas la sound logic). En + * défférant la résolution à l'appel de {@link #playEquipSound}, les tests + * restent ignorants du MC runtime pour les helpers pure-Java.

+ */ + private static SoundEvent defaultEquipSound() { + return SoundEvents.ARMOR_EQUIP_LEATHER; + } + + /** + * Snapshot du set d'items équipés par player lors du dernier + * {@link #rebuildBondageAnimations} — utilisé pour calculer les diffs + * {@code newlyEquipped} et {@code newlyUnequipped} tick-to-tick afin de + * déclencher les one-shots {@link AnimationBindings#onEquip} / + * {@link AnimationBindings#onUnequip}. + * + *

WeakHashMap : key = {@link Player}, weak ref → l'entrée est + * GC'd automatiquement quand le player se déconnecte et n'est plus + * référencé nulle part ailleurs. Évite la fuite mémoire sur serveur + * intégré à la longue.

+ * + *

Semantics du set : identity-based (via + * {@link Collections#newSetFromMap(Map)} + {@link IdentityHashMap}). Deux + * {@link ItemStack} avec NBT identique mais instances distinctes sont + * traités comme différents items — convention V2 (un stack = une + * occurrence équipée). Permet de détecter {@code split stack} / + * {@code re-equip} même si le nouveau stack est content-equals à l'ancien.

+ * + *

Thread-safety : lu/écrit exclusivement depuis le client main + * thread (même contrat que {@link #rebuildBondageAnimations}). Pas de + * synchronisation nécessaire.

+ */ + private static final WeakHashMap> LAST_EQUIPPED = + new WeakHashMap<>(); + /** * Functional adapter autour de {@link ClientAnimator#addLivingAnimation} * pour permettre l'injection en tests sans bootstrap MC. @@ -131,6 +192,16 @@ public final class ClientRigEquipmentHandler { void add(LivingMotion motion, AssetAccessor accessor); } + /** + * Functional adapter autour de {@link ClientAnimator#playAnimation} pour + * permettre l'injection en tests sans bootstrap MC. Équivalent à + * {@code animator::playAnimation} côté prod. + */ + @FunctionalInterface + interface OneshotPlayer { + void play(AssetAccessor accessor, float transitionTime); + } + /** * Entry point publique — rebuild la map {@code livingAnimations} du * {@link ClientAnimator} du player en fonction de ses items bondage V2 @@ -177,6 +248,42 @@ public final class ClientRigEquipmentHandler { Map equipped = V2EquipmentHelper.getAllEquipped(player); + // === P3-07 : diff detection tick-to-tick pour on_equip / on_unequip === + // + // On snapshot l'ensemble des items équipés avant le rebuild + on compare + // au dernier snapshot pour ce player. Semantic identity (IdentityHashMap) : + // un stack content-equals mais instance différente compte comme un + // nouvel item équipé (convention V2, un stack = une occurrence). + Set currentUnique = uniqueByIdentity(equipped.values()); + Set previousUnique = + LAST_EQUIPPED.getOrDefault(player, Collections.emptySet()); + + Set newlyEquipped = diffBy(currentUnique, previousUnique); + Set newlyUnequipped = diffBy(previousUnique, currentUnique); + + // Unequip AVANT rebuild — l'anim one-shot joue sur "l'ancien état" du + // rig pendant que la map livingAnimations est encore celle de l'ancien + // équipement. Pragmatique : à ce stade le rebuild n'a pas encore tourné, + // donc les defaults EF sont toujours en place de la passe précédente. + OneshotPlayer oneshotPlayer = animator::playAnimation; + Consumer soundPlayer = stack -> playEquipSound(player, stack); + + for (ItemStack stack : newlyUnequipped) { + triggerOneshot( + oneshotPlayer, + stack, + AnimationBindings::onUnequip, + DataDrivenItemRegistry::get, + TiedUpAnimationRegistry::resolveWithFallback + ); + soundPlayer.accept(stack); + } + + // Update snapshot AVANT le rebuild — si rebuild throw, on a quand même + // consigné l'état courant pour le tick suivant (évite un double-fire + // au prochain rebuild après exception). + LAST_EQUIPPED.put(player, currentUnique); + // La capability dédupe déjà par identity (cf V2BondageEquipment.getAllEquipped, // IdentityHashMap seen+LinkedHashMap result) — on applique néanmoins notre // propre dédup défensive pour ne pas dépendre d'un invariant qu'un refactor @@ -202,14 +309,209 @@ public final class ClientRigEquipmentHandler { TiedUpAnimationRegistry::resolveWithFallback ); + // Equip APRÈS rebuild — l'anim one-shot joue sur "le nouvel état" du + // rig, après que les bindings de l'item nouvellement équipé ont été + // poussés dans livingAnimations. Cohérent avec l'intention visuelle : + // on voit l'item se mettre en place puis le one-shot l'enrichit. + for (ItemStack stack : newlyEquipped) { + triggerOneshot( + oneshotPlayer, + stack, + AnimationBindings::onEquip, + DataDrivenItemRegistry::get, + TiedUpAnimationRegistry::resolveWithFallback + ); + soundPlayer.accept(stack); + } + TiedUpRigConstants.LOGGER.debug( "[ClientRigEquipmentHandler] Rebuilt livingAnimations for player {} " - + "({} data-driven items processed)", + + "({} data-driven items processed, {} equip / {} unequip one-shots)", player.getName().getString(), - sortedDefs.size() + sortedDefs.size(), + newlyEquipped.size(), + newlyUnequipped.size() ); } + /** + * Collecte un {@link Set} identity-based à partir d'un iterable. Deux + * éléments {@code equals()} mais avec instances distinctes restent + * distincts. Null skippés. + * + *

Encapsule le pattern {@code Collections.newSetFromMap(new IdentityHashMap<>())} + * + filter null pour la réutilisation dans {@link #rebuildBondageAnimations} + * et les tests.

+ * + *

Générique : prod l'utilise avec {@link ItemStack}, tests avec + * {@link Object} dummy (aucune méthode d'instance appelée — seule + * l'identité est utilisée).

+ * + * @param type des éléments + * @param items iterable potentiellement avec duplicats identity + nulls + * @return nouveau set identity-based, ordre non garanti + */ + static Set uniqueByIdentity(Iterable items) { + Set out = Collections.newSetFromMap(new IdentityHashMap<>()); + for (T item : items) { + if (item == null) continue; + out.add(item); + } + return out; + } + + /** + * Retourne les éléments de {@code a} qui ne sont pas (par identité objet, + * {@code ==}) dans {@code b}. Utilisé pour calculer {@code newlyEquipped} + * = current - previous et {@code newlyUnequipped} = previous - current. + * + *

Identity semantics (pas {@link Object#equals}) : deux éléments + * {@code equals()} mais références distinctes sont considérés comme + * différents. Volontaire — convention V2 : un stack = une + * occurrence équipée. Permet de détecter correctement :

+ *
    + *
  • Un re-equip où le joueur déséquipe puis rééquipe le même item + * (le stack est potentiellement la même instance Minecraft ; + * dans ce cas pas de one-shot, ok).
  • + *
  • Un remplacement par un item visuellement identique mais stack + * différent (nouveau padlock, NBT patch) → on-shot fire, correct.
  • + *
+ * + *

Ordre de sortie : suit l'ordre d'itération de {@code a} (pour + * {@link IdentityHashMap} non-déterministe mais stable dans un même run). + * Suffisant pour le use case : les one-shots jouent indépendamment sur + * des layers distinctes, l'ordre ne change pas le rendu.

+ * + * @param type des éléments + * @param a set d'origine + * @param b set à soustraire (identity-based) + * @return nouveau set {@code a \ b} (identity difference) + */ + static Set diffBy(Set a, Set b) { + // IdentityHashMap-based set pour respecter la semantic identity. + Set out = Collections.newSetFromMap(new IdentityHashMap<>()); + for (T x : a) { + boolean found = false; + for (T y : b) { + if (x == y) { + found = true; + break; + } + } + if (!found) out.add(x); + } + return out; + } + + /** + * Trigger un one-shot animation pour un item donné via le callback + * {@code oneshotPlayer}. Extrait soit {@code onEquip} soit {@code onUnequip} + * de {@link AnimationBindings} via {@code extractor}. + * + *

No-op silencieux si :

+ *
    + *
  • Le stack n'a pas de {@link DataDrivenItemDefinition} (item V2 legacy) ;
  • + *
  • La définition n'a pas d'{@link AnimationBindings} (animations==null) ;
  • + *
  • Le binding ne déclare pas de one-shot pour ce trigger (onEquip ou + * onUnequip == null dans le JSON).
  • + *
+ * + *

Si un one-shot est déclaré, il est résolu via {@code animResolver} + * ({@link TiedUpAnimationRegistry#resolveWithFallback} en prod) et joué + * via {@code oneshotPlayer} avec {@link #ONESHOT_TRANSITION_TIME}.

+ * + *

Priority forwarding : on NE passe PAS de priority override à + * {@link ClientAnimator#playAnimation(AssetAccessor, float)} (signature + * limitée). La priority vient de + * {@link StaticAnimation#getPriority()} intrinsèque (property JSON du + * fichier anim). Cohérent avec la décision P3-11 : + * {@code PacketPlayRigAnim.priorityOrdinal} reste informationnel / loggable, + * priority override est TODO Phase 4 si besoin.

+ * + *

Package-private + générique {@code } pour unit test avec + * callbacks injectés et stacks {@link Object} dummy (la prod passe + * {@link ItemStack}).

+ * + * @param type de l'item (prod : {@link ItemStack}, + * tests : {@link Object} dummy) + * @param oneshotPlayer callback play(accessor, transitionTime) ; + * en prod {@code animator::playAnimation} + * @param stack item en cours de trigger ; null toléré (no-op) + * @param extractor fonction de sélection du trigger, typiquement + * {@code AnimationBindings::onEquip} ou {@code ::onUnequip} + * @param defResolver resolver stack → {@link DataDrivenItemDefinition} + * (null pour items V2 legacy) + * @param animResolver resolver {@link ResourceLocation} → accessor, + * ex. {@link TiedUpAnimationRegistry#resolveWithFallback} + */ + static void triggerOneshot( + OneshotPlayer oneshotPlayer, + T stack, + Function extractor, + Function defResolver, + Function> animResolver + ) { + if (stack == null) return; + DataDrivenItemDefinition def = defResolver.apply(stack); + if (def == null) return; + AnimationBindings bindings = def.animations(); + if (bindings == null) return; + ResourceLocation oneshotId = extractor.apply(bindings); + if (oneshotId == null) return; + + AssetAccessor accessor = animResolver.apply(oneshotId); + if (accessor == null) return; // defensive — resolveWithFallback ne renvoie jamais null en prod + oneshotPlayer.play(accessor, ONESHOT_TRANSITION_TIME); + } + + /** + * Joue le son equip/unequip vanilla pour un stack donné au voisinage du + * player. Client-only ({@link net.minecraft.world.level.Level#playLocalSound} + * n'émet aucun paquet réseau). + * + *

MVP : son {@link #DEFAULT_EQUIP_SOUND} pour tous les items. + * Future : lire un éventuel champ {@code equip_sound} dans + * {@link DataDrivenItemDefinition} pour customiser per-item.

+ * + *

Non testable unit (nécessite {@link Player} live + level) — skip + * dans la suite de tests.

+ * + * @param player le player source du son (position utilisée pour la + * spatialisation 3D) + * @param stack l'item en cours d'equip/unequip ; jamais null quand appelé + * depuis {@link #rebuildBondageAnimations} mais on check + * défensivement + */ + private static void playEquipSound(Player player, ItemStack stack) { + if (player == null || stack == null || stack.isEmpty()) return; + player.level().playLocalSound( + player.getX(), + player.getY(), + player.getZ(), + defaultEquipSound(), + SoundSource.PLAYERS, + /* volume */ 1.0F, + /* pitch */ 1.0F, + /* distanceDelay */ false + ); + } + + /** + * Vide le cache de snapshots {@link #LAST_EQUIPPED}. Utilisé dans : + *
    + *
  • Tests unitaires — pour isoler les cas entre tests (sinon un test + * qui popule le cache pollue les suivants via l'état statique).
  • + *
  • Client logout / world unload — hook optionnel si on observe des + * fuites ou des triggers fantômes après reconnection (actuellement + * non câblé car WeakHashMap GC suffit).
  • + *
+ * + *

Package-private — pas destiné à la prod.

+ */ + static void clearEquipmentSnapshotsForTesting() { + LAST_EQUIPPED.clear(); + } + /** * Collecte les {@link DataDrivenItemDefinition} associées aux items * d'entrée, dédupe par identity, et trie par {@code posePriority} diff --git a/src/test/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandlerTest.java b/src/test/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandlerTest.java index 97ce04e..2e0dea8 100644 --- a/src/test/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandlerTest.java +++ b/src/test/java/com/tiedup/remake/v2/client/ClientRigEquipmentHandlerTest.java @@ -552,4 +552,269 @@ class ClientRigEquipmentHandlerTest { // un seul occupiedRegion par défaut assertFalse(def.occupiedRegions().isEmpty()); } + + // ========== P3-07 : diffBy identity semantics ========== + + /** Diff de deux sets vides → set vide, no throw. */ + @Test + void diffBy_emptySets_returnsEmpty() { + Set result = ClientRigEquipmentHandler.diffBy(Set.of(), Set.of()); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + /** + * Un item présent dans {@code a} mais absent de {@code b} apparaît dans le + * diff. Cas {@code newlyEquipped = current \ previous}. + */ + @Test + void diffBy_itemOnlyInA_appearsInResult() { + Object item1 = new Object(); + Object item2 = new Object(); + + Set a = Collections.newSetFromMap(new IdentityHashMap<>()); + a.add(item1); + a.add(item2); + Set b = Collections.newSetFromMap(new IdentityHashMap<>()); + b.add(item1); + + Set result = ClientRigEquipmentHandler.diffBy(a, b); + + assertEquals(1, result.size(), "item2 nouveau doit etre detecte"); + assertTrue(result.contains(item2)); + assertFalse(result.contains(item1), "item1 present dans les deux ne doit PAS etre dans le diff"); + } + + /** + * Deux instances {@link Object#equals equals} mais identity-different → + * considérées comme distinctes. Critical pour la convention "un stack = + * une occurrence". + */ + @Test + void diffBy_identitySemantics_equalsObjectsAreDistinct() { + // Deux String content-equal mais avec new String(...) pour garantir + // des instances distinctes (String pool interning contourné). + String a1 = new String("same"); + String b1 = new String("same"); + assertTrue(a1.equals(b1)); + assertFalse(a1 == b1, "guard : les deux Strings doivent etre des instances distinctes"); + + Set setA = Collections.newSetFromMap(new IdentityHashMap<>()); + setA.add(a1); + Set setB = Collections.newSetFromMap(new IdentityHashMap<>()); + setB.add(b1); + + Set result = ClientRigEquipmentHandler.diffBy(setA, setB); + + assertEquals(1, result.size(), + "deux strings equals() mais != doivent etre traites comme distincts (identity)"); + assertSame(a1, result.iterator().next(), + "le diff doit retenir l'instance de setA"); + } + + /** Quand a == b (mêmes instances), diff vide. */ + @Test + void diffBy_identicalSets_returnsEmpty() { + Object item = new Object(); + Set a = Collections.newSetFromMap(new IdentityHashMap<>()); + a.add(item); + Set b = Collections.newSetFromMap(new IdentityHashMap<>()); + b.add(item); + + Set result = ClientRigEquipmentHandler.diffBy(a, b); + assertTrue(result.isEmpty(), "meme instance dans les deux → diff vide"); + } + + // ========== P3-07 : uniqueByIdentity ========== + + /** Iterable avec duplicats identity → un seul élément retenu. */ + @Test + void uniqueByIdentity_duplicateIdentity_dedups() { + Object item = new Object(); + Set result = + ClientRigEquipmentHandler.uniqueByIdentity(List.of(item, item, item)); + assertEquals(1, result.size()); + assertSame(item, result.iterator().next()); + } + + /** Les null de l'iterable sont skippés silencieusement. */ + @Test + void uniqueByIdentity_nullsSkipped() { + Object item = new Object(); + List input = Arrays.asList(null, item, null); + Set result = ClientRigEquipmentHandler.uniqueByIdentity(input); + assertEquals(1, result.size()); + assertSame(item, result.iterator().next()); + } + + // ========== P3-07 : triggerOneshot ========== + + /** + * Fake {@link ClientRigEquipmentHandler.OneshotPlayer} qui capture chaque + * play(accessor, transitionTime) dans une liste. + */ + private static final class CapturingOneshotPlayer + implements ClientRigEquipmentHandler.OneshotPlayer { + + final List> calls = new ArrayList<>(); + + @Override + public void play(AssetAccessor accessor, float transitionTime) { + ResourceLocation id = accessor == null ? null : accessor.registryName(); + calls.add(Map.entry(id == null ? ResourceLocation.fromNamespaceAndPath("test", "null") : id, + transitionTime)); + } + } + + /** + * Stack sans {@link DataDrivenItemDefinition} (resolver → null) → skip + * silencieusement, aucun appel au oneshotPlayer. + */ + @Test + void triggerOneshot_noDefinition_skipsSilently() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + Object stack = new Object(); + + ClientRigEquipmentHandler.triggerOneshot( + player, + stack, + AnimationBindings::onEquip, + any -> null, // resolver retourne null : item V2 legacy + PASSTHROUGH_RESOLVER + ); + + assertTrue(player.calls.isEmpty(), + "stack sans definition → pas de trigger"); + } + + /** + * Définition dont {@code animations() == null} → skip silencieusement. + */ + @Test + void triggerOneshot_nullBindings_skipsSilently() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + Object stack = new Object(); + DataDrivenItemDefinition def = makeDef(ITEM_A, 10, /* animations */ null); + + ClientRigEquipmentHandler.triggerOneshot( + player, + stack, + AnimationBindings::onEquip, + any -> def, + PASSTHROUGH_RESOLVER + ); + + assertTrue(player.calls.isEmpty(), + "animations()==null → pas de trigger"); + } + + /** + * Bindings présents mais {@code onEquip == null} → skip silencieusement. + * Le cas dominant : 99% des items data-driven n'ont que des + * {@code living_motions}, pas de one-shots. + */ + @Test + void triggerOneshot_onEquipNull_skipsSilently() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + Object stack = new Object(); + AnimationBindings bindings = new AnimationBindings( + Map.of(LivingMotions.WALK, ANIM_WALK_A), + /* onEquip */ null, + /* onUnequip */ null + ); + DataDrivenItemDefinition def = makeDef(ITEM_A, 10, bindings); + + ClientRigEquipmentHandler.triggerOneshot( + player, + stack, + AnimationBindings::onEquip, + any -> def, + PASSTHROUGH_RESOLVER + ); + + assertTrue(player.calls.isEmpty(), + "onEquip==null → pas de trigger"); + } + + /** + * Happy path — bindings avec {@code onEquip} défini → le oneshotPlayer + * est appelé avec l'accessor résolu + la transition time canonique + * (0.15s, {@code ONESHOT_TRANSITION_TIME}). + */ + @Test + void triggerOneshot_happyPath_callsPlayerWithResolvedAccessor() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + Object stack = new Object(); + + ResourceLocation oneshotId = + ResourceLocation.fromNamespaceAndPath("tiedup", "cuffs_equip_oneshot"); + AnimationBindings bindings = new AnimationBindings( + Map.of(), + /* onEquip */ oneshotId, + /* onUnequip */ null + ); + DataDrivenItemDefinition def = makeDef(ITEM_A, 10, bindings); + + ClientRigEquipmentHandler.triggerOneshot( + player, + stack, + AnimationBindings::onEquip, + any -> def, + PASSTHROUGH_RESOLVER + ); + + assertEquals(1, player.calls.size(), + "happy path : un seul trigger"); + assertEquals(oneshotId, player.calls.get(0).getKey(), + "l'accessor resolu doit porter l'ID du one-shot"); + assertEquals(0.15F, player.calls.get(0).getValue(), + 0.0001F, + "transition time doit etre 0.15s (ONESHOT_TRANSITION_TIME)"); + } + + /** + * Happy path côté {@code onUnequip} : même flow, mais l'extractor pointe + * sur {@link AnimationBindings#onUnequip}. + */ + @Test + void triggerOneshot_onUnequip_callsPlayerWithResolvedAccessor() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + Object stack = new Object(); + + ResourceLocation unequipId = + ResourceLocation.fromNamespaceAndPath("tiedup", "cuffs_unequip_oneshot"); + AnimationBindings bindings = new AnimationBindings( + Map.of(), + /* onEquip */ null, + /* onUnequip */ unequipId + ); + DataDrivenItemDefinition def = makeDef(ITEM_A, 10, bindings); + + ClientRigEquipmentHandler.triggerOneshot( + player, + stack, + AnimationBindings::onUnequip, + any -> def, + PASSTHROUGH_RESOLVER + ); + + assertEquals(1, player.calls.size()); + assertEquals(unequipId, player.calls.get(0).getKey()); + } + + /** Stack null est toléré — pas de NPE, pas d'appel player. */ + @Test + void triggerOneshot_nullStack_noThrow() { + CapturingOneshotPlayer player = new CapturingOneshotPlayer(); + assertDoesNotThrow(() -> + ClientRigEquipmentHandler.triggerOneshot( + player, + /* stack */ null, + AnimationBindings::onEquip, + any -> { throw new AssertionError("resolver ne doit pas etre appele pour null"); }, + PASSTHROUGH_RESOLVER + ) + ); + assertTrue(player.calls.isEmpty()); + } }