Skip to content
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

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

notTamion
Copy link
Contributor

@notTamion notTamion commented Feb 11, 2025

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

@notTamion notTamion requested a review from a team as a code owner February 11, 2025 21:17
Copy link
Member

@electronicboy electronicboy left a 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"

Copy link
Member

@Owen1212055 Owen1212055 left a 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.

@notTamion
Copy link
Contributor Author

The only real concern I see would be getAttachedBlock not returning the block plugins are expecting in relation to the hit event.

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

Copy link
Contributor

@lynxplay lynxplay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest lgtm

@notTamion notTamion force-pushed the proper-attachedblocks-api branch from d36aa47 to ae017cd Compare February 12, 2025 16:33
@lynxplay lynxplay merged commit 072a831 into PaperMC:main Feb 12, 2025
3 checks passed
Y2Kwastaken pushed a commit to Y2Kwastaken/Paper that referenced this pull request Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

AbstractArrow#getAttachedBlock() returns wrong block
4 participants