Swapping items and attacking will (for a split second) make player.getAttackCooldown() return 1.0 instantly using EntityDamageByEntityEvent

[ILikePtatoes]

Expected behavior

Code

Using EntityDamageByEntityEvent:

            if (attackingPlayer.getAttackCooldown() == 1.0) {
                  damagedPlayer.setFoodLevel(Math.max(damagedPlayer.getFoodLevel() - foodLevelReduction, 0));
                  Sound.KIGA_HIT.play(damagedPlayer.getLocation());
            }

Player draws attack cooldown attributed item from a non-attack cooldown attributed item and instantly attacks another player -> player.getAttackCooldown() != 1.0

If player.getAttackCooldown() returns 1.0 at this stage, players can use macros, or manually swap quickly from an item with no attack cooldown attribute to an item with an attack cooldown attribute. This would effectively bypass any usage of player.getAttackCooldown() checks making the method obsolete in most situations.

Expected behaviour is that client attributes and server attributes are synchronous upon swapping and do not result in a desynchronisation between expected attack cooldown values and actual attack cooldown values.

Using this specific example of code, the player holds a golden carrot, swaps to this axe, and instantly hits. This effect should not trigger, as upon drawing of a separate item with an attack cooldown attribute, the item should have the initial attack cooldown.

Observed/Actual behavior

Code

Using EntityDamageByEntityEvent:

            if (attackingPlayer.getAttackCooldown() == 1.0) {
                  damagedPlayer.setFoodLevel(Math.max(damagedPlayer.getFoodLevel() - foodLevelReduction, 0));
                  Sound.KIGA_HIT.play(damagedPlayer.getLocation());
            }

https://streamable.com/q4qgp1

Player draws item and instantly attacks another player -> player.getAttackCooldown() == 1.0 (for a split second, most likely dependent on ping, however regardless allows people to attack with something that triggers an EntityDamageByEntityEvent, and bypass any player.getAttackCooldown() check), then actual item swap player.getAttackCooldown() is applied, properly 'drawing' the weapon and initialising cooldown.

Overall the handling of the actual weapon used when EntityDamageByEntityEvent is poor, Minecraft will denote that the weapon used when attacking is actually the golden-carrot at point of attack (seen by the damage done in the video, for reference, the axe that was getting quick-swapped to had Sharpness 25, yet damage was little). This is most likely due to client/server desync regarding attributes (seen in another issue raised on here where players could quickswap from an efficiency 5 pickaxe to a regular diamond axe, and mine wood very quickly for a split second due to desynchronisation).

In this video, you can see that the player swaps from golden carrot to the axe, and instantly calls EntityDamageByEntityEvent. There is a check in place to ensure the players attack cooldown is equal to 1.0 (player is able to do a 'full-attack'), however this is circumvented.

Steps/models to reproduce

Step 1: Hold any item without an attack speed attribute
Step 2: Swap to axe, sword or any item with an attack speed attribute
Step 3: Instantly swing, player.getAttackCooldown() at this moment should not return 1.0, as the player is still 'drawing' the item, however it will return 1.0
Step 4: Item will pop up instantly for a second, however then go back down, and repeat it's pop up animation (assumingly initialising the proper attributes on the item (resynchronisation after a period of desync)

Plugin and Datapack List

N/A

Paper version

This server is running Paper version 1.21.1-130-ver/1.21.1@4d2672e (2024-10-30T18:33:50Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are 1 version(s) behind
Download the new version at: https://papermc.io/downloads/paper
Previous version: 1.21.1-DEV-fcedb49 (MC: 1.21.1)

Other

https://streamable.com/q4qgp1