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

Allow listening for cancelled events #5891

Closed
1 task done
Test-Account666 opened this issue Aug 8, 2023 · 4 comments
Closed
1 task done

Allow listening for cancelled events #5891

Test-Account666 opened this issue Aug 8, 2023 · 4 comments
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).

Comments

@Test-Account666
Copy link

Suggestion

— A way to specifically listen for cancelled events
— A way to listen for all events - No matter if they're cancelled or not

Why?

Sometimes you have an event cancelled by a java plugin, but still want to execute code with your Skript.

As an example, we have a Lobby System which cancels ANY interaction with inventories.

Skript will no longer listen to those.

If we now want to make something gui based with Skript, we either have to edit Skript's source code or the Lobby System's source code.

Both is not ideal for people who don't know how to do this

Other

No response

Agreement

  • I have read the guidelines above and affirm I am following them with this suggestion.
@AyhamAl-Ali
Copy link
Member

AyhamAl-Ali commented Aug 8, 2023

Already possible, change plugin priority in config.sk

plugin priority: high
# Allowed values: lowest, low, normal, high, highest
# Change this if you encounter problems with other plugins, e.g.:
# - cancelled events: The event is cancelled by another plugin, but Skript handles the event nonetheless => increase priority
# (e.g. WorldGuard cancels events if a player doesn't have permission for the given region,
# and you made some 'place' triggers which should not bypass WorldGuard's protection)
# - effects '(un)cancel event': Another plugin should/should not process the event, but does/does not => decrease priority
# - effect 'drop': Another plugin doesn't process added drops => decrease priority
# - effects 'remove ... from drops'/'clear drops': Drops added by other plugins are not removed => increase priority
# Skript removes drops it shouldn't => decrease priority or specify which item types to remove

There is also per event priority
image

@AyhamAl-Ali AyhamAl-Ali added the invalid Not a Skript issue, already implemented, does not apply, etc. label Aug 8, 2023
@sovdeeth
Copy link
Member

sovdeeth commented Aug 8, 2023

I don't think that's quite the same, ayham. That's just trying to get in front of the other plugins. Skript still hears about cancelled events, there's just a conscious choice to ignore them in the code. I think this capability would be valuable for those who want it.

@sovdeeth
Copy link
Member

sovdeeth commented Aug 8, 2023

After discussion:

Ok normal bukkit event operation should work like this, to the best of my knowledge:
For each priority level, starting at lowest, give those listeners a chance to do something:
So at LOWEST, a plguin can trigger something, or cancel the event, or change an event value.
then at NORMAL, another plugin can do the same thing and overwrite,
then at HIGHEST, a final say on what this event does
MONITOR can't affect it, but it's useful for reading the final state.

Skript normally runs at priority HIGH. This means that currently, any event cancelled prior to HIGH will simply not be passed on to the user of Skript. This incentivizes users to lower their listener priority to get ahead of the cancelling event, which is misuse of the system. If they're acting on this event, it may still be cancelled after their trigger runs, leading to weird behavior. Proper use of the system should be staying at HIGH or HIGHEST and uncanceling the event.

Basically, what happens at LOWEST may have no effect on the final event state. If you care about knowing the final state, you use MONITOR (or HIGHEST). But since skript doesn't pass on cancelled events, users who are listening to HIGHEST or MONITOR don't get a chance to uncancel or modify, or even read the data of cancelled events, making it pretty useless, which kind of forces Skript users into misuse of the priority system.

A solution would be giving a config option to allow listening to cancelled events, as switching default behavior would be a very breaking change.

@sovdeeth sovdeeth reopened this Aug 8, 2023
@AyhamAl-Ali AyhamAl-Ali added enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). and removed invalid Not a Skript issue, already implemented, does not apply, etc. labels Aug 8, 2023
@Test-Account666
Copy link
Author

Test-Account666 commented Aug 8, 2023

I'd just add something like this, so users don't need to mess with the config file when, for example, using other people's skripts:

on cancelled ...

on any ...

(But also keep the normal syntax for default behavior)

@AyhamAl-Ali AyhamAl-Ali added the PR available Issues which have a yet-to-be merged PR resolving it label Aug 9, 2023
@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 Apr 7, 2024
@APickledWalrus APickledWalrus added completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).
Projects
None yet
Development

No branches or pull requests

4 participants