From b8bbf950a9d563d9ed23a7aee14aca5a5395eb6d Mon Sep 17 00:00:00 2001 From: Jonathing Date: Mon, 17 Feb 2025 22:09:58 -0500 Subject: [PATCH] Re-introduce IForgeItem.damageItem when an item takes damage, Fixes #10344 (#10374) --- .../world/entity/player/Player.java.patch | 13 ++ .../minecraft/world/item/ItemStack.java.patch | 11 +- .../extensions/IForgeGameTestHelper.java | 98 +++++++++++- .../common/extensions/IForgeItem.java | 22 +++ .../common/extensions/IForgeItemStack.java | 24 +++ .../gameplay/item/PreventItemDamageTest.java | 145 ++++++++++++++++++ .../gameplay/item/ShieldDisablingTest.java | 17 +- 7 files changed, 321 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 b7837446a6..c39e921938 100644 --- a/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch +++ b/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch @@ -137,6 +137,19 @@ if (!this.level().isClientSide) { this.awardStat(Stats.ITEM_USED.get(this.useItem.getItem())); } +@@ -919,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 { @@ -937,10 +_,13 @@ @Override protected void actuallyHurt(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 12015bd19e..31bc38e97b 100644 --- a/patches/minecraft/net/minecraft/world/item/ItemStack.java.patch +++ b/patches/minecraft/net/minecraft/world/item/ItemStack.java.patch @@ -33,13 +33,22 @@ if (player != null && interactionresult.indicateItemUse()) { player.awardStat(Stats.ITEM_USED.get(item)); } +@@ -412,6 +_,8 @@ + + public void hurtAndBreak(int p_220158_, RandomSource p_220159_, @Nullable ServerPlayer p_220160_, Runnable p_331215_) { + if (this.isDamageableItem()) { ++ // FORGE: modify the base damage based on the item's impl of IForgeItem.damageItem ++ p_220158_ = this.damageItem(p_220158_, p_220159_, p_220160_, p_331215_); + if (p_220158_ > 0) { + int i = EnchantmentHelper.getItemEnchantmentLevel(Enchantments.UNBREAKING, this); + int j = 0; @@ -447,6 +_,10 @@ } this.hurtAndBreak(p_41623_, p_41624_.getRandom(), p_41624_ instanceof ServerPlayer serverplayer ? serverplayer : null, () -> { + 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_.broadcastBreakEvent(p_335324_); Item item = this.getItem(); diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java b/src/main/java/net/minecraftforge/common/extensions/IForgeGameTestHelper.java index 06d52d2989..0763246954 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,28 @@ 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 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); @@ -158,19 +184,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); + } } } @@ -195,9 +251,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 { @@ -216,5 +292,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 20c1df15be..3dd92acb7e 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java +++ b/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java @@ -7,6 +7,9 @@ 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.util.RandomSource; import net.minecraft.world.damagesource.DamageSource; import net.minecraft.world.entity.player.Inventory; import net.minecraft.world.item.*; @@ -521,6 +524,25 @@ 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 random The random used where the damage is taking place (typically from the level) + * @param player The player holding the item + * @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}, this method will not be + * called. + */ + default int damageItem(ItemStack stack, int damage, RandomSource random, @Nullable ServerPlayer player, Runnable 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 da973f3542..fc5cf8b468 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java +++ b/src/main/java/net/minecraftforge/common/extensions/IForgeItemStack.java @@ -5,6 +5,9 @@ package net.minecraftforge.common.extensions; +import net.minecraft.server.level.ServerLevel; +import net.minecraft.server.level.ServerPlayer; +import net.minecraft.util.RandomSource; import net.minecraft.world.damagesource.DamageSource; import net.minecraft.world.entity.monster.EnderMan; import net.minecraft.world.entity.player.Inventory; @@ -31,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. */ @@ -398,6 +403,25 @@ 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 random The random used where the damage is taking place (typically from the level) + * @param player The player holding the item + * @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}, this method will not be + * called. + * @see IForgeItem#damageItem(ItemStack, int, RandomSource, ServerPlayer, Runnable) + */ + default int damageItem(int damage, RandomSource random, @Nullable ServerPlayer player, Runnable onBroken) { + return self().getItem().damageItem(self(), damage, random, player, 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..3f3932e59b --- /dev/null +++ b/src/test/java/net/minecraftforge/debug/gameplay/item/PreventItemDamageTest.java @@ -0,0 +1,145 @@ +/* + * 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.ServerPlayer; +import net.minecraft.util.RandomSource; +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.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; + +@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() { + this.testItem(() -> 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).getHolderOrThrow(DamageTypes.MOB_ATTACK); + var damage = new DamageSource(attack, enemy) { + @Override + public boolean scalesWithDifficulty() { + return false; + } + }; + player.hurt(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.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.boolFlag("damaged shield"); + shield.hurtAndBreak(1, helper.getLevel().random, player, () -> damaged.set(true)); + damaged.assertEquals(true, "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."); + + // HURT WITHOUT BREAK WAS ADDED IN 1.21.3 + /* + // 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().durability(10)); + } + + @Override + public int damageItem(ItemStack stack, int damage, RandomSource random, @Nullable ServerPlayer player, Runnable onBroken) { + onBroken.run(); + return 1; + } + } +} 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 3efde936b9..de568dccc0 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; @@ -62,7 +70,14 @@ public final class ShieldDisablingTest extends BaseTestMod { player.lookAt(EntityAnchorArgument.Anchor.EYES, enemy.position()); // hit the player - player.hurt(enemy.damageSources().mobAttack(enemy), 5.0F); + var attack = helper.registryLookup(Registries.DAMAGE_TYPE).getHolderOrThrow(DamageTypes.MOB_ATTACK); + var damage = new DamageSource(attack, enemy) { + @Override + public boolean scalesWithDifficulty() { + return false; + } + }; + player.hurt(damage, 5.0F); // shield on cooldown? helper.assertTrue(player.getCooldowns().isOnCooldown(shield.getItem()), "shield should be on cooldown");