From 41148ad75dcb5ba9bfbff7a6bfa03d861f50cf03 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/net/minecraft/server/ItemStack.java b/src/main/java/net/minecraft/server/ItemStack.java index 0dcea61d..a8f7ff98 100644 --- a/src/main/java/net/minecraft/server/ItemStack.java +++ b/src/main/java/net/minecraft/server/ItemStack.java @@ -56,6 +56,22 @@ public final class ItemStack { } // CraftBukkit start + // Paper start + private static final java.util.Comparator enchantSorter = java.util.Comparator.comparingInt(o -> o.getShort("id")); + private void processEnchantOrder(NBTTagCompound tag) { + if (tag == null || !tag.hasKeyOfType("ench", 9)) { + return; + } + NBTTagList list = tag.getList("ench", 10); + if (list.size() < 2) { + return; + } + try { + list.sort(enchantSorter); // Paper + } catch (Exception ignored) {} + } + // Paper end + public ItemStack(Item item, int i, int j) { this(item, i, j, true); } @@ -113,6 +129,7 @@ public final class ItemStack { if (nbttagcompound.hasKeyOfType("tag", 10)) { // CraftBukkit start - make defensive copy as this data may be coming from the save thread this.tag = (NBTTagCompound) nbttagcompound.getCompound("tag").clone(); + processEnchantOrder(this.tag); // Paper if (this.item != null) { this.item.a(this.tag); // CraftBukkit end @@ -585,6 +602,7 @@ public final class ItemStack { public void setTag(@Nullable NBTTagCompound nbttagcompound) { this.tag = nbttagcompound; + processEnchantOrder(this.tag); // Paper } public String getName() { @@ -658,6 +676,7 @@ public final class ItemStack { nbttagcompound.setShort("id", (short) Enchantment.getId(enchantment)); nbttagcompound.setShort("lvl", (short) ((byte) i)); nbttaglist.add(nbttagcompound); + processEnchantOrder(nbttagcompound); // Paper } public boolean hasEnchantments() { diff --git a/src/main/java/net/minecraft/server/NBTTagList.java b/src/main/java/net/minecraft/server/NBTTagList.java index ca9eb2f3..576c3b71 100644 --- a/src/main/java/net/minecraft/server/NBTTagList.java +++ b/src/main/java/net/minecraft/server/NBTTagList.java @@ -14,6 +14,12 @@ public class NBTTagList extends NBTBase { private static final Logger b = LogManager.getLogger(); public List list = Lists.newArrayList(); // Paper + // Paper start + public void sort(java.util.Comparator comparator) { + //noinspection unchecked + java.util.Collections.sort(list, (java.util.Comparator) comparator); + } + // Paper end private byte type = 0; public NBTTagList() {} diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java index fb1dc542..cdf16e15 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; @@ -183,28 +184,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) { @@ -221,66 +205,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 c743ae06..0cdc8007 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; @@ -38,10 +33,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.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; @@ -226,7 +229,7 @@ class CraftMetaItem implements ItemMeta, Repairable { private String displayName; private String locName; private List lore; - private Map enchantments; + private EnchantmentMap enchantments; // Paper private int repairCost; private int hideFlag; private boolean unbreakable; @@ -234,7 +237,7 @@ class CraftMetaItem implements ItemMeta, Repairable { private static final Set HANDLED_TAGS = Sets.newHashSet(); private NBTTagCompound internalTag; - private final Map unhandledTags = new HashMap(); + private final Map unhandledTags = new TreeMap<>(); // Paper CraftMetaItem(CraftMetaItem meta) { if (meta == null) { @@ -249,7 +252,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; @@ -470,13 +473,13 @@ class CraftMetaItem implements ItemMeta, Repairable { } } - 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, CraftMagicNumbers.NBT.TAG_COMPOUND); - 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); @@ -546,13 +549,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()) { // Doctor older enchants String enchantKey = entry.getKey().toString(); @@ -703,13 +706,13 @@ 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) { Validate.notNull(ench, "Enchantment cannot be null"); if (enchantments == null) { - enchantments = new HashMap(4); + enchantments = new EnchantmentMap(); // Paper } if (ignoreRestrictions || level >= ench.getStartLevel() && level <= ench.getMaxLevel()) { @@ -880,7 +883,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; clone.unbreakable = this.unbreakable; @@ -1038,6 +1041,23 @@ class CraftMetaItem implements ItemMeta, Repairable { } } + // Paper start + private static class EnchantmentMap extends TreeMap { + private EnchantmentMap(Map enchantments) { + this(); + putAll(enchantments); + } + + private EnchantmentMap() { + super((o1, o2) -> ((Integer) o1.getId()).compareTo(o2.getId())); + } + + public EnchantmentMap clone() { + return (EnchantmentMap) super.clone(); + } + } + // Paper end + // Spigot start private final Spigot spigot = new Spigot() { -- 2.14.3