Re-introduce IForgeItem.damageItem when an item takes damage, Fixes #10344 (#10372)

patches/minecraft/net/minecraft/world/entity/player/Player.java.patch

@@ -142,6 +142,19 @@
              if (!this.level().isClientSide) {
                  this.awardStat(Stats.ITEM_USED.get(this.useItem.getItem()));
              }
+@@ -928,8 +_,11 @@
+             if (p_36383_ >= 3.0F) {
+                 int i = 1 + Mth.floor(p_36383_);
+                 InteractionHand interactionhand = this.getUsedItemHand();
++                // FORGE: cache this.useItem -- if this.stopUsingItem() is called, it will be set to ItemStack.EMPTY
++                ItemStack currentItem = this.useItem;
+                 this.useItem.hurtAndBreak(i, this, getSlotForHand(interactionhand));
+-                if (this.useItem.isEmpty()) {
++                // FORGE: use cached item since this.useItem could be ItemStack.EMPTY
++                if (currentItem.isEmpty()) {
+                     if (interactionhand == InteractionHand.MAIN_HAND) {
+                         this.setItemSlot(EquipmentSlot.MAINHAND, ItemStack.EMPTY);
+                     } else {
 @@ -946,10 +_,13 @@
      @Override
      protected void actuallyHurt(ServerLevel p_365751_, DamageSource p_36312_, float p_36313_) {

patches/minecraft/net/minecraft/world/item/ItemStack.java.patch

@@ -33,6 +33,34 @@
              if (player != null && interactionresult instanceof InteractionResult.Success interactionresult$success && interactionresult$success.wasItemInteraction()) {
                  player.awardStat(Stats.ITEM_USED.get(item));
              }
+@@ -459,18 +_,26 @@
+     }
+ 
+     public void hurtAndBreak(int p_220158_, ServerLevel p_342197_, @Nullable ServerPlayer p_220160_, Consumer<Item> p_343361_) {
+-        int i = this.processDurabilityChange(p_220158_, p_342197_, p_220160_);
++        // FORGE: use context-sensitive sister of processDurabilityChange that calls IForgeItem.damageItem
++        int i = this.processDurabilityChange(p_220158_, p_342197_, p_220160_, true, p_343361_);
+         if (i != 0) {
+             this.applyDamage(this.getDamageValue() + i, p_220160_, p_343361_);
+         }
+     }
+ 
+     private int processDurabilityChange(int p_362423_, ServerLevel p_364910_, @Nullable ServerPlayer p_365570_) {
++        return this.processDurabilityChange(p_362423_, p_364910_, p_365570_, false, p_359411_ -> { });
++    }
++
++    /** FORGE: context-sensitive sister of processDurabilityChange that calls IForgeItem.damageItem */
++    private int processDurabilityChange(int p_362423_, ServerLevel p_364910_, @Nullable ServerPlayer p_365570_, boolean canBreak, Consumer<Item> onBreak) {
+         if (!this.isDamageableItem()) {
+             return 0;
+         } else if (p_365570_ != null && p_365570_.hasInfiniteMaterials()) {
+             return 0;
+         } else {
++            // FORGE: modify the base damage based on the item's impl of IForgeItem.damageItem
++            p_362423_ = this.damageItem(p_362423_, p_364910_, p_365570_, canBreak, onBreak);
+             return p_362423_ > 0 ? EnchantmentHelper.processDurabilityChange(p_364910_, this, p_362423_) : p_362423_;
+         }
+     }
 @@ -507,7 +_,13 @@
                  p_41623_,
                  serverlevel,
@@ -41,7 +69,7 @@
 +                p_341563_ -> {
 +                    if (p_41624_ instanceof Player player) {
 +                        net.minecraftforge.event.ForgeEventFactory.onPlayerDestroyItem(player, this, p_335324_);
-+                        player.stopUsingItem(); // Forge: fix MC-168573
++                        if (player.getUseItem() == this) player.stopUsingItem(); // Forge: fix MC-168573
 +                    }
 +                    p_41624_.onEquippedItemBroken(p_341563_, p_335324_);
 +                }

src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java

@@ -5,6 +5,8 @@
 
 package net.minecraftforge.common.extensions;
 
+import java.util.Arrays;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -12,6 +14,8 @@ import java.util.function.Supplier;
 import net.minecraft.ChatFormatting;
 import net.minecraft.Util;
 import net.minecraft.network.chat.MutableComponent;
+import net.minecraft.core.Registry;
+import net.minecraft.resources.ResourceKey;
 import org.jetbrains.annotations.Nullable;
 
 import com.mojang.authlib.GameProfile;
@@ -74,6 +78,46 @@ public interface IForgeGameTestHelper {
             throw new GameTestAssertException(message.get());
     }
 
+    default <N> void assertValueEqual(N expected, N actual, String name, String message) {
+        this.assertValueEqual(expected, actual, name, () -> message);
+    }
+
+    default <N> void assertValueEqual(N expected, N actual, String name, Supplier<String> message) {
+        if (!Objects.equals(expected, actual))
+            throw new GameTestAssertException("%s -- Expected %s to be %s, but was %s".formatted(message.get(), name, expected, actual));
+    }
+
+    default <N> void assertValueEqual(N[] expected, N[] actual, String name, String message) {
+        this.assertValueEqual(expected, actual, name, () -> message);
+    }
+
+    default <N> void assertValueEqual(N[] expected, N[] actual, String name, Supplier<String> message) {
+        if (!Objects.deepEquals(expected, actual))
+            throw new GameTestAssertException("%s -- Expected %s to be %s, but was %s".formatted(message.get(), name, Arrays.toString(expected), Arrays.toString(actual)));
+    }
+
+    default <N> void assertValueNotEqual(N expected, N actual, String name, String message) {
+        this.assertValueNotEqual(expected, actual, name, () -> message);
+    }
+
+    default <N> void assertValueNotEqual(N expected, N actual, String name, Supplier<String> message) {
+        if (Objects.equals(expected, actual))
+            throw new GameTestAssertException("%s -- Expected %s to NOT be %s, but was".formatted(message.get(), name, expected));
+    }
+
+    default <N> void assertValueNotEqual(N[] expected, N[] actual, String name, String message) {
+        this.assertValueNotEqual(expected, actual, name, () -> message);
+    }
+
+    default <N> void assertValueNotEqual(N[] expected, N[] actual, String name, Supplier<String> message) {
+        if (!Objects.deepEquals(expected, actual))
+            throw new GameTestAssertException("%s -- Expected %s to NOT be %s, but was".formatted(message.get(), name, Arrays.toString(expected)));
+    }
+
+    default <E> Registry<E> registryLookup(ResourceKey<? extends Registry<? extends E>> registryKey) {
+        return this.self().getLevel().registryAccess().lookupOrThrow(registryKey);
+    }
+
     default ServerPlayer makeMockServerPlayer() {
         var level = self().getLevel();
         var cookie = CommonListenerCookie.createInitial(new GameProfile(UUID.randomUUID(), "test-mock-player"), false);
@@ -181,19 +225,49 @@ public interface IForgeGameTestHelper {
         }
 
         public void assertUnset() {
-            if (this.value != null)
-                throw new GameTestAssertException("Expected " + name + " to be null, but was " + this.value);
+            this.assertUnset((Supplier<String>) null);
+        }
+
+        public void assertUnset(String message) {
+            this.assertUnset(message != null ? () -> message : null);
+        }
+
+        public void assertUnset(Supplier<String> message) {
+            if (this.value != null) {
+                String s = message != null ? message.get() + " -- " : "";
+                throw new GameTestAssertException(s + "Expected " + name + " to be null, but was " + this.value);
+            }
         }
 
         public void assertSet() {
-            if (this.value == null)
-                throw new GameTestAssertException("Flag " + name + " was never set");
+            this.assertSet((Supplier<String>) null);
+        }
+
+        public void assertSet(String message) {
+            this.assertSet(message != null ? () -> message : null);
+        }
+
+        public void assertSet(Supplier<String> message) {
+            if (this.value == null) {
+                String s = message != null ? message.get() + " -- " : "";
+                throw new GameTestAssertException(s + "Flag " + name + " was never set");
+            }
         }
 
         public void assertEquals(T expected) {
-            assertSet();
-            if (expected != null && !expected.equals(this.value))
-                throw new GameTestAssertException("Expected " + name + " to be " + expected + ", but was " + this.value);
+            this.assertEquals(expected, (Supplier<String>) null);
+        }
+
+        public void assertEquals(T expected, String message) {
+            this.assertEquals(expected, message != null ? () -> message : null);
+        }
+
+        public void assertEquals(T expected, Supplier<String> message) {
+            assertSet(message);
+            if (expected != null && !expected.equals(this.value)) {
+                String s = message != null ? message.get() + " -- " : "";
+                throw new GameTestAssertException(s + "Expected " + name + " to be " + expected + ", but was " + this.value);
+            }
         }
     }
 
@@ -244,9 +318,29 @@ public interface IForgeGameTestHelper {
             return this.value == null ? -1 : this.value.longValue();
         }
 
+        public void assertEquals(int expected) {
+            super.assertEquals((long) expected);
+        }
+
         public void assertEquals(long expected) {
             super.assertEquals(expected);
         }
+
+        public void assertEquals(int expected, String message) {
+            super.assertEquals((long) expected, message);
+        }
+
+        public void assertEquals(long expected, String message) {
+            super.assertEquals(expected, message);
+        }
+
+        public void assertEquals(int expected, Supplier<String> message) {
+            super.assertEquals((long) expected, message);
+        }
+
+        public void assertEquals(long expected, Supplier<String> message) {
+            super.assertEquals(expected, message);
+        }
     }
 
     public static class BoolFlag extends Flag<Boolean> {
@@ -265,5 +359,13 @@ public interface IForgeGameTestHelper {
         public void assertEquals(boolean expected) {
             super.assertEquals(expected);
         }
+
+        public void assertEquals(boolean expected, String message) {
+            super.assertEquals(expected, message);
+        }
+
+        public void assertEquals(boolean expected, Supplier<String> message) {
+            super.assertEquals(expected, message);
+        }
     }
 }

src/main/java/net/minecraftforge/common/extensions/IForgeItem.java

@@ -7,6 +7,8 @@ package net.minecraftforge.common.extensions;
 
 import java.util.function.Consumer;
 
+import net.minecraft.server.level.ServerLevel;
+import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.world.damagesource.DamageSource;
 import net.minecraft.world.entity.player.Inventory;
 import net.minecraft.world.item.*;
@@ -481,6 +483,28 @@ public interface IForgeItem {
      */
     default void onHorseArmorTick(ItemStack stack, Level level, Mob horse) { }
 
+    /**
+     * Called when this item is to be damaged, such as use a tool being used or a shield blocking damage. The damage
+     * parameter has not yet been processed, so enchantments are not taken into account.
+     *
+     * @param stack    The processed amount of damage the item will take
+     * @param damage   The amount of damage the item will take before processing
+     * @param level    The level where the damage is taking place
+     * @param player   The player holding the item
+     * @param canBreak If the item can break from this damage instance ({@code true} if this is called from
+     *                 {@link ItemStack#hurtAndBreak(int, ServerLevel, ServerPlayer, Consumer)}, {@code false} if from
+     *                 {@link ItemStack#hurtWithoutBreaking(int, Player)})
+     * @param onBroken The callback for when an item is broken (use this if you plan on cancelling damage that will
+     *                 break an item)
+     * @return The amount of damage the item should take, after processing
+     *
+     * @apiNote If the item stack is not {@linkplain ItemStack#isDamageableItem() damageable} or the player
+     * {@linkplain Player#hasInfiniteMaterials() has infinite materials}, this method will not be called.
+     */
+    default int damageItem(ItemStack stack, int damage, ServerLevel level, @Nullable ServerPlayer player, boolean canBreak, Consumer<Item> onBroken) {
+        return damage;
+    }
+
     /**
      * Called when an item entity for this stack is destroyed. Note: The {@link ItemStack} can be retrieved from the item entity.
      *

src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java

@@ -5,6 +5,8 @@
 
 package net.minecraftforge.common.extensions;
 
+import net.minecraft.server.level.ServerLevel;
+import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.world.damagesource.DamageSource;
 import net.minecraft.world.entity.monster.Monster;
 import net.minecraft.world.entity.player.Inventory;
@@ -32,6 +34,8 @@ import net.minecraftforge.common.ToolActions;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.function.Consumer;
+
 /*
  * Extension added to ItemStack that bounces to ItemSack sensitive Item methods. Typically this is just for convince.
  */
@@ -377,6 +381,28 @@ public interface IForgeItemStack {
         return self().getItem().getSweepHitBox(self(), player, target);
     }
 
+    /**
+     * Called when this item is to be damaged, such as use a tool being used or a shield blocking damage. The damage
+     * parameter has not yet been processed, so enchantments are not taken into account.
+     *
+     * @param damage   The amount of damage the item will take before processing
+     * @param level    The level where the damage is taking place
+     * @param player   The player holding the item
+     * @param canBreak If the item can break from this damage instance ({@code true} if this is called from
+     *                 {@link ItemStack#hurtAndBreak(int, ServerLevel, ServerPlayer, Consumer)}, {@code false} if from
+     *                 {@link ItemStack#hurtWithoutBreaking(int, Player)})
+     * @param onBroken The callback for when an item is broken (use this if you plan on cancelling damage that will
+     *                 break an item)
+     * @return The amount of damage the item should take
+     *
+     * @apiNote If the item stack is not {@linkplain ItemStack#isDamageableItem() damageable} or the player
+     * {@linkplain Player#hasInfiniteMaterials() has infinite materials}, this method will not be called.
+     * @see IForgeItem#damageItem(ItemStack, int, ServerLevel, ServerPlayer, boolean, Consumer)
+     */
+    default int damageItem(int damage, ServerLevel level, @Nullable ServerPlayer player, boolean canBreak, Consumer<Item> onBroken) {
+        return self().getItem().damageItem(self(), damage, level, player, canBreak, onBroken);
+    }
+
     /**
      * Called when an item entity for this stack is destroyed. Note: The {@link ItemStack} can be retrieved from the item entity.
      *

src/test/java/net/minecraftforge/debug/gameplay/item/PreventItemDamageTest.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.gameplay.item;
+
+import net.minecraft.commands.arguments.EntityAnchorArgument;
+import net.minecraft.core.BlockPos;
+import net.minecraft.core.registries.Registries;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+import net.minecraft.server.level.ServerLevel;
+import net.minecraft.server.level.ServerPlayer;
+import net.minecraft.world.InteractionHand;
+import net.minecraft.world.damagesource.DamageSource;
+import net.minecraft.world.damagesource.DamageTypes;
+import net.minecraft.world.entity.EntityType;
+import net.minecraft.world.item.Item;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.ShieldItem;
+import net.minecraft.world.level.GameType;
+import net.minecraftforge.event.entity.living.LivingEntityUseItemEvent;
+import net.minecraftforge.event.entity.player.PlayerDestroyItemEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.registries.DeferredRegister;
+import net.minecraftforge.registries.ForgeRegistries;
+import net.minecraftforge.registries.RegistryObject;
+import net.minecraftforge.test.BaseTestMod;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.function.Consumer;
+
+@Mod(PreventItemDamageTest.MOD_ID)
+@GameTestHolder("forge." + PreventItemDamageTest.MOD_ID)
+public class PreventItemDamageTest extends BaseTestMod {
+    static final String MOD_ID = "prevent_item_damage";
+
+    private static final DeferredRegister<Item> ITEMS = DeferredRegister.create(ForgeRegistries.ITEMS, MOD_ID);
+    private static final RegistryObject<FakeShieldItem> FAKE_SHIELD = ITEMS.register("fake_shield", FakeShieldItem::new);
+
+    public PreventItemDamageTest(FMLJavaModLoadingContext context) {
+        super(context);
+        this.testItem(lookup -> FAKE_SHIELD.get().getDefaultInstance());
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void player_fake_shield_took_modified_damage(GameTestHelper helper) {
+        helper.makeFloor();
+
+        // setup player
+        var player = helper.makeMockPlayer(GameType.SURVIVAL);
+
+        // setup shield
+        var shield = FAKE_SHIELD.get().getDefaultInstance();
+
+        // setup events
+        var firedLivingEntityUseItem = helper.boolFlag("fired LivingEntityUseItemEvent");
+        var firedPlayerDestroyItem = helper.boolFlag("fired PlayerDestroyItemEvent");
+        helper.<LivingEntityUseItemEvent.Start>addEventListener(event -> {
+            if (event.getEntity() != player) return;
+
+            if (event.getItem() != shield)
+                helper.fail("Player is using an item, but it's not the fake shield! Check the game test impl.");
+
+            // Artificially pass 5 seconds from start of the shield
+            // This is because the first 5 ticks, the player is still vulnerable
+            firedLivingEntityUseItem.set(true);
+            event.setDuration(event.getDuration() - 100);
+        });
+        helper.<PlayerDestroyItemEvent>addEventListener(event -> {
+            if (event.getEntity() != player) return;
+
+            if (event.getOriginal() != shield)
+                helper.fail("Player destroyed an item, but it's not the fake shield! Check the game test impl.");
+
+            firedPlayerDestroyItem.set(true);
+        });
+
+        // start using shield
+        player.setItemInHand(InteractionHand.MAIN_HAND, shield);
+        player.startUsingItem(InteractionHand.MAIN_HAND);
+        int initialDamage = shield.getDamageValue();
+
+        // setup enemy
+        var enemy = helper.spawnWithNoFreeWill(EntityType.HUSK, new BlockPos(2, 0, 2));
+        player.lookAt(EntityAnchorArgument.Anchor.EYES, enemy.position());
+
+        // hit the player
+        var attack = helper.registryLookup(Registries.DAMAGE_TYPE).getOrThrow(DamageTypes.MOB_ATTACK);
+        var damage = new DamageSource(attack, enemy) {
+            @Override
+            public boolean scalesWithDifficulty() {
+                return false;
+            }
+        };
+        player.hurtServer(helper.getLevel(), damage, 5.0F);
+
+        // ok, run the tests
+        firedLivingEntityUseItem.assertEquals(true);
+        firedPlayerDestroyItem.assertEquals(true);
+        helper.assertValueEqual(initialDamage + 1, shield.getDamageValue(), "shield damage value", "Fake shield did not take precisely 1 damage! Check IForgeItem#damageItem.");
+        helper.assertValueEqual(player.getItemInHand(InteractionHand.MAIN_HAND), shield, "player shield", "Fake shield was removed from player's hand! Check Player#hurtCurrentlyUsedShield.");
+        helper.assertValueNotEqual(player.getUseItem(), shield, "player use item", "Player should not be using the shield! The onBreak callback was never invoked. Check FakeShieldItem or IForgeItem#damageItem.");
+        helper.succeed();
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void fake_shield_damage_item_impl(GameTestHelper helper) {
+        helper.makeFloor();
+
+        // setup player
+        var player = helper.makeMockServerPlayer();
+
+        // setup shield
+        var shield = FAKE_SHIELD.get().getDefaultInstance();
+        int initialDamage = shield.getDamageValue();
+
+        // test hurt and break
+        var damaged = helper.<Item>flag("damaged shield");
+        shield.hurtAndBreak(1, helper.getLevel(), player, damaged::set);
+        damaged.assertEquals(FAKE_SHIELD.get(), "Fake shield was not damaged! Check IForgeItem#damageItem.");
+        helper.assertValueEqual(initialDamage + 1, shield.getDamageValue(), "shield damage value", "Fake shield did not take precisely 1 damage! Check IForgeItem#damageItem.");
+
+        // test hurt without breaking
+        initialDamage = shield.getDamageValue();
+        shield.hurtWithoutBreaking(1, player);
+        helper.assertValueEqual(initialDamage, shield.getDamageValue(), "shield damage value", "Fake shield took damage even though hurtWithoutBreak test should set damage taken to 0! Check FakeShieldItem or IForgeItem#damageItem.");
+
+        // finished
+        helper.succeed();
+    }
+
+    private static final class FakeShieldItem extends ShieldItem {
+        public FakeShieldItem() {
+            super(new Item.Properties().setId(ITEMS.key("fake_shield")).durability(10));
+        }
+
+        @Override
+        public int damageItem(ItemStack stack, int damage, ServerLevel level, @Nullable ServerPlayer player, boolean canBreak, Consumer<Item> onBroken) {
+            if (canBreak) {
+                onBroken.accept(this);
+                return 1;
+            }
+
+            return 0;
+        }
+    }
+}

src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java

@@ -1,11 +1,19 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
 package net.minecraftforge.debug.gameplay.item;
 
 import net.minecraft.Util;
 import net.minecraft.commands.arguments.EntityAnchorArgument;
 import net.minecraft.core.BlockPos;
+import net.minecraft.core.registries.Registries;
 import net.minecraft.gametest.framework.GameTest;
 import net.minecraft.gametest.framework.GameTestHelper;
 import net.minecraft.world.InteractionHand;
+import net.minecraft.world.damagesource.DamageSource;
+import net.minecraft.world.damagesource.DamageTypes;
 import net.minecraft.world.entity.EntityType;
 import net.minecraft.world.entity.LivingEntity;
 import net.minecraft.world.item.ItemStack;
@@ -64,7 +72,14 @@ public final class ShieldDisablingTest extends BaseTestMod {
         player.lookAt(EntityAnchorArgument.Anchor.EYES, enemy.position());
 
         // hit the player
-        player.hurtServer(helper.getLevel(), enemy.damageSources().mobAttack(enemy), 5.0F);
+        var attack = helper.registryLookup(Registries.DAMAGE_TYPE).getOrThrow(DamageTypes.MOB_ATTACK);
+        var damage = new DamageSource(attack, enemy) {
+            @Override
+            public boolean scalesWithDifficulty() {
+                return false;
+            }
+        };
+        player.hurtServer(helper.getLevel(), damage, 5.0F);
 
         // shield on cooldown?
         helper.assertTrue(player.getCooldowns().isOnCooldown(shield), "shield should be on cooldown");