P3-19 review polish : drop redundant distinct + reorder keybind guard

MEDIUM SMELL-01 reviewer : V2EquipmentHelper.getAllEquipped already
identity-dedupes (IdentityHashMap upstream in V2BondageEquipment:84).
The extra .mapToInt(identityHashCode).distinct() was dead defensive
code. Simplified to direct .count().

MEDIUM SMELL-02 reviewer : keybind guard 'mc.player == null || mc.level
== null' fired BEFORE RIG_DEBUG_KEY consumeClick loop. Main menu clicks
were queued and flushed on world-join (phantom toggle). Move toggle
consumption BEFORE the guard — it's a client-side boolean flip, needs
no world context.

LOW RISK-01 skipped : pin test on Layer.toString() format would require
either registry init (TiedUpRigRegistry.EMPTY_ANIMATION in AnimationPlayer
ctor) or Mockito ; test env doesn't currently init the RIG registry and
layerPriorityName() already has a defensive "?" fallback. Gameday check
is a tighter feedback loop than a flaky unit test here.
This commit is contained in:
notevil
2026-04-24 01:05:33 +02:00
parent cae3572488
commit e15ab7b831
2 changed files with 17 additions and 13 deletions

View File

@@ -225,6 +225,19 @@ public class ModKeybindings {
return;
}
// RIG debug overlay toggle (P3-19) — F6 by default.
// Consumed BEFORE the player/level guard: it's a pure client-side
// boolean flip, needs no world context. Otherwise clicks queued on
// main menu / loading screen would flush on world-join → phantom
// toggle (reviewer SMELL-02).
while (RIG_DEBUG_KEY.consumeClick()) {
boolean nowOn = com.tiedup.remake.rig.debug.RigDebugOverlay.toggle();
TiedUpMod.LOGGER.debug(
"[CLIENT] RIG debug overlay: {}",
nowOn ? "ENABLED" : "DISABLED"
);
}
Minecraft mc = Minecraft.getInstance();
if (mc.player == null || mc.level == null) {
return;
@@ -293,15 +306,6 @@ public class ModKeybindings {
);
}
}
// RIG debug overlay toggle (P3-19) — F6 by default
while (RIG_DEBUG_KEY.consumeClick()) {
boolean nowOn = com.tiedup.remake.rig.debug.RigDebugOverlay.toggle();
TiedUpMod.LOGGER.debug(
"[CLIENT] RIG debug overlay: {}",
nowOn ? "ENABLED" : "DISABLED"
);
}
}
/**

View File

@@ -215,16 +215,16 @@ public final class RigDebugOverlay {
/**
* Compte les items data-driven équipés. Un armbinder occupant 3 régions
* compte pour 1 (dédup identity via {@code distinct()} sur
* {@code identityHashCode} des stacks).
* compte pour 1 — la dédup identity est déjà garantie upstream par
* {@link com.tiedup.remake.v2.bondage.capability.V2BondageEquipment#getAllEquipped()}
* via {@code IdentityHashMap}. Pas de {@code .distinct()} ici : serait
* du dead defensive code.
*/
private static void appendItemCountLine(List<String> lines, Player player) {
Map<BodyRegionV2, ItemStack> equipped = V2EquipmentHelper.getAllEquipped(player);
long count = equipped.values().stream()
.filter(s -> s != null && !s.isEmpty())
.filter(s -> DataDrivenItemRegistry.get(s) != null)
.mapToInt(System::identityHashCode)
.distinct()
.count();
lines.add(String.format("items: §e%d§r", count));
}