Paper icon indicating copy to clipboard operation
Paper copied to clipboard

EntityKnockbackByEntityEvent called with wrong entities

Open vytskalt opened this issue 3 years ago • 8 comments

Expected behavior

The EntityKnockbackByEntityEvent#hitBy() entity should be called with the entity that did the knockback

Observed/Actual behavior

Instead, if the entity that did the knockback is a projectile or a TNT, the #hitBy entity is a player (shooter of the projectile or the one that lit the TNT) instead of the entity that actually did the knockback

Steps/models to reproduce

Example 1:

  1. Throw a snowball
  2. EntityKnockbackByEntityEvent will be called with #hitBy set to the shooter of the snowball instead of the actual snowball entity

Example 2:

  1. Get a TNT and flint and steel
  2. Prime the TNT
  3. EntityKnockbackByEntityEvent will be called with #hitBy set to the player that primed the TNT instead of the TNT entity

Example 3:

  1. /summon tnt
  2. EntityKnockbackByEntityEvent will not be called because there was no entity that primed the TNT

Plugin and Datapack List

Only a plugin that was printing the #hitBy entity into the console. No datapacks.

Paper version

This server is running Paper version git-Paper-85 (MC: 1.18.1) (Implementing API version 1.18.1-R0.1-SNAPSHOT) (Git: df8d28a) You are running the latest version

Other

No response

vytskalt avatar Dec 21 '21 20:12 vytskalt

I'm pretty sure this is intentional. The game tracks the source entity for the damage sources you named, tnt being ignited by a player or a projectile fired from a player/entity.

That being said, I'm open to a nullable method in that event to get the actual entity. It should've been the actual entity before cause there are methods to get the "shooter" or a projectile if you actually wanted a reference to that entity, but I don't think it can be changed now.

Machine-Maker avatar Dec 21 '21 21:12 Machine-Maker

I'm pretty sure this is intentional. The game tracks the source entity for the damage sources you named, tnt being ignited by a player or a projectile fired from a player/entity.

That being said, I'm open to a nullable method in that event to get the actual entity. It should've been the actual entity before cause there are methods to get the "shooter" or a projectile if you actually wanted a reference to that entity, but I don't think it can be changed now.

That nullable method would still not fix the issue where the event isn't called if there's no source entity. I don't see any way of fixing this issue without breaking plugins or creating a new event.

vytskalt avatar Dec 21 '21 21:12 vytskalt

Yeah, so that's really a separate issue. You've got two issues here, one where you can't get the projectile if it has a shooter, and one where its tnt that didnt come from a player in some way. (I think that's the only was an entity can create knockback without having a source mob)

Machine-Maker avatar Dec 21 '21 21:12 Machine-Maker

Yeah, so that's really a separate issue. You've got two issues here, one where you can't get the projectile if it has a shooter, and one where its tnt that didnt come from a player in some way. (I think that's the only was an entity can create knockback without having a source mob)

Well, they're both related so I made an issue for both of them. I can create a separate issue if you want though.

vytskalt avatar Dec 21 '21 21:12 vytskalt

Paper has a changed behavior from Spigot regarding the EntityKnockbackByEntityEvent from Bukkit. This event is deprecated for Paper but this event shouldn't behave different than on Spigot.

The method org.bukkit.event.entity.EntityKnockbackByEntityEvent#getSourceEntity() returns different entities on Spigot than on Paper:

Used code:

    @EventHandler(ignoreCancelled = true)
    public void onEntityKnockbackByEntity(EntityKnockbackByEntityEvent event) {
        Entity damager = event.getSourceEntity();

        WorldGuardPlugin.inst().getLogger().info("Knockback from damager: " + damager.getClass().getCanonicalName());
    }

Spigot will call two events. I'm not sure why there are two events called.

[16:42:08] [Server thread/INFO]: [WorldGuard] Knockback from damager: org.bukkit.craftbukkit.v1_21_R1.entity.CraftPlayer
[16:42:08] [Server thread/INFO]: [WorldGuard] Knockback from damager: org.bukkit.craftbukkit.v1_21_R1.entity.CraftWindCharge

Paper calls only one event:

[16:46:42 INFO]: [WorldGuard] Knockback from damager: org.bukkit.craftbukkit.entity.CraftPlayer

If I want to cancel only knockbacks created by exploded windcharges I can't get the source entity correctly. Using Spigot and cancel for entities instance of AbstractWindCharge it works fine. On paper this doesn't work.

Edit: Small opinion:

Returning the effective entity which fired the wind charge in paper's EntityPushedByEntityAttackEvent#getPushedBy() is misleading. The entity is not pushed by the player, it's pushed by an explosion caused by the wind charge. I agree on using the wind charge as entity there, but getPushedBy shouldn't return the player.

Joo200 avatar Jul 27 '24 14:07 Joo200

Well we cannot really change the returned value of our own events, eventho I would somewhat agree that getPushedBy is somewhat missleading to return the causing entity and not the direct one.

Fixing this for legacy events however is certainly something we should be doing.

lynxplay avatar Jul 27 '24 15:07 lynxplay

The problem here is the disambiguation between what vanilla considers the causing entity and what bukkit considers the causing entity, and the widening gap between the two. part of the reality is that we just need to expose the real full context as much as possible and let people figure what they're after there

electronicboy avatar Jul 27 '24 15:07 electronicboy

Opened PR #11176 to fix the Bukkit behavior. More API in the paper events would be great but that would be a new feature. With more to do (and a duplicate of #7170 .

Joo200 avatar Jul 27 '24 15:07 Joo200

Will this be released in the next version?

paaulhier avatar Feb 04 '25 22:02 paaulhier