Bulk bugfixes for itemstack damage API (#11063)

A general set of bugfixes for itemstack damage related logic.

1. Prevent NPE when calling deprecated ItemStack#getMaxItemUseDuration()
2. Do not apply enchantments when damaging items via API
3. Do not error when passing a null equipment slot to hurtAndBreak
4. Correctly call PlayerItemBreakEvent

3 files changed, 47 insertions, 7 deletions

diff --git a/patches/server/0194-ItemStack-getMaxItemUseDuration.patch b/patches/server/0194-ItemStack-getMaxItemUseDuration.patch
index 8e84109de0..7bd4cd1c1b 100644
--- a/patches/server/0194-ItemStack-getMaxItemUseDuration.patch
+++ b/patches/server/0194-ItemStack-getMaxItemUseDuration.patch
@@ -6,7 +6,7 @@ Subject: [PATCH] ItemStack#getMaxItemUseDuration
 Allows you to determine how long it takes to use a usable/consumable item
 
 diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
-index 9fa993ac05092170794911394c994fcad33d648f..23d61d553be3ab0a62624e469b2782baa2b075db 100644
+index 9fa993ac05092170794911394c994fcad33d648f..75f9405ee3453620e1561857575cc8700971a865 100644
 --- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
 +++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
 @@ -7,14 +7,17 @@ import net.minecraft.core.Holder;
@@ -42,7 +42,7 @@ index 9fa993ac05092170794911394c994fcad33d648f..23d61d553be3ab0a62624e469b2782ba
 +        if (entity == null && handle.is(Items.CROSSBOW)) {
 +            throw new UnsupportedOperationException("This item requires an entity to determine the max use duration");
 +        }
-+        return handle.getUseDuration(((CraftLivingEntity) entity).getHandle());
++        return handle.getUseDuration(entity != null ? ((CraftLivingEntity) entity).getHandle() : null);
 +    }
 +    // Paper end
 +
diff --git a/patches/server/0775-ItemStack-damage-API.patch b/patches/server/0775-ItemStack-damage-API.patch
index 7e85a68861..24e6ad94dd 100644
--- a/patches/server/0775-ItemStack-damage-API.patch
+++ b/patches/server/0775-ItemStack-damage-API.patch
@@ -11,10 +11,10 @@ the logic associated with damaging them
 public net.minecraft.world.entity.LivingEntity entityEventForEquipmentBreak(Lnet/minecraft/world/entity/EquipmentSlot;)B
 
 diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
-index d825c2e3808e44db9935dab4e7b528146c6d83c2..96a7e80e3efab1bf626fb7aff61e095ca09789d4 100644
+index d825c2e3808e44db9935dab4e7b528146c6d83c2..57510b85caf8914290ab0afb89cfb773158715b8 100644
 --- a/src/main/java/net/minecraft/world/item/ItemStack.java
 +++ b/src/main/java/net/minecraft/world/item/ItemStack.java
