From 55862dfd1d916d0bb7695cfecf8a67f8d7070000 Mon Sep 17 00:00:00 2001 From: Jonathing Date: Mon, 3 Feb 2025 16:17:38 -0500 Subject: [PATCH] Re-introduce IForgeItem.damageItem when an item takes damage, Fixes #10344 (#10371) --- .../world/entity/player/Player.java.patch | 13 ++ .../minecraft/world/item/ItemStack.java.patch | 30 +++- .../extensions/IForgeGameTestHelper.java | 116 +++++++++++++- .../common/extensions/IForgeItem.java | 24 +++ .../common/extensions/IForgeItemStack.java | 26 +++ .../gameplay/item/PreventItemDamageTest.java | 151 ++++++++++++++++++ .../gameplay/item/ShieldDisablingTest.java | 17 +- 7 files changed, 368 insertions(+), 9 deletions(-) create mode 100644 src/test/java/net/minecraftforge/debug/gameplay/item/PreventItemDamageTest.java diff --git a/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch b/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch index f355b62bf3..f85ad116b2 100644 --- a/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch +++ b/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())); } +@@ -931,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 { @@ -949,10 +_,13 @@ @Override protected void actuallyHurt(ServerLevel p_365751_, DamageSource p_36312_, float p_36313_) { diff --git a/patches/minecraft/net/minecraft/world/item/ItemStack.java.patch b/patches/minecraft/net/minecraft/world/item/ItemStack.java.patch index 68aa435c7b..b8f8b61f00 100644 --- a/patches/minecraft/net/minecraft/world/item/ItemStack.java.patch +++ b/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)); } +@@ -468,18 +_,26 @@ + } + + public void hurtAndBreak(int p_220158_, ServerLevel p_342197_, @Nullable ServerPlayer p_220160_, Consumer 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 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_; + } + } @@ -516,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_); + } diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java b/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java index da036fe062..55678e2427 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java +++ b/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java @@ -5,10 +5,14 @@ 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; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; import org.jetbrains.annotations.Nullable; import com.mojang.authlib.GameProfile; @@ -60,6 +64,46 @@ public interface IForgeGameTestHelper { throw new GameTestAssertException(message.get()); } + default void assertValueEqual(N expected, N actual, String name, String message) { + this.assertValueEqual(expected, actual, name, () -> message); + } + + default void assertValueEqual(N expected, N actual, String name, Supplier 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 void assertValueEqual(N[] expected, N[] actual, String name, String message) { + this.assertValueEqual(expected, actual, name, () -> message); + } + + default void assertValueEqual(N[] expected, N[] actual, String name, Supplier 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 void assertValueNotEqual(N expected, N actual, String name, String message) { + this.assertValueNotEqual(expected, actual, name, () -> message); + } + + default void assertValueNotEqual(N expected, N actual, String name, Supplier message) { + if (Objects.equals(expected, actual)) + throw new GameTestAssertException("%s -- Expected %s to NOT be %s, but was".formatted(message.get(), name, expected)); + } + + default void assertValueNotEqual(N[] expected, N[] actual, String name, String message) { + this.assertValueNotEqual(expected, actual, name, () -> message); + } + + default void assertValueNotEqual(N[] expected, N[] actual, String name, Supplier 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 Registry registryLookup(ResourceKey> 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); @@ -159,19 +203,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) null); + } + + public void assertUnset(String message) { + this.assertUnset(message != null ? () -> message : null); + } + + public void assertUnset(Supplier 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) null); + } + + public void assertSet(String message) { + this.assertSet(message != null ? () -> message : null); + } + + public void assertSet(Supplier 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) null); + } + + public void assertEquals(T expected, String message) { + this.assertEquals(expected, message != null ? () -> message : null); + } + + public void assertEquals(T expected, Supplier 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); + } } } @@ -196,9 +270,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 message) { + super.assertEquals((long) expected, message); + } + + public void assertEquals(long expected, Supplier message) { + super.assertEquals(expected, message); + } } public static class BoolFlag extends Flag { @@ -217,5 +311,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 message) { + super.assertEquals(expected, message); + } } } diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java b/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java index cdacf432e1..6feb89fcdf 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java +++ b/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 onBroken) { + return damage; + } + /** * Called when an item entity for this stack is destroyed. Note: The {@link ItemStack} can be retrieved from the item entity. * diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java b/src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java index 77cb69d3b2..dbcd1ecfa1 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java +++ b/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 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. * diff --git a/src/test/java/net/minecraftforge/debug/gameplay/item/PreventItemDamageTest.java b/src/test/java/net/minecraftforge/debug/gameplay/item/PreventItemDamageTest.java new file mode 100644 index 0000000000..abf3870717 --- /dev/null +++ b/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 ITEMS = DeferredRegister.create(ForgeRegistries.ITEMS, MOD_ID); + private static final RegistryObject 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.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.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.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 onBroken) { + if (canBreak) { + onBroken.accept(this); + return 1; + } + + return 0; + } + } +} diff --git a/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java b/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java index ad292542d8..cd0fb7a7c0 100644 --- a/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java +++ b/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");