Essentials icon indicating copy to clipboard operation
Essentials copied to clipboard

`/kill` doesn't call `minecraft:entity_hurt_player` advancement trigger (breaking data packs)

Open GrantGryczan opened this issue 1 year ago • 7 comments

Type of bug

Compatibility issue, Other unexpected behaviour

/ess dump all output

https://essentialsx.net/dump.html?id=4688c1ddb0b848c7a0ed7437d7a5c9e0

Error log (if applicable)

No response

Bug description

In vanilla, an advancement is granted by what's called an advancement trigger. Advancement triggers are like game events. When something occurs in-game that a particular advancement trigger was meant to detect, it triggers all advancements that hook onto that trigger in their data.

One such trigger is minecraft:entity_hurt_player. Despite the name, this is triggered when the player takes any form of damage, including from /kill, and even while in creative mode. But with EssentialsX installed, /kill doesn't trigger minecraft:entity_hurt_player.

If data packs want to detect certain in-game events efficiently (i.e. without checking things every tick), they must use hidden advancements with these triggers. So to detect a player dying, data packs have to use the minecraft:entity_hurt_player advancement trigger. It's the only way for a data pack to detect player deaths efficiently. Therefore, this bug makes /kill able to break any data pack that relies on this to efficiently detect deaths, such as the Graves data pack from Vanilla Tweaks.

Steps to reproduce

In MC 1.20.6, install this data pack by copying it into the datapacks folder in your world save and then reloading the world: test.zip. Here is a version for 1.21: test.zip.

This is a minimal reproduction data pack which simply hooks onto the minecraft:entity_hurt_player advancement trigger, making it so whenever it triggers, it prints entity_hurt_player in chat to the player who triggered it.

With this data pack installed and enabled, enter /kill. (With EssentialsX installed, you can also use /suicide.)

Expected behaviour

Without EssentialsX, /kill prints entity_hurt_player in your chat, just like /minecraft:kill.

Actual behaviour

With EssentialsX, /kill doesn't print entity_hurt_player in your chat, contrary to the vanilla behavior.

Additional Information

This applies to /suicide as well.

See https://github.com/EssentialsX/Essentials/pull/5850#issuecomment-2223132714 for important notes on how to fix this.

GrantGryczan avatar Jun 26 '24 09:06 GrantGryczan

I was able to replicate this. Seems that setHealth(double) does not call that trigger. Changing it to use damage(double) seems to work. Will do some testing then submit a PR

abulujayn avatar Jun 28 '24 01:06 abulujayn

Just to dispel any perception that this issue isn't that big of a deal, I still deal with people running into this and reporting our Graves data pack not working because of it every week. It's been driving me crazy for months now and makes giving technical support for our data packs significantly more time-consuming.

GrantGryczan avatar Jan 13 '25 01:01 GrantGryczan

Could you clarify what the issue is here? The fix in #5850 appears to address the damage cause issue with EssentialsX's own /kill command (which we'd recommend server owners don't use when using datapacks anyway, via command aliases and/or command-block-overrides).

The steps to reproduce here imply that EssentialsX breaks the vanilla /minecraft:kill command, which is not something EssentialsX should even able to interfere with (barring Spigot event handler issues, which should be reported upstream).

mdcfe avatar Jan 13 '25 08:01 mdcfe

Correct. It interferes with /minecraft:kill. Try the steps for yourself and see! This issue does not occur without the EssentialsX plugin enabled.

But /essentials:kill and /essentials:suicide also exhibit this issue, and that should be fixed too.

GrantGryczan avatar Jan 16 '25 22:01 GrantGryczan

Yet another user reported this issue to us again today, which led me to notice the issue is still labeled "not enough info"--is there any more information I can provide to progress this?

If it helps, additional information regarding the fix can be found here: https://github.com/EssentialsX/Essentials/pull/5850#issuecomment-2223132714

We've had countless players report this to us in the past, and while it's possible this is an upstream issue, I find that relatively unlikely considering not a single plugin other than EssentialsX has ever triggered it. If it is found to be an upstream issue, I would be happy to report it, but as I'm not familiar with plugin development enough to confirm it as an upstream issue, that would have to be investigated by people who are more familiar with plugin development first.

GrantGryczan avatar Mar 07 '25 20:03 GrantGryczan

4 different players have reported running into this issue since my last comment, one of them today. It's nearly every week. For almost the last year. I don't even use EssentialsX, and yet it is sinking a great deal of my time, diagnosing and explaining this issue to a new person in our data pack help channel every single time it happens.

What do I need to do to get EssentialsX developers to investigate this issue? Again, I've provided plenty of info regarding the fix in https://github.com/EssentialsX/Essentials/pull/5850#issuecomment-2223132714. I'm confused why the issue is still labeled "not enough info".

GrantGryczan avatar Apr 05 '25 17:04 GrantGryczan

Update: It seems /minecraft:kill actually does not trigger this issue! I'm not sure why I thought it did initially. I definitely tested it when reporting, but I must have made a mistake. My apologies. I have updated the issue accordingly.

This should address your confusion earlier.

GrantGryczan avatar Aug 14 '25 09:08 GrantGryczan