mirror of
https://github.com/PaperMC/Paper.git
synced 2024-12-23 17:37:45 +01:00
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.
This commit is contained in:
Aikar
parent
dc477811ed
commit
e9222c0be7
@ -0,0 +1,297 @@
|
||||
From 5207c4aef31a69a4f9cded63c9b90e056c7f9d50 Mon Sep 17 00:00:00 2001
|
||||
From: Aikar <aikar@aikar.co>
|
||||
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<Enchantment> 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<Enchantment, Integer> getEnchantments() {
|
||||
- return getEnchantments(handle);
|
||||
+ return hasItemMeta() ? getItemMeta().getEnchants() : ImmutableMap.<Enchantment, Integer>of(); // Paper - use Item Meta
|
||||
}
|
||||
|
||||
static Map<Enchantment, Integer> 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<String> lore;
|
||||
- private Map<Enchantment, Integer> enchantments;
|
||||
+ private EnchantmentMap enchantments; // Paper
|
||||
private int repairCost;
|
||||
private int hideFlag;
|
||||
|
||||
private static final Set<String> HANDLED_TAGS = Sets.newHashSet();
|
||||
|
||||
- private final Map<String, NBTBase> unhandledTags = new HashMap<String, NBTBase>();
|
||||
+ private final Map<String, NBTBase> 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<Enchantment, Integer>(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<Enchantment, Integer> 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<Enchantment, Integer> enchantments = new HashMap<Enchantment, Integer>(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<Enchantment, Integer> buildEnchantments(Map<String, Object> map, ItemMetaKey key) {
|
||||
+ static EnchantmentMap buildEnchantments(Map<String, Object> map, ItemMetaKey key) { // Paper
|
||||
Map<?, ?> ench = SerializableMeta.getObject(Map.class, map, key.BUKKIT, true);
|
||||
if (ench == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
- Map<Enchantment, Integer> enchantments = new HashMap<Enchantment, Integer>(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<Enchantment, Integer> getEnchants() {
|
||||
- return hasEnchants() ? ImmutableMap.copyOf(enchantments) : ImmutableMap.<Enchantment, Integer>of();
|
||||
+ return hasEnchants() ? ImmutableSortedMap.copyOfSorted(enchantments) : ImmutableMap.<Enchantment, Integer>of(); // Paper
|
||||
}
|
||||
|
||||
public boolean addEnchant(Enchantment ench, int level, boolean ignoreRestrictions) {
|
||||
if (enchantments == null) {
|
||||
- enchantments = new HashMap<Enchantment, Integer>(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<String>(this.lore);
|
||||
}
|
||||
if (this.enchantments != null) {
|
||||
- clone.enchantments = new HashMap<Enchantment, Integer>(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<Enchantment, Integer> {
|
||||
+ private EnchantmentMap(Map<Enchantment, Integer> enchantments) {
|
||||
+ this();
|
||||
+ putAll(enchantments);
|
||||
+ }
|
||||
+
|
||||
+ private EnchantmentMap() {
|
||||
+ super(new Comparator<Enchantment>() {
|
||||
+ @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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user