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 VaultStateChangeEvent #1621

Open
wants to merge 2 commits into
base: ver/1.21.3
Choose a base branch
from

Conversation

Grabsky
Copy link

@Grabsky Grabsky commented Dec 2, 2024

Simplified version of PaperMC/Paper#11679. May extend this API further in the future but it is already good enough for use-case described in the discussion thread.

I don't know if I put the implementation in the right place because currently - I'm afraid - the event is also called when Vault.State is changed via API and doing so inside the event can create an indefinite loop if handled incorrectly. I'm not sure what should I do about this.

@Onako2
Copy link
Contributor

Onako2 commented Dec 2, 2024

Is there a reason behind not creating a PR at Paper?
Just asking because Paper is more wide-spread and would profit more people

@Grabsky
Copy link
Author

Grabsky commented Dec 2, 2024

@Onako2: Is there a reason behind not creating a PR at Paper?
Just asking because Paper is more wide-spread and would profit more people

Linked discussion have not received any feedback ever since it's creation and because Paper is generally bigger, I think this PR might have taken much longer than I'd like to. I agree however that more people could benefit from it but I'm currently unable to implement the full suggestion. When I'm a bit more familiar with how patches and such work, I might PR the full version to the Paper instead (assuming nobody else would get to it before me!). Just wanted to start with Purpur, as it seems smaller and more open for small changes like this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants