PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
10.06k stars 2.34k forks source link

Cancelling ProjectileHitEvent of arrow with piercing prevents it from hitting the next target #11132

Open woodyn1002 opened 4 months ago

woodyn1002 commented 4 months ago

Expected behavior

Cancelling ProjectileHitEvent of an arrow shouldn't affect its next target

Observed/Actual behavior

Once ProjectileHitEvent of an arrow with piercing has been cancelled, the next hit entity doesn't hurt at all

On AbstractArrow.java:

@Override
public ProjectileDeflection preHitTargetOrDeflectSelf(HitResult hitResult) {
    if (hitResult instanceof EntityHitResult entityHitResult && this.hitCancelled && this.getPierceLevel() > 0) {
        if (this.piercingIgnoreEntityIds == null) {
            this.piercingIgnoreEntityIds = new IntOpenHashSet(5);
        }
        this.piercingIgnoreEntityIds.add(entityHitResult.getEntity().getId());
    }
    return super.preHitTargetOrDeflectSelf(hitResult);
}

this.hitCancelled is checked to add the hit entity to piercing-ignoring entities, but it's BEFORE super.preHitTargetOrDeflectSelf

On Projectile.java:

public ProjectileDeflection preHitTargetOrDeflectSelf(HitResult movingobjectposition) { // Paper - protected -> public
    org.bukkit.event.entity.ProjectileHitEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callProjectileHitEvent(this, movingobjectposition);
    this.hitCancelled = event != null && event.isCancelled();
    if (movingobjectposition.getType() == HitResult.Type.BLOCK || !this.hitCancelled) {
        return this.hitTargetOrDeflectSelf(movingobjectposition);
    }
    return ProjectileDeflection.NONE;
}

And here 'this.hitCancelled' is set by the ProjectileHitEvent result, but it's AFTER adding piercing-ignoring entity.

On AbstractArrow::onHitEntity:

if (this.piercingIgnoreEntityIds.size() >= this.getPierceLevel() + 1) {
    this.discard(EntityRemoveEvent.Cause.HIT); // CraftBukkit - add Bukkit remove cause
    return;
}
// ...hit the entity...

If the piercing-ignoring entities is enough, the arrow is discarded before hitting entity.

Steps/models to reproduce

@EventHandler
fun onProjectileHit(event: ProjectileHitEvent) {
  if (event.hitEntity is Enemy) {
    event.isCancelled = true
  }
}

Plugin and Datapack List

My plugin only

Paper version

This server is running Paper version 1.21-107-master@aa36ae6 (2024-07-21T10:39:28Z) (Implementing API version 1.21-R0.1-SNAPSHOT) You are running the latest version Previous version: 1.21-99-f1f01a1 (MC: 1.21)

Other

No response

notTamion commented 4 months ago

once a design choice has been made this will be fixed in https://github.com/PaperMC/Paper/pull/10804