Fix invulnerability damage and armour#12190
Merged
Merged
Conversation
15bce09 to
8bf47fc
Compare
f264c31 to
cdf07ab
Compare
lynxplay
approved these changes
Feb 26, 2025
kennytv
reviewed
Feb 26, 2025
…Entity.java.patch Co-authored-by: Nassim Jahnke <jahnke.nassim@gmail.com>
kennytv
approved these changes
Feb 26, 2025
| + // Apply damage to armor | ||
| + if (!damageSource.is(DamageTypeTags.BYPASSES_ARMOR)) { | ||
| + float armorDamage = (float) (event.getDamage() + event.getDamage(DamageModifier.BLOCKING) + event.getDamage(DamageModifier.HARD_HAT)); | ||
| + float armorDamage = (float) event.getDamage(); |
Contributor
There was a problem hiding this comment.
This computation is flawed anyway since it doesn't take in account the recent damage modifier added by Spigot (freezing) accounted in Vanilla. So the armor generally take less damage in some environment.
For example with a datapack that clear the #bypasses_armor damage type tag and add the player to the #freeze_hurts_extra_types entity type tag.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If a plugin changes the damage in
EntityDamageEvent, it can cause theINVULNERABILITY_REDUCTIONdamage modifier to be set to 0 and render invulnerability damage reduction, added in #11599, ineffective.This happens because a float (the
lastHurtvalue in a Player) is being compared to a double (stored in the modifiers of EntityDamageEvent), and a float can be greater than a double, even if they are the same noiminal value:Due to lossy floating point precision, 3.2 becomes 3.200000047683716, which indeed would be greater than 3.2.
float lastHurtis the result of casting the double fromEntityDamageEvent#getFinalDamage, which causes this imprecision. The fix is to cast the doubles fromEntityDamageEventto floats when comparing them here. This ensures that if they are the same value, then one will not be less than the other, because they have both been cast from the samedoublevalue. The opposite approach, casting adoubleto afloatand back to adoubleagain, does not work, because is lossy. We cannot really convertlastHurtto adoublewithout touching a lot of the damage processing code.I have also taken the liberty of fixing how armour damage is calculated. In the #11599 PR, armour would still take damage even if the damage should have been blocked due to invulnerability. This change will now take that into account. It's also necessary to individual cast each component to a float, rather than adding the doubles like before, otherwise the armourDamage may be marginally greater than 0. I've done this in a similar way to the
LivingEntity#computeAmountFromEntityDamageEventfunction.The BASE damage and INVULNERABILITY_DAMAGE modifers may be slightly different doubles, but represent the same float. The INVULNERABILITY_DAMAGE represents the BASE modifier, but casted to a float and back to a double. Therefore we must cast BASE and INVULNERABILITY_DAMAGE to floats before adding them.
To test this PR, you can use the following code and run the
/testcommand (plugin.yml not included)Without this fix:
With this fix:
cc @lynxplay