Improve Item Comparisons

src/main/java/ch/njol/skript/aliases/ItemData.java

@@ -156,6 +156,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.
 	 */
@@ -186,6 +194,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;
 	}
 
@@ -331,13 +340,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;
 			}
 		}
@@ -403,33 +410,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<String> ourLore = first.hasLore() ? first.getLore() : null;
 		List<String> 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<Enchantment, Integer> ourEnchants = first.getEnchants();
 		Map<Enchantment, Integer> 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<ItemFlag> ourFlags = first.getItemFlags();
 		Set<ItemFlag> 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
@@ -548,6 +564,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;
 	}
 
@@ -558,8 +575,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:
+	 * <ul>
+	 * <li>This ItemData is plain AND the other ItemData is plain</li>
+	 * <li>This ItemData is plain AND the other ItemData is an alias</li>
+	 * <li>This ItemData is an alias AND the other ItemData is plain</li>
+	 * </ul>
+	 * @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 {

src/main/java/ch/njol/skript/aliases/ItemType.java

@@ -762,7 +762,11 @@ public final boolean removeFrom(final List<ItemStack>... 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;

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<Slot, ItemType>() {
 			@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<ItemStack, ItemType>() {
 			@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;
 						}
 					}

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 <http://www.gnu.org/licenses/>.
+ *
+ * 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<ItemType> {
+
+	@SuppressWarnings("null")
+	private Expression<ItemType> 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<ItemType>) 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<? extends ItemType> getReturnType() {
+		return ItemType.class;
+	}
+
+	@Override
+	public String toString(@Nullable Event e, boolean debug) {
+		return "plain " + item.toString(e, debug);
+	}
+
+}

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"""