From e15ab7b83176b16bb6c1b0c951ff9f8a347fdf92 Mon Sep 17 00:00:00 2001 From: notevil Date: Fri, 24 Apr 2026 01:05:33 +0200 Subject: [PATCH] P3-19 review polish : drop redundant distinct + reorder keybind guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../tiedup/remake/client/ModKeybindings.java | 22 +++++++++++-------- .../remake/rig/debug/RigDebugOverlay.java | 8 +++---- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/tiedup/remake/client/ModKeybindings.java b/src/main/java/com/tiedup/remake/client/ModKeybindings.java index 4be2114..ca06670 100644 --- a/src/main/java/com/tiedup/remake/client/ModKeybindings.java +++ b/src/main/java/com/tiedup/remake/client/ModKeybindings.java @@ -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" - ); - } } /** diff --git a/src/main/java/com/tiedup/remake/rig/debug/RigDebugOverlay.java b/src/main/java/com/tiedup/remake/rig/debug/RigDebugOverlay.java index 3b45688..4e77c67 100644 --- a/src/main/java/com/tiedup/remake/rig/debug/RigDebugOverlay.java +++ b/src/main/java/com/tiedup/remake/rig/debug/RigDebugOverlay.java @@ -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 lines, Player player) { Map 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)); }