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 QuarterDeleteEvent #31

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

galacticwarrior9
Copy link
Contributor

This PR adds a cancellable QuarterDeleteEvent which is fired when a quarter is deleted. The event is fired with a Cause so that plugins can see the deletion reason. If the event is cancelled, a message is sent to the deleting player if one exists. Quarters will be deleted regardless of the event's cancellation state if the Cause is Cause.CONTAINS_WILDERNESS.

I have kept the original deletion method since I am unsure what to do with it. All of its internal uses have been replaced with a new deletion method which fires this event.

@jwkerr
Copy link
Owner

jwkerr commented Sep 8, 2024

i will probably add a few events in the next few days and I can make this part of it, but I will want to rewrite the logic around it

what does this event need to be able to do so I can make sure I add it properly and what other events would you like added if any

@galacticwarrior9
Copy link
Contributor Author

Ideally:

  • A way to see the cause of a quarter's deletion.
    • For example, a plugin might not want to prevent an admin from deleting a quarter.
  • A way to set a cancellation message.

Other events:

  • A cancellable QuarterCreateEvent
  • A cancellable QuarterCuboidAddEvent and QuarterCuboidRemoveEvent
  • A PlayerEntersQuarterEvent
    • It would be nice if the quarter entry notification was exposed so we can add information to it. Could be a separate event like Towny's ChunkNotificationEvent.

@jwkerr
Copy link
Owner

jwkerr commented Sep 8, 2024

for the cuboid ones would that be for the edit commands?

@galacticwarrior9
Copy link
Contributor Author

That's correct

@jwkerr
Copy link
Owner

jwkerr commented Sep 11, 2024

@galacticwarrior9 i made some changes, thinking i should probably add the delete plot and delete all ones to their own event though, i also made the decision to remove CONTAINS_WILDERNESS and will make that throw a non-cancellable event through the delete method with no args, other than that let me know if there's any issues before i merge it

@jwkerr
Copy link
Owner

jwkerr commented Sep 11, 2024

going to think about how i'll change around the old delete method to handle adding a cause to the post quarter delete event, might deprecate the no args one

@galacticwarrior9
Copy link
Contributor Author

@galacticwarrior9 i made some changes, thinking i should probably add the delete plot and delete all ones to their own event though, i also made the decision to remove CONTAINS_WILDERNESS and will make that throw a non-cancellable event through the delete method with no args, other than that let me know if there's any issues before i merge it

This all looks fine to me.

This fixes the event throwing an java.lang.IllegalStateException if the building is deleted off the main thread
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