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 method to register Cancelled result callbacks for Cancellable. #11927

Open
LamGC opened this issue Jan 7, 2025 · 2 comments
Open

Add a method to register Cancelled result callbacks for Cancellable. #11927

LamGC opened this issue Jan 7, 2025 · 2 comments

Comments

@LamGC
Copy link

LamGC commented Jan 7, 2025

Is your feature request related to a problem?

During the development process, I may encounter some Post operations that depend on whether the event is ultimately cancelled or not.

Example, After listening to EntityDamageEvent and processing it at LOW priority level, it may be cancelled later, but I still play some special effects.

Alternatively, I don't really want to recalculate certain values between two listening methods, or require some fields to pass them.

Describe the solution you'd like.

This is a very sudden idea, and I want to know if it is possible to implement it, or what is the foolishness of this idea?

Can we add a method for setting the final cancellation result callback for event listeners to the Cancellable interface? This can allow listeners whose listening level is not in MONITOR to re determine whether the event was ultimately cancelled.

So, may this feature help change this situation?

Just like this

Cancellable:

/**
 * A type characterizing events that may be cancelled by a plugin or the server.
 */
public interface Cancellable {

    /**
     * Gets the cancellation state of this event. A cancelled event will not
     * be executed in the server, but will still pass to other plugins.
     *
     * @return true if this event is cancelled
     */
    public boolean isCancelled();

    /**
     * Sets the cancellation state of this event. A cancelled event will not
     * be executed in the server, but will still pass to other plugins.
     *
     * @param cancel true if you wish to cancel this event
     */
    public void setCancelled(boolean cancel);

    /**
     * Register a callback to determine whether the event is ultimately cancelled.
     * 
     * @param callback A callback used to receive cancel results.
     */
    public void onResult(Consumer<Boolean> callback);

    /**
     * Get the registered result callbacks.
     * 
     * @return The list of registered result callbacks.
     */
    public List<Consumer<Boolean>> getResultCallbacks();
}

A simple event class:

public class SimpleCancellableEvent extends Event implements Cancellable {

    private final static HandlerList HANDLER_LIST = new HandlerList();

    public static HandlerList getHandlerList() {
        return HANDLER_LIST;
    }

    // Something...

    private boolean cancel;
    private final List<Consumer<Boolean>> resultCallbacks = new ArrayList<>();
    
    @Override
    public boolean isCancelled() {
        return this.cancel;
    }

    @Override
    public void setCancelled(boolean cancel) {
        this.cancel = cancel;
    }

    @Override
    public void onResult(Consumer<Boolean> callback) {
        this.resultCallbacks.add(Objects.requireNonNull(callback));
    }

    @Override
    public List<Consumer<Boolean>> getResultCallbacks() {
        return this.resultCallbacks;
    }

    @Override
    public HandlerList getHandlers() {
        return HANDLER_LIST;
    }
}

And a example listener:

public class SimpleListener implements EventListener {

    // Before:
    @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true)
    public void prohibitPlayerSomething(SimpleCancellableEvent event) {
        // Check conditions...
        if (true) {
            event.setCancelled(true);
        }
    }

    @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
    public void onPlayerCancelledSomething(SimpleCancellableEvent event) {
        event.getPlayer().sendMessage("Hey, You can't do this!");
    }

    // After:
    @EventHandler(priority = EventPriority.LOW)
    public void prohibitPlayerSomething(SimpleCancellableEvent event) {
        // Check conditions...
        if (true && !event.isCancelled()) {
            event.setCancelled(true);
        }

        event.onResult(cancelled -> {
            if (cancelled) {
                event.getPlayer().sendMessage("Hey, You can't do this!");
            }
        });
    }
}

Describe alternatives you've considered.

I am currently using lightweight metadata based on WeakHashMap to solve the problem I am encountering.

Other

No response

@Machine-Maker
Copy link
Member

I think there are some fundamental issues with this concept. I do not like introducing any way to thwart/bypass the priority system while we still have modifiable events. In your example, you are re-querying the player from the event. Imagine an event with a mutable field with a getter/setter. This would enable you to modify the event from that callback. That shouldn't be allowed. I much prefer you just have 2 listeners on different priorities.

@LamGC
Copy link
Author

LamGC commented Jan 9, 2025

@Machine-Maker Yes, I agree with your concerns because we can only request that cannot do so, but the request is clearly not as restrictive as the limit.

Therefore, I also agree that this change should not be implemented, at least not in the Paper API. But I also want to know if using it while adhering to immutable requirements would result in performance issues?

Sorry for the late reply, there was an issue with Github when I was replying this morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants