From 6beda019a03959570c26720c578bfa26515beffd Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sat, 12 Dec 2020 17:18:16 -0500 Subject: [PATCH] Improve Item Comparisons Hopefully they are good once and for all. --- .../java/ch/njol/skript/aliases/ItemData.java | 61 +++++++++++-- .../java/ch/njol/skript/aliases/ItemType.java | 6 +- .../classes/data/DefaultComparators.java | 14 ++- .../ch/njol/skript/expressions/ExprPlain.java | 85 +++++++++++++++++++ .../regressions/3419-item comparisons.sk | 24 ++++-- 5 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 src/main/java/ch/njol/skript/expressions/ExprPlain.java diff --git a/src/main/java/ch/njol/skript/aliases/ItemData.java b/src/main/java/ch/njol/skript/aliases/ItemData.java index ce9a2da9128..30d94e9220c 100644 --- a/src/main/java/ch/njol/skript/aliases/ItemData.java +++ b/src/main/java/ch/njol/skript/aliases/ItemData.java @@ -155,6 +155,14 @@ public static class OldItemData { */ boolean isAlias = false; + /** + * Whether this item is a 'plain' item type. + * This is used for comparison, as any item type matched to this item type must also be plain OR be an alias. + * This variable should not be used directly. Use {@link #isPlain()}. + * @see ch.njol.skript.expressions.ExprPlain + */ + private boolean plain = false; + /** * Some properties about this item. */ @@ -185,6 +193,7 @@ public ItemData(ItemData data) { this.type = data.type; this.blockValues = data.blockValues; this.isAlias = data.isAlias; + this.plain = data.plain; this.itemFlags = data.itemFlags; } @@ -330,13 +339,11 @@ public MatchQuality matchAlias(ItemData item) { if (item.getType() != getType()) { return MatchQuality.DIFFERENT; } - BlockValues values = blockValues; if (!itemDataValues) { // Items (held in inventories) don't have block values // If this is an item, given item must not have them either - if (itemForm && item.blockValues != null - && !item.blockValues.isDefault()) { + if (itemForm && item.blockValues != null && !item.blockValues.isDefault()) { return MatchQuality.SAME_MATERIAL; } } @@ -402,33 +409,42 @@ private boolean hasFlag(int flag) { */ private static MatchQuality compareItemMetas(ItemMeta first, ItemMeta second) { MatchQuality quality = MatchQuality.EXACT; // Lowered as we go on + MatchQuality newQuality; // Used to prevent upgrading the quality // Display name String ourName = first.hasDisplayName() ? first.getDisplayName() : null; String theirName = second.hasDisplayName() ? second.getDisplayName() : null; if (!Objects.equals(ourName, theirName)) { - quality = ourName != null ? MatchQuality.SAME_MATERIAL : theirName != null ? MatchQuality.SAME_MATERIAL : quality; + newQuality = (ourName != null && theirName == null) ? MatchQuality.SAME_ITEM : MatchQuality.SAME_MATERIAL; + if (!newQuality.isBetter(quality)) + quality = newQuality; } // Lore List ourLore = first.hasLore() ? first.getLore() : null; List theirLore = second.hasLore() ? second.getLore() : null; if (!Objects.equals(ourLore, theirLore)) { - quality = ourLore != null ? MatchQuality.SAME_MATERIAL : theirLore != null ? MatchQuality.SAME_MATERIAL : quality; + newQuality = (ourLore != null && theirLore == null) ? MatchQuality.SAME_ITEM : MatchQuality.SAME_MATERIAL; + if (!newQuality.isBetter(quality)) + quality = newQuality; } // Enchantments Map ourEnchants = first.getEnchants(); Map theirEnchants = second.getEnchants(); if (!Objects.equals(ourEnchants, theirEnchants)) { - quality = !ourEnchants.isEmpty() ? MatchQuality.SAME_MATERIAL : !theirEnchants.isEmpty() ? MatchQuality.SAME_MATERIAL : quality; + newQuality = (!ourEnchants.isEmpty() && theirEnchants.isEmpty()) ? MatchQuality.SAME_ITEM : MatchQuality.SAME_MATERIAL; + if (!newQuality.isBetter(quality)) + quality = newQuality; } // Item flags Set ourFlags = first.getItemFlags(); Set theirFlags = second.getItemFlags(); if (!Objects.equals(ourFlags, theirFlags)) { - quality = !ourFlags.isEmpty() ? MatchQuality.SAME_MATERIAL : !theirFlags.isEmpty() ? MatchQuality.SAME_MATERIAL : quality; + newQuality = (!ourFlags.isEmpty() && theirFlags.isEmpty()) ? MatchQuality.SAME_ITEM : MatchQuality.SAME_MATERIAL; + if (!newQuality.isBetter(quality)) + quality = newQuality; } // Potion data @@ -547,6 +563,7 @@ public ItemMeta getItemMeta() { public void setItemMeta(ItemMeta meta) { stack.setItemMeta(meta); isAlias = false; // This is no longer exact alias + plain = false; // This is no longer a plain item itemFlags |= ItemFlags.CHANGED_TAGS; } @@ -557,8 +574,38 @@ public int getDurability() { public void setDurability(int durability) { ItemUtils.setDamage(stack, durability); isAlias = false; // Change happened + plain = false; // This is no longer a plain item itemFlags |= ItemFlags.CHANGED_DURABILITY; } + + /** + * Checks if this item type was created through {@link ch.njol.skript.expressions.ExprPlain} + * and thus has no modifications made to it. + * @return Whether this item type is 'plain' + */ + public boolean isPlain() { + return plain; + } + + public void setPlain(boolean plain) { + this.plain = plain; + } + + /** + * Compares this ItemData with another to determine if they are matching "plain" items. + * For these ItemDatas to match, they must share a {@link Material}. One of the following must also be true: + *
    + *
  • This ItemData is plain AND the other ItemData is plain
    • + *
  • This ItemData is plain AND the other ItemData is an alias
    • + *
  • This ItemData is an alias AND the other ItemData is plain
    • + *
+ * @param other The ItemData to compare with. + * @return Whether these items can be "plain matched" + * @see ch.njol.skript.expressions.ExprPlain + */ + public boolean matchPlain(ItemData other) { + return getType() == other.getType() && ((isPlain() && other.isPlain()) || (isPlain() && other.isAlias()) || (isAlias() && other.isPlain())); + } @Override public Fields serialize() throws NotSerializableException { diff --git a/src/main/java/ch/njol/skript/aliases/ItemType.java b/src/main/java/ch/njol/skript/aliases/ItemType.java index cd9ca48b21f..f460e81baa3 100644 --- a/src/main/java/ch/njol/skript/aliases/ItemType.java +++ b/src/main/java/ch/njol/skript/aliases/ItemType.java @@ -762,7 +762,11 @@ public final boolean removeFrom(final List... lists) { * item meta is completely different. */ ItemData other = is != null ? new ItemData(is) : null; - if (other != null && other.matchAlias(d).isAtLeast(((d.isAlias()) && !other.isAlias()) ? MatchQuality.SAME_MATERIAL : MatchQuality.EXACT)) { + if (other == null) { + continue; + } + boolean plain = d.isPlain() != other.isPlain(); + if (d.matchPlain(other) || other.matchAlias(d).isAtLeast(plain ? MatchQuality.EXACT : (d.isAlias() && !other.isAlias() ? MatchQuality.SAME_MATERIAL : MatchQuality.SAME_ITEM))) { if (all && amount == -1) { list.set(i, null); removed = 1; diff --git a/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java b/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java index 2b74c445b16..a2fbdf453b3 100644 --- a/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java +++ b/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java @@ -46,6 +46,7 @@ import org.bukkit.entity.Wither; import org.bukkit.event.entity.EntityDamageEvent.DamageCause; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; import ch.njol.skript.Skript; import ch.njol.skript.SkriptConfig; @@ -143,7 +144,10 @@ public boolean supportsOrdering() { Comparators.registerComparator(Slot.class, ItemType.class, new Comparator() { @Override public Relation compare(Slot slot, ItemType item) { - return Relation.get(item.isOfType(slot.getItem())); + ItemStack stack = slot.getItem(); + if (stack == null) + return Comparators.compare(new ItemType(Material.AIR), item); + return Comparators.compare(new ItemType(stack), item); } @Override @@ -156,7 +160,7 @@ public boolean supportsOrdering() { Comparators.registerComparator(ItemStack.class, ItemType.class, new Comparator() { @Override public Relation compare(final ItemStack is, final ItemType it) { - return Relation.get(it.isOfType(is)); + return Comparators.compare(new ItemType(is), it); } @Override @@ -203,8 +207,12 @@ public Relation compare(final ItemType i1, final ItemType i2) { return Relation.NOT_EQUAL; for (ItemData myType : i1.getTypes()) { for (ItemData otherType : i2.getTypes()) { + if (myType.matchPlain(otherType)) { + return Relation.EQUAL; + } + boolean plain = myType.isPlain() != otherType.isPlain(); // Don't require an EXACT match if the other ItemData is an alias. They only need to share a material. - if (myType.matchAlias(otherType).isAtLeast((otherType.isAlias() && !myType.isAlias()) ? MatchQuality.SAME_ITEM : MatchQuality.EXACT)) { + if (myType.matchAlias(otherType).isAtLeast(plain ? MatchQuality.EXACT : otherType.isAlias() && !myType.isAlias() ? MatchQuality.SAME_MATERIAL : MatchQuality.SAME_ITEM)) { return Relation.EQUAL; } } diff --git a/src/main/java/ch/njol/skript/expressions/ExprPlain.java b/src/main/java/ch/njol/skript/expressions/ExprPlain.java new file mode 100644 index 00000000000..ef0794877bc --- /dev/null +++ b/src/main/java/ch/njol/skript/expressions/ExprPlain.java @@ -0,0 +1,85 @@ +/** + * This file is part of Skript. + * + * Skript is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Skript is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Skript. If not, see . + * + * Copyright Peter Güttinger, SkriptLang team and contributors + */ +package ch.njol.skript.expressions; + +import ch.njol.skript.Skript; +import ch.njol.skript.aliases.ItemData; +import ch.njol.skript.aliases.ItemType; +import ch.njol.skript.doc.Description; +import ch.njol.skript.doc.Examples; +import ch.njol.skript.doc.Name; +import ch.njol.skript.doc.Since; +import ch.njol.skript.lang.Expression; +import ch.njol.skript.lang.ExpressionType; +import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.util.SimpleExpression; +import ch.njol.util.Kleenean; + +import org.bukkit.event.Event; +import org.eclipse.jdt.annotation.Nullable; + +@Name("Plain Item") +@Description("A plain item is an item with no modifications. It can be used to convert items to their default state or to match with other default items.") +@Examples({"if the player's tool is a plain diamond: # check if player's tool has no modifications", + "\tsend \"You are holding a plain diamond!\""}) +@Since("INSERT VERSION") +public class ExprPlain extends SimpleExpression { + + @SuppressWarnings("null") + private Expression item; + + static { + Skript.registerExpression(ExprPlain.class, ItemType.class, ExpressionType.COMBINED, "[a[n]] (plain|unmodified) %itemtype%"); + } + + @SuppressWarnings("unchecked") + @Override + public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { + item = (Expression) exprs[0]; + return true; + } + + @Override + @Nullable + protected ItemType[] get(Event e) { + ItemType itemType = item.getSingle(e); + if (itemType == null) + return new ItemType[0]; + ItemData data = new ItemData(itemType.getMaterial()); + data.setPlain(true); + ItemType plain = new ItemType(data); + return new ItemType[]{plain}; + } + + @Override + public boolean isSingle() { + return true; + } + + @Override + public Class getReturnType() { + return ItemType.class; + } + + @Override + public String toString(@Nullable Event e, boolean debug) { + return "plain " + item.toString(e, debug); + } + +} diff --git a/src/test/skript/tests/regressions/3419-item comparisons.sk b/src/test/skript/tests/regressions/3419-item comparisons.sk index 9428daa0f7e..8b08fac3705 100644 --- a/src/test/skript/tests/regressions/3419-item comparisons.sk +++ b/src/test/skript/tests/regressions/3419-item comparisons.sk @@ -20,11 +20,11 @@ test "item comparisons": set {_b} to a diamond named "Fake Diamond" assert {_a} is a diamond with "{_a} is not a diamond (it should be)" - assert {_a} is a diamond named "" with "{_a} is not an unnamed diamond (it should be)" + assert {_a} is a plain diamond with "{_a} is not an unnamed diamond (it should be)" assert {_a} is not a diamond named "Fake Diamond" with "{_a} is a diamond named ""Fake Diamond"" (it shouldn't be)" assert {_b} is a diamond with "{_b} is not a diamond (it should be)" - assert {_b} is not a diamond named "" with "{_b} is an unnamed diamond (it shouldn't be)" + assert {_b} is not a plain diamond with "{_b} is an unnamed diamond (it shouldn't be)" assert {_b} is a diamond named "Fake Diamond" with "{_b} is not a diamond named ""Fake Diamond"" (it should be)" assert {_a} is not {_b} with "{_a} is {_b} (it shouldn't be) (Different Name Comparisons)" @@ -68,14 +68,14 @@ test "item comparisons": set {_b} to an ender dragon skull named "Skully Skull" assert {_a} is a skeleton skull with "{_a} is not a skeleton skull (it should be)" - assert {_a} is not a skeleton skull named "" with "{_a} is an unnamed skeleton skull (it shouldn't be)" + assert {_a} is not a plain skeleton skull with "{_a} is an unnamed skeleton skull (it shouldn't be)" assert {_a} is a skeleton skull named "Skulliest Skull" with "{_a} is not a skeleton skull named ""Skulliest Skull"" (it should be)" assert {_a} is not an ender dragon skull with "{_a} is an ender dragon skull (it should't be)" assert {_a} is not an ender dragon skull named "Skulliest Skull" with "{_a} is an ender dragon skull named ""Skulliest Skull"" (it shouldn't be)" assert {_b} is an ender dragon skull with "{_b} is not an ender dragon skull (it should be)" - assert {_b} is not an ender dragon skull named "" with "{_b} is an unnamed ender dragon skull (it shouldn't be)" + assert {_b} is not a plain ender dragon skull with "{_b} is an unnamed ender dragon skull (it shouldn't be)" assert {_b} is an ender dragon skull named "Skully Skull" with "{_b} is not an ender dragon skull named ""Skully Skull"" (it should be)" assert {_b} is not a skeleton skull with "{_b} is a skeleton skull (it shouldn't be)" @@ -110,7 +110,7 @@ test "item comparisons": set {_inventory} to inventory of block at spawn of world "world" add 64 diamonds and 64 diamonds named "Fake Diamond" to {_inventory} - remove 4 diamonds named "" from {_inventory} + remove 4 of plain diamonds from {_inventory} assert {_inventory} contains 60 diamonds and 64 diamonds named "Fake Diamond" with "4 unnamed diamonds were not properly removed." remove 4 diamonds named "Fake Diamond" from {_inventory} @@ -124,3 +124,17 @@ test "item comparisons": assert {_inventory} is empty with "{_inventory} still has diamonds, but it shouldn't." set block at spawn of world "world" to air + + # Different Meta Comparisons + + set {_one} to a diamond sword of sharpness 1 named "TEST" + set {_two} to a diamond sword of sharpness 1 + + assert {_one} is {_two} with "a diamond sword of sharpness 1 named ""TEST"" should be a diamond sword of sharpness 1" + assert {_two} is not {_one} with "a diamond sword of sharpness 1 should NOT be a diamond sword of sharpness 1 named ""TEST""" + + set {_three} to a sand block of unbreaking 3 with lore "Sandy Block" + set {_four} to a sand block of unbreaking 3 + + assert {_three} is {_four} with "a sand block of unbreaking 3 with lore ""Sandy Block"" should be a sand block of unbreaking 3" + assert {_four} is not {_three} with "a sand block of unbreaking 3 should NOT be a sand block of unbreaking 3 with lore ""Sandy Block"""