Skip to content

Fix EffKill failing to kill entities#8505

Open
bluelhf wants to merge 1 commit intoSkriptLang:dev/patchfrom
bluelhf:patch/fix-eff-kill-damage
Open

Fix EffKill failing to kill entities#8505
bluelhf wants to merge 1 commit intoSkriptLang:dev/patchfrom
bluelhf:patch/fix-eff-kill-damage

Conversation

@bluelhf
Copy link
Copy Markdown
Contributor

@bluelhf bluelhf commented Mar 16, 2026

Problem

Currently, using the kill effect to cause an entity to die does not guarantee that the entity dies. In server versions where damage sources are supported, Skript attempts to deal a high amount of damage (100 hearts plus twice the hearts of the entity being killed). However, if an event listener reduces the damage to a number lower than the entity's health, then the entity does not die.

For entities other than players, the effect correctly remove()s the entity if they fail to die. However, since removing players is not possible (throws UnsupportedOperationException), they are exempt from this last-ditch effort, and end up not dying at all.

The bug, also described in #8441, can be reproduced with this code:

command /slay:
  trigger:
    kill player

on damage:
  set damage to 0.01 * damage
  send "Reduced damage is %damage%, your health is %2 * victim' health%" to victim

Without this PR, the /slay command does not kill the player. With the addition of this PR, /slay kills the player correctly.

Solution

This PR fixes the issue by dealing Double.POSITIVE_INFINITY damage instead, which event listeners are more likely to handle correctly.

It is impossible to fix the root cause of the problem without changing the Paper API to implement a proper kill() method that bypasses event listeners. It should be noted that for living entities, /minecraft:kill behaves identically to the new implementation added in this PR.

Testing Completed

Verified that the PR fixes the issue using the aforementioned script

Supporting Information

Before PR After PR
The player does not die The player dies

Completes: #8441
Related: none
AI assistance: none

all damage event listeners process the damage before it is actually processed, so having a "normal" damage value both conveys the wrong meaning and leaves us susceptible to bugs where `EffKill` does not actually kill anything
@bluelhf bluelhf requested a review from a team as a code owner March 16, 2026 18:32
@bluelhf bluelhf requested review from Absolutionism and cheeezburga and removed request for a team March 16, 2026 18:32
@sovdeeth
Copy link
Copy Markdown
Member

Can you confirm this doesn't apply crazy amounts of durability damage to armor? I don't think it should but better to double check.

@sovdeeth sovdeeth changed the base branch from master to dev/patch March 16, 2026 19:05
@bluelhf
Copy link
Copy Markdown
Contributor Author

bluelhf commented Mar 16, 2026

Can you confirm this doesn't apply crazy amounts of durability damage to armor? I don't think it should but better to double check.

It does not, since the minecraft:generic_kill damage type has tag #minecraft:bypasses_armor

It also bypasses resistance, invulnerability (incl. totems of undying), and knockback

@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Mar 17, 2026
@sovdeeth sovdeeth linked an issue Mar 17, 2026 that may be closed by this pull request
1 task
@sovdeeth sovdeeth moved this to In Review in 2.15 Releases Mar 17, 2026
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Mar 17, 2026
@sovdeeth sovdeeth added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Mar 23, 2026
@APickledWalrus APickledWalrus added the don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. label Mar 27, 2026
@APickledWalrus
Copy link
Copy Markdown
Member

APickledWalrus commented Mar 27, 2026

Adding label pending confirmation of the issue being fixed by the reporting user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.

Projects

Status: Awaiting Merge

Development

Successfully merging this pull request may close these issues.

"kill" can`t kill player

4 participants