P3-05 review fixes : deterministic tiebreaker + reset contract doc

HIGH RISK-001 : secondary comparator on def.id() prevents UB on
equal-priority items — no longer depends on BodyRegionV2 enum order.
New test asserts lexicographic ordering.

HIGH RISK-002 : resetLivingAnimations restores defaultLivingAnimations
snapshot. Javadoc now warns against calling setCurrentMotionsAsDefault
anywhere in the TiedUp pipeline (would leak custom bindings into
defaults, breaking unbind). Grep confirms no call site exists today.

Polish : rename tautological null test, precise null-safety doc.
This commit is contained in:
notevil
2026-04-23 22:11:33 +02:00
parent 9a31f21b55
commit e37dad18aa
2 changed files with 106 additions and 8 deletions

View File

@@ -245,6 +245,44 @@ class ClientRigEquipmentHandlerTest {
assertSame(high, result.get(1), "posePriority=20 doit sortir en dernier (ASC)");
}
/**
* Deux items distincts avec la MÊME {@code posePriority} doivent être
* triés de manière déterministe via le tiebreaker secondaire
* {@code id.toString()} lexicographique. Ainsi l'ordre final ne dépend
* pas de {@link BodyRegionV2} declaration order (invariant fragile
* qu'un refactor pourrait casser silencieusement).
*/
@Test
void extractSortedDefinitions_equalPriority_usesIdLexicographicTiebreaker() {
ResourceLocation idAlpha =
ResourceLocation.fromNamespaceAndPath("tiedup", "alpha_cuffs");
ResourceLocation idBeta =
ResourceLocation.fromNamespaceAndPath("tiedup", "beta_cuffs");
ResourceLocation idCharlie =
ResourceLocation.fromNamespaceAndPath("tiedup", "charlie_cuffs");
DataDrivenItemDefinition alpha = makeDef(idAlpha, /* priority */ 10, null);
DataDrivenItemDefinition beta = makeDef(idBeta, 10, null);
DataDrivenItemDefinition charlie = makeDef(idCharlie, 10, null);
// Input volontairement dans l'ordre inverse — [charlie, alpha, beta]
// — pour vérifier que le sort le réorganise bien sur id
// lexicographique et non sur ordre d'insertion.
List<DataDrivenItemDefinition> sorted =
ClientRigEquipmentHandler.extractSortedDefinitions(
List.of(charlie, alpha, beta),
Function.identity()
);
assertEquals(3, sorted.size());
assertSame(alpha, sorted.get(0),
"alpha_cuffs doit sortir en premier (id.toString() min)");
assertSame(beta, sorted.get(1),
"beta_cuffs doit sortir au milieu");
assertSame(charlie, sorted.get(2),
"charlie_cuffs doit sortir en dernier (id.toString() max)");
}
/** Un {@code null} dans l'iterable est silencieusement skip. */
@Test
void extractSortedDefinitions_nullItemInList_skips() {
@@ -462,19 +500,25 @@ class ClientRigEquipmentHandlerTest {
}
/**
* Sanity : si animResolver retourne un accessor pour l'ID fourni, le
* handler ne suppose pas de non-nullité — contract de
* {@link com.tiedup.remake.rig.TiedUpAnimationRegistry#resolveWithFallback}
* garantit non-null en prod, mais on vérifie qu'aucune assertion plus
* stricte n'a été introduite dans le handler.
* Sanity : si {@code animResolver} retourne {@code null} pour un id, le
* handler le forwarde tel quel à l'adder sans dereferencer. La gestion
* réelle des accessors null est la responsabilité de
* {@link com.tiedup.remake.rig.anim.client.ClientAnimator#addLivingAnimation}
* (out of scope de ce test — seule l'absence de NPE côté handler est
* vérifiée ici).
*
* <p>En prod, {@link com.tiedup.remake.rig.TiedUpAnimationRegistry#resolveWithFallback}
* garantit non-null (fallback {@code EMPTY_ANIMATION} + WARN). Le test
* simule un resolver "buggy" pour s'assurer qu'aucune assertion plus
* stricte n'a été introduite dans le handler côté TiedUp.</p>
*/
@Test
void applyDefinitions_respectsAnimResolverContract() {
void applyDefinitions_nullAnimAccessor_forwardsToAdder() {
AtomicInteger resolverCalls = new AtomicInteger();
ClientRigEquipmentHandler.LivingAnimationAdder adder = (motion, accessor) -> {
// accessor vient du resolver, peut être null si resolver est buggy —
// le handler ne doit pas le dereferencer lui-même
assertNull(accessor, "resolver renvoie null ici");
assertNull(accessor, "resolver renvoie null ici, le handler forward tel quel");
};
AnimationBindings bindings = new AnimationBindings(