From 0701389f25b6f43caad79f557e1f79dd259aa7b5 Mon Sep 17 00:00:00 2001 From: tastybento Date: Fri, 10 Jul 2026 14:09:04 -0700 Subject: [PATCH 1/2] Compare base potion type when metadata is ignored When a potion material was in the ignore-metadata list, items were matched by Material alone, so any potion matched any other potion and removal could consume the wrong ones. Now potion-like items (POTION, SPLASH_POTION, LINGERING_POTION, TIPPED_ARROW) still compare their base PotionType while ignoring other metadata. Fixes #320 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NKxodNE4h3TsSHMqDEeC8v --- .../challenges/tasks/TryToComplete.java | 83 ++++++++----- .../bentobox/challenges/utils/Utils.java | 84 ++++++++++++- .../challenges/tasks/TryToCompleteTest.java | 116 ++++++++++++++++++ 3 files changed, 248 insertions(+), 35 deletions(-) diff --git a/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java b/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java index 1281baf5..de29306b 100644 --- a/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java +++ b/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Set; import java.util.PriorityQueue; import java.util.Queue; import java.util.UUID; @@ -1072,18 +1073,13 @@ Map removeItems(List requiredItemList, int factor // Sanity check. User always has inventory at this point of code. itemsInInventory = Collections.emptyList(); } - else if (this.getInventoryRequirements().getIgnoreMetaData().contains(required.getType())) - { - // Use collecting method that ignores item meta. - itemsInInventory = Arrays.stream(user.getInventory().getContents()). - filter(Objects::nonNull).filter(i -> i.getType().equals(required.getType())) - .collect(Collectors.toList()); - } else { - // Use collecting method that compares item meta. + // Use helper method that handles ignore-metadata logic including potion types. itemsInInventory = Arrays.stream(user.getInventory().getContents()). - filter(Objects::nonNull).filter(i -> i.isSimilar(required)).collect(Collectors.toList()); + filter(Objects::nonNull). + filter(i -> itemsMatch(i, required, this.getInventoryRequirements().getIgnoreMetaData())). + collect(Collectors.toList()); } for (ItemStack itemStack : itemsInInventory) @@ -1876,6 +1872,45 @@ private boolean hasRequiredTeamPresence() } + /** + * Checks if two items match, considering the ignore-metadata setting. For potion-like materials + * that are in the ignore-metadata set, compares the base potion type while ignoring other metadata. + * For non-potion materials in the ignore-metadata set, uses type-only comparison. For materials + * not in the ignore-metadata set, uses full similarity comparison. + * + * @param candidate candidate item from inventory + * @param required required item template + * @param ignoreMetaData set of materials to ignore metadata for + * @return true if the items match + */ + private static boolean itemsMatch(ItemStack candidate, ItemStack required, Set ignoreMetaData) + { + if (candidate == null || required == null) + { + return false; + } + + if (!candidate.getType().equals(required.getType())) + { + return false; + } + + // If metadata should not be ignored, use full similarity check + if (!ignoreMetaData.contains(required.getType())) + { + return candidate.isSimilar(required); + } + + // Metadata is being ignored. For potion-like materials, still compare base potion type. + if (Utils.isPotionLike(required.getType())) + { + return Utils.comparePotionType(candidate, required); + } + + // For non-potion materials, type-only matching is sufficient + return true; + } + /** * Counts how many of {@code required} a single player holds, honouring the challenge's * ignore-meta-data setting. @@ -1886,17 +1921,9 @@ private boolean hasRequiredTeamPresence() */ private int countInInventory(Player player, ItemStack required) { - if (this.getInventoryRequirements().getIgnoreMetaData().contains(required.getType())) - { - return Arrays.stream(player.getInventory().getContents()). - filter(Objects::nonNull). - filter(i -> i.getType().equals(required.getType())). - mapToInt(ItemStack::getAmount).sum(); - } - return Arrays.stream(player.getInventory().getContents()). filter(Objects::nonNull). - filter(i -> i.isSimilar(required)). + filter(i -> itemsMatch(i, required, this.getInventoryRequirements().getIgnoreMetaData())). mapToInt(ItemStack::getAmount).sum(); } @@ -1916,22 +1943,10 @@ private int removeFromInventory(Player player, ItemStack required, int amount) return 0; } - List itemsInInventory; - - if (this.getInventoryRequirements().getIgnoreMetaData().contains(required.getType())) - { - itemsInInventory = Arrays.stream(player.getInventory().getContents()). - filter(Objects::nonNull). - filter(i -> i.getType().equals(required.getType())). - toList(); - } - else - { - itemsInInventory = Arrays.stream(player.getInventory().getContents()). - filter(Objects::nonNull). - filter(i -> i.isSimilar(required)). - toList(); - } + List itemsInInventory = Arrays.stream(player.getInventory().getContents()). + filter(Objects::nonNull). + filter(i -> itemsMatch(i, required, this.getInventoryRequirements().getIgnoreMetaData())). + toList(); int toRemove = amount; diff --git a/src/main/java/world/bentobox/challenges/utils/Utils.java b/src/main/java/world/bentobox/challenges/utils/Utils.java index ab0f62f5..b3ccfe54 100644 --- a/src/main/java/world/bentobox/challenges/utils/Utils.java +++ b/src/main/java/world/bentobox/challenges/utils/Utils.java @@ -59,6 +59,52 @@ else if (stack == input) } + /** + * Checks if a material is potion-like (potions, splash potions, lingering potions, tipped arrows). + * + * @param material the material to check + * @return true if the material is potion-like + */ + public static boolean isPotionLike(Material material) + { + return material == Material.POTION || + material == Material.SPLASH_POTION || + material == Material.LINGERING_POTION || + material == Material.TIPPED_ARROW; + } + + /** + * Compares the base potion type of two potion items. Returns true if they have the same + * base potion type, ignoring custom effects, lore, and other metadata. + * + * @param first first potion item + * @param second second potion item + * @return true if both items have the same base potion type + */ + public static boolean comparePotionType(@Nullable ItemStack first, @Nullable ItemStack second) + { + if (first == null || second == null) + { + return false; + } + + PotionType firstType = null; + PotionType secondType = null; + + if (first.hasItemMeta() && first.getItemMeta() instanceof PotionMeta) + { + firstType = ((PotionMeta) first.getItemMeta()).getBasePotionType(); + } + + if (second.hasItemMeta() && second.getItemMeta() instanceof PotionMeta) + { + secondType = ((PotionMeta) second.getItemMeta()).getBasePotionType(); + } + + // If either has no potion meta, they're only equal if both are missing the meta + return java.util.Objects.equals(firstType, secondType); + } + /** * This method groups input items in single itemstack with correct amount and returns it. * Allows to remove duplicate items from list. @@ -84,7 +130,7 @@ public static List groupEqualItems(List requiredItems, Set // Merge items which meta can be ignored or is similar to item in required list. if (Utils.isSimilarNoDurability(required, item) || - ignoreMetaData.contains(item.getType()) && item.getType().equals(required.getType())) + itemsMatchIgnoreMetadata(required, item, ignoreMetaData)) { required.setAmount(required.getAmount() + item.getAmount()); isUnique = false; @@ -103,6 +149,42 @@ public static List groupEqualItems(List requiredItems, Set return returnItems; } + /** + * Checks if two items match when metadata should be ignored. For potion-like materials, + * compares the base potion type. For non-potion materials, uses type-only comparison. + * + * @param first first item + * @param second second item + * @param ignoreMetaData set of materials to ignore metadata for + * @return true if items match according to the ignore-metadata rules + */ + private static boolean itemsMatchIgnoreMetadata(@Nullable ItemStack first, @Nullable ItemStack second, Set ignoreMetaData) + { + if (first == null || second == null) + { + return false; + } + + if (!first.getType().equals(second.getType())) + { + return false; + } + + if (!ignoreMetaData.contains(first.getType())) + { + return false; + } + + // Metadata is being ignored. For potion-like materials, still compare base potion type. + if (isPotionLike(first.getType())) + { + return comparePotionType(first, second); + } + + // For non-potion materials, type-only matching is sufficient + return true; + } + /** * This method transforms given World into GameMode name. If world is not a GameMode diff --git a/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java b/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java index 1d616e51..fc201491 100644 --- a/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java +++ b/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java @@ -33,6 +33,8 @@ import org.bukkit.entity.EntityType; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.PotionMeta; +import org.bukkit.potion.PotionType; import org.bukkit.util.BoundingBox; import org.eclipse.jdt.annotation.NonNull; import org.junit.jupiter.api.AfterEach; @@ -55,6 +57,7 @@ import world.bentobox.challenges.database.object.requirements.StatisticRequirements.StatisticRec; import world.bentobox.challenges.managers.ChallengesManager; import world.bentobox.challenges.tasks.TryToComplete.ChallengeResult; +import world.bentobox.challenges.utils.Utils; import world.bentobox.level.Level; /** @@ -849,4 +852,117 @@ void testFreeChallengeNoLevelCheck() { assertTrue(TryToComplete.complete(addon, user, challenge, world, topLabel, permissionPrefix)); verify(cm, never()).getLevel(any(Challenge.class)); } + + // ------------------------------------------------------------------------- + // Tests for issue #320: Potion type comparison when metadata is ignored + // ------------------------------------------------------------------------- + + @Test + void testPotionComparisonIgnoreMetadataSameType() { + // Test that potions with same base type match when metadata is ignored + // by using Utils.groupEqualItems which should group them together + ItemStack swiftnessPotion1 = createPotion(Material.POTION, PotionType.SWIFTNESS); + swiftnessPotion1.setAmount(1); + ItemStack swiftnessPotion2 = createPotion(Material.POTION, PotionType.SWIFTNESS); + swiftnessPotion2.setAmount(1); + // Add different custom name to swiftnessPotion2 to ensure metadata differs + PotionMeta meta = (PotionMeta) swiftnessPotion2.getItemMeta(); + if (meta != null) { + meta.setDisplayName("Fancy Swiftness"); + swiftnessPotion2.setItemMeta(meta); + } + + // When grouping items with ignore-metadata set for potions, + // potions with the same base type should be grouped together + Set ignoreMetaData = Set.of(Material.POTION); + List requiredItems = Arrays.asList(swiftnessPotion1, swiftnessPotion2); + List grouped = Utils.groupEqualItems(requiredItems, ignoreMetaData); + + // Both swiftness potions should be grouped into one stack with amount 2 + assertEquals(1, grouped.size(), "Potions with same base type should be grouped together"); + assertEquals(2, grouped.get(0).getAmount(), "Grouped potion should have combined amount"); + assertEquals(Material.POTION, grouped.get(0).getType(), "Grouped potion should be POTION"); + } + + @Test + void testPotionComparisonIgnoreMetadataDifferentType() { + // Test that potions with different base types DON'T group when metadata is ignored + ItemStack swiftnessPotion = createPotion(Material.POTION, PotionType.SWIFTNESS); + swiftnessPotion.setAmount(1); + ItemStack strengthPotion = createPotion(Material.POTION, PotionType.STRENGTH); + strengthPotion.setAmount(1); + + // When grouping potions with different base types and ignore-metadata set, + // they should NOT be grouped together + Set ignoreMetaData = Set.of(Material.POTION); + List requiredItems = Arrays.asList(swiftnessPotion, strengthPotion); + List grouped = Utils.groupEqualItems(requiredItems, ignoreMetaData); + + // Different potion types should NOT be grouped + assertEquals(2, grouped.size(), "Potions with different base types should NOT be grouped"); + assertEquals(1, grouped.get(0).getAmount(), "First potion should keep original amount"); + assertEquals(1, grouped.get(1).getAmount(), "Second potion should keep original amount"); + } + + @Test + void testPotionComparisonHelperMethod() { + // Unit test for the helper method that checks if items match with potion type comparison + ItemStack swiftnessPotion1 = createPotion(Material.POTION, PotionType.SWIFTNESS); + ItemStack swiftnessPotion2 = createPotion(Material.POTION, PotionType.SWIFTNESS); + ItemStack strengthPotion = createPotion(Material.POTION, PotionType.STRENGTH); + + Set ignoreMetaData = Set.of(Material.POTION); + + // Test that same potion type matches + assertTrue(Utils.comparePotionType(swiftnessPotion1, swiftnessPotion2), + "Potions with same base type should compare as equal"); + + // Test that different potion types don't match + assertFalse(Utils.comparePotionType(swiftnessPotion1, strengthPotion), + "Potions with different base types should not compare as equal"); + + // Test that potion-like check works + assertTrue(Utils.isPotionLike(Material.POTION), + "POTION should be recognized as potion-like"); + assertTrue(Utils.isPotionLike(Material.SPLASH_POTION), + "SPLASH_POTION should be recognized as potion-like"); + assertTrue(Utils.isPotionLike(Material.LINGERING_POTION), + "LINGERING_POTION should be recognized as potion-like"); + assertTrue(Utils.isPotionLike(Material.TIPPED_ARROW), + "TIPPED_ARROW should be recognized as potion-like"); + assertFalse(Utils.isPotionLike(Material.DIRT), + "DIRT should not be recognized as potion-like"); + } + + @Test + void testNonPotionIgnoreMetadataUnchanged() { + // Test that non-potion materials still use type-only comparison + ItemStack dirt1 = new ItemStack(Material.DIRT); + dirt1.setAmount(1); + ItemStack dirt2 = new ItemStack(Material.DIRT); + dirt2.setAmount(1); + + // When grouping non-potion items with ignore-metadata set, + // they should be grouped together by type alone + Set ignoreMetaData = Set.of(Material.DIRT); + List requiredItems = Arrays.asList(dirt1, dirt2); + List grouped = Utils.groupEqualItems(requiredItems, ignoreMetaData); + + // Both dirt items should be grouped into one stack with amount 2 + assertEquals(1, grouped.size(), "Non-potion items with same type should be grouped"); + assertEquals(2, grouped.get(0).getAmount(), "Grouped items should have combined amount"); + } + + /** + * Helper method to create a potion ItemStack with specified base potion type + */ + private ItemStack createPotion(Material material, PotionType potionType) { + ItemStack potion = new ItemStack(material); + PotionMeta meta = (PotionMeta) potion.getItemMeta(); + if (meta != null) { + meta.setBasePotionType(potionType); + potion.setItemMeta(meta); + } + return potion; + } } From a3a8f3235b0382200ca11232b6e4843bd4e3227e Mon Sep 17 00:00:00 2001 From: tastybento Date: Fri, 10 Jul 2026 15:41:21 -0700 Subject: [PATCH 2/2] Address SonarCloud findings on PR #409 Remove the always-false inventory null check (User#getInventory is non-null for players), use Stream.toList(), use pattern-matching instanceof in Utils, and drop an unused test variable. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NKxodNE4h3TsSHMqDEeC8v --- .../challenges/tasks/TryToComplete.java | 20 +++++-------------- .../bentobox/challenges/utils/Utils.java | 8 ++++---- .../challenges/tasks/TryToCompleteTest.java | 2 -- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java b/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java index de29306b..46f66bcd 100644 --- a/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java +++ b/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java @@ -18,7 +18,6 @@ import java.util.Queue; import java.util.UUID; import java.util.function.BiPredicate; -import java.util.stream.Collectors; import org.bukkit.Bukkit; import org.bukkit.ChatColor; @@ -1066,21 +1065,12 @@ Map removeItems(List requiredItemList, int factor for (ItemStack required : requiredItemList) { int amountToBeRemoved = required.getAmount() * factor; - List itemsInInventory; - if (this.user.getInventory() == null) - { - // Sanity check. User always has inventory at this point of code. - itemsInInventory = Collections.emptyList(); - } - else - { - // Use helper method that handles ignore-metadata logic including potion types. - itemsInInventory = Arrays.stream(user.getInventory().getContents()). - filter(Objects::nonNull). - filter(i -> itemsMatch(i, required, this.getInventoryRequirements().getIgnoreMetaData())). - collect(Collectors.toList()); - } + // Use helper method that handles ignore-metadata logic including potion types. + List itemsInInventory = Arrays.stream(user.getInventory().getContents()). + filter(Objects::nonNull). + filter(i -> itemsMatch(i, required, this.getInventoryRequirements().getIgnoreMetaData())). + toList(); for (ItemStack itemStack : itemsInInventory) { diff --git a/src/main/java/world/bentobox/challenges/utils/Utils.java b/src/main/java/world/bentobox/challenges/utils/Utils.java index b3ccfe54..c9b8f525 100644 --- a/src/main/java/world/bentobox/challenges/utils/Utils.java +++ b/src/main/java/world/bentobox/challenges/utils/Utils.java @@ -91,14 +91,14 @@ public static boolean comparePotionType(@Nullable ItemStack first, @Nullable Ite PotionType firstType = null; PotionType secondType = null; - if (first.hasItemMeta() && first.getItemMeta() instanceof PotionMeta) + if (first.hasItemMeta() && first.getItemMeta() instanceof PotionMeta potionMeta) { - firstType = ((PotionMeta) first.getItemMeta()).getBasePotionType(); + firstType = potionMeta.getBasePotionType(); } - if (second.hasItemMeta() && second.getItemMeta() instanceof PotionMeta) + if (second.hasItemMeta() && second.getItemMeta() instanceof PotionMeta potionMeta) { - secondType = ((PotionMeta) second.getItemMeta()).getBasePotionType(); + secondType = potionMeta.getBasePotionType(); } // If either has no potion meta, they're only equal if both are missing the meta diff --git a/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java b/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java index fc201491..e4ffd75d 100644 --- a/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java +++ b/src/test/java/world/bentobox/challenges/tasks/TryToCompleteTest.java @@ -911,8 +911,6 @@ void testPotionComparisonHelperMethod() { ItemStack swiftnessPotion2 = createPotion(Material.POTION, PotionType.SWIFTNESS); ItemStack strengthPotion = createPotion(Material.POTION, PotionType.STRENGTH); - Set ignoreMetaData = Set.of(Material.POTION); - // Test that same potion type matches assertTrue(Utils.comparePotionType(swiftnessPotion1, swiftnessPotion2), "Potions with same base type should compare as equal");