Repository: MinecraftForge/MinecraftForge (language: java)

Issue:
Warden attacks do not disable shield
**Minecraft Version:** 1.19
**Forge Version:** 41.0.62
**Logs:** _not applicable due to in-game behavior_

**Steps to Reproduce:**
 1. Obtain a shield
 2. Spawn a Warden and anger it
 3. Block the Warden's attacks with the shield
    - _Observe as the shield does not go on cooldown._
 4. Repeat the same steps on vanilla 1.19.
 	- _Observe as the shield does go on cooldown._

**Description of issue:**
_Originally [reported by `GotdMän#0384` in the r/feedthebeast Discord server](https://canary.discord.com/channels/166630061217153024/418125698486632448/992393209193967677) and [relayed by](https://canary.discord.com/channels/313125603924639766/983834532904042537/992394920113483878) @TelepathicGrunt._

When attacked by the Warden while blocking using the shield, the shield is not disabled and does not go on cooldown. This is contrary to the behavior exhibited in vanilla, where the Warden's attacks disable the player's shield.

Investigations reveal that the issue is caused by an outdated patch at [`net/minecraft/world/entity/player/Player` (L120-L128)](https://github.com/MinecraftForge/MinecraftForge/blob/bb03b1a7d19f03d79b2236716b9b4621cf344b87/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch#L120-L128) which hooks in the `IForgeItemStack#canDisableShield` method.

In the [1.18.2 version of the patch](https://github.com/MinecraftForge/MinecraftForge/blob/dbf1c8f10fe235f16f3c7b66b617ab6bd35bd655/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch#L120-L128), the unpatched area is `p_36295_.getMainHandItem().getItem() instanceof AxeItem`, which is correctly replicated in the [default implementation of `IForgeItem#canDisableShield`](https://github.com/MinecraftForge/MinecraftForge/blob/dbf1c8f10fe235f16f3c7b66b617ab6bd35bd655/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java#L641-L644).

In 1.19 however, the check is now `p_36295_.canDisableShield()` -- the check is delegated to `LivingEntity#canDisableShield`, whose base implementation is `return this.getMainHandItem().getItem() instanceof AxeItem`. The Warden overrides this method and makes it `return true`, which causes the shield disabling behavior from the Warden's attacks. However, the 1.19 update and patch cycle did not account for this change in the base implementation of `LivingEntity#canDisableShield` or [the default implementation of `IForgeItem#canDisableShield`](https://github.com/MinecraftForge/MinecraftForge/blob/bb03b1a7d19f03d79b2236716b9b4621cf344b87/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java#L669-L672).

There are two solutions here:
 - Change the default implementation of `IForgeItem#canDisableShield` to redirect to `LivingEntity#canDisableShield`, allowing the held item of an entity to determine whether the entity's inherent capability to disable shields will have an effect (though this may be of limited value).
 - Change the patched area to call `LivingEntity#canDisableShield`, and patch that method to redirect to `IForgeItem#canDisableShield`, allowing entities to determine whether they are capable of disabling the shield regardless of the held item or not (as is already done by the Warden).


Discussion / hints:
I would suggest option 2, it's basically just pushing our change down the stack one. And giving the entity the ability to check.
This one's still an issue as of today/latest forge

Fix patch:
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 76ec5e43443..f355b62bf34 100644
--- a/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch
+++ b/patches/minecraft/net/minecraft/world/entity/player/Player.java.patch
@@ -129,7 +129,7 @@
          super.blockUsingShield(p_36295_);
          ItemStack itemstack = this.getItemBlockingWith();
 -        if (p_36295_.canDisableShield() && itemstack != null) {
-+        if (itemstack != null && itemstack.canDisableShield(this.useItem, this, p_36295_)) {
++        if (itemstack != null && p_36295_.getWeaponItem().canDisableShield(itemstack, this, p_36295_)) {
              this.disableShield(itemstack);
          }
      }
diff --git a/patches/minecraft/net/minecraft/world/item/AxeItem.java.patch b/patches/minecraft/net/minecraft/world/item/AxeItem.java.patch
index 744b70fda68..f802e5bc9e7 100644
--- a/patches/minecraft/net/minecraft/world/item/AxeItem.java.patch
+++ b/patches/minecraft/net/minecraft/world/item/AxeItem.java.patch
@@ -10,7 +10,7 @@
              if (optional.isEmpty()) {
                  return InteractionResult.PASS;
              } else {
-@@ -115,5 +_,16 @@
+@@ -115,5 +_,21 @@
      private Optional<BlockState> getStripped(BlockState p_150691_) {
          return Optional.ofNullable(STRIPPABLES.get(p_150691_.getBlock()))
              .map(p_359378_ -> p_359378_.defaultBlockState().setValue(RotatedPillarBlock.AXIS, p_150691_.getValue(RotatedPillarBlock.AXIS)));
@@ -25,5 +25,10 @@
 +    public static BlockState getAxeStrippingState(BlockState originalState) {
 +        Block block = STRIPPABLES.get(originalState.getBlock());
 +        return block != null ? block.defaultBlockState().setValue(RotatedPillarBlock.AXIS, originalState.getValue(RotatedPillarBlock.AXIS)) : null;
++    }
++
++    @Override
++    public boolean canDisableShield(ItemStack stack, ItemStack shield, LivingEntity entity, LivingEntity attacker) {
++        return true;
      }
  }
diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java b/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java
index c7362ed2f98..cdacf432e1b 100644
--- a/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java
+++ b/src/main/java/net/minecraftforge/common/extensions/IForgeItem.java
@@ -459,7 +459,7 @@ default ICapabilityProvider initCapabilities(ItemStack stack, @Nullable Compound
      * @return True if this ItemStack can disable the shield in question.
      */
     default boolean canDisableShield(ItemStack stack, ItemStack shield, LivingEntity entity, LivingEntity attacker) {
-        return this instanceof AxeItem;
+        return attacker.canDisableShield();
     }
 
     /**


Test patch:
diff --git a/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java b/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java
new file mode 100644
index 00000000000..ad292542d8f
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/gameplay/item/ShieldDisablingTest.java
@@ -0,0 +1,73 @@
+package net.minecraftforge.debug.gameplay.item;
+
+import net.minecraft.Util;
+import net.minecraft.commands.arguments.EntityAnchorArgument;
+import net.minecraft.core.BlockPos;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+import net.minecraft.world.InteractionHand;
+import net.minecraft.world.entity.EntityType;
+import net.minecraft.world.entity.LivingEntity;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.minecraft.world.level.GameType;
+import net.minecraftforge.event.entity.living.LivingEntityUseItemEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.test.BaseTestMod;
+
+import java.util.function.Function;
+
+@Mod(ShieldDisablingTest.MOD_ID)
+@GameTestHolder("forge." + ShieldDisablingTest.MOD_ID)
+public final class ShieldDisablingTest extends BaseTestMod {
+    public static final String MOD_ID = "shield_disabling";
+
+    public ShieldDisablingTest(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void player_shield_disabled_by_axe(GameTestHelper helper) {
+        player_shield_disabled_common(helper, h -> Util.make(
+            h.spawnWithNoFreeWill(EntityType.HUSK, new BlockPos(2, 0, 2)),
+            enemy -> enemy.setItemInHand(InteractionHand.MAIN_HAND, new ItemStack(Items.IRON_AXE))
+        ));
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void player_shield_disabled_by_warden(GameTestHelper helper) {
+        player_shield_disabled_common(helper, h -> h.spawnWithNoFreeWill(EntityType.WARDEN, new BlockPos(2, 0, 2)));
+    }
+
+    private static void player_shield_disabled_common(GameTestHelper helper, Function<GameTestHelper, LivingEntity> enemyGetter) {
+        helper.makeFloor();
+
+        // setup player
+        var player = helper.makeMockPlayer(GameType.SURVIVAL);
+        helper.<LivingEntityUseItemEvent.Start>addEventListener(event -> {
+            // Artificially pass 5 seconds from start of the shield
+            // This is because the first 5 ticks, the player is still vulnerable
+            if (event.getEntity() == player) {
+                event.setDuration(event.getDuration() - 100);
+            }
+        });
+
+        // start using shield
+        var shield = Items.SHIELD.getDefaultInstance();
+        player.setItemInHand(InteractionHand.MAIN_HAND, shield);
+        player.startUsingItem(InteractionHand.MAIN_HAND);
+
+        // setup enemy
+        var enemy = enemyGetter.apply(helper);
+        player.lookAt(EntityAnchorArgument.Anchor.EYES, enemy.position());
+
+        // hit the player
+        player.hurtServer(helper.getLevel(), enemy.damageSources().mobAttack(enemy), 5.0F);
+
+        // shield on cooldown?
+        helper.assertTrue(player.getCooldowns().isOnCooldown(shield), "shield should be on cooldown");
+        helper.succeed();
+    }
+}