Config: Consider NBT tag when item is in multiple categories. (#101)

2008Choco · Aug 24, 2023 · 39b82aa · 39b82aa
1 parent 22de608
commit 39b82aa
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 32 deletions.
diff --git a/veinminer-bukkit/src/main/java/wtf/choco/veinminer/listener/BreakBlockListener.java b/veinminer-bukkit/src/main/java/wtf/choco/veinminer/listener/BreakBlockListener.java
@@ -22,7 +22,6 @@
 import org.bukkit.metadata.FixedMetadataValue;
 import org.bukkit.metadata.LazyMetadataValue;
 import org.bukkit.metadata.LazyMetadataValue.CacheStrategy;
-import org.bukkit.persistence.PersistentDataType;
 import org.bukkit.util.RayTraceResult;
 import org.jetbrains.annotations.NotNull;
 
@@ -72,20 +71,11 @@ private void onBlockBreak(BlockBreakEvent event) {
         Player player = event.getPlayer();
         ItemStack item = player.getInventory().getItemInMainHand();
 
-        VeinMinerToolCategory category = plugin.getToolCategoryRegistry().get(BukkitAdapter.adaptItem(item.getType()), cat -> player.hasPermission(VMConstants.PERMISSION_VEINMINE.apply(cat)));
+        VeinMinerToolCategory category = plugin.getToolCategoryRegistry().get(BukkitAdapter.adapt(item), cat -> player.hasPermission(VMConstants.PERMISSION_VEINMINE.apply(cat)));
         if (category == null) {
             return;
         }
 
-        // Check for the NBT value is one is present
-        String nbtValue = category.getNBTValue();
-        if (nbtValue != null) {
-            ItemMeta meta = item.getItemMeta();
-            if (meta != null && !nbtValue.equals(meta.getPersistentDataContainer().get(VMConstants.getVeinMinerNBTKey(), PersistentDataType.STRING))) {
-                return;
-            }
-        }
-
         VeinMinerManager veinMinerManager = plugin.getVeinMinerManager();
         BlockData originBlockData = origin.getBlockData();
         BlockState originBlockState = BukkitAdapter.adapt(originBlockData);

diff --git a/veinminer-server/src/main/java/wtf/choco/veinminer/VeinMinerPlayer.java b/veinminer-server/src/main/java/wtf/choco/veinminer/VeinMinerPlayer.java
@@ -504,19 +504,13 @@ public void handleToggleVeinMiner(@NotNull PluginMessageServerboundToggleVeinMin
     public void handleRequestVeinMine(@NotNull PluginMessageServerboundRequestVeinMine message) {
         ItemStack itemStack = player.getItemInMainHand();
         VeinMinerServer veinMiner = VeinMinerServer.getInstance();
-        VeinMinerToolCategory category = veinMiner.getToolCategoryRegistry().get(itemStack.getType());
+        VeinMinerToolCategory category = veinMiner.getToolCategoryRegistry().get(itemStack);
 
         if (category == null) {
             this.sendMessage(new PluginMessageClientboundVeinMineResults());
             return;
         }
 
-        // Check for the NBT value is one is present
-        String nbtValue = category.getNBTValue();
-        if (nbtValue != null && !nbtValue.equals(itemStack.getVeinMinerNBTValue())) {
-            return;
-        }
-
         RayTraceResult rayTraceResult = player.getTargetBlock(6);
         BlockPosition targetBlock = rayTraceResult.getHitBlock();
         BlockFace targetBlockFace = rayTraceResult.getHitBlockFace();

diff --git a/veinminer-server/src/main/java/wtf/choco/veinminer/tool/ToolCategoryRegistry.java b/veinminer-server/src/main/java/wtf/choco/veinminer/tool/ToolCategoryRegistry.java
@@ -10,6 +10,7 @@
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.UnmodifiableView;
 
+import wtf.choco.veinminer.platform.world.ItemStack;
 import wtf.choco.veinminer.platform.world.ItemType;
 
 /**
@@ -42,43 +43,86 @@ public VeinMinerToolCategory get(@NotNull String id) {
         return categories.get(id.toLowerCase());
     }
 
+    @Nullable
+    private VeinMinerToolCategory get(@NotNull ItemType itemType, @Nullable String itemNbtValue, @NotNull Predicate<VeinMinerToolCategory> categoryPredicate) {
+        VeinMinerToolCategory resultCategory = null;
+        for (VeinMinerToolCategory category : categories.values()) {
+            if (category.containsItem(itemType) && (category.getNBTValue() == null || category.getNBTValue().equals(itemNbtValue)) && (resultCategory == null || category.compareTo(resultCategory) > 1) && categoryPredicate.test(category)) {
+                resultCategory = category;
+            }
+        }
+
+        return resultCategory;
+    }
+
     /**
-     * Get the {@link VeinMinerToolCategory} that contains the given {@link ItemType} and matches
-     * the given {@link Predicate}. There is no guarantee as to which category will be returned if
-     * more than one category contains the provided ItemType.
+     * Get the {@link VeinMinerToolCategory} that contains the given {@link ItemType}. There is
+     * no guarantee as to which category will be returned if more than one category contains the
+     * provided ItemType.
+     *
+     * @param itemType the item type
+     *
+     * @return the corresponding tool category, or null if no category contains the item
+     *
+     * @deprecated Using this leads to incorrect behaviour when an NBT tag is set
+     * in config. Pass ItemStack instead of ItemType.
+     */
+    @Deprecated
+    @Nullable
+    public VeinMinerToolCategory get(@NotNull ItemType itemType) {
+        return get(itemType, null, PREDICATE_ALWAYS_TRUE);
+    }
+
+    /**
+     * Get the {@link VeinMinerToolCategory} that contains the given {@link ItemType}. There is
+     * no guarantee as to which category will be returned if more than one category contains the
+     * provided ItemType.
      *
      * @param itemType the item type
      * @param categoryPredicate a predicate to apply on top of the item condition. If the predicate
      * returns false for any given category, it will not be returned by this method. Useful for an
      * additional permission check on a category
      *
      * @return the corresponding tool category, or null if no category contains the item
+     *
+     * @deprecated Using this leads to incorrect behaviour when an NBT tag is set
+     * in config. Pass ItemStack instead of ItemType.
      */
+    @Deprecated
     @Nullable
     public VeinMinerToolCategory get(@NotNull ItemType itemType, @NotNull Predicate<VeinMinerToolCategory> categoryPredicate) {
-        VeinMinerToolCategory resultCategory = null;
-
-        for (VeinMinerToolCategory category : categories.values()) {
-            if (category.containsItem(itemType) && (resultCategory == null || category.compareTo(resultCategory) > 1) && categoryPredicate.test(category)) {
-                resultCategory = category;
-            }
-        }
+        return get(itemType, null, categoryPredicate);
+    }
 
-        return resultCategory;
+    /**
+     * Get the {@link VeinMinerToolCategory} that contains the given {@link ItemType}. There is
+     * no guarantee as to which category will be returned if more than one category contains the
+     * provided ItemType.
+     *
+     * @param itemStack the item
+     *
+     * @return the corresponding tool category, or null if no category contains the item
+     */
+    @Nullable
+    public VeinMinerToolCategory get(@NotNull ItemStack itemStack) {
+        return get(itemStack.getType(), itemStack.getVeinMinerNBTValue(), PREDICATE_ALWAYS_TRUE);
     }
 
     /**
      * Get the {@link VeinMinerToolCategory} that contains the given {@link ItemType}. There is
      * no guarantee as to which category will be returned if more than one category contains the
      * provided ItemType.
      *
-     * @param itemType the item type
+     * @param itemStack the item
+     * @param categoryPredicate a predicate to apply on top of the item condition. If the predicate
+     * returns false for any given category, it will not be returned by this method. Useful for an
+     * additional permission check on a category
      *
      * @return the corresponding tool category, or null if no category contains the item
      */
     @Nullable
-    public VeinMinerToolCategory get(@NotNull ItemType itemType) {
-        return get(itemType, PREDICATE_ALWAYS_TRUE);
+    public VeinMinerToolCategory get(@NotNull ItemStack itemStack, @NotNull Predicate<VeinMinerToolCategory> categoryPredicate) {
+        return get(itemStack.getType(), itemStack.getVeinMinerNBTValue(), categoryPredicate);
     }
 
     /**