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

[GrantGryczan]

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

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 /minecraft:kill. (Or if testing in vanilla, just use /kill. With EssentialsX installed, you can also use /kill <username> or /suicide.)

Expected behaviour

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

Actual behaviour

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

Additional Information

This applies to /suicide as well.

See #5850 (comment) for important notes on how to fix this.

[zp4rker]

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

[GrantGryczan]

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.

[mdcfe]

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).

[GrantGryczan]

Correct. It interferes with /minecraft:kill (when entered in chat, not when ran by data packs). 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.