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

A way for event handlers to see why an event was cancelled #6542

Open
dktapps opened this issue Nov 29, 2024 · 0 comments
Open

A way for event handlers to see why an event was cancelled #6542

dktapps opened this issue Nov 29, 2024 · 0 comments
Labels
Category: API Related to the plugin API Opinions Wanted Request for comments & opinions from the community Type: Enhancement Contributes features or other improvements to PocketMine-MP

Comments

@dktapps
Copy link
Member

dktapps commented Nov 29, 2024

Problem description

There's currently no way for plugins to know why an event was cancelled.

In the event of stuff like player interactions, this leads to plugins either blindly uncancelling events (which may lead to problems like spectator players being able to do stuff they aren't supposed to), or having to bake in extra checks to avoid uncancelling the event in certain conditions (e.g. checking if the player's gamemode is spectator to avoid uncancelling PlayerInteractEvent).

Proposed solution

Two possible options have appeared so far:

Event cancellation events

This would involve removing Cancellable->uncancel() as per #3823.
Plugins cancelling events, if they want to allow uncancelling, would dispatch events notifying other plugins of the event cancellation.

While this initially seems like a good solution, it presents a couple of problems:

  • Plugins would have to hard-depend on plugins whose events they wanted to uncancel, as there's no obvious way to generalize such a solution. This is described in more detail here: [RFC] Disallow uncancelling events by default #3823 (comment)
  • It makes the cancelling plugin the authority over what happens, which is inconsistent with the way the priority system works (later priorities are supposed to get the final say). If the cancelling plugin doesn't dispatch a cancellation event, other plugins have no way to modify the outcome, regardless of the reason for cancellation.

Add cause info to Cancellable

Require passing a cause parameter to Cancellable->cancel(). This would allow plugins to decide if they want to uncancel an event or not.

This might look something like how PlayerPreLoginEvent's kick flag system works in PM4+ (implemented in 9f4bb44). However, in the event of KICK_FLAG_PLUGIN being set, there's no way to tell any more information besides this, so a more fine-grained version is probably called for.

Further notes

This seems most commonly a problem for events which can be fired in a pre-cancelled state by the server itself, like PlayerInteractEvent. Unclear if anything more specific than a flag for "cancelled by plugin" is actually even necessary. Please discuss.

@dktapps dktapps added Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP Opinions Wanted Request for comments & opinions from the community labels Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Related to the plugin API Opinions Wanted Request for comments & opinions from the community Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

No branches or pull requests

1 participant