Skip to content

Call EntityDeathEvent for EnderDragon#kill#8839

Closed
alfiejfs wants to merge 1 commit into
PaperMC:masterfrom
alfiejfs:ender-dragon-kill
Closed

Call EntityDeathEvent for EnderDragon#kill#8839
alfiejfs wants to merge 1 commit into
PaperMC:masterfrom
alfiejfs:ender-dragon-kill

Conversation

@alfiejfs
Copy link
Copy Markdown

Fixes #8836.

@alfiejfs alfiejfs requested a review from a team as a code owner February 10, 2023 06:34
@Override
public void kill() {
+ // Paper start
+ org.bukkit.event.entity.EntityDeathEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callEntityDeathEvent(this, drops); // Paper - call event
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should pass in the drops field here. It looks like the ender dragon should drop nothing here, so passing in a list that might have something in it, might not is misleading.

Also, the callEntityDeathEvent method inside CraftEventFactory has some logic for playing death sounds. I think you need to set the silentDeath field on LivingEntity to true before calling the event for EnderDragon so it doesn't play the death sound by default (which would be a behavior change).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix this in a few hours. I passed in the drops as a convention (the same is done for armor stand, but I suppose that has drops) - surely it is always empty for the dragon?

Will change the silentDeath field also.

Copy link
Copy Markdown
Member

@Machine-Maker Machine-Maker Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as far as I can tell it is always going to be empty there, but we should just pass an empty list regardless, so its clear its always empty. If you pass the drops list, it might look like its supposed to have drops.

EDIT: As I'm typing this, I'm realizing that datapacks might be able to change the loot table of the dragon so it does have drops, but it looks like the kill command bypasses those in vanilla anyways. So my point still stands.

@cunhar
Copy link
Copy Markdown

cunhar commented Mar 5, 2023

how can we get this blessed by the merge gods?

@lynxplay lynxplay enabled auto-merge (squash) March 5, 2023 13:08
@lynxplay lynxplay disabled auto-merge March 5, 2023 13:08
@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Mar 5, 2023

I can look at it later today specifically in regards to MMs comment on drops👍

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Mar 5, 2023

Well, the merge gods cannot give a blessing to a PR that has not addressed the issues outlined 👍

@alfiejfs
Copy link
Copy Markdown
Author

alfiejfs commented Mar 5, 2023

Apologies, have been very busy. Might get to look at it today!

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Mar 5, 2023

No worries! Take your time :) My comments were just for cunhar 😅

@kennytv
Copy link
Copy Markdown
Member

kennytv commented Aug 21, 2023

Superseded by #9495

@kennytv kennytv closed this Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

/kill on EnderDragon doesnt fire EntityDeathEvent

5 participants