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

Adds ability to listen to cancelled events #6207

Merged
merged 18 commits into from
Apr 7, 2024

Conversation

sovdeeth
Copy link
Member

@sovdeeth sovdeeth commented Nov 30, 2023

Description

Continuation of #5896.

Adds the ability to listen to events cancelled by other plugins by using on all <event>, on any <event>, and on cancelled <event>.
Previously, Skript would simply return before executing any triggers if the event was cancelled. Now it checks each trigger to see if each one is ignoring or listening to cancelled events based on the SkriptEvent's ListeningBehavior.

This should maintain current behavior without breaking changes.

Also fixes a bug with #5900 where event priority would leak from one event to following events.

Docs update:

For another PR - Note this requires updating docs event pattern to add [all]


Target Minecraft Versions: any
Requirements: none
Related Issues: #5891 #5908

Adds functionality to allow events to listen to cancelled events only, uncancelled only (previous behavior), and both.
Deprecates listenCancelled (breaking change for EvtResurrect)
@sovdeeth sovdeeth added enhancement Feature request, an issue about something that could be improved, or a PR improving something. 2.8 Targeting a 2.8.X version release require docs update A PR that requires Skript docs to be updated to reflect the related changes. labels Nov 30, 2023
Removes breaking change from EvtResurrect.
Refactors some of SkriptEventHandler
Errors for events that don't support cancelling
Bug fix for data leak via EventData
…led. Reduce local complexity in SkriptEventHandler
@sovdeeth sovdeeth requested a review from Fusezion December 3, 2023 05:24
Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Member

@Pikachu920 Pikachu920 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is lovely, can we add tests?

Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool beans

Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good, just a few minor thoughts

@sovdeeth sovdeeth added 2.9 Targeting a 2.9.X version release and removed 2.8 Targeting a 2.8.X version release labels Jan 1, 2024
I need to stop using the browser for merge conflicts
@sovdeeth sovdeeth added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Mar 21, 2024
@sovdeeth sovdeeth merged commit 33d0fb1 into SkriptLang:dev/feature Apr 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. require docs update A PR that requires Skript docs to be updated to reflect the related changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants