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 a base event to handle multiple events at once + two new events making use of it #112

Draft
wants to merge 16 commits into
base: 1.19.4
Choose a base branch
from

Conversation

bl4ckscor3
Copy link
Contributor

@bl4ckscor3 bl4ckscor3 commented Mar 21, 2023

This pull request adds a new abstract event AbstractMultiEvent as well as two new events TwoAtOnceEvent and FiveAtOnceEvent that make use of it - they run two/five events at the same time respectively. AbstractMultiEvent is useful for handling multiple events at once. Subclasses can select events they want to run, and AbstractMultiEvent handles the rest. The events get rendered as a group in the queue as well. I have at least two more events planned that can make use of this. Here's a showcase of how it looks:

This PR is a draft because there are still some issues with this, but I still wanted to gather some feedback on this idea as well as perhaps get help with fixing these issues. I currently know of:

  • Having very few events active including both of the new onces leads to a StackOverflowException when activating one of the new events (tested with only the two new events, as well as the new ones + another one for a total of 3)
  • Sometimes with just a few events enabled that include the new events, the queue will be incorrect (Note the first group of elements has 6 instead of 5. Timers are not working because I am in spectator mode). (tested with the two new events + 3 others for a total of 5)

@juancarloscp52 juancarloscp52 self-requested a review March 25, 2023 12:14
@bl4ckscor3
Copy link
Contributor Author

I think I have ironed out the two problems I explained in my opening post. The NothingEvent came into good use as a technical help! I am still leaving this as a draft for now incase any more issues pop up or more feedback is received.

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.

1 participant