Add Paper Only TargetHitScriptEvent #2604
Conversation
| public void onProjectileHit(TargetHitEvent event) { | ||
| this.event = event; | ||
| projectile = new EntityTag(event.getEntity()); | ||
|
|
| if (projectile.getLocation() == null) { | ||
| return; | ||
| } | ||
| if (Double.isNaN(projectile.getLocation().getDirection().normalize().getX())) { |
There was a problem hiding this comment.
This is clearly copypasted from the projectile hit event - is it even relevant to have here?
There was a problem hiding this comment.
Relatedly - is this actually a unique event? ie is it not already covered by the existing projectile hit event?
There was a problem hiding this comment.
Glance at source code public class TargetHitEvent extends ProjectileHitEvent indicates it's not unique
There was a problem hiding this comment.
I don't see a reason why this should be needed. I copied it from the ProjectileHitEvent. In paper the Location of an entity is marked with @NotNull so I removed those checks
There was a problem hiding this comment.
Glance at source code
public class TargetHitEvent extends ProjectileHitEventindicates it's not unique
It has a different HandlerList so it's a different event
| import org.bukkit.event.Listener; | ||
|
|
||
|
|
||
| public class TargetHitScriptEvent extends BukkitScriptEvent implements Listener { |
There was a problem hiding this comment.
classname should match the Denizen name not the paper name - ie TargetBlockHitScriptEvent
|
|
||
| @Override | ||
| public boolean couldMatch(ScriptPath path) { | ||
| return path.eventLower.matches("targetblock hit"); |
There was a problem hiding this comment.
This is the legacy couldMatch handling, should use the modern register handling
paper/src/main/java/com/denizenscript/denizen/paper/events/TargetBlockHitScriptEvent.java
Show resolved
Hide resolved
| projectile = new EntityTag(event.getEntity()); | ||
| hitBlock = new LocationTag(event.getHitBlock().getLocation()); | ||
| shooter = projectile.getShooter(); | ||
| strength = new ElementTag(event.getSignalStrength()); |
There was a problem hiding this comment.
This doesn't need to be read in advance, should just be in the getContext call
There was a problem hiding this comment.
I moved the Strengh attribute to the getContext call. The other Tags are used in other methods. If you want to move them too I would have to create new Tags every call.
Joo200
force-pushed
the
feat/targethitevent
branch
from
10973ad
to
aeb41cf
Compare
|
Sorry for the delay of this PR. I rebased this PR and applied all PR comments |
|
Why the rebase? |
| // @Context | ||
| // <context.projectile> returns an EntityTag of the projectile. | ||
| // <context.shooter> returns a EntityTag of the shooter of the projectile. | ||
| // <context.hit_block> returns a LocationTag of the block |
There was a problem hiding this comment.
Needs periods at end of line
There was a problem hiding this comment.
... oh and, for that matter, the rest of the text lol.
Force of habit |
| // <context.projectile> returns an EntityTag of the projectile. | ||
| // <context.shooter> returns a EntityTag of the shooter of the projectile. | ||
| // <context.hit_block> returns a LocationTag of the block. | ||
| // <context.hit_face> returns a LocationTag of the face. |
There was a problem hiding this comment.
These were copied from the projectile hits event, but cut short -- should probably just restore the full text from the other events docs into here
|
|
||
| // <--[event] | ||
| // @Events | ||
| // targetblock hit |
There was a problem hiding this comment.
Event name should have a space (target block hit)
Add the Event from Paper when a Target block is hit by an arrow to Denizen.