Paper icon indicating copy to clipboard operation
Paper copied to clipboard

Old EntityKnockbackByEntityEvent lacks KnockbackCause

Open LoliColleen opened this issue 1 year ago • 10 comments

Expected behavior

EntityKnockbackByEntityEvent should contain a knock back cause just like what is it be like in Spigot. Be like: @ApiStatus.Internal public EntityKnockbackByEntityEvent(@NotNull LivingEntity entity, @NotNull Entity hitBy, float knockbackStrength, @NotNull Vector acceleration, KnockbackCause knockbackCause) { super(entity, hitBy, acceleration); this.knockbackStrength = knockbackStrength; this.knockbackCause = knockbackCause; }

Observed/Actual behavior

@ApiStatus.Internal public EntityKnockbackByEntityEvent(@NotNull LivingEntity entity, @NotNull Entity hitBy, float knockbackStrength, @NotNull Vector acceleration) { super(entity, hitBy, acceleration); this.knockbackStrength = knockbackStrength; } There isn't any cause here so that I can't get the cause of this event

Steps/models to reproduce

@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onEntityKnockbackByEntity(EntityKnockbackByEntityEvent event) { // if (event.getCause()... ok there isn't any cause :( }

Plugin and Datapack List

This issue isn't based on any plugin or datapack

Paper version

version [23:53:34 INFO]: This server is running Paper version git-Paper-"6ad63fb" (MC: 1.20.4) (Implementing API version 1.20.4-R0.1-SNAPSHOT) (Git: 6ad63fb) You are running the latest version Previous version: git-Paper-446 (MC: 1.20.4)

Other

No response

LoliColleen avatar Mar 04 '24 15:03 LoliColleen

This is not the new event but the old one, the cause stuff needs transporting over, basically

electronicboy avatar Mar 04 '24 15:03 electronicboy

Is the new one added right now? The following is the reason of deprecated: @deprecated use {@link com.destroystokyo.paper.event.entity.EntityKnockbackByEntityEvent} or {@link io.papermc.paper.event.entity.EntityPushedByEntityAttackEvent} I havent seen any cause from neither EntityPushedByEntityAttackEvent nor EntityKnockbackByEntityEvent

LoliColleen avatar Mar 04 '24 16:03 LoliColleen

The new one spigot added is the deprecated one, forcing people off events we've had for 6 years is generally not a goal here

electronicboy avatar Mar 04 '24 16:03 electronicboy

I see. However, the new one has causes but is deprecated(forRemoval=true) so that the building of plugin cant work correctly. I tried @SuppressWarnings("deprecation") to use the new one but the building still failed. And the old one doesn't have any cause. Maybe you can remove the "(forRemoval = true)" from the new one or just add some causes to the old one instead.

LoliColleen avatar Mar 04 '24 16:03 LoliColleen

Yes, the goal would be to backport the causes to the older event, hence why this is marked as a feature request

electronicboy avatar Mar 04 '24 16:03 electronicboy

Do you mind if I send a Pull Request to join causes to solve the problem as soon as possible? I really need this feature

LoliColleen avatar Mar 04 '24 16:03 LoliColleen

You are more than welcome to submit a PR

electronicboy avatar Mar 04 '24 16:03 electronicboy

By the way, shield item has two bugs caused by Vanilla and Spigot's code. Both of these bugs I think can be fixed together. But I'm not sure if this kind of change is in accordance with your regulations.

LoliColleen avatar Mar 04 '24 16:03 LoliColleen

such a thing should generally be within a seperate PR

electronicboy avatar Mar 04 '24 16:03 electronicboy

Do I need to remove the previous "cause"? Like the method: public void knockback(double d0, double d1, double d2, @Nullable Entity attacker, EntityKnockbackEvent.KnockbackCause cause)

Also I m not sure which way is the best:

  1. Add a convert method to convert the Bukkit causes to the Paper causes
  2. Add both causes to all methods that contain the cause parameter
  3. Change Bukkit cause paramter to Paper one to all methods that contain the cause parameter (2 and 3 do break the nms user's code)

LoliColleen avatar Mar 04 '24 16:03 LoliColleen