-@@ -647,8 +647,13 @@ public final class ItemStack implements DataComponentHolder {
+@@ -647,11 +647,16 @@ public final class ItemStack implements DataComponentHolder {
      }
  
      public void hurtAndBreak(int amount, ServerLevel world, @Nullable LivingEntity player, Consumer<Item> breakCallback) { // Paper - Add EntityDamageItemEvent
@@ -28,7 +28,11 @@ index d825c2e3808e44db9935dab4e7b528146c6d83c2..96a7e80e3efab1bf626fb7aff61e095c
 +            if (player == null || !player.hasInfiniteMaterials() || force) { // Paper
                  if (amount > 0) {
                      int originalDamage = amount; // Paper - Expand PlayerItemDamageEvent
-                     amount = EnchantmentHelper.processDurabilityChange(world, this, amount);
+-                    amount = EnchantmentHelper.processDurabilityChange(world, this, amount);
++                    if (!force) amount = EnchantmentHelper.processDurabilityChange(world, this, amount); // Paper - itemstack damage API - do not consider enchantments when damaging from API
+                     // CraftBukkit start
+                     if (player instanceof ServerPlayer serverPlayer) { // Paper - Add EntityDamageItemEvent
+                         PlayerItemDamageEvent event = new PlayerItemDamageEvent(serverPlayer.getBukkitEntity(), CraftItemStack.asCraftMirror(this), amount, originalDamage); // Paper - Add EntityDamageItemEvent & Expand PlayerItemDamageEvent
 @@ -699,6 +704,11 @@ public final class ItemStack implements DataComponentHolder {
      }
  
@@ -41,11 +45,13 @@ index d825c2e3808e44db9935dab4e7b528146c6d83c2..96a7e80e3efab1bf626fb7aff61e095c
          Level world = entity.level();
  
          if (world instanceof ServerLevel worldserver) {
-@@ -717,7 +727,7 @@ public final class ItemStack implements DataComponentHolder {
+@@ -716,8 +726,8 @@ public final class ItemStack implements DataComponentHolder {
+                     org.bukkit.craftbukkit.event.CraftEventFactory.callPlayerItemBreakEvent((net.minecraft.world.entity.player.Player) entity, this);
                  }
                  // CraftBukkit end
-                 entity.onEquippedItemBroken(item, slot);
+-                entity.onEquippedItemBroken(item, slot);
 -            });
++                if (slot != null) entity.onEquippedItemBroken(item, slot); // Paper - itemstack damage API - do not process entity related callbacks when damaging from API
 +            }, force); // Paper
          }
  
diff --git a/patches/server/1040-Correctly-call-PlayerItemBreakEvent.patch b/patches/server/1040-Correctly-call-PlayerItemBreakEvent.patch
new file mode 100644
index 0000000000..6637cd6cd0
--- /dev/null
+++ b/patches/server/1040-Correctly-call-PlayerItemBreakEvent.patch
@@ -0,0 +1,34 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Bjarne Koll <[email protected]>
+Date: Fri, 12 Jul 2024 19:09:44 +0200
+Subject: [PATCH] Correctly call PlayerItemBreakEvent
+
+The minecraft 1.21 update changed the invocation order in
+ItemStack#hurtAndBreak, now first shrinking the itemstack to a count of
+zero and then invoking the break callback.
+
+This leads to spigots logic no longer firing at all. This patch now
+correctly executes on count of zero and temporarily bumps the count to
+one before passing it to event handlers to maintain compatibility with
+the event contracts.
+
+This fix was chosen over invoking the callback prior to shrinking the
+stack to not disrupt potential new vanilla changes that might depend on
+this behaviour.
+
+diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
+index 0f2b3c5ca88478a541bf9e61ae61cc99a7d08836..2c312c0b741fb96a008881e9e01fa660a1fb63ab 100644
+--- a/src/main/java/net/minecraft/world/item/ItemStack.java
++++ b/src/main/java/net/minecraft/world/item/ItemStack.java
+@@ -731,8 +731,10 @@ public final class ItemStack implements DataComponentHolder {
+ 
+             this.hurtAndBreak(amount, worldserver, entity, (item) -> { // Paper - Add EntityDamageItemEvent
+                 // CraftBukkit start - Check for item breaking
+-                if (this.count == 1 && entity instanceof net.minecraft.world.entity.player.Player) {
++                if (this.count == 0 && entity instanceof net.minecraft.world.entity.player.Player) { // Paper - correctly call item break event - run if count reached 0
++                    this.setCount(1); // Paper - correctly call item break event - grow to count 1
+                     org.bukkit.craftbukkit.event.CraftEventFactory.callPlayerItemBreakEvent((net.minecraft.world.entity.player.Player) entity, this);
++                    this.setCount(0); // Paper - correctly call item break event - reset to count 0
+                 }
+                 // CraftBukkit end
+                 if (slot != null) entity.onEquippedItemBroken(item, slot); // Paper - itemstack damage API - do not process entity related callbacks when damaging from API