Better ProjectileHitEvent API

[posterhusky]

• made ProjectileHitEvent cancellable
• new ProjectileHitBlockEvent extending ProjectileHitEvent with Block hitBlock and BlockFace hitFace
• new ProjectileHitEntityEvent extending ProjectileHitEvent with Entity hitEntity
• deprecated ProjectileCollideEvent beacuse of the new ProjectileHitEntityEvent
• reworked the projectiles' collision calculation to call events and allow noclipping through blocks and entities

⚠ The ProjectileHitEvent now fires before the collision (like in new mc versions) and not after the projectile collided and died.

[roccodev]

Thank you and nice work!

I left some notes to simplify the implementation, however I'd also like to highlight something important with the design:

As you mentioned this changes the order of events, so I don't think it makes sense to reuse the existing ProjectileHitEvent, doing so would also break plugins designed for 1.8.

Instead, I'd create a new base event class and call it something like PreHit.

[roccodev]

Can't this whole patch simply be:

diff --git a/src/main/java/net/minecraft/server/EntityArrow.java b/src/main/java/net/minecraft/server/EntityArrow.java
--- a/src/main/java/net/minecraft/server/EntityArrow.java	(revision fcd4bd805787519acff9ae7a6f70b646a21dc287)
+++ b/src/main/java/net/minecraft/server/EntityArrow.java	(date 1768936446917)
@@ -265,6 +265,11 @@
                 ProjectileCollideEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callProjectileCollideEvent(this, movingobjectposition.entity);
                 if (event.isCancelled()) movingobjectposition = null;
             }
+
+            if (movingobjectposition != null && movingobjectposition.entity == null) {
+                ProjectileHitBlockEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callProjectileHitBlockEvent(this, movingobjectposition);
+                if (event.isCancelled()) movingobjectposition = null;
+            }
             // KigPaper end
 
             if (movingobjectposition != null) {

I don't think the separate function is needed, it also had some issues, for example:

• The 5 ticks lived check for self damage should check for this.as, not this.ar (which is the ground stuck timer)
• If an entity collision fails it should move to block collision, not step out of the function

Also, the first part of the arrow tick logic checks for the block the arrow is in to be non-air and non-passable to stick the arrow to it. Wouldn't that cause arrows with speed < 1 block/tick to still hit blocks even if events are cancelled?

[roccodev]

Same as for arrows, I guess

[roccodev]

I think the simple arrow patch works here too

[roccodev]

For fishing hooks, you could also use the simple patch while also setting a flag if the event is cancelled that allows you to noclip the hook.

[roccodev]

I'd also add docs to the events, for example I think if you set the block type in a hit-block listener to one that the projectile would pass through, you still need to cancel the event

[posterhusky]

I made a separate function bcs i was acc thinking of doing a more complex implementation with ranked collisions, however i thought it would add too much complexity, especially for my inital goal of just being able to know what block was hit by the projectile.
Suppose the following scenario: a projectile travels fast enough to go through multiple blocks in 1 tick, and you have a listener that cancels collisions with green blocks only:

The current "basic" implementation would call the event once and ignore all the subsequent block collisions. i had an idea in mind to make a list of entities ranked by distance (squared) and make a while loop that goes through each collision recasting a ray for each cancelled block. here's a quick pesudeo code of how such an algorithm would work:

var block: MovingObjectPosition? = // hit block or null
var bDistSquared = // dist squared or something else (null, 0.0, or any value)
val collisionList: MutableList<Pair<Double, MovingObjectPosition>>

// same code here except all entities are stored in the list

var i = 0
val ignoredBlockList = mutableListOf()
while (i < collisionList.size || block != null) {
    if (block == null || collisionList[i].first < bDistSquared) { // if the next collision is an entity
        // call ProjectilePreHitEntityEvent
        if (!event.isCancelled) return collisionList[i].second
        i++
        continue
    }
    // call ProjectilePreHitBlockEvent
    if (!event.isCancelled) return block
    ignoredBlockList.append(block)
    (block, bDistSquared) = // recast ray ingoring blocks in ignoredBlockList and get distance
}
return null

the algorithm would also require the modification of the raytracing function to add support for ignored blocs so idk if this whole thing is worth the effort

[roccodev]

I think that could be done by plugins instead. When the pre-hit and hit events are fired, you can access the motion vector with getVelocity(), so you can re-calculate manually. NMS would also be an option

[posterhusky]

• made patches simpler
• added new ProjectilePreHitEvent and restored ProjectileHitEvent's old functionality
• also added some docs idk if elaboration is needed tho

[roccodev]

Is moving needed here? What I suggested instead was to move the PreHitBlockEvent below this one. I think it's better to get the block data after it may have been potentially modified by the event... (then if for example plugins don't want the new block to be hit by the projectile, e.g. .setType(Material.WOOD_BUTTON), they can delay it by a tick).
Though I don't think this was an intentional change because the other projectile classes do get the data after the event has fired.

As a side note the // KigPaper comments are only useful in the context of the patched code (i.e. after all patches have been applied), ProjectileCollideEvent is a KigPaper API so the comment would not be needed here

[posterhusky]

I moved it bcs it simplified the following:

If an entity collision fails it should move to block collision, not step out of the function

otherwise i'd have to create 2 variables to store both potential collisions bcs the entity collision overwrites the block collsion
what im currently doing is calling the event even before the overwrite happens, if its cancelled i do nothing (potential block collision remains in movingobjectposition), otherwise i proceed with the overwrite.

[posterhusky]

the block data does not affect the projectile's physics, it is only used to tell what block is the arrow stuck in and for special interactions (like priming tnt with flame bow).
i mainly changed this to prevent arrows being stuck mid-air: the next tick it will compare the new block data with the old one and if the event changed the block it will realise this and recalculate collisions
for special interactions imo it makes sense to apply to the old block, and the a() function can be ez called for the new block by the plugin with nms

[posterhusky]

Also is modifying 0187-Add-ProjectileCollideEvent.patch an option? Only the craft event factory changes can be left

[roccodev]

Yeah, this is for latest Paper but the steps described here should still work: https://github.com/PaperMC/Paper/blob/main/CONTRIBUTING.md#modifying-larger-feature-patches

[roccodev]

I moved it bcs it simplified the following:

If an entity collision fails it should move to block collision, not step out of the function

otherwise i'd have to create 2 variables to store both potential collisions bcs the entity collision overwrites the block collsion
what im currently doing is calling the event even before the overwrite happens, if its cancelled i do nothing (potential block collision remains in movingobjectposition), otherwise i proceed with the overwrite.

Checking the source again I realised the vanilla server doesn't move on to block collision when ignoring the entity (e.g. when the entity is invulnerable or on the same team as the shooter, that's a vanilla check)

[roccodev]

...so this change is probably not needed here (and the hitBlockData variable can be removed)

[roccodev]

The scope here is unnecessary. While this is probably cleaner, in KigPaper we prefer to keep patches simpler and with minimal modifications to vanilla code

[roccodev]

From what I understand these are equivalent (even when it comes to getting the block data before/after the event), but leaving the CollideEvent call where it was results in a simpler patch.

[roccodev]

Other than the same suggestions pointed out above, this looks good

[roccodev]

Same here

[posterhusky]

made patches smaller and reverted checking block collision after entity is cancelled
changing 0187-Add-ProjectileCollideEvent.patch wont be necessary then