Allow handling PlayerChatEvent asynchronously

[SOF3]

Two simple code snippets for TLDR:

function handle(PlayerChatEvent $event) : void{
    $event->getWaitGroup()->add();
    callTranslateApi($event->getMessage(), function(string $message) {
        $event->setMessage($message);
        $event->getWaitGroup()->done();
    }, function(Exception $error){
        $this->getLogger()->error($error->getMessage();
        $event->getWaitGroup()->done();
    });
}

await-generator users:

function handle(PlayerChatEvent $event) : void{
    Await::f2c(function() use($event) : void{
        try{
           $event->getWaitGroup()->add();
           $message = yield from callTranslateApi($event->getMessage();
           $event->setMessage($message);
       }finally{
           $event->getWaitGroup()->done();
       }
    });
}

Introduction

PlayerChatEvent is a simple event that can be handled asynchronously. As an early prototype, this PR implements a minimal, non-intrusive, backwards-compatible approach to enable asynchronous capabilities without introducing any changes to the event API in general (and does not even impact existing handlers of PlayerChatEvent, unless they have some unfortunate logic that expects the message to be sent immediately, in the same tick).

Relevant issues

There are multiple prior attempts to implement asynchronous events, most of which have failed due to excessive complexity.

The initial attempt was proposed in #1881 and implemented in #2096. It adopted a more sophisticated approach that adds a generic pause function to all events and requires callAsyncEvent instead of callEvent to be used. It stops the whole handler chain to wait for a single function.

The more recent attempt was #4120, which enforced concurrency isolation at each priority.

In contrast to the previous attempts, this PR lacks the following features:

• If a WaitGroup is never marked as done(), user will never be able to figure out what's wrong. This can be improved in the future when we have better async framework in PocketMine that allows easily scheduling a task to check that a called WaitGroup event is not complete after an expected duration and generate warnings.
• There is no concurrency control between multiple event handlers. This means all handlers get called together, and asynchronous modifications in one handler does not immediately notify another handler. Considering lower latency is more important, and that there is little mutual dependency between multiple event callers anyway (usually async is only used for calling translation/profanity checking APIs), this is currently not a concern.
• Handlers cannot observe the final state of a PlayerChatEvent. This can be a future enhancement to add another set of hooks called after event completion. Note that unlike Go sync.WaitGroup, my WaitGroup::wait() does not allow calling multiple times in favor of more explicit state control.

Changes

API changes

Adds a WaitGroup class, and initializes it as a field in PlayerChatEvent. Users can defer execution of the event with the following code:

function handle(PlayerChatEvent $event) : void{
    $event->getWaitGroup()->add();
    callTranslateApi($event->getMessage(), function(string $message) {
        $event->setMessage($message);
        $event->getWaitGroup()->done();
    }, function(Exception $error){
        $this->getLogger()->error($error->getMessage();
        $event->getWaitGroup()->done();
    });
}

It is tempting to call done before setting the message in the example above. Calling an event setter after event completion, just as it currently is to modify a normal synchronous event in an async callback, is a bug, and should lead to an error, which should be addressed in a separate issue.

Since there is no async runtime involved, forgetting to call done() on a WaitGroup will lead to the event getting silently dropped. Although this is not desired (because no handler got notified of the cancellation and is most likely a bug anyway), this does not lead to memory leak because the WaitGroup and the executor are only referenced from the context that holds the WaitGroup. The most common case of forgetting to call done() is the error case, in which I would recommend using await-generator instead:

function handle(PlayerChatEvent $event) : void{
    Await::f2c(function() use($event) : void{
        try{
           $event->getWaitGroup()->add();
           $message = yield from callTranslateApi($event->getMessage();
           $event->setMessage($message);
       }finally{
           $event->getWaitGroup()->done();
       }
    });
}

Another advantage of using await-generator is that the developer would not accidentally call done before using setMessage in the case above.

Behavioural changes

If no plugins handle the event asynchronously, there are no behavioral changes.

For plugins that handle the event asynchronously but resolve correctly, the only change is slightly delayed message broadcast, which is expected and is barely noticeable for users.

For plugins that do not resolve correctly, see the analysis above.

Backwards compatibility

PlayerChatEvent cannot be invoked multiple times. Plugins have never been expected to leak the PlayerchatEvent and call the same instance anyway, as there is practically no advantage to this.

Follow-up

Tests

I would add tests after the API design has been reviewed.

[dktapps]

Seems to me like this violates the MONITOR priority's expected conditions (that the event won't be modified on or after MONITOR).

[Gewinum]

Seems to me like this violates the MONITOR priority's expected conditions (that the event won't be modified on or after MONITOR).

what if we use sofe's mutex for this?

[Gewinum]

Seems to me like this violates the MONITOR priority's expected conditions (that the event won't be modified on or after MONITOR).

something like mutex to be right

[Gewinum]

Seems to me like this violates the MONITOR priority's expected conditions (that the event won't be modified on or after MONITOR).

something like mutex to be right

or.. nah im not sure im just async fanboy

[SOF3]

@Gewinum the problem is that you can't guarantee that handlers use the mutex

[Gewinum]

@Gewinum the problem is that you can't guarantee that handlers use the mutex

what if we call playerchatevent and check every listener? after calling every playerchatevent we wait for the waitgroup and when its done we continue calling chatevent same way. if code in playerchatevent is entirely sync and doesnt use waitgroup it would be not a problem anymore. if waitgroup is empty we just dont wait for it and continue calling these playerchatevents, so nothing would be ruined, right?

[Gewinum]

@Gewinum the problem is that you can't guarantee that handlers use the mutex

yeah, my solution could have some type of latency but does it matter with playerchatevent by the way?

[SOF3]

it has nothing to do with latency. the problem is correctness, because this PR cannot ensure that MONITOR handlers will use the mutex, so it would result in incorrect behavior.