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
This commit is contained in:
Bjarne Koll 2024-07-12 20:47:08 +02:00 committed by GitHub
parent ffe31a1cd0
commit 5a503d7db4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 47 additions and 7 deletions

View file

@ -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
+

View file

@ -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
}

View file

@ -0,0 +1,34 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Bjarne Koll <lynxplay101@gmail.com>
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