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 ExplodeEvent and Improvement in Spigot Explode events #11840

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Doc94
Copy link
Contributor

@Doc94 Doc94 commented Dec 27, 2024

This PR is a "copy" from a 2 years old PR in Spigot related to explosion, when this start the issue is how the gamerule for avoid mob griefing make the explosion event not being called, with recent changes now CraftBukkit pass the explosion behavior for make the user know what is the behaviour of this explosion.. with this in mind this PR takes that for well.. call the event for that also handle a issue where "explosions" created by Wind Burst call the block explosion with not data.

This PR implement a new "generic" event for any explosion generated by the server, this brings the same behaviour of the Spigot explode events (block,entity) this this event is intented for being called for all the explosions.. affects or not the environment.

- Handle KEEP explosion behaviour (gamerule or explosion by Wind Burst)
- Call custom event damager for allow CraftBukkit know who make the explosion by Wind Burst
@Doc94 Doc94 requested a review from a team as a code owner December 27, 2024 18:25
@Doc94
Copy link
Contributor Author

Doc94 commented Dec 27, 2024

PS: Very new with the new contribution system.. currently the ExplosionServer has unused CB imports but if remove that the fixupSourcePatches just fails

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.

Alright, thank you for the PR!
We have reviewed it but are a bit (a bit too) hesitant to call these events when their javadocs do define them to not be called in these exact spots.

We could fire them pre-cancelled, but block break related events are obviously rather important for plugin data, and I'd rather avoid plugins dying/duping because they did not correctly check cancelled here.

Instead, Machine and I think it'd be best to use this API downside to finally create a better version of the "SomethingExplodeEvent" (better name pending) that covers both entities, blocks and nothing (like in the mace's case). Such an event can be called for these cases, specifying that no blocks will be broken etc.
The current events types could then be marked as Obsolete and the new event would be called after to allow people to take priority with the new API.

If you are up for implementing such a change yourself, feel free to just use this API, otherwise we can close it and open a feature request issue 👍

@Doc94
Copy link
Contributor Author

Doc94 commented Dec 29, 2024

Then you mean a new event "ExplodeEvent" for handle all the explosions, like a merge of the current entity and block not?
This require send a canceled state of this new event?
PD in teory just need call this new event in the sane place but later of the bukkit?

@Doc94 Doc94 changed the title Improvement Explosion events Add ExplodeEvent and Improvement in Spigot Explode events Dec 30, 2024
@Doc94
Copy link
Contributor Author

Doc94 commented Dec 30, 2024

Ok based in the comments here and in discord i rework this PR for keep the changes but also the new calls just being called in the new event called...
i dont like "duplicate" the CB code for the paper event but dont wanna mess with strange behaviours for use the same method for call the paper event and make changes...

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

Successfully merging this pull request may close these issues.

3 participants