Skip to content

Expose block, entity and flag context to PlayerPickItemEvent#12425

Merged
lynxplay merged 6 commits into
PaperMC:mainfrom
NonSwag:pick-item
May 2, 2025
Merged

Expose block, entity and flag context to PlayerPickItemEvent#12425
lynxplay merged 6 commits into
PaperMC:mainfrom
NonSwag:pick-item

Conversation

@NonSwag
Copy link
Copy Markdown
Contributor

@NonSwag NonSwag commented Apr 13, 2025

Enhanced event to include associated block, entity, and data flag.

Enhanced event to include associated block, entity, and data flag.
@NonSwag NonSwag requested a review from a team as a code owner April 13, 2025 11:26
@github-project-automation github-project-automation Bot moved this to Awaiting review in Paper PR Queue Apr 13, 2025
@kennytv
Copy link
Copy Markdown
Member

kennytv commented Apr 22, 2025

Maybe this should just be separate events from an abstract class

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Apr 22, 2025

Initially I had the same idea, but I wasn't sure how backwards compat would work
Would PlayerPickItemEvent listeners still be called if the exact event isn't called anymore?

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Apr 24, 2025

Would PlayerPickItemEvent listeners still be called if the exact event isn't called anymore?

To answer my own question, yes the event is still called

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Apr 24, 2025

I made the PlayerPickItemEvent abstract, would that cause issues with plugins that listen to the event?
I am not sure if that would cause an IncompatibleClassChangeError

@github-project-automation github-project-automation Bot moved this from Awaiting review to Awaiting final testing in Paper PR Queue May 1, 2025
Copy link
Copy Markdown
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.

A little on the fence regarding the naming for PlayerPickEntityEvent, as it does not really convey that the player is picking an item by clicking the entity.
It instead sounds a bit like the player is picking an entity itself (somehow). Open for naming changes but, if team is fine with this, also down with merging this this weekend.

@lynxplay lynxplay merged commit 825685f into PaperMC:main May 2, 2025
@github-project-automation github-project-automation Bot moved this from Awaiting final testing to Merged in Paper PR Queue May 2, 2025
@NonSwag NonSwag deleted the pick-item branch May 2, 2025 20:18
@Vrganj
Copy link
Copy Markdown

Vrganj commented Jul 16, 2025

This now requires a BlockPos/Entity be passed to the tryPickItem method and gets rid of the old signature. Could this be reviewed as I and others have relied on the method without a relevant BlockPos or Entity?

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.

5 participants