From e9222c0be71e8819ff455c03d0caacb9c215f858 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 18 Mar 2016 14:56:16 -0400 Subject: [PATCH] Handle Item Meta Inconsistencies First, Enchantment order would blow away seeing 2 items as the same, however the Client forces enchantment list in a certain order, as well as does the /enchant command. Anvils can insert it into forced order, causing 2 same items to be considered different. This change makes unhandled NBT Tags and Enchantments use a sorted tree map, so they will always be in a consistent order. Additionally, the old enchantment API was never updated when ItemMeta was added, resulting in 2 different ways to modify an items enchantments. For consistency, the old API methods now forward to use the ItemMeta API equivalents, and should deprecate the old API's. --- ...089-Handle-Item-Meta-Inconsistencies.patch | 297 ++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 Spigot-Server-Patches/0089-Handle-Item-Meta-Inconsistencies.patch diff --git a/Spigot-Server-Patches/0089-Handle-Item-Meta-Inconsistencies.patch b/Spigot-Server-Patches/0089-Handle-Item-Meta-Inconsistencies.patch new file mode 100644 index 0000000000..4374a54baa --- /dev/null +++ b/Spigot-Server-Patches/0089-Handle-Item-Meta-Inconsistencies.patch @@ -0,0 +1,297 @@ +From 5207c4aef31a69a4f9cded63c9b90e056c7f9d50 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Thu, 28 May 2015 23:00:19 -0400 +Subject: [PATCH] Handle Item Meta Inconsistencies + +First, Enchantment order would blow away seeing 2 items as the same, +however the Client forces enchantment list in a certain order, as well +as does the /enchant command. Anvils can insert it into forced order, +causing 2 same items to be considered different. + +This change makes unhandled NBT Tags and Enchantments use a sorted tree map, +so they will always be in a consistent order. + +Additionally, the old enchantment API was never updated when ItemMeta +was added, resulting in 2 different ways to modify an items enchantments. + +For consistency, the old API methods now forward to use the +ItemMeta API equivalents, and should deprecate the old API's. + +diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java +index 8b6fd4f..b397ccc 100644 +--- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java ++++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java +@@ -4,6 +4,7 @@ import static org.bukkit.craftbukkit.inventory.CraftMetaItem.ENCHANTMENTS; + import static org.bukkit.craftbukkit.inventory.CraftMetaItem.ENCHANTMENTS_ID; + import static org.bukkit.craftbukkit.inventory.CraftMetaItem.ENCHANTMENTS_LVL; + ++import java.util.Iterator; + import java.util.Map; + + import net.minecraft.server.EnchantmentManager; +@@ -184,28 +185,11 @@ public final class CraftItemStack extends ItemStack { + public void addUnsafeEnchantment(Enchantment ench, int level) { + Validate.notNull(ench, "Cannot add null enchantment"); + +- if (!makeTag(handle)) { +- return; +- } +- NBTTagList list = getEnchantmentList(handle); +- if (list == null) { +- list = new NBTTagList(); +- handle.getTag().set(ENCHANTMENTS.NBT, list); +- } +- int size = list.size(); +- +- for (int i = 0; i < size; i++) { +- NBTTagCompound tag = (NBTTagCompound) list.get(i); +- short id = tag.getShort(ENCHANTMENTS_ID.NBT); +- if (id == ench.getId()) { +- tag.setShort(ENCHANTMENTS_LVL.NBT, (short) level); +- return; +- } +- } +- NBTTagCompound tag = new NBTTagCompound(); +- tag.setShort(ENCHANTMENTS_ID.NBT, (short) ench.getId()); +- tag.setShort(ENCHANTMENTS_LVL.NBT, (short) level); +- list.add(tag); ++ // Paper start - Replace whole method ++ final ItemMeta itemMeta = getItemMeta(); ++ itemMeta.addEnchant(ench, level, true); ++ setItemMeta(itemMeta); ++ // Paper end + } + + static boolean makeTag(net.minecraft.server.ItemStack item) { +@@ -222,66 +206,34 @@ public final class CraftItemStack extends ItemStack { + + @Override + public boolean containsEnchantment(Enchantment ench) { +- return getEnchantmentLevel(ench) > 0; ++ return hasItemMeta() && getItemMeta().hasEnchant(ench); // Paper - use meta + } + + @Override + public int getEnchantmentLevel(Enchantment ench) { +- Validate.notNull(ench, "Cannot find null enchantment"); +- if (handle == null) { +- return 0; +- } +- return EnchantmentManager.getEnchantmentLevel(CraftEnchantment.getRaw(ench), handle); ++ return hasItemMeta() ? getItemMeta().getEnchantLevel(ench) : 0; // Pape - replace entire method with meta + } + + @Override + public int removeEnchantment(Enchantment ench) { + Validate.notNull(ench, "Cannot remove null enchantment"); +- +- NBTTagList list = getEnchantmentList(handle), listCopy; +- if (list == null) { +- return 0; +- } +- int index = Integer.MIN_VALUE; +- int level = Integer.MIN_VALUE; +- int size = list.size(); +- +- for (int i = 0; i < size; i++) { +- NBTTagCompound enchantment = (NBTTagCompound) list.get(i); +- int id = 0xffff & enchantment.getShort(ENCHANTMENTS_ID.NBT); +- if (id == ench.getId()) { +- index = i; +- level = 0xffff & enchantment.getShort(ENCHANTMENTS_LVL.NBT); +- break; +- } +- } +- +- if (index == Integer.MIN_VALUE) { +- return 0; +- } +- if (size == 1) { +- handle.getTag().remove(ENCHANTMENTS.NBT); +- if (handle.getTag().isEmpty()) { +- handle.setTag(null); ++ // Paper start - replace entire method, maintain backwards compat of returning previous level. ++ final ItemMeta itemMeta = getItemMeta(); ++ final Iterator iterator = itemMeta.getEnchants().keySet().iterator(); ++ for (int i = 0; iterator.hasNext(); i++) { ++ if (iterator.next().equals(ench)) { ++ itemMeta.removeEnchant(ench); ++ setItemMeta(itemMeta); ++ return i; + } +- return level; + } +- +- // This is workaround for not having an index removal +- listCopy = new NBTTagList(); +- for (int i = 0; i < size; i++) { +- if (i != index) { +- listCopy.add(list.get(i)); +- } +- } +- handle.getTag().set(ENCHANTMENTS.NBT, listCopy); +- +- return level; ++ // Paper end ++ return 0; + } + + @Override + public Map getEnchantments() { +- return getEnchantments(handle); ++ return hasItemMeta() ? getItemMeta().getEnchants() : ImmutableMap.of(); // Paper - use Item Meta + } + + static Map getEnchantments(net.minecraft.server.ItemStack item) { +diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java +index 22cc267..94f2ba0 100644 +--- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java ++++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java +@@ -6,13 +6,8 @@ import java.lang.annotation.RetentionPolicy; + import java.lang.annotation.Target; + import java.lang.reflect.Constructor; + import java.lang.reflect.InvocationTargetException; +-import java.util.ArrayList; +-import java.util.Collection; +-import java.util.HashMap; +-import java.util.List; +-import java.util.Map; +-import java.util.NoSuchElementException; + ++import com.google.common.collect.ImmutableSortedMap; + import net.minecraft.server.NBTBase; + import net.minecraft.server.NBTTagCompound; + import net.minecraft.server.NBTTagList; +@@ -37,11 +32,18 @@ import com.google.common.collect.Sets; + import java.io.ByteArrayInputStream; + import java.io.ByteArrayOutputStream; + import java.io.IOException; ++import java.util.ArrayList; + import java.util.Arrays; ++import java.util.Collection; ++import java.util.Comparator; + import java.util.EnumSet; ++import java.util.HashMap; + import java.util.HashSet; +-import java.util.Iterator; ++import java.util.List; ++import java.util.Map; ++import java.util.NoSuchElementException; + import java.util.Set; ++import java.util.TreeMap; + import java.util.logging.Level; + import java.util.logging.Logger; + import net.minecraft.server.NBTCompressedStreamTools; +@@ -222,13 +224,13 @@ class CraftMetaItem implements ItemMeta, Repairable { + + private String displayName; + private List lore; +- private Map enchantments; ++ private EnchantmentMap enchantments; // Paper + private int repairCost; + private int hideFlag; + + private static final Set HANDLED_TAGS = Sets.newHashSet(); + +- private final Map unhandledTags = new HashMap(); ++ private final Map unhandledTags = new TreeMap<>(); // Paper + + CraftMetaItem(CraftMetaItem meta) { + if (meta == null) { +@@ -242,7 +244,7 @@ class CraftMetaItem implements ItemMeta, Repairable { + } + + if (meta.enchantments != null) { // Spigot +- this.enchantments = new HashMap(meta.enchantments); ++ this.enchantments = new EnchantmentMap(meta.enchantments); // Paper + } + + this.repairCost = meta.repairCost; +@@ -457,13 +459,13 @@ class CraftMetaItem implements ItemMeta, Repairable { + // Spigot end + } + +- static Map buildEnchantments(NBTTagCompound tag, ItemMetaKey key) { ++ static EnchantmentMap buildEnchantments(NBTTagCompound tag, ItemMetaKey key) { // Paper + if (!tag.hasKey(key.NBT)) { + return null; + } + + NBTTagList ench = tag.getList(key.NBT, 10); +- Map enchantments = new HashMap(ench.size()); ++ EnchantmentMap enchantments = new EnchantmentMap(); // Paper + + for (int i = 0; i < ench.size(); i++) { + int id = 0xffff & ((NBTTagCompound) ench.get(i)).getShort(ENCHANTMENTS_ID.NBT); +@@ -536,13 +538,13 @@ class CraftMetaItem implements ItemMeta, Repairable { + void deserializeInternal(NBTTagCompound tag) { + } + +- static Map buildEnchantments(Map map, ItemMetaKey key) { ++ static EnchantmentMap buildEnchantments(Map map, ItemMetaKey key) { // Paper + Map ench = SerializableMeta.getObject(Map.class, map, key.BUKKIT, true); + if (ench == null) { + return null; + } + +- Map enchantments = new HashMap(ench.size()); ++ EnchantmentMap enchantments = new EnchantmentMap(); // Paper + for (Map.Entry entry : ench.entrySet()) { + Enchantment enchantment = Enchantment.getByName(entry.getKey().toString()); + +@@ -672,12 +674,12 @@ class CraftMetaItem implements ItemMeta, Repairable { + } + + public Map getEnchants() { +- return hasEnchants() ? ImmutableMap.copyOf(enchantments) : ImmutableMap.of(); ++ return hasEnchants() ? ImmutableSortedMap.copyOfSorted(enchantments) : ImmutableMap.of(); // Paper + } + + public boolean addEnchant(Enchantment ench, int level, boolean ignoreRestrictions) { + if (enchantments == null) { +- enchantments = new HashMap(4); ++ enchantments = new EnchantmentMap(); // Paper + } + + if (ignoreRestrictions || level >= ench.getStartLevel() && level <= ench.getMaxLevel()) { +@@ -835,7 +837,7 @@ class CraftMetaItem implements ItemMeta, Repairable { + clone.lore = new ArrayList(this.lore); + } + if (this.enchantments != null) { +- clone.enchantments = new HashMap(this.enchantments); ++ clone.enchantments = new EnchantmentMap(this.enchantments); // Paper + } + clone.hideFlag = this.hideFlag; + return clone; +@@ -991,6 +993,28 @@ class CraftMetaItem implements ItemMeta, Repairable { + } + } + ++ // Paper start ++ private static class EnchantmentMap extends TreeMap { ++ private EnchantmentMap(Map enchantments) { ++ this(); ++ putAll(enchantments); ++ } ++ ++ private EnchantmentMap() { ++ super(new Comparator() { ++ @Override ++ public int compare(Enchantment o1, Enchantment o2) { ++ return ((Integer) o1.getId()).compareTo(o2.getId()); ++ } ++ }); ++ } ++ ++ public EnchantmentMap clone() { ++ return (EnchantmentMap) super.clone(); ++ } ++ } ++ // Paper end ++ + // Spigot start + private final Spigot spigot = new Spigot() + { +-- +2.7.3 +