Repository navigation
Conversation
me4502
left a comment
me4502
left a comment
There was a problem hiding this comment.
Thanks for the PR! I've left a few comments as I'm not sure this is actually the best solution for the given issue.
| } | ||
|
|
||
| @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) | ||
| public void onEntityTarget(EntityTargetEvent event) { |
There was a problem hiding this comment.
This should probably be in WorldGuardEntityListener, EventAbstractionListener isn't the right place for this one as it's not calling out to an internal event.
|
|
||
| if (event.getTarget() instanceof Player player) { | ||
| Cause.trackParentCause(creeper, player); | ||
| } else if (event.getReason() != EntityTargetEvent.TargetReason.TARGET_DIED) { |
There was a problem hiding this comment.
I'm a bit concerned about the potential side effects this could have. The PR is made from the assumption that a player death is directly caused by the creeper itself, and therefore likely to trigger further block explosions afterwards. In most cases though, the creeper is likely to not actually die afterwards and therefore potentially forever have a stale reference around to a player.
Would potentially solving this in ExplosionPrimeEvent rather than the entity target event be a narrower approach? It still has the potential to leak a player if the explosion never occurs, but it at least is adding it at a more causally-linked location rather than for every random target action
There was a problem hiding this comment.
Good call, the target event was too broad. I looked into this and found why it broke: on 26.x, Creeper#getTarget() returns null once the target is dead, so the player is already gone by the time the explode event runs.
I'm thinking of doing this:
-
In
ExplosionPrimeEvent: If the creeper's target is a player, track them as the parent cause. -
In
EntityRemoveEvent: Clear the stored player reference. The creeper is removed right after it explodes, and this event also covers death, despawn, and chunk unloads—ensuring the player is only stored for that single explosion. (I'd rather avoid clearing inEntityExplodeEvent, since that never fires for creepers whenmobGriefingis set tofalse.) -
Handler location: Both handlers would go in
WorldGuardEntityListener.
Does that sound good, or would you rather avoid metadata entirely (e.g., with a weak-key map in Cause)?
Fixes #2284
If a creeper's explosion kills its target before blocks are broken, the creeper is used as the cause instead of the player, so non-members can blow up protected blocks. The creeper's player target is now tracked so it's still used as the cause after they die.