mirrors/papermc
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Optimize hasItemMeta (remove getItemMeta call) (#1279)

Browse source 
Spigot 1.13 checks if any field (which are manually copied from the ItemStack's "tag" NBT tag) on the ItemMeta class of an ItemStack is set.

We could just check if the "tag" NBT tag is empty, albeit that would break some plugins. The only general tag added on 1.13 is "Damage", and we can just check if the "tag" NBT tag contains any other tag that's not "Damage" (https://minecraft.gamepedia.com/Player.dat_format#Item_structure) making the `hasItemStack` method behave as before.

Check the `ItemMetaTest#testTaggedButNotMeta` method to see how this method behaves. (I also added some extra tests).

`hasItemMeta()` will return true if `ItemStack.getDamage() != 0` or it has the `Damage` tag or any other tag is set.

Closes #1222
This commit is contained in:
Hugo Manrique 2018-07-29 18:30:01 +02:00 committed by Daniel Ennis
parent fa9224721e
commit 6d166798e6
1 changed files with 88 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

88
Spigot-Server-Patches/0315-Don-t-call-getItemMeta-on-hasItemMeta.patch Normal file
View file

@ -0,0 +1,88 @@
From 78471ebdec4229b758d9217bb793434fccef264f Mon Sep 17 00:00:00 2001
From: Hugo Manrique <hugmanrique@gmail.com>
Date: Thu, 26 Jul 2018 14:10:23 +0200
Subject: [PATCH] Don't call getItemMeta on hasItemMeta
Spigot 1.13 checks if any field (which are manually copied from the ItemStack's "tag" NBT tag) on the ItemMeta class of an ItemStack is set.
We could just check if the "tag" NBT tag is empty, albeit that would break some plugins. The only general tag added on 1.13 is "Damage", and we can just check if the "tag" NBT tag contains any other tag that's not "Damage" (https://minecraft.gamepedia.com/Player.dat_format#Item_structure) making the `hasItemStack` method behave as before.
Returns true if getDamage() == 0 or has damage tag or other tag is set.
Check the `ItemMetaTest#testTaggedButNotMeta` method to see how this method behaves.
diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
index 5f3331de..ae217809 100644
--- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
+++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
@@ -517,7 +517,7 @@ public final class CraftItemStack extends ItemStack {
@Override
public boolean hasItemMeta() {
- return hasItemMeta(handle) && !CraftItemFactory.instance().equals(getItemMeta(), null);
+ return hasItemMeta(handle) && (handle.getDamage() != 0 || (handle.getTag() != null && handle.getTag().map.size() >= (handle.getTag().hasKey(CraftMetaItem.DAMAGE.NBT) ? 2 : 1))); // Paper - keep 1.12 CraftBukkit behavior without calling getItemMeta
}
static boolean hasItemMeta(net.minecraft.server.ItemStack item) {
diff --git a/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java b/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java
index eb6cf1bb..79ce3752 100644
--- a/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java
+++ b/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java
@@ -5,6 +5,7 @@ import static org.hamcrest.Matchers.*;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections; // Paper
import java.util.List;
import com.destroystokyo.paper.inventory.meta.ArmorStandMeta; // Paper
import net.minecraft.server.Block;
@@ -154,8 +155,47 @@ public class ItemMetaTest extends AbstractTestingBase {
ItemStack pureBukkit = new ItemStack(Material.SHEARS);
assertThat("Bukkit and craft stacks should be similar", craft.isSimilar(pureBukkit), is(true));
assertThat("Bukkit and craft stacks should be equal", craft.equals(pureBukkit), is(true));
+ // Paper start - test additional ItemMeta damage cases
+ ItemStack clone = CraftItemStack.asBukkitCopy(CraftItemStack.asNMSCopy(craft));
+ assertThat("Bukkit and craft stacks should be similar", craft.isSimilar(clone), is(true));
+ assertThat("Bukkit and craft stacks should be equal", craft.equals(clone), is(true));
+
+ pureBukkit = new ItemStack(Material.DIAMOND_SWORD);
+ pureBukkit.setDurability((short) 2);
+ net.minecraft.server.ItemStack nms = CraftItemStack.asNMSCopy(pureBukkit);
+ ItemStack other = CraftItemStack.asBukkitCopy(nms);
+
+ assertThat("Bukkit and NMS ItemStack copies should be similar", pureBukkit.isSimilar(other), is(true));
+ assertThat("Bukkit and NMS ItemStack copies should be equal", pureBukkit.equals(other), is(true));
}
+ private void testItemMeta(ItemStack stack) {
+ assertThat("Should not have ItemMeta", stack.hasItemMeta(), is(false));
+
+ stack.setDurability((short) 0);
+ assertThat("ItemStack with zero durability should not have ItemMeta", stack.hasItemMeta(), is(false));
+
+ stack.setDurability((short) 2);
+ assertThat("ItemStack with non-zero durability should have ItemMeta", stack.hasItemMeta(), is(true));
+
+ stack.setLore(Collections.singletonList("Lore"));
+ assertThat("ItemStack with lore and durability should have ItemMeta", stack.hasItemMeta(), is(true));
+
+ stack.setDurability((short) 0);
+ assertThat("ItemStack with lore should have ItemMeta", stack.hasItemMeta(), is(true));
+
+ stack.setLore(null);
+ }
+
+ @Test
+ public void testHasItemMeta() {
+ ItemStack itemStack = new ItemStack(Material.SHEARS);
+
+ testItemMeta(itemStack);
+ testItemMeta(CraftItemStack.asCraftCopy(itemStack));
+ }
+ // Paper end
+
@Test
public void testBlockStateMeta() {
List<Block> queue = new ArrayList<>();
--
2.18.0.windows.1