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

@@ -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.c("RepairCost"); // PAIL removeTag
-+                this.tag.remove("RepairCost");
-+            }
 +            return;
 +        }
 +        // CraftBukkit end
          this.getOrCreateTag().setInt("RepairCost", i);
      }
  
-@@ -521,6 +740,13 @@
+@@ -521,6 +744,13 @@
          nbttaglist.add((NBTBase) nbttagcompound);
      }
  

src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java

@@ -147,17 +147,41 @@ public class ItemMetaTest extends AbstractTestingBase {
     }
 
     @Test
-    public void testTaggedButNotMeta() {
+    public void testNoDamageEquality() {
-        CraftItemStack craft = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS));
+        CraftItemStack noTag = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS));
-        craft.handle.setDamage(0);
 
-        assertThat("Should have NBT tag", CraftItemStack.hasItemMeta(craft.handle), is(true));
+        CraftItemStack noDamage = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS));
-        assertThat("NBT Tag should contain Damage", craft.handle.getTag().get("Damage"), instanceOf(NBTTagInt.class));
+        noDamage.handle.setDamage(0);
-        assertThat("But we should not have meta", craft.hasItemMeta(), is(false));
 
-        ItemStack pureBukkit = new ItemStack(Material.SHEARS);
+        CraftItemStack enchanted = CraftItemStack.asCraftCopy(new ItemStack(Material.SHEARS));
-        assertThat("Bukkit and craft stacks should be similar", craft.isSimilar(pureBukkit), is(true));
+        enchanted.addEnchantment(Enchantment.DIG_SPEED, 1);
-        assertThat("Bukkit and craft stacks should be equal", craft.equals(pureBukkit), is(true));
+
+        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