Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some item comparison issues #2834

Merged
merged 3 commits into from
Mar 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/main/java/ch/njol/skript/aliases/ItemData.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,11 @@ public ItemData(ItemStack stack, @Nullable BlockValues values) {
// Play safe and mark ALL items that may have durability to have it changed
itemFlags |= ItemFlags.CHANGED_DURABILITY;
}
if (stack.hasItemMeta()) {
itemFlags |= ItemFlags.CHANGED_TAGS;
}
// All data made from stacks may have changed tags
// We cannot assume that lack of tags indicates that they can be
// ignored in comparisons; they may well have been explicitly removed
// See issue #2714 for examples of bad things that this causes
itemFlags |= ItemFlags.CHANGED_TAGS;
}

public ItemData(ItemStack stack) {
Expand Down Expand Up @@ -413,28 +415,28 @@ private static MatchQuality compareItemMetas(ItemMeta first, ItemMeta second) {
String ourName = first.hasDisplayName() ? first.getDisplayName() : null;
String theirName = second.hasDisplayName() ? second.getDisplayName() : null;
if (!Objects.equals(ourName, theirName)) {
quality = theirName != null ? MatchQuality.SAME_MATERIAL : quality;
quality = ourName != null ? MatchQuality.SAME_MATERIAL : quality;
}

// Lore
List<String> ourLore = first.hasLore() ? first.getLore() : null;
List<String> theirLore = second.hasLore() ? second.getLore() : null;
if (!Objects.equals(ourLore, theirLore)) {
quality = theirLore != null ? MatchQuality.SAME_MATERIAL : quality;
quality = ourLore != null ? MatchQuality.SAME_MATERIAL : quality;
}

// Enchantments
Map<Enchantment, Integer> ourEnchants = first.getEnchants();
Map<Enchantment, Integer> theirEnchants = second.getEnchants();
if (!Objects.equals(ourEnchants, theirEnchants)) {
quality = !theirEnchants.isEmpty() ? MatchQuality.SAME_MATERIAL : quality;
quality = !ourEnchants.isEmpty() ? MatchQuality.SAME_MATERIAL : quality;
}

// Item flags
Set<ItemFlag> ourFlags = first.getItemFlags();
Set<ItemFlag> theirFlags = second.getItemFlags();
if (!Objects.equals(ourFlags, theirFlags)) {
quality = !theirFlags.isEmpty() ? MatchQuality.SAME_MATERIAL : quality;
quality = !ourFlags.isEmpty() ? MatchQuality.SAME_MATERIAL : quality;
}

// Potion data
Expand Down
25 changes: 5 additions & 20 deletions src/main/java/ch/njol/skript/aliases/ItemType.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,25 +276,9 @@ public void setAll(final boolean all) {
}

public boolean isOfType(@Nullable ItemStack item) {
// Duplicate code to avoid creating ItemData
for (ItemData myType : types) {
if (item == null) { // Given item null
if (myType.type == Material.AIR)
return true; // Both items AIR/null
} else if (myType.isAlias) {
if (!myType.isOfType(item))
continue;
return true;
} else {
return item.isSimilar(myType.stack);
}
}
return false;

// Alternative, simpler implementation
// if (item == null)
// return isOfType(Material.AIR, null);
// return isOfType(new ItemData(item));
if (item == null)
return isOfType(Material.AIR, null);
return isOfType(new ItemData(item));
}

public boolean isOfType(@Nullable BlockState block) {
Expand All @@ -312,8 +296,9 @@ public boolean isOfType(@Nullable Block block) {

public boolean isOfType(ItemData type) {
for (final ItemData myType : types) {
if (myType.equals(type))
if (myType.equals(type)) {
return true;
}
}
return false;
}
Expand Down