Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect invulnerability damage reduction #11599

Merged

Conversation

lynxplay
Copy link
Contributor

@lynxplay lynxplay commented Nov 9, 2024

Fixes incorrect spigot handling of the invulnerability damage
reduction applied when an already invulnerable entity is damaged with a
larger damage amount than the initial damage.
Vanilla still damages entities even if invulnerable if the damage to be
applied is larger than the previous damage taken. In that case, vanilla
applies the difference between the previous damage taken and the
proposed damage.

Spigot's damage modifier API takes over the computation of damage
reducing effects, however spigot invokes this handling with the initial
damage before computing the difference to the previous damage amount.
This leads to the reduction values to generally be larger than expected,
as they are computed on the not-yet-reduced value.
Spigot applies these reductions after calling the EntityDamageEvent and
then subtracts the previous damage point, leading to the final damage
amount being smaller than expected.

This patch cannot simply call the EntityDamageEvent with the reduced
damage, as that would lead to EntityDamageEvent#getDamage() returning
the already reduced damage, which breaks its method contract.
Instead, this patch makes use of the DamageModifier API, implementing
the last-damage-reduction as a DamageModifier.


Download the paperclip jar for this pull request: paper-11599.zip

@lynxplay lynxplay requested a review from a team as a code owner November 9, 2024 22:50
@electronicboy electronicboy added the build-pr-jar Enables a workflow to build Paperclip jars on the pull request. label Nov 11, 2024
@Malfrador
Copy link
Member

Malfrador commented Nov 11, 2024

Just so it doesn't get lost:
I've tested this PR and it seems to fix the issue. Damage is behaving correctly again, as it does in Vanila. In my tests, changing damage in the EntityDamageEvent now also works correctly, which fixes #11480 as well.

Further testing with the build attached to the PR would be welcome though, especially if you have a plugin that changes damage in the EntityDamageEvent/EntityDamageByEntityEvent, to make sure this change doesn't cause unexpected behaviour there.

@lynxplay lynxplay force-pushed the bugfix/fix-invuln-reduction-in-damage-event branch 2 times, most recently from aec70d4 to 0c2b4e9 Compare November 11, 2024 20:37
@lynxplay lynxplay changed the title Maybe fix invuln reduction bug Fix incorrect invulnerability damage reduction Nov 11, 2024
Fixes incorrect spigot handling of the invulnerability damage
reduction applied when an already invulnerable entity is damaged with a
larger damage amount than the initial damage.
Vanilla still damages entities even if invulnerable if the damage to be
applied is larger than the previous damage taken. In that case, vanilla
applies the difference between the previous damage taken and the
proposed damage.

Spigot's damage modifier API takes over the computation of damage
reducing effects, however spigot invokes this handling with the initial
damage before computing the difference to the previous damage amount.
This leads to the reduction values to generally be larger than expected,
as they are computed on the not-yet-reduced value.
Spigot applies these reductions after calling the EntityDamageEvent and
*then* subtracts the previous damage point, leading to the final damage
amount being smaller than expected.

This patch cannot simply call the EntityDamageEvent with the reduced
damage, as that would lead to EntityDamageEvent#getDamage() returning
the already reduced damage, which breaks its method contract.
Instead, this patch makes use of the DamageModifier API, implementing
the last-damage-reduction as a DamageModifier.
@lynxplay lynxplay force-pushed the bugfix/fix-invuln-reduction-in-damage-event branch from 0c2b4e9 to fc21cd8 Compare November 19, 2024 10:39
@lynxplay lynxplay merged commit d0dcd7d into PaperMC:master Nov 19, 2024
3 checks passed
@lynxplay lynxplay deleted the bugfix/fix-invuln-reduction-in-damage-event branch November 19, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-pr-jar Enables a workflow to build Paperclip jars on the pull request.
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Damage ticks producing non vanilla behaviour, vanilla overlapping removed/canceled
5 participants