Prioritise direct entity for Damager in damage event and avoid redundant causing entity

[Doc94]

This fix #10273 making the damager selector use the same logic than Projectile for every case, directEntity is who cause the damage (the damager).

if any can enable the jar for allow users test another bahaviours.. i test the basic based in my upstream PR for make sure this change not break any thing.

---

Download the paperclip jar for this pull request: paper-10275.zip

[TonytheMacaroni]

This does indeed fix the problem with fireworks.

The issue with the causing entity for some other explosions (i.e. TNT) being wrong is still a thing, though. As far as I can tell, the usage of DamageSource#customCausingEntity here: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/nms-patches/net/minecraft/world/level/Explosion.patch#42-43 Is unnecessary as a result of this patch, as the direct entity and the set custom causing entity here are always the same, after checking the usages of Explosion. Whenever the damagesource is non-null, its direct entity is always the source entity passed to the constructor, and world.damageSources().explosion(this) uses the Explosion's source entity for the direct entity. Reverting this line should fix that issue.

[Doc94]

This does indeed fix the problem with fireworks.

The issue with the causing entity for some other explosions (i.e. TNT) being wrong is still a thing, though. As far as I can tell, the usage of DamageSource#customCausingEntity here: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/nms-patches/net/minecraft/world/level/Explosion.patch#42-43 Is unnecessary as a result of this patch, as the direct entity and the set custom causing entity here are always the same, after checking the usages of Explosion. Whenever the damagesource is non-null, its direct entity is always the source entity passed to the constructor, and world.damageSources().explosion(this) uses the Explosion's source entity for the direct entity. Reverting this line should fix that issue.

Yeah i notice that in a few test... I remember set that for a reason.. and i forgot that reason now xd My approach is more edit the method where the custom causing entity is set and make ignore the set if the entity passed is the direct entity, or the customCausingEntity is already set or if the entity passed is null for avoid mess in the CB bahaviour

[Doc94]

@TonytheMacaroni PR updated for avoid the customCausingMethod set the value if.

• The customCausingEntity is already set
• The causingEntity is the same than the entity passed
• The directEntity is the same than the entity passed

this can solve the most of issues related to this auxiliar field...

[TonytheMacaroni]

That would fix the issue with TNT, but still, the call to customCausingEntity isn't needed in Explosion? That's the only area that that method causes this specific issue.

And the behavior of customCausingEntity is still quite suspect. Take evoker fangs, if it has an owner, the direct entity is the fangs and the owner is the causing entity. But without an owner, customCausingEntity makes the fangs the causing entity and doesn't set a direct entity. Similar issue with its usage with ender pearls and lightning - they each can have their own owners that should be the causing entity, and the direct entity is also never set in these cases. The inconsistency wouldn't really matter if net.minecraft.world.damagesource.DamageSource#getCausingEntity was only used for the damage events, but it's also exposed in the API as org.bukkit.damage.DamageSource#getCausingEntity.

[Doc94]

That would fix the issue with TNT, but still, the call to customCausingEntity isn't needed in Explosion? That's the only area that that method causes this specific issue.

And the behavior of customCausingEntity is still quite suspect. Take evoker fangs, if it has an owner, the direct entity is the fangs and the owner is the causing entity. But without an owner, customCausingEntity makes the fangs the causing entity and doesn't set a direct entity. Similar issue with its usage with ender pearls and lightning - they each have their own owners that should be the causing entity, and the direct entity is also never set in these cases. The inconsistency wouldn't really matter if net.minecraft.world.damagesource.DamageSource#getCausingEntity was only used for the damage events, but it's also exposed in the API as org.bukkit.damage.DamageSource#getCausingEntity.

The WindCharge make a few things to the damagesource... i point to the upstream PR where talking with Machine suggest a form to handle the static field in the explosion. https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/1180/overview?commentId=12404

about the Bukkit class and the using of getCausingEntity add the another comment where this was mentioned too. https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/1180/overview?commentId=12426

[TonytheMacaroni]

Just to note, I don't have an account to view the upstream comments you're referring to.

[Doc94]

Just to note, I don't have an account to view the upstream comments you're referring to.

oh...

• The customCausingEntity was make for allow the old static damager set using a static field in CB then need handle that in all places where was placed.. in the case of explosions in the constructor or in another case pass a custom damagesource for that, for this i rewrite a little the method where set the custom causing entity for avoid mistakes
• This customCausingEntity is taken over the causingEntity from vanilla for make DamageSource in bukkit side reflect the custom changes in CB/Paper and avoid things like DamageSource show WindCharge with not causing or direct entity and CB set the causing entity for that

i hope all this can help... i make more tests and the results for projectiles and another explosions looks valid.

[JasperLorelai]

Minor spelling issue: priorice > prioritise/prioritize. Present both in comment, PR title, and commit message, though only the first might matter for future context.

[Owen1212055]

Superseded by https://github.com/PaperMC/Paper/pull/10307 , Thank you for your contribution regardless. 😄