P3-07 review fixes : key-based diff + first-rebuild guard
CRITICAL BUG-001 reviewer : identity-based diff triggered sound 2x per sync because V2BondageEquipment.deserializeNBT regenerates ItemStack instances. 19 call-sites (respawn, dialogue, lock toggle, whip) do full resync → every interaction = sound cacophony. Switch LAST_EQUIPPED from Set<ItemStack> (identity) to Map<BodyRegionV2, ResourceLocation> keyed by (region, itemId). Same region+item = no change, ignored. HIGH RISK-001 reviewer : first rebuild per player had LAST_EQUIPPED empty → all items detected as newly equipped → sound spam at login (5 items = 5 sounds). Add FIRST_REBUILD_DONE WeakHashMap flag to skip oneshots+sounds on very first rebuild per player. MEDIUM SMELL-001/002 : javadoc of diffBy removed (assumed identity sharing which never happens). Padlock NBT patch detection promise removed — would require NBT hash in snapshot key (Phase 4). Tests refactored : key-based (region, itemId) semantics.
This commit is contained in:
@@ -7,12 +7,13 @@ package com.tiedup.remake.v2.client;
|
|||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.Comparator;
|
import java.util.Comparator;
|
||||||
|
import java.util.EnumMap;
|
||||||
import java.util.IdentityHashMap;
|
import java.util.IdentityHashMap;
|
||||||
|
import java.util.LinkedHashMap;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.WeakHashMap;
|
import java.util.WeakHashMap;
|
||||||
import java.util.function.Consumer;
|
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
|
|
||||||
import net.minecraft.resources.ResourceLocation;
|
import net.minecraft.resources.ResourceLocation;
|
||||||
@@ -102,7 +103,7 @@ import com.tiedup.remake.v2.bondage.datadriven.DataDrivenItemRegistry;
|
|||||||
*
|
*
|
||||||
* <p>L'entrée publique {@link #rebuildBondageAnimations(Player)} est
|
* <p>L'entrée publique {@link #rebuildBondageAnimations(Player)} est
|
||||||
* side-gated mais non testable sans MC runtime (player réel, capability
|
* side-gated mais non testable sans MC runtime (player réel, capability
|
||||||
* réelle, animator réel). La logique pure est extraite dans deux helpers
|
* réelle, animator réel). La logique pure est extraite dans plusieurs helpers
|
||||||
* package-private :</p>
|
* package-private :</p>
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li>{@link #extractSortedDefinitions} — dedup identity + filter +
|
* <li>{@link #extractSortedDefinitions} — dedup identity + filter +
|
||||||
@@ -111,6 +112,12 @@ import com.tiedup.remake.v2.bondage.datadriven.DataDrivenItemRegistry;
|
|||||||
* callbacks injectables ({@link Runnable} +
|
* callbacks injectables ({@link Runnable} +
|
||||||
* {@link LivingAnimationAdder}). Testable avec un fake adder qui
|
* {@link LivingAnimationAdder}). Testable avec un fake adder qui
|
||||||
* capture les appels dans une list.</li>
|
* capture les appels dans une list.</li>
|
||||||
|
* <li>{@link #buildKeySnapshot} — construit le snapshot stable
|
||||||
|
* {@code (region → itemId)} utilisé pour le diff tick-to-tick
|
||||||
|
* résistant aux re-sync réseau.</li>
|
||||||
|
* <li>{@link #diffKeys} — calcule newly-entered entries d'une map
|
||||||
|
* {@code a \ b} par comparaison (key, value) — base de détection
|
||||||
|
* equip / unequip.</li>
|
||||||
* </ul>
|
* </ul>
|
||||||
*
|
*
|
||||||
* @see AnimationBindings
|
* @see AnimationBindings
|
||||||
@@ -154,29 +161,58 @@ public final class ClientRigEquipmentHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Snapshot du set d'items équipés par player lors du dernier
|
* Snapshot stable {@code (region → itemId)} — utilisé pour calculer les
|
||||||
* {@link #rebuildBondageAnimations} — utilisé pour calculer les diffs
|
* diffs {@code newlyEquipped} / {@code newlyUnequipped} tick-to-tick afin
|
||||||
* {@code newlyEquipped} et {@code newlyUnequipped} tick-to-tick afin de
|
* de déclencher les one-shots {@link AnimationBindings#onEquip} /
|
||||||
* déclencher les one-shots {@link AnimationBindings#onEquip} /
|
|
||||||
* {@link AnimationBindings#onUnequip}.
|
* {@link AnimationBindings#onUnequip}.
|
||||||
*
|
*
|
||||||
|
* <p><b>Pourquoi key-based et non identity-based</b> : le packet
|
||||||
|
* {@code PacketSyncV2Equipment} appelle {@code V2BondageEquipment.deserializeNBT}
|
||||||
|
* qui recrée systématiquement de nouvelles instances {@link ItemStack} via
|
||||||
|
* {@code ItemStack.of(stackTag)}. Les {@literal ~}19 call sites server-side
|
||||||
|
* qui appellent {@code V2EquipmentHelper.sync()} (respawn, dialogue,
|
||||||
|
* struggle, lock toggle, whip, etc.) produisent chacun un full resync, donc
|
||||||
|
* un diff basé sur l'identité des stacks verrait tous les items comme
|
||||||
|
* "nouvellement équipés" + "nouvellement déséquipés" à chaque sync —
|
||||||
|
* double-fire du son à chaque interaction (BUG-001 reviewer P3-07).</p>
|
||||||
|
*
|
||||||
|
* <p>Le snapshot est maintenant keyé par {@code (BodyRegionV2, itemId)}
|
||||||
|
* où {@code itemId} est extrait via
|
||||||
|
* {@link DataDrivenItemRegistry#get(ItemStack)} → {@code def.id()}. Tant
|
||||||
|
* que le même item reste au même slot, le diff est vide même si
|
||||||
|
* l'{@link ItemStack} est une nouvelle instance après re-sync.</p>
|
||||||
|
*
|
||||||
|
* <p><b>Limitation connue</b> : une mutation NBT silencieuse (ex. padlock
|
||||||
|
* posé sur des cuffs existants qui ne change pas {@code tiedup_item_id})
|
||||||
|
* ne déclenche PAS de diff. Si un tel usage devient critical, hash du
|
||||||
|
* NBT à inclure dans la key — TODO Phase 4.</p>
|
||||||
|
*
|
||||||
* <p><b>WeakHashMap</b> : key = {@link Player}, weak ref → l'entrée est
|
* <p><b>WeakHashMap</b> : key = {@link Player}, weak ref → l'entrée est
|
||||||
* GC'd automatiquement quand le player se déconnecte et n'est plus
|
* 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
|
* référencé nulle part ailleurs. Évite la fuite mémoire sur serveur
|
||||||
* intégré à la longue.</p>
|
* intégré à la longue.</p>
|
||||||
*
|
*
|
||||||
* <p><b>Semantics du set</b> : 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.</p>
|
|
||||||
*
|
|
||||||
* <p><b>Thread-safety</b> : lu/écrit exclusivement depuis le client main
|
* <p><b>Thread-safety</b> : lu/écrit exclusivement depuis le client main
|
||||||
* thread (même contrat que {@link #rebuildBondageAnimations}). Pas de
|
* thread (même contrat que {@link #rebuildBondageAnimations}). Pas de
|
||||||
* synchronisation nécessaire.</p>
|
* synchronisation nécessaire.</p>
|
||||||
*/
|
*/
|
||||||
private static final WeakHashMap<Player, Set<ItemStack>> LAST_EQUIPPED =
|
private static final WeakHashMap<Player, Map<BodyRegionV2, ResourceLocation>>
|
||||||
|
LAST_EQUIPPED_KEYS = new WeakHashMap<>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Flag par-player indiquant que {@link #rebuildBondageAnimations} a déjà
|
||||||
|
* tourné une fois pour ce player. Skip des sounds + one-shots sur le
|
||||||
|
* premier rebuild — sinon, à chaque login / join, tous les items équipés
|
||||||
|
* seraient vus comme "nouveaux" (previous={}) et fireraient le son en
|
||||||
|
* cascade (RISK-001 reviewer P3-07 : 5 items = 5 sons cuir simultanés
|
||||||
|
* à l'arrivée, désagréable auditivement).
|
||||||
|
*
|
||||||
|
* <p><b>WeakHashMap</b> : même justification que
|
||||||
|
* {@link #LAST_EQUIPPED_KEYS} — GC'd quand le player part. Un reconnect
|
||||||
|
* retraite donc comme un "premier rebuild" (prévu : on ne veut pas spammer
|
||||||
|
* le son au retour).</p>
|
||||||
|
*/
|
||||||
|
private static final WeakHashMap<Player, Boolean> FIRST_REBUILD_DONE =
|
||||||
new WeakHashMap<>();
|
new WeakHashMap<>();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -248,42 +284,37 @@ public final class ClientRigEquipmentHandler {
|
|||||||
|
|
||||||
Map<BodyRegionV2, ItemStack> equipped = V2EquipmentHelper.getAllEquipped(player);
|
Map<BodyRegionV2, ItemStack> equipped = V2EquipmentHelper.getAllEquipped(player);
|
||||||
|
|
||||||
// === P3-07 : diff detection tick-to-tick pour on_equip / on_unequip ===
|
// === P3-07 : diff detection key-based résistant aux re-sync réseau ===
|
||||||
//
|
//
|
||||||
// On snapshot l'ensemble des items équipés avant le rebuild + on compare
|
// On keye par (region, itemId) plutôt que par identity d'ItemStack :
|
||||||
// au dernier snapshot pour ce player. Semantic identity (IdentityHashMap) :
|
// V2BondageEquipment.deserializeNBT recrée systématiquement de nouvelles
|
||||||
// un stack content-equals mais instance différente compte comme un
|
// instances à chaque packet, donc une comparaison identity verrait tous
|
||||||
// nouvel item équipé (convention V2, un stack = une occurrence).
|
// les items comme "nouveaux" à chaque full-resync (dialogue, respawn,
|
||||||
Set<ItemStack> currentUnique = uniqueByIdentity(equipped.values());
|
// lock toggle, etc.) → son dédoublé. Key-based : même itemId au même
|
||||||
Set<ItemStack> previousUnique =
|
// slot → pas de change, silence, quelle que soit l'instance.
|
||||||
LAST_EQUIPPED.getOrDefault(player, Collections.emptySet());
|
Map<BodyRegionV2, ResourceLocation> currentKeys =
|
||||||
|
buildKeySnapshot(equipped, DataDrivenItemRegistry::get);
|
||||||
|
Map<BodyRegionV2, ResourceLocation> previousKeys =
|
||||||
|
LAST_EQUIPPED_KEYS.getOrDefault(player, Map.of());
|
||||||
|
|
||||||
Set<ItemStack> newlyEquipped = diffBy(currentUnique, previousUnique);
|
Map<BodyRegionV2, ResourceLocation> newlyEquipped =
|
||||||
Set<ItemStack> newlyUnequipped = diffBy(previousUnique, currentUnique);
|
diffKeys(currentKeys, previousKeys);
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyUnequipped =
|
||||||
|
diffKeys(previousKeys, currentKeys);
|
||||||
|
|
||||||
// Unequip AVANT rebuild — l'anim one-shot joue sur "l'ancien état" du
|
// Mapping itemId → stack pour les entries nouvellement équipées :
|
||||||
// rig pendant que la map livingAnimations est encore celle de l'ancien
|
// le callsite du one-shot / sound a besoin du stack concret (ou de la
|
||||||
// équipement. Pragmatique : à ce stade le rebuild n'a pas encore tourné,
|
// définition pour le oneshot, cf plus bas). Pour les unequip, le stack
|
||||||
// donc les defaults EF sont toujours en place de la passe précédente.
|
// est parti — on passe directement l'itemId et on lookup la définition
|
||||||
OneshotPlayer oneshotPlayer = animator::playAnimation;
|
// dans le registry (qui garde la def en cache).
|
||||||
Consumer<ItemStack> soundPlayer = stack -> playEquipSound(player, stack);
|
Map<ResourceLocation, ItemStack> itemIdToStack = new LinkedHashMap<>();
|
||||||
|
for (Map.Entry<BodyRegionV2, ItemStack> entry : equipped.entrySet()) {
|
||||||
for (ItemStack stack : newlyUnequipped) {
|
ResourceLocation id = extractItemId(entry.getValue(), DataDrivenItemRegistry::get);
|
||||||
triggerOneshot(
|
if (id != null && !itemIdToStack.containsKey(id)) {
|
||||||
oneshotPlayer,
|
itemIdToStack.put(id, entry.getValue());
|
||||||
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,
|
// La capability dédupe déjà par identity (cf V2BondageEquipment.getAllEquipped,
|
||||||
// IdentityHashMap seen+LinkedHashMap result) — on applique néanmoins notre
|
// 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
|
// propre dédup défensive pour ne pas dépendre d'un invariant qu'un refactor
|
||||||
@@ -291,6 +322,37 @@ public final class ClientRigEquipmentHandler {
|
|||||||
List<DataDrivenItemDefinition> sortedDefs =
|
List<DataDrivenItemDefinition> sortedDefs =
|
||||||
extractSortedDefinitions(equipped.values(), DataDrivenItemRegistry::get);
|
extractSortedDefinitions(equipped.values(), DataDrivenItemRegistry::get);
|
||||||
|
|
||||||
|
// Suppress sounds + one-shots sur le PREMIER rebuild par player :
|
||||||
|
// previousKeys sera empty au login → sans garde, tous les items seraient
|
||||||
|
// vus comme newly equipped → 5 items = 5 sons simultanés à l'arrivée
|
||||||
|
// (RISK-001 reviewer). Le flag FIRST_REBUILD_DONE évite ce spam.
|
||||||
|
boolean isFirstRebuild = !FIRST_REBUILD_DONE.containsKey(player);
|
||||||
|
|
||||||
|
OneshotPlayer oneshotPlayer = animator::playAnimation;
|
||||||
|
|
||||||
|
if (!isFirstRebuild) {
|
||||||
|
// 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.
|
||||||
|
for (Map.Entry<BodyRegionV2, ResourceLocation> entry : newlyUnequipped.entrySet()) {
|
||||||
|
ResourceLocation itemId = entry.getValue();
|
||||||
|
triggerOneshotById(
|
||||||
|
oneshotPlayer,
|
||||||
|
itemId,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
DataDrivenItemRegistry::get,
|
||||||
|
TiedUpAnimationRegistry::resolveWithFallback
|
||||||
|
);
|
||||||
|
playEquipSound(player);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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_KEYS.put(player, Map.copyOf(currentKeys));
|
||||||
|
|
||||||
// Reset + restore des defaults EF (IDLE, WALK, RUN, etc.). Les bindings
|
// Reset + restore des defaults EF (IDLE, WALK, RUN, etc.). Les bindings
|
||||||
// custom qu'on ajoute après prennent le pas sur ces defaults via la
|
// custom qu'on ajoute après prennent le pas sur ces defaults via la
|
||||||
// map livingAnimations (dernière put gagne pour une même LivingMotion).
|
// map livingAnimations (dernière put gagne pour une même LivingMotion).
|
||||||
@@ -309,100 +371,185 @@ public final class ClientRigEquipmentHandler {
|
|||||||
TiedUpAnimationRegistry::resolveWithFallback
|
TiedUpAnimationRegistry::resolveWithFallback
|
||||||
);
|
);
|
||||||
|
|
||||||
// Equip APRÈS rebuild — l'anim one-shot joue sur "le nouvel état" du
|
if (!isFirstRebuild) {
|
||||||
// rig, après que les bindings de l'item nouvellement équipé ont été
|
// Equip APRÈS rebuild — l'anim one-shot joue sur "le nouvel état" du
|
||||||
// poussés dans livingAnimations. Cohérent avec l'intention visuelle :
|
// rig, après que les bindings de l'item nouvellement équipé ont été
|
||||||
// on voit l'item se mettre en place puis le one-shot l'enrichit.
|
// poussés dans livingAnimations. Cohérent avec l'intention visuelle :
|
||||||
for (ItemStack stack : newlyEquipped) {
|
// on voit l'item se mettre en place puis le one-shot l'enrichit.
|
||||||
triggerOneshot(
|
for (Map.Entry<BodyRegionV2, ResourceLocation> entry : newlyEquipped.entrySet()) {
|
||||||
oneshotPlayer,
|
ResourceLocation itemId = entry.getValue();
|
||||||
stack,
|
triggerOneshotById(
|
||||||
AnimationBindings::onEquip,
|
oneshotPlayer,
|
||||||
DataDrivenItemRegistry::get,
|
itemId,
|
||||||
TiedUpAnimationRegistry::resolveWithFallback
|
AnimationBindings::onEquip,
|
||||||
);
|
DataDrivenItemRegistry::get,
|
||||||
soundPlayer.accept(stack);
|
TiedUpAnimationRegistry::resolveWithFallback
|
||||||
|
);
|
||||||
|
playEquipSound(player);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
FIRST_REBUILD_DONE.put(player, Boolean.TRUE);
|
||||||
|
|
||||||
TiedUpRigConstants.LOGGER.debug(
|
TiedUpRigConstants.LOGGER.debug(
|
||||||
"[ClientRigEquipmentHandler] Rebuilt livingAnimations for player {} "
|
"[ClientRigEquipmentHandler] Rebuilt livingAnimations for player {} "
|
||||||
+ "({} data-driven items processed, {} equip / {} unequip one-shots)",
|
+ "({} data-driven items processed, {} equip / {} unequip one-shots,"
|
||||||
|
+ " firstRebuild={})",
|
||||||
player.getName().getString(),
|
player.getName().getString(),
|
||||||
sortedDefs.size(),
|
sortedDefs.size(),
|
||||||
newlyEquipped.size(),
|
newlyEquipped.size(),
|
||||||
newlyUnequipped.size()
|
newlyUnequipped.size(),
|
||||||
|
isFirstRebuild
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Collecte un {@link Set} identity-based à partir d'un iterable. Deux
|
* Extrait l'identifiant data-driven ({@link DataDrivenItemDefinition#id()})
|
||||||
* éléments {@code equals()} mais avec instances distinctes restent
|
* d'un {@link ItemStack}. Retourne {@code null} pour les stacks
|
||||||
* distincts. Null skippés.
|
* vides, legacy ou sans definition.
|
||||||
*
|
*
|
||||||
* <p>Encapsule le pattern {@code Collections.newSetFromMap(new IdentityHashMap<>())}
|
* <p>Package-private + resolver injecté pour permettre un unit test sans
|
||||||
* + filter null pour la réutilisation dans {@link #rebuildBondageAnimations}
|
* bootstrap MC (test : {@code Object} stack + resolver custom ; prod :
|
||||||
* et les tests.</p>
|
* {@link ItemStack} + {@link DataDrivenItemRegistry#get(ItemStack)}).</p>
|
||||||
*
|
*
|
||||||
* <p><b>Générique</b> : prod l'utilise avec {@link ItemStack}, tests avec
|
* @param <T> type de l'item (prod : {@link ItemStack}, tests : dummy)
|
||||||
* {@link Object} dummy (aucune méthode d'instance appelée — seule
|
* @param stack le stack à inspecter, null toléré
|
||||||
* l'identité est utilisée).</p>
|
* @param resolver resolver {@code stack → DataDrivenItemDefinition} (null pour legacy)
|
||||||
*
|
* @return l'itemId, ou null si non data-driven
|
||||||
* @param <T> type des éléments
|
|
||||||
* @param items iterable potentiellement avec duplicats identity + nulls
|
|
||||||
* @return nouveau set identity-based, ordre non garanti
|
|
||||||
*/
|
*/
|
||||||
static <T> Set<T> uniqueByIdentity(Iterable<T> items) {
|
static <T> ResourceLocation extractItemId(
|
||||||
Set<T> out = Collections.newSetFromMap(new IdentityHashMap<>());
|
T stack,
|
||||||
for (T item : items) {
|
Function<T, DataDrivenItemDefinition> resolver
|
||||||
if (item == null) continue;
|
) {
|
||||||
out.add(item);
|
if (stack == null) return null;
|
||||||
|
DataDrivenItemDefinition def = resolver.apply(stack);
|
||||||
|
return def != null ? def.id() : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Construit un snapshot stable {@code (BodyRegionV2 → itemId)} à partir
|
||||||
|
* d'un {@code Map<BodyRegionV2, ItemStack>}. Les slots dont le stack n'est
|
||||||
|
* pas data-driven (resolver → null) sont omis.
|
||||||
|
*
|
||||||
|
* <p>Ce snapshot est la base du diff key-based : deux snapshots pris à
|
||||||
|
* deux instants différents sont comparés par {@code (region, itemId)}. Un
|
||||||
|
* resync réseau qui régénère les instances {@link ItemStack} produit le
|
||||||
|
* MÊME snapshot tant que les items occupant les slots restent les mêmes
|
||||||
|
* par itemId — d'où l'immunité au double-fire du son P3-07.</p>
|
||||||
|
*
|
||||||
|
* <p>Package-private + generic + resolver injecté → testable sans bootstrap
|
||||||
|
* MC.</p>
|
||||||
|
*
|
||||||
|
* @param <T> type de l'item (prod : {@link ItemStack}, tests : dummy)
|
||||||
|
* @param equipped map du callsite ({@link V2EquipmentHelper#getAllEquipped}
|
||||||
|
* en prod)
|
||||||
|
* @param resolver resolver {@code stack → DataDrivenItemDefinition}
|
||||||
|
* @return map {@code (region → itemId)} possiblement vide, never null
|
||||||
|
*/
|
||||||
|
static <T> Map<BodyRegionV2, ResourceLocation> buildKeySnapshot(
|
||||||
|
Map<BodyRegionV2, T> equipped,
|
||||||
|
Function<T, DataDrivenItemDefinition> resolver
|
||||||
|
) {
|
||||||
|
Map<BodyRegionV2, ResourceLocation> out = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
for (Map.Entry<BodyRegionV2, T> entry : equipped.entrySet()) {
|
||||||
|
ResourceLocation id = extractItemId(entry.getValue(), resolver);
|
||||||
|
if (id != null) out.put(entry.getKey(), id);
|
||||||
}
|
}
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Retourne les éléments de {@code a} qui ne sont pas (par identité objet,
|
* Calcule les entrées de {@code a} qui n'existent pas à l'identique
|
||||||
* {@code ==}) dans {@code b}. Utilisé pour calculer {@code newlyEquipped}
|
* dans {@code b}, keyées {@code (region, itemId)}. Utilisé pour détecter
|
||||||
* = current - previous et {@code newlyUnequipped} = previous - current.
|
* {@code newlyEquipped = current \ previous} et
|
||||||
|
* {@code newlyUnequipped = previous \ current}.
|
||||||
*
|
*
|
||||||
* <p>Identity semantics (pas {@link Object#equals}) : deux éléments
|
* <p>Sémantique : une entrée {@code (region, itemId)} de {@code a} est
|
||||||
* {@code equals()} mais références distinctes sont considérés comme
|
* incluse dans le diff si {@code b} :</p>
|
||||||
* <b>différents</b>. Volontaire — convention V2 : un stack = une
|
|
||||||
* occurrence équipée. Permet de détecter correctement :</p>
|
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li>Un re-equip où le joueur déséquipe puis rééquipe le même item
|
* <li>ne contient pas la {@code region}, OU</li>
|
||||||
* (le stack est potentiellement la même instance Minecraft ;
|
* <li>contient la {@code region} mais avec un {@code itemId} différent
|
||||||
* dans ce cas pas de one-shot, ok).</li>
|
* (via {@link Object#equals}).</li>
|
||||||
* <li>Un remplacement par un item visuellement identique mais stack
|
|
||||||
* différent (nouveau padlock, NBT patch) → on-shot fire, correct.</li>
|
|
||||||
* </ul>
|
* </ul>
|
||||||
*
|
*
|
||||||
* <p>Ordre de sortie : suit l'ordre d'itération de {@code a} (pour
|
* <p>Ainsi :</p>
|
||||||
* {@link IdentityHashMap} non-déterministe mais stable dans un même run).
|
* <ul>
|
||||||
* Suffisant pour le use case : les one-shots jouent indépendamment sur
|
* <li>{@code previous={ARMS→cuffs}, current={ARMS→cuffs}} →
|
||||||
* des layers distinctes, l'ordre ne change pas le rendu.</p>
|
* {@code diffKeys(current, previous) = {}} — pas de change malgré
|
||||||
|
* ItemStack réinstancié par {@code deserializeNBT}.</li>
|
||||||
|
* <li>{@code previous={ARMS→cuffs}, current={ARMS→shackles}} →
|
||||||
|
* {@code diffKeys(current, previous) = {ARMS→shackles}} — change
|
||||||
|
* détecté.</li>
|
||||||
|
* </ul>
|
||||||
*
|
*
|
||||||
* @param <T> type des éléments
|
* <p>Package-private → testable.</p>
|
||||||
* @param a set d'origine
|
*
|
||||||
* @param b set à soustraire (identity-based)
|
* @param a la map source
|
||||||
* @return nouveau set {@code a \ b} (identity difference)
|
* @param b la map à soustraire
|
||||||
|
* @return nouvelle map, entrées de {@code a} absentes de {@code b}
|
||||||
*/
|
*/
|
||||||
static <T> Set<T> diffBy(Set<T> a, Set<T> b) {
|
static Map<BodyRegionV2, ResourceLocation> diffKeys(
|
||||||
// IdentityHashMap-based set pour respecter la semantic identity.
|
Map<BodyRegionV2, ResourceLocation> a,
|
||||||
Set<T> out = Collections.newSetFromMap(new IdentityHashMap<>());
|
Map<BodyRegionV2, ResourceLocation> b
|
||||||
for (T x : a) {
|
) {
|
||||||
boolean found = false;
|
Map<BodyRegionV2, ResourceLocation> out = new EnumMap<>(BodyRegionV2.class);
|
||||||
for (T y : b) {
|
for (Map.Entry<BodyRegionV2, ResourceLocation> entry : a.entrySet()) {
|
||||||
if (x == y) {
|
ResourceLocation bVal = b.get(entry.getKey());
|
||||||
found = true;
|
if (bVal == null || !bVal.equals(entry.getValue())) {
|
||||||
break;
|
out.put(entry.getKey(), entry.getValue());
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if (!found) out.add(x);
|
|
||||||
}
|
}
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Trigger un one-shot pour un {@code itemId} donné. Resolve la
|
||||||
|
* {@link DataDrivenItemDefinition} via le registry, extrait le trigger
|
||||||
|
* (on_equip ou on_unequip) via l'{@code extractor}, et joue l'animation
|
||||||
|
* via le {@code oneshotPlayer}.
|
||||||
|
*
|
||||||
|
* <p>Utilisé à la place de {@link #triggerOneshot} quand on n'a plus le
|
||||||
|
* stack concret (cas unequip : le stack vient de quitter le slot). La
|
||||||
|
* {@link DataDrivenItemDefinition} reste accessible via
|
||||||
|
* {@link DataDrivenItemRegistry#get(ResourceLocation)} tant que le
|
||||||
|
* registry n'a pas été rechargé ({@code /reload}).</p>
|
||||||
|
*
|
||||||
|
* <p>No-op silencieux si :</p>
|
||||||
|
* <ul>
|
||||||
|
* <li>L'{@code itemId} est inconnu du registry (item supprimé / mod
|
||||||
|
* unloaded) ;</li>
|
||||||
|
* <li>La définition n'a pas d'{@link AnimationBindings} ;</li>
|
||||||
|
* <li>Le binding ne déclare pas de one-shot pour ce trigger.</li>
|
||||||
|
* </ul>
|
||||||
|
*
|
||||||
|
* <p>Package-private + resolvers injectés → testable unit sans bootstrap.</p>
|
||||||
|
*
|
||||||
|
* @param oneshotPlayer callback play(accessor, transitionTime)
|
||||||
|
* @param itemId identifiant de l'item ; null toléré (no-op)
|
||||||
|
* @param extractor fonction de sélection du trigger ({@code onEquip} ou {@code onUnequip})
|
||||||
|
* @param defResolver resolver {@code itemId → DataDrivenItemDefinition} (null = inconnu)
|
||||||
|
* @param animResolver resolver {@code animId → AssetAccessor}
|
||||||
|
*/
|
||||||
|
static void triggerOneshotById(
|
||||||
|
OneshotPlayer oneshotPlayer,
|
||||||
|
ResourceLocation itemId,
|
||||||
|
Function<AnimationBindings, ResourceLocation> extractor,
|
||||||
|
Function<ResourceLocation, DataDrivenItemDefinition> defResolver,
|
||||||
|
Function<ResourceLocation, AssetAccessor<? extends StaticAnimation>> animResolver
|
||||||
|
) {
|
||||||
|
if (itemId == null) return;
|
||||||
|
DataDrivenItemDefinition def = defResolver.apply(itemId);
|
||||||
|
if (def == null) return;
|
||||||
|
AnimationBindings bindings = def.animations();
|
||||||
|
if (bindings == null) return;
|
||||||
|
ResourceLocation oneshotId = extractor.apply(bindings);
|
||||||
|
if (oneshotId == null) return;
|
||||||
|
|
||||||
|
AssetAccessor<? extends StaticAnimation> accessor = animResolver.apply(oneshotId);
|
||||||
|
if (accessor == null) return;
|
||||||
|
oneshotPlayer.play(accessor, ONESHOT_TRANSITION_TIME);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Trigger un one-shot animation pour un item donné via le callback
|
* Trigger un one-shot animation pour un item donné via le callback
|
||||||
* {@code oneshotPlayer}. Extrait soit {@code onEquip} soit {@code onUnequip}
|
* {@code oneshotPlayer}. Extrait soit {@code onEquip} soit {@code onUnequip}
|
||||||
@@ -465,25 +612,25 @@ public final class ClientRigEquipmentHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Joue le son equip/unequip vanilla pour un stack donné au voisinage du
|
* Joue le son equip/unequip vanilla au voisinage du player. Client-only
|
||||||
* player. Client-only ({@link net.minecraft.world.level.Level#playLocalSound}
|
* ({@link net.minecraft.world.level.Level#playLocalSound} n'émet aucun
|
||||||
* n'émet aucun paquet réseau).
|
* paquet réseau).
|
||||||
*
|
*
|
||||||
* <p>MVP : son {@link #DEFAULT_EQUIP_SOUND} pour tous les items.
|
* <p>MVP : son {@link #defaultEquipSound} pour tous les items. Le son
|
||||||
* Future : lire un éventuel champ {@code equip_sound} dans
|
* n'a plus besoin du stack ({@code ItemStack}) en entrée depuis le switch
|
||||||
* {@link DataDrivenItemDefinition} pour customiser per-item.</p>
|
* key-based : le callsite unequip ne possède plus le stack (disparu), et
|
||||||
|
* le son vanilla choisi ne varie pas per-item. Future évolution : lire un
|
||||||
|
* champ {@code equip_sound} dans {@link DataDrivenItemDefinition} via
|
||||||
|
* l'itemId + registry lookup, sans besoin du stack concret.</p>
|
||||||
*
|
*
|
||||||
* <p>Non testable unit (nécessite {@link Player} live + level) — skip
|
* <p>Non testable unit (nécessite {@link Player} live + level) — skip
|
||||||
* dans la suite de tests.</p>
|
* dans la suite de tests.</p>
|
||||||
*
|
*
|
||||||
* @param player le player source du son (position utilisée pour la
|
* @param player le player source du son (position utilisée pour la
|
||||||
* spatialisation 3D)
|
* 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) {
|
private static void playEquipSound(Player player) {
|
||||||
if (player == null || stack == null || stack.isEmpty()) return;
|
if (player == null) return;
|
||||||
player.level().playLocalSound(
|
player.level().playLocalSound(
|
||||||
player.getX(),
|
player.getX(),
|
||||||
player.getY(),
|
player.getY(),
|
||||||
@@ -497,7 +644,8 @@ public final class ClientRigEquipmentHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Vide le cache de snapshots {@link #LAST_EQUIPPED}. Utilisé dans :
|
* Vide les caches de snapshots {@link #LAST_EQUIPPED_KEYS} et
|
||||||
|
* {@link #FIRST_REBUILD_DONE}. Utilisé dans :
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li>Tests unitaires — pour isoler les cas entre tests (sinon un test
|
* <li>Tests unitaires — pour isoler les cas entre tests (sinon un test
|
||||||
* qui popule le cache pollue les suivants via l'état statique).</li>
|
* qui popule le cache pollue les suivants via l'état statique).</li>
|
||||||
@@ -509,7 +657,8 @@ public final class ClientRigEquipmentHandler {
|
|||||||
* <p>Package-private — pas destiné à la prod.</p>
|
* <p>Package-private — pas destiné à la prod.</p>
|
||||||
*/
|
*/
|
||||||
static void clearEquipmentSnapshotsForTesting() {
|
static void clearEquipmentSnapshotsForTesting() {
|
||||||
LAST_EQUIPPED.clear();
|
LAST_EQUIPPED_KEYS.clear();
|
||||||
|
FIRST_REBUILD_DONE.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
|
|||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
|
import java.util.EnumMap;
|
||||||
import java.util.IdentityHashMap;
|
import java.util.IdentityHashMap;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@@ -39,25 +40,32 @@ import com.tiedup.remake.v2.bondage.movement.MovementModifier;
|
|||||||
import com.tiedup.remake.v2.bondage.movement.MovementStyle;
|
import com.tiedup.remake.v2.bondage.movement.MovementStyle;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Tests unitaires pour {@link ClientRigEquipmentHandler} (P3-05).
|
* Tests unitaires pour {@link ClientRigEquipmentHandler} (P3-05 + P3-07).
|
||||||
*
|
*
|
||||||
* <h2>Stratégie de test</h2>
|
* <h2>Stratégie de test</h2>
|
||||||
* <p>L'entrée publique {@link ClientRigEquipmentHandler#rebuildBondageAnimations}
|
* <p>L'entrée publique {@link ClientRigEquipmentHandler#rebuildBondageAnimations}
|
||||||
* nécessite un {@code Player} réel + capability MC — non testable sans
|
* nécessite un {@code Player} réel + capability MC — non testable sans
|
||||||
* bootstrap. On vérifie uniquement son null-safety. La logique métier est
|
* bootstrap. On vérifie uniquement son null-safety. La logique métier est
|
||||||
* extraite dans deux méthodes package-private pures :</p>
|
* extraite dans plusieurs méthodes package-private pures :</p>
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li>{@link ClientRigEquipmentHandler#extractSortedDefinitions} —
|
* <li>{@link ClientRigEquipmentHandler#extractSortedDefinitions} —
|
||||||
* dedup identity, filter null, sort by posePriority ASC.</li>
|
* dedup identity, filter null, sort by posePriority ASC.</li>
|
||||||
* <li>{@link ClientRigEquipmentHandler#applyDefinitions} — itère +
|
* <li>{@link ClientRigEquipmentHandler#applyDefinitions} — itère +
|
||||||
* adder pour chaque binding non-null/non-vide, reset d'abord.</li>
|
* adder pour chaque binding non-null/non-vide, reset d'abord.</li>
|
||||||
|
* <li>{@link ClientRigEquipmentHandler#buildKeySnapshot} — snapshot
|
||||||
|
* stable {@code (region → itemId)} résistant aux re-sync.</li>
|
||||||
|
* <li>{@link ClientRigEquipmentHandler#diffKeys} — diff key-based,
|
||||||
|
* base du détection equip / unequip post-P3-07.</li>
|
||||||
|
* <li>{@link ClientRigEquipmentHandler#triggerOneshotById} — fire
|
||||||
|
* one-shot à partir d'un itemId (cas unequip post-P3-07 où le
|
||||||
|
* stack est parti).</li>
|
||||||
* </ul>
|
* </ul>
|
||||||
*
|
*
|
||||||
* <p>Les ItemStack sont remplacés par des {@link Object} dummy : la logique
|
* <p>Les ItemStack sont remplacés par des {@link Object} dummy : la logique
|
||||||
* ne lit aucune méthode d'instance, seule l'identité objet compte pour la
|
* ne lit aucune méthode d'instance, seule l'identité objet compte pour la
|
||||||
* dédup. Les {@link DataDrivenItemDefinition} sont construites via le
|
* dédup ou la key (itemId) pour le diff. Les {@link DataDrivenItemDefinition}
|
||||||
* {@link #makeDef} helper avec des defaults neutres (seuls posePriority +
|
* sont construites via le {@link #makeDef} helper avec des defaults neutres
|
||||||
* animations comptent pour ces tests).</p>
|
* (seuls posePriority + animations comptent pour ces tests).</p>
|
||||||
*/
|
*/
|
||||||
class ClientRigEquipmentHandlerTest {
|
class ClientRigEquipmentHandlerTest {
|
||||||
|
|
||||||
@@ -553,101 +561,260 @@ class ClientRigEquipmentHandlerTest {
|
|||||||
assertFalse(def.occupiedRegions().isEmpty());
|
assertFalse(def.occupiedRegions().isEmpty());
|
||||||
}
|
}
|
||||||
|
|
||||||
// ========== P3-07 : diffBy identity semantics ==========
|
// ========== P3-07 review : extractItemId ==========
|
||||||
|
|
||||||
/** Diff de deux sets vides → set vide, no throw. */
|
/** Stack {@code null} → id {@code null}, no throw. */
|
||||||
@Test
|
@Test
|
||||||
void diffBy_emptySets_returnsEmpty() {
|
void extractItemId_nullStack_returnsNull() {
|
||||||
Set<Object> result = ClientRigEquipmentHandler.diffBy(Set.of(), Set.of());
|
ResourceLocation id = ClientRigEquipmentHandler.extractItemId(
|
||||||
assertNotNull(result);
|
(Object) null,
|
||||||
assertTrue(result.isEmpty());
|
stack -> { throw new AssertionError("resolver ne doit pas etre appele"); }
|
||||||
|
);
|
||||||
|
assertNull(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Item legacy (resolver → null) → id {@code null}. */
|
||||||
|
@Test
|
||||||
|
void extractItemId_nonDataDrivenStack_returnsNull() {
|
||||||
|
Object stack = new Object();
|
||||||
|
ResourceLocation id = ClientRigEquipmentHandler.extractItemId(
|
||||||
|
stack,
|
||||||
|
any -> null
|
||||||
|
);
|
||||||
|
assertNull(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Stack data-driven → id extrait de la définition. */
|
||||||
|
@Test
|
||||||
|
void extractItemId_dataDrivenStack_returnsDefinitionId() {
|
||||||
|
Object stack = new Object();
|
||||||
|
DataDrivenItemDefinition def = makeDef(ITEM_A, 10, null);
|
||||||
|
ResourceLocation id = ClientRigEquipmentHandler.extractItemId(
|
||||||
|
stack,
|
||||||
|
any -> def
|
||||||
|
);
|
||||||
|
assertEquals(ITEM_A, id);
|
||||||
|
}
|
||||||
|
|
||||||
|
// ========== P3-07 review : buildKeySnapshot ==========
|
||||||
|
|
||||||
|
/** Map vide → snapshot vide. */
|
||||||
|
@Test
|
||||||
|
void buildKeySnapshot_emptyMap_returnsEmpty() {
|
||||||
|
Map<BodyRegionV2, ResourceLocation> snap =
|
||||||
|
ClientRigEquipmentHandler.buildKeySnapshot(
|
||||||
|
Collections.emptyMap(),
|
||||||
|
stack -> { throw new AssertionError("resolver ne doit pas etre appele"); }
|
||||||
|
);
|
||||||
|
assertNotNull(snap);
|
||||||
|
assertTrue(snap.isEmpty());
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Un item présent dans {@code a} mais absent de {@code b} apparaît dans le
|
* Map {@code (region → stack)} → {@code (region → itemId)} pour les
|
||||||
* diff. Cas {@code newlyEquipped = current \ previous}.
|
* stacks data-driven. Les entrées dont le stack n'est pas data-driven
|
||||||
|
* (resolver → null) sont omises du snapshot.
|
||||||
*/
|
*/
|
||||||
@Test
|
@Test
|
||||||
void diffBy_itemOnlyInA_appearsInResult() {
|
void buildKeySnapshot_extractsRegionToItemIdMap() {
|
||||||
Object item1 = new Object();
|
Object stackArms = new Object();
|
||||||
Object item2 = new Object();
|
Object stackNeck = new Object();
|
||||||
|
DataDrivenItemDefinition defArms = makeDef(ITEM_A, 10, null);
|
||||||
|
DataDrivenItemDefinition defNeck = makeDef(ITEM_B, 10, null);
|
||||||
|
|
||||||
Set<Object> a = Collections.newSetFromMap(new IdentityHashMap<>());
|
IdentityHashMap<Object, DataDrivenItemDefinition> resolverMap = new IdentityHashMap<>();
|
||||||
a.add(item1);
|
resolverMap.put(stackArms, defArms);
|
||||||
a.add(item2);
|
resolverMap.put(stackNeck, defNeck);
|
||||||
Set<Object> b = Collections.newSetFromMap(new IdentityHashMap<>());
|
|
||||||
b.add(item1);
|
|
||||||
|
|
||||||
Set<Object> result = ClientRigEquipmentHandler.diffBy(a, b);
|
Map<BodyRegionV2, Object> equipped = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
equipped.put(BodyRegionV2.ARMS, stackArms);
|
||||||
|
equipped.put(BodyRegionV2.NECK, stackNeck);
|
||||||
|
|
||||||
assertEquals(1, result.size(), "item2 nouveau doit etre detecte");
|
Map<BodyRegionV2, ResourceLocation> snap =
|
||||||
assertTrue(result.contains(item2));
|
ClientRigEquipmentHandler.buildKeySnapshot(equipped, resolverMap::get);
|
||||||
assertFalse(result.contains(item1), "item1 present dans les deux ne doit PAS etre dans le diff");
|
|
||||||
|
assertEquals(2, snap.size());
|
||||||
|
assertEquals(ITEM_A, snap.get(BodyRegionV2.ARMS));
|
||||||
|
assertEquals(ITEM_B, snap.get(BodyRegionV2.NECK));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Deux instances {@link Object#equals equals} mais identity-different →
|
* Les stacks non data-driven (legacy V2, resolver → null) ne sont pas
|
||||||
* considérées comme distinctes. Critical pour la convention "un stack =
|
* dans le snapshot — ainsi le diff ne les considère jamais, et aucun
|
||||||
* une occurrence".
|
* sound ne joue pour leur equip/unequip (cohérent avec l'usage : les
|
||||||
|
* items sans JSON n'ont aucun binding, pas d'intérêt à sounder).
|
||||||
*/
|
*/
|
||||||
@Test
|
@Test
|
||||||
void diffBy_identitySemantics_equalsObjectsAreDistinct() {
|
void buildKeySnapshot_skipsNonDataDrivenStacks() {
|
||||||
// Deux String content-equal mais avec new String(...) pour garantir
|
Object stackArms = new Object();
|
||||||
// des instances distinctes (String pool interning contourné).
|
Object stackLegs = new Object(); // non data-driven
|
||||||
String a1 = new String("same");
|
DataDrivenItemDefinition defArms = makeDef(ITEM_A, 10, null);
|
||||||
String b1 = new String("same");
|
|
||||||
assertTrue(a1.equals(b1));
|
|
||||||
assertFalse(a1 == b1, "guard : les deux Strings doivent etre des instances distinctes");
|
|
||||||
|
|
||||||
Set<String> setA = Collections.newSetFromMap(new IdentityHashMap<>());
|
IdentityHashMap<Object, DataDrivenItemDefinition> resolverMap = new IdentityHashMap<>();
|
||||||
setA.add(a1);
|
resolverMap.put(stackArms, defArms);
|
||||||
Set<String> setB = Collections.newSetFromMap(new IdentityHashMap<>());
|
// stackLegs n'est pas dans le resolver → resolver.apply(stackLegs) == null
|
||||||
setB.add(b1);
|
|
||||||
|
|
||||||
Set<String> result = ClientRigEquipmentHandler.diffBy(setA, setB);
|
Map<BodyRegionV2, Object> equipped = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
equipped.put(BodyRegionV2.ARMS, stackArms);
|
||||||
|
equipped.put(BodyRegionV2.LEGS, stackLegs);
|
||||||
|
|
||||||
assertEquals(1, result.size(),
|
Map<BodyRegionV2, ResourceLocation> snap =
|
||||||
"deux strings equals() mais != doivent etre traites comme distincts (identity)");
|
ClientRigEquipmentHandler.buildKeySnapshot(equipped, resolverMap::get);
|
||||||
assertSame(a1, result.iterator().next(),
|
|
||||||
"le diff doit retenir l'instance de setA");
|
assertEquals(1, snap.size(), "stack legacy doit etre skip");
|
||||||
|
assertEquals(ITEM_A, snap.get(BodyRegionV2.ARMS));
|
||||||
|
assertFalse(snap.containsKey(BodyRegionV2.LEGS),
|
||||||
|
"LEGS ne doit pas etre dans le snapshot (item non data-driven)");
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Quand a == b (mêmes instances), diff vide. */
|
// ========== P3-07 review : diffKeys ==========
|
||||||
|
|
||||||
|
/** Deux maps vides → diff vide. */
|
||||||
@Test
|
@Test
|
||||||
void diffBy_identicalSets_returnsEmpty() {
|
void diffKeys_emptyMaps_returnsEmpty() {
|
||||||
Object item = new Object();
|
Map<BodyRegionV2, ResourceLocation> diff =
|
||||||
Set<Object> a = Collections.newSetFromMap(new IdentityHashMap<>());
|
ClientRigEquipmentHandler.diffKeys(Map.of(), Map.of());
|
||||||
a.add(item);
|
assertNotNull(diff);
|
||||||
Set<Object> b = Collections.newSetFromMap(new IdentityHashMap<>());
|
assertTrue(diff.isEmpty());
|
||||||
b.add(item);
|
|
||||||
|
|
||||||
Set<Object> result = ClientRigEquipmentHandler.diffBy(a, b);
|
|
||||||
assertTrue(result.isEmpty(), "meme instance dans les deux → diff vide");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ========== P3-07 : uniqueByIdentity ==========
|
/**
|
||||||
|
* previous={ARMS→cuffs}, current={ARMS→cuffs, NECK→collar} →
|
||||||
/** Iterable avec duplicats identity → un seul élément retenu. */
|
* {@code diffKeys(current, previous)} = {NECK→collar} — nouvelle entrée
|
||||||
|
* détectée.
|
||||||
|
*/
|
||||||
@Test
|
@Test
|
||||||
void uniqueByIdentity_duplicateIdentity_dedups() {
|
void diffKeys_newEntry_returnsNewlyEquipped() {
|
||||||
Object item = new Object();
|
Map<BodyRegionV2, ResourceLocation> previous = new EnumMap<>(BodyRegionV2.class);
|
||||||
Set<Object> result =
|
previous.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
ClientRigEquipmentHandler.uniqueByIdentity(List.of(item, item, item));
|
|
||||||
assertEquals(1, result.size());
|
Map<BodyRegionV2, ResourceLocation> current = new EnumMap<>(BodyRegionV2.class);
|
||||||
assertSame(item, result.iterator().next());
|
current.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
|
current.put(BodyRegionV2.NECK, ITEM_B);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> diff =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(current, previous);
|
||||||
|
|
||||||
|
assertEquals(1, diff.size());
|
||||||
|
assertEquals(ITEM_B, diff.get(BodyRegionV2.NECK));
|
||||||
|
assertFalse(diff.containsKey(BodyRegionV2.ARMS),
|
||||||
|
"ARMS inchangé ne doit PAS etre dans le diff");
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Les null de l'iterable sont skippés silencieusement. */
|
/**
|
||||||
|
* previous={ARMS→cuffs, NECK→collar}, current={ARMS→cuffs} →
|
||||||
|
* {@code diffKeys(previous, current)} = {NECK→collar} — entrée
|
||||||
|
* déséquipée détectée.
|
||||||
|
*/
|
||||||
@Test
|
@Test
|
||||||
void uniqueByIdentity_nullsSkipped() {
|
void diffKeys_removedEntry_returnsNewlyUnequipped() {
|
||||||
Object item = new Object();
|
Map<BodyRegionV2, ResourceLocation> previous = new EnumMap<>(BodyRegionV2.class);
|
||||||
List<Object> input = Arrays.asList(null, item, null);
|
previous.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
Set<Object> result = ClientRigEquipmentHandler.uniqueByIdentity(input);
|
previous.put(BodyRegionV2.NECK, ITEM_B);
|
||||||
assertEquals(1, result.size());
|
|
||||||
assertSame(item, result.iterator().next());
|
Map<BodyRegionV2, ResourceLocation> current = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
current.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> diff =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(previous, current);
|
||||||
|
|
||||||
|
assertEquals(1, diff.size());
|
||||||
|
assertEquals(ITEM_B, diff.get(BodyRegionV2.NECK));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ========== P3-07 : triggerOneshot ==========
|
/**
|
||||||
|
* Remplacement in-place sur le même slot : previous={ARMS→cuffs},
|
||||||
|
* current={ARMS→shackles} → (a) diff current\previous = {ARMS→shackles},
|
||||||
|
* (b) diff previous\current = {ARMS→cuffs}. Les deux sens retournent
|
||||||
|
* l'entrée différente.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void diffKeys_sameRegionDifferentItem_returnsBoth() {
|
||||||
|
Map<BodyRegionV2, ResourceLocation> previous = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
previous.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> current = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
current.put(BodyRegionV2.ARMS, ITEM_B);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyEquipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(current, previous);
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyUnequipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(previous, current);
|
||||||
|
|
||||||
|
assertEquals(1, newlyEquipped.size());
|
||||||
|
assertEquals(ITEM_B, newlyEquipped.get(BodyRegionV2.ARMS),
|
||||||
|
"remplacement : shackles est newly equipped sur ARMS");
|
||||||
|
|
||||||
|
assertEquals(1, newlyUnequipped.size());
|
||||||
|
assertEquals(ITEM_A, newlyUnequipped.get(BodyRegionV2.ARMS),
|
||||||
|
"remplacement : cuffs est newly unequipped sur ARMS");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* <b>FIX PRINCIPAL P3-07 (BUG-001)</b> — previous={ARMS→cuffs},
|
||||||
|
* current={ARMS→cuffs} avec des instances {@link ResourceLocation}
|
||||||
|
* distinctes (scénario {@link net.minecraft.world.item.ItemStack#of}
|
||||||
|
* qui recrée une ItemStack après deserializeNBT). Le diff doit être
|
||||||
|
* vide car les itemIds sont {@code equals()} même si les ItemStacks
|
||||||
|
* sous-jacents étaient différents. Sans ce comportement, chaque
|
||||||
|
* {@code V2EquipmentHelper.sync()} fire un son à chaque item — 19
|
||||||
|
* call-sites server-side au total.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void diffKeys_sameRegionSameItem_noChange() {
|
||||||
|
// ResourceLocation instances distinctes mais content-equal
|
||||||
|
ResourceLocation prevId =
|
||||||
|
ResourceLocation.fromNamespaceAndPath("tiedup", "cuffs");
|
||||||
|
ResourceLocation currId =
|
||||||
|
ResourceLocation.fromNamespaceAndPath("tiedup", "cuffs");
|
||||||
|
assertEquals(prevId, currId, "sanity : les deux ResourceLocation sont equals");
|
||||||
|
// Note: Forge peut intern-er les ResourceLocation, donc == peut aussi
|
||||||
|
// etre true. L'important est que equals() l'est (garanti par le
|
||||||
|
// contract de ResourceLocation).
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> previous = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
previous.put(BodyRegionV2.ARMS, prevId);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> current = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
current.put(BodyRegionV2.ARMS, currId);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyEquipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(current, previous);
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyUnequipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(previous, current);
|
||||||
|
|
||||||
|
assertTrue(newlyEquipped.isEmpty(),
|
||||||
|
"meme item sur meme region → pas de newly equipped (BUG-001 fix)");
|
||||||
|
assertTrue(newlyUnequipped.isEmpty(),
|
||||||
|
"meme item sur meme region → pas de newly unequipped (BUG-001 fix)");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Cas bizarre mais possible : un même itemId apparaît dans deux régions
|
||||||
|
* différentes entre previous et current (ex. un item multi-region dont
|
||||||
|
* le set d'occupied regions change après refactor data-pack). Le diff
|
||||||
|
* doit traiter chaque region indépendamment.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void diffKeys_sameItemDifferentRegion_treatsAsMovement() {
|
||||||
|
Map<BodyRegionV2, ResourceLocation> previous = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
previous.put(BodyRegionV2.ARMS, ITEM_A);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> current = new EnumMap<>(BodyRegionV2.class);
|
||||||
|
current.put(BodyRegionV2.LEGS, ITEM_A);
|
||||||
|
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyEquipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(current, previous);
|
||||||
|
Map<BodyRegionV2, ResourceLocation> newlyUnequipped =
|
||||||
|
ClientRigEquipmentHandler.diffKeys(previous, current);
|
||||||
|
|
||||||
|
assertEquals(1, newlyEquipped.size());
|
||||||
|
assertEquals(ITEM_A, newlyEquipped.get(BodyRegionV2.LEGS));
|
||||||
|
assertEquals(1, newlyUnequipped.size());
|
||||||
|
assertEquals(ITEM_A, newlyUnequipped.get(BodyRegionV2.ARMS));
|
||||||
|
}
|
||||||
|
|
||||||
|
// ========== P3-07 review : triggerOneshotById ==========
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fake {@link ClientRigEquipmentHandler.OneshotPlayer} qui capture chaque
|
* Fake {@link ClientRigEquipmentHandler.OneshotPlayer} qui capture chaque
|
||||||
@@ -666,6 +833,110 @@ class ClientRigEquipmentHandlerTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** {@code itemId == null} → skip silencieux, aucun trigger. */
|
||||||
|
@Test
|
||||||
|
void triggerOneshotById_nullItemId_skipsSilently() {
|
||||||
|
CapturingOneshotPlayer player = new CapturingOneshotPlayer();
|
||||||
|
|
||||||
|
ClientRigEquipmentHandler.triggerOneshotById(
|
||||||
|
player,
|
||||||
|
/* itemId */ null,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
id -> { throw new AssertionError("resolver ne doit pas etre appele pour null"); },
|
||||||
|
PASSTHROUGH_RESOLVER
|
||||||
|
);
|
||||||
|
|
||||||
|
assertTrue(player.calls.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Registry lookup retourne null (itemId inconnu) → skip silencieux. */
|
||||||
|
@Test
|
||||||
|
void triggerOneshotById_unknownItemId_skipsSilently() {
|
||||||
|
CapturingOneshotPlayer player = new CapturingOneshotPlayer();
|
||||||
|
|
||||||
|
ClientRigEquipmentHandler.triggerOneshotById(
|
||||||
|
player,
|
||||||
|
ITEM_A,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
id -> null, // inconnu
|
||||||
|
PASSTHROUGH_RESOLVER
|
||||||
|
);
|
||||||
|
|
||||||
|
assertTrue(player.calls.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Définition sans {@code animations} → skip silencieux. */
|
||||||
|
@Test
|
||||||
|
void triggerOneshotById_nullBindings_skipsSilently() {
|
||||||
|
CapturingOneshotPlayer player = new CapturingOneshotPlayer();
|
||||||
|
DataDrivenItemDefinition def = makeDef(ITEM_A, 10, /* animations */ null);
|
||||||
|
|
||||||
|
ClientRigEquipmentHandler.triggerOneshotById(
|
||||||
|
player,
|
||||||
|
ITEM_A,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
id -> def,
|
||||||
|
PASSTHROUGH_RESOLVER
|
||||||
|
);
|
||||||
|
|
||||||
|
assertTrue(player.calls.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
/** {@code onUnequip == null} dans les bindings → skip silencieux. */
|
||||||
|
@Test
|
||||||
|
void triggerOneshotById_onUnequipNull_skipsSilently() {
|
||||||
|
CapturingOneshotPlayer player = new CapturingOneshotPlayer();
|
||||||
|
AnimationBindings bindings = new AnimationBindings(
|
||||||
|
Map.of(LivingMotions.WALK, ANIM_WALK_A),
|
||||||
|
/* onEquip */ null,
|
||||||
|
/* onUnequip */ null
|
||||||
|
);
|
||||||
|
DataDrivenItemDefinition def = makeDef(ITEM_A, 10, bindings);
|
||||||
|
|
||||||
|
ClientRigEquipmentHandler.triggerOneshotById(
|
||||||
|
player,
|
||||||
|
ITEM_A,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
id -> def,
|
||||||
|
PASSTHROUGH_RESOLVER
|
||||||
|
);
|
||||||
|
|
||||||
|
assertTrue(player.calls.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Happy path : bindings.onUnequip non-null → oneshotPlayer appelé avec
|
||||||
|
* l'accessor résolu + transition time canonique.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void triggerOneshotById_happyPath_callsPlayerWithResolvedAccessor() {
|
||||||
|
CapturingOneshotPlayer player = new CapturingOneshotPlayer();
|
||||||
|
|
||||||
|
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.triggerOneshotById(
|
||||||
|
player,
|
||||||
|
ITEM_A,
|
||||||
|
AnimationBindings::onUnequip,
|
||||||
|
id -> def,
|
||||||
|
PASSTHROUGH_RESOLVER
|
||||||
|
);
|
||||||
|
|
||||||
|
assertEquals(1, player.calls.size());
|
||||||
|
assertEquals(unequipId, player.calls.get(0).getKey());
|
||||||
|
assertEquals(0.15F, player.calls.get(0).getValue(), 0.0001F,
|
||||||
|
"transition time doit etre 0.15s (ONESHOT_TRANSITION_TIME)");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ========== P3-07 legacy : triggerOneshot (stack-based, still used for equip) ==========
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Stack sans {@link DataDrivenItemDefinition} (resolver → null) → skip
|
* Stack sans {@link DataDrivenItemDefinition} (resolver → null) → skip
|
||||||
* silencieusement, aucun appel au oneshotPlayer.
|
* silencieusement, aucun appel au oneshotPlayer.
|
||||||
|
|||||||
Reference in New Issue
Block a user