From c3749a2358d93d272702a20fa6d36b08722f9ea3 Mon Sep 17 00:00:00 2001 From: blablubbabc Date: Thu, 18 Apr 2019 11:56:42 +0200 Subject: [PATCH] Remove the Damage tag from items when it is 0. CraftBukkit (and Minecraft as well in some cases, such as when getting an item from the creative inventory menu) will omit the Damage tag when it is zero. However, minecraft will add the tag in some situations nevertheless, such as when loading the ItemStack, or when explictly setting the item undamaged. These items (with and without the Damage tag for undamaged items) will be considered as different by minecraft and CraftBukkit in various situations, even though they should not. In CraftBukkit these items will actually only be considered unsimilar if the items' metadata is not 'empty' (if it contains other additional metadata, such as enchantments, etc.). If the item's tag is empty after removing the Damage tag, it gets completely removed. The setRepairCost function was adapted to behave in the same way (removal of the tag if it becomes empty). --- nms-patches/ItemStack.patch | 27 ++++++++---- .../craftbukkit/inventory/ItemMetaTest.java | 42 +++++++++++++++---- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/nms-patches/ItemStack.patch b/nms-patches/ItemStack.patch index 2a65e3be6a..d82038e8d0 100644 --- a/nms-patches/ItemStack.patch +++ b/nms-patches/ItemStack.patch @@ -248,7 +248,20 @@ } return nbttagcompound; -@@ -191,6 +371,21 @@ +@@ -167,6 +347,12 @@ + } + + public void setDamage(int i) { ++ // CraftBukkit start - remove Damage tag if 0 ++ if (i <= 0) { ++ this.c("Damage"); // PAIL removeTag ++ return; ++ } ++ // CraftBukkit end + this.getOrCreateTag().setInt("Damage", Math.max(0, i)); + } + +@@ -191,6 +377,21 @@ } i -= k; @@ -270,7 +283,7 @@ if (i <= 0) { return false; } -@@ -212,6 +407,11 @@ +@@ -212,6 +413,11 @@ if (this.isDamaged(i, entityliving.getRandom(), entityliving instanceof EntityPlayer ? (EntityPlayer) entityliving : null)) { entityliving.c(this); Item item = this.getItem(); @@ -282,7 +295,7 @@ this.subtract(1); if (entityliving instanceof EntityHuman) { -@@ -335,6 +535,17 @@ +@@ -335,6 +541,17 @@ return this.tag; } @@ -300,22 +313,20 @@ public NBTTagCompound getOrCreateTag() { if (this.tag == null) { this.setTag(new NBTTagCompound()); -@@ -479,6 +690,14 @@ +@@ -479,6 +696,12 @@ } public void setRepairCost(int i) { + // CraftBukkit start - remove RepairCost tag when 0 (SPIGOT-3945) + if (i == 0) { -+ if (this.hasTag()) { -+ this.tag.remove("RepairCost"); -+ } ++ this.c("RepairCost"); // PAIL removeTag + return; + } + // CraftBukkit end this.getOrCreateTag().setInt("RepairCost", i); } -@@ -521,6 +740,13 @@ +@@ -521,6 +744,13 @@ nbttaglist.add((NBTBase) nbttagcompound); } diff --git a/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java b/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java index 65b8123a8b..5a2de14fa5 100644 --- a/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java +++ b/src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java @@ -147,17 +147,41 @@ public class ItemMetaTest extends AbstractTestingBase { } @Test - public void testTaggedButNotMeta() { - CraftItemStack craft = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS)); - craft.handle.setDamage(0); + public void testNoDamageEquality() { + CraftItemStack noTag = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS)); - assertThat("Should have NBT tag", CraftItemStack.hasItemMeta(craft.handle), is(true)); - assertThat("NBT Tag should contain Damage", craft.handle.getTag().get("Damage"), instanceOf(NBTTagInt.class)); - assertThat("But we should not have meta", craft.hasItemMeta(), is(false)); + CraftItemStack noDamage = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS)); + noDamage.handle.setDamage(0); - 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)); + CraftItemStack enchanted = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS)); + enchanted.addEnchantment(Enchantment.DIG_SPEED, 1); + + CraftItemStack enchantedNoDamage = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS)); + enchantedNoDamage.addEnchantment(Enchantment.DIG_SPEED, 1); + enchantedNoDamage.handle.setDamage(0); + + // check tags: + assertThat("noTag should have no NBT tag", CraftItemStack.hasItemMeta(noTag.handle), is(false)); + assertThat("noTag should have no meta", noTag.hasItemMeta(), is(false)); + + assertThat("noDamage should have no NBT tag", CraftItemStack.hasItemMeta(noDamage.handle), is(false)); + assertThat("noDamage should have no meta", noDamage.hasItemMeta(), is(false)); + + assertThat("enchanted should have NBT tag", CraftItemStack.hasItemMeta(enchanted.handle), is(true)); + assertThat("enchanted should have meta", enchanted.hasItemMeta(), is(true)); + + assertThat("enchantedNoDamage should have NBT tag", CraftItemStack.hasItemMeta(enchantedNoDamage.handle), is(true)); + assertThat("enchantedNoDamage should have meta", enchantedNoDamage.hasItemMeta(), is(true)); + + // check comparisons: + assertThat("noTag and noDamage stacks should be similar", noTag.isSimilar(noDamage), is(true)); + assertThat("noTag and noDamage stacks should be equal", noTag.equals(noDamage), is(true)); + + assertThat("enchanted and enchantedNoDamage stacks should be similar", enchanted.isSimilar(enchantedNoDamage), is(true)); + assertThat("enchanted and enchantedNoDamage stacks should be equal", enchanted.equals(enchantedNoDamage), is(true)); + + assertThat("noTag and enchanted stacks should not be similar", noTag.isSimilar(enchanted), is(false)); + assertThat("noTag and enchanted stacks should not be equal", noTag.equals(enchanted), is(false)); } @Test