-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add proper attached blocks API to AbstractArrow #12099
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only real concern I see would be getAttachedBlock not returning the block plugins are expecting in relation to the hit event. I would ponder deprecating the old method just for the "you should probably consider them all, or default to the first if that fits your needs"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future api, but may be nice to have a method to get all the blocks in a provided bounding box in a world.
The Block that was hit in the hitevent is already Stored in a Variable (i think). could return that in the old method if it is still there |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm
paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftAbstractArrow.java
Outdated
Show resolved
Hide resolved
d36aa47
to
ae017cd
Compare
AbstractArrows don't just have one block they are attached to. i definitely think we should expose all the blocks to the api. whether or not the existing method should be deprecated is up for debate though the current jd for it is misleading as it makes the user think there might only be one block keeping the arrow from falling
fixes #12096