Fix clicking protection blocks next to a door also opening that door. F…

…ixes #208
rutgerkok · Dec 7, 2024 · c97d7ac · c97d7ac
1 parent a385557
commit c97d7ac
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 55 deletions.
diff --git a/src/main/java/nl/rutgerkok/blocklocker/impl/event/InteractListener.java b/src/main/java/nl/rutgerkok/blocklocker/impl/event/InteractListener.java
@@ -3,6 +3,7 @@
 import java.util.Optional;
 import java.util.Set;
 
+import nl.rutgerkok.blocklocker.*;
 import org.bukkit.Bukkit;
 import org.bukkit.GameMode;
 import org.bukkit.Location;
@@ -33,10 +34,6 @@
 
 import com.google.common.collect.ImmutableSet;
 
-import nl.rutgerkok.blocklocker.AttackType;
-import nl.rutgerkok.blocklocker.Permissions;
-import nl.rutgerkok.blocklocker.ProtectionSign;
-import nl.rutgerkok.blocklocker.SignType;
 import nl.rutgerkok.blocklocker.Translator.Translation;
 import nl.rutgerkok.blocklocker.event.PlayerProtectionCreateEvent;
 import nl.rutgerkok.blocklocker.impl.BlockLockerPluginImpl;
@@ -48,7 +45,7 @@
 
 public final class InteractListener extends EventListener {
 
-    private static Set<BlockFace> AUTOPLACE_BLOCK_FACES = ImmutableSet.of(BlockFace.NORTH, BlockFace.EAST,
+    private static final Set<BlockFace> AUTOPLACE_BLOCK_FACES = ImmutableSet.of(BlockFace.NORTH, BlockFace.EAST,
             BlockFace.SOUTH, BlockFace.WEST, BlockFace.UP);
 
     public InteractListener(BlockLockerPluginImpl plugin) {
@@ -85,18 +82,10 @@ private boolean canBuildInMode(GameMode gameMode) {
         if (gameMode == null) {
             return false;
         }
-        switch (gameMode) {
-            case ADVENTURE:
-                return false;
-            case CREATIVE:
-                return true;
-            case SPECTATOR:
-                return false;
-            case SURVIVAL:
-                return true;
-            default:
-                return false; // Speculative
-        }
+        return switch (gameMode) {
+            case ADVENTURE, SPECTATOR -> false;
+            case CREATIVE, SURVIVAL -> true;
+        };
     }
 
     private boolean checkAllowed(Player player, Protection protection, boolean clickedSign) {
@@ -194,7 +183,8 @@ private void handleAllowed(PlayerInteractEvent event, Protection protection, boo
         }
 
         // Open (double/trap/fence) doors manually
-        boolean clickedMainBlock = plugin.getChestSettings().canProtect(clickedBlock);
+        boolean clickedMainBlock = plugin.getProtectionFinder().findProtection(clickedBlock, SearchMode.NO_SUPPORTING_BLOCKS)
+                .filter(p -> p.equals(protection)).isPresent();
         if (protection.canBeOpened() && !isSneakPlacing(player) && clickedMainBlock) {
             event.setCancelled(true);
 
@@ -299,14 +289,17 @@ public void onPlayerInteract(PlayerInteractEvent event) {
 
         Player player = event.getPlayer();
         Block block = event.getClickedBlock();
+        if (block == null) {
+            return;
+        }
         Material material = block.getType();
         boolean clickedSign = Tag.STANDING_SIGNS.isTagged(material) || Tag.WALL_SIGNS.isTagged(material);
         // When using the offhand check, access checks must still be performed,
         // but no messages must be sent
         boolean usedOffHand = event.getHand() == EquipmentSlot.OFF_HAND;
         Optional<Protection> protection = plugin.getProtectionFinder().findProtection(block);
 
-        if (!protection.isPresent()) {
+        if (protection.isEmpty()) {
             if (tryPlaceSign(event.getPlayer(), block, event.getBlockFace(), event.getHand(), SignType.PRIVATE)) {
                 event.setCancelled(true);
             }

diff --git a/src/main/java/nl/rutgerkok/blocklocker/impl/protection/AbstractProtection.java b/src/main/java/nl/rutgerkok/blocklocker/impl/protection/AbstractProtection.java
@@ -1,58 +1,34 @@
 package nl.rutgerkok.blocklocker.impl.protection;
 
-import java.util.Collection;
-import java.util.Date;
-import java.util.List;
-import java.util.Optional;
-
-import org.apache.commons.lang.Validate;
-import org.bukkit.Material;
-import org.bukkit.block.data.BlockData;
-import org.bukkit.block.data.Openable;
-
 import com.google.common.collect.Lists;
-
 import nl.rutgerkok.blocklocker.ProtectionSign;
 import nl.rutgerkok.blocklocker.profile.Profile;
 import nl.rutgerkok.blocklocker.profile.TimerProfile;
 import nl.rutgerkok.blocklocker.protection.Protection;
+import org.apache.commons.lang.Validate;
+import org.bukkit.Material;
+import org.bukkit.block.data.BlockData;
+import org.bukkit.block.data.Openable;
+
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
 
 /**
  * Base class for protection implementations.
- *
  */
 abstract class AbstractProtection implements Protection {
 
-    /**
-     * Checks if the instance is of {@link Openable} that will open correctly when
-     * you call {@link Openable#setOpen(boolean)}. For barrels, opening them using
-     * {@link Openable#setOpen(boolean)} won't show the inventory, so those
-     * shouldn't be considered openable.
-     *
-     * @param blockData
-     *            The block data.
-     * @return True if the block data is openable in a way that is functional.
-     */
-    protected static boolean isFunctionalOpenable(BlockData blockData) {
-        if (!(blockData instanceof Openable)) {
-            return false;
-        }
-        if (blockData.getMaterial() == Material.BARREL) {
-            return false; // Barrels are openable in Bukkit since 1.19, but then the inventory won't show up
-        }
-        return true;
-    }
     private Optional<Collection<Profile>> allAllowed = Optional.empty();
     private Optional<Profile> owner = Optional.empty();
-
     private Optional<Collection<ProtectionSign>> allSigns = Optional.empty();
 
     /**
      * Constructor for creating the protection with all signs already looked up.
      * Collection may not be empty.
      *
-     * @param signs
-     *            All signs.
+     * @param signs All signs.
      */
     AbstractProtection(Collection<ProtectionSign> signs) {
         Validate.notEmpty(signs);
@@ -64,8 +40,7 @@ protected static boolean isFunctionalOpenable(BlockData blockData) {
      * up. Not all signs are found yet. If it turns out that all signs are
      * needed, {@link #fetchSigns()} will be called.
      *
-     * @param sign
-     *            A sign attached to the protection.
+     * @param sign A sign attached to the protection.
      */
     AbstractProtection(ProtectionSign sign) {
         if (sign.getType().isMainSign()) {
@@ -76,6 +51,25 @@ protected static boolean isFunctionalOpenable(BlockData blockData) {
         }
     }
 
+    /**
+     * Checks if the instance is of {@link Openable} that will open correctly when
+     * you call {@link Openable#setOpen(boolean)}. For barrels, opening them using
+     * {@link Openable#setOpen(boolean)} won't show the inventory, so those
+     * shouldn't be considered openable.
+     *
+     * @param blockData The block data.
+     * @return True if the block data is openable in a way that is functional.
+     */
+    protected static boolean isFunctionalOpenable(BlockData blockData) {
+        if (!(blockData instanceof Openable)) {
+            return false;
+        }
+        if (blockData.getMaterial() == Material.BARREL) {
+            return false; // Barrels are openable in Bukkit since 1.19, but then the inventory won't show up
+        }
+        return true;
+    }
+
     private Collection<Profile> fetchAllowed(Collection<ProtectionSign> signs) {
         Collection<Profile> allAllowed = Lists.newArrayList();
         for (ProtectionSign sign : signs) {
@@ -192,4 +186,29 @@ public final boolean isOwner(Profile profile) {
 
         return owner.get().includes(profile);
     }
+
+    @Override
+    public boolean equals(Object other) {
+        if (other == this) {
+            return true;
+        }
+        if (!(other instanceof AbstractProtection that)) {
+            return false;
+        }
+
+        // Check if both sign lists are equal, ignoring order
+        Collection<ProtectionSign> otherSigns = that.getSigns();
+        Collection<ProtectionSign> ourSigns = this.getSigns();
+        return otherSigns.containsAll(ourSigns) && ourSigns.containsAll(otherSigns);
+    }
+
+    @Override
+    public int hashCode() {
+        // Make sure the hash code does not depend on the order of the signs, to mirror this.equals
+        int hashCode = 0;
+        for (ProtectionSign sign : this.getSigns()) {
+            hashCode += sign.hashCode();
+        }
+        return hashCode;
+    }
 }