From eb7f06bfc889d4c626fd9f7a19a6bbe31d461424 Mon Sep 17 00:00:00 2001 From: NotEvil Date: Tue, 14 Apr 2026 16:38:09 +0200 Subject: [PATCH] fix(D-01/A): 3 review bugs + null guards (BUG-001, BUG-002, BUG-003, RISK-003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG-001: TyingInteractionHelper swap now checks V2EquipResult — on failure, rolls back by re-equipping the old bind instead of leaving target untied BUG-002: OwnershipComponent.onUnequipped no longer double-calls unregisterWearer — onCollarRemoved already handles it. Suppressed-alert path calls unregister directly since onCollarRemoved is skipped. BUG-003: PacketSelfBondage handleV2SelfBind now clears completed tying task from PlayerBindState to prevent blocking future tying interactions RISK-003: StruggleCollar get/setResistanceState null-guard on player --- .../selfbondage/PacketSelfBondage.java | 6 ++++ .../remake/state/struggle/StruggleCollar.java | 2 ++ .../v2/bondage/TyingInteractionHelper.java | 18 +++++++--- .../bondage/component/OwnershipComponent.java | 33 ++++++++++--------- 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/tiedup/remake/network/selfbondage/PacketSelfBondage.java b/src/main/java/com/tiedup/remake/network/selfbondage/PacketSelfBondage.java index e65c9d3..51f48ee 100644 --- a/src/main/java/com/tiedup/remake/network/selfbondage/PacketSelfBondage.java +++ b/src/main/java/com/tiedup/remake/network/selfbondage/PacketSelfBondage.java @@ -291,6 +291,12 @@ public class PacketSelfBondage { if (playerState.getCurrentTyingTask() == newTask) { newTask.update(); } + + // Clear completed task to prevent blocking future tying interactions + TyingTask activeTask = playerState.getCurrentTyingTask(); + if (activeTask != null && activeTask.isStopped()) { + playerState.setCurrentTyingTask(null); + } } private static void handleV2SelfAccessory( diff --git a/src/main/java/com/tiedup/remake/state/struggle/StruggleCollar.java b/src/main/java/com/tiedup/remake/state/struggle/StruggleCollar.java index 3432141..b75a5a7 100644 --- a/src/main/java/com/tiedup/remake/state/struggle/StruggleCollar.java +++ b/src/main/java/com/tiedup/remake/state/struggle/StruggleCollar.java @@ -47,6 +47,7 @@ public class StruggleCollar extends StruggleState { @Override protected int getResistanceState(PlayerBindState state) { Player player = state.getPlayer(); + if (player == null) return 0; ItemStack collar = V2EquipmentHelper.getInRegion(player, BodyRegionV2.NECK); if (collar.isEmpty() || !CollarHelper.isCollar(collar)) return 0; @@ -66,6 +67,7 @@ public class StruggleCollar extends StruggleState { @Override protected void setResistanceState(PlayerBindState state, int resistance) { Player player = state.getPlayer(); + if (player == null) return; ItemStack collar = V2EquipmentHelper.getInRegion(player, BodyRegionV2.NECK); if (collar.isEmpty() || !CollarHelper.isCollar(collar)) return; diff --git a/src/main/java/com/tiedup/remake/v2/bondage/TyingInteractionHelper.java b/src/main/java/com/tiedup/remake/v2/bondage/TyingInteractionHelper.java index 99350de..f75a08c 100644 --- a/src/main/java/com/tiedup/remake/v2/bondage/TyingInteractionHelper.java +++ b/src/main/java/com/tiedup/remake/v2/bondage/TyingInteractionHelper.java @@ -9,6 +9,7 @@ import com.tiedup.remake.tasks.V2TyingPlayerTask; import com.tiedup.remake.util.KidnappedHelper; import com.tiedup.remake.v2.BodyRegionV2; import com.tiedup.remake.v2.bondage.capability.V2EquipmentHelper; +import com.tiedup.remake.v2.bondage.V2EquipResult; import net.minecraft.server.level.ServerPlayer; import net.minecraft.world.InteractionHand; import net.minecraft.world.InteractionResult; @@ -50,11 +51,18 @@ public final class TyingInteractionHelper { if (stack.isEmpty()) return InteractionResult.PASS; ItemStack oldBind = V2EquipmentHelper.unequipFromRegion(target, BodyRegionV2.ARMS); if (!oldBind.isEmpty()) { - V2EquipmentHelper.equipItem(target, stack.copy()); - stack.shrink(1); - target.spawnAtLocation(oldBind); - TiedUpMod.LOGGER.debug("[TyingInteraction] Swapped bind on {}", target.getName().getString()); - return InteractionResult.SUCCESS; + V2EquipResult result = V2EquipmentHelper.equipItem(target, stack.copy()); + if (result.isSuccess()) { + stack.shrink(1); + target.spawnAtLocation(oldBind); + TiedUpMod.LOGGER.debug("[TyingInteraction] Swapped bind on {}", target.getName().getString()); + return InteractionResult.SUCCESS; + } else { + // Equip failed — rollback: re-equip old bind + V2EquipmentHelper.equipItem(target, oldBind); + TiedUpMod.LOGGER.debug("[TyingInteraction] Swap failed, rolled back old bind on {}", target.getName().getString()); + return InteractionResult.PASS; + } } return InteractionResult.PASS; } diff --git a/src/main/java/com/tiedup/remake/v2/bondage/component/OwnershipComponent.java b/src/main/java/com/tiedup/remake/v2/bondage/component/OwnershipComponent.java index 2d6e632..07be4a8 100644 --- a/src/main/java/com/tiedup/remake/v2/bondage/component/OwnershipComponent.java +++ b/src/main/java/com/tiedup/remake/v2/bondage/component/OwnershipComponent.java @@ -67,23 +67,26 @@ public class OwnershipComponent implements IItemComponent { if (entity.level().isClientSide()) return; if (!(entity.level() instanceof ServerLevel serverLevel)) return; - // Alert kidnappers if removal wasn't suppressed + // Alert kidnappers + unregister from CollarRegistry + // onCollarRemoved handles both the alert AND the unregister call internally, + // so we do NOT call registry.unregisterWearer() separately to avoid double unregister. if (!CollarHelper.isRemovalAlertSuppressed()) { ItemCollar.onCollarRemoved(entity, true); - } - - try { - CollarRegistry registry = CollarRegistry.get(serverLevel); - registry.unregisterWearer(entity.getUUID()); - TiedUpMod.LOGGER.debug( - "[OwnershipComponent] Unregistered collar for {}", - entity.getName().getString() - ); - } catch (Exception e) { - TiedUpMod.LOGGER.warn( - "[OwnershipComponent] Failed to unregister collar for {}: {}", - entity.getName().getString(), e.getMessage() - ); + } else { + // Suppressed alert path: still need to unregister, just skip the alert + try { + CollarRegistry registry = CollarRegistry.get(serverLevel); + registry.unregisterWearer(entity.getUUID()); + TiedUpMod.LOGGER.debug( + "[OwnershipComponent] Unregistered collar for {} (alert suppressed)", + entity.getName().getString() + ); + } catch (Exception e) { + TiedUpMod.LOGGER.warn( + "[OwnershipComponent] Failed to unregister collar for {}: {}", + entity.getName().getString(), e.getMessage() + ); + } } }