-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Generalize asynchronous events #6092
base: minor-next
Are you sure you want to change the base?
Conversation
code came from #6015 and will be subject to probable changes
An asynchronous event is one that allows the addition of promises to be resolved before being completed. This implementation integrates priority levels, allowing you to wait for all promises added to one level before moving on to the next. This is made possible by separating the event call logic into several functions, which can then be integrated into AsyncEventTrait::callAsync()
If the event is async and the handler return Promise, it will be handle as an async event. However, if the event is async and the handler return smth different than Promise, it will be handle synchronously. An async handler can specify if it wishes to be called with no concurrent handlers with the tag @noConcurrentCall
New behaviourAbove all, the logic of synchronous events remains the same, even for asynchronous events, as long as they are called synchronously. When a listener of an asynchronous event returns a
In this way, each listener can add only one promise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tldr AsyncEventDelegate, will look again when PR is ready
Co-authored-by: Javier León <[email protected]>
As I understand, now, we will accept async listeners to return null or Promise ? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Yep, they'll allow the next handler in the queue to be called immediately if returning null |
this required mocking to get around #6524. Longer term we should make improvements to avoid the need for mocking here.
I think the main thing still on the TODO list is to buffer incomplete events. The existing mechanism using stack depth won't cut it. We need a way to track which handlers didn't fulfil their promises so that the correct plugin can be properly blamed when an event fails to complete. |
Introduction
This PR would be more of a foretaste for #6015 in that it provides a means of generalizing asynchronous event calling.
An asynchronous event in this context is one whose action can be deferred by plugins specifying promises to be resolved.
The integrity of event priorities is guaranteed by calling them one by one. Before moving on to the next priority, we wait until all the promises of the current one have been fulfilled.
Only the state of the server and player context can't be guaranteed. However, the state of the event is unaffected, whether it is called asynchronous or synchronous.
Behaviour
When a listener of an asynchronous event returns a
Promise
, the event is marked as asynchronous and placed at the end of the call stack for its priority.When a callAsync is called, the call flow is as follows:
In this way, each listener can add only one promise.
If, on the other hand, the handler is sensitive to changes in the event, it is preferable to add the @exclusiveCall tag, which specifies that it must be the only one being called.
Event transformation
If we wanted to implement the asynchronous chat event, this would look like this:
When calling:
Changes
API changes
AsyncEvent
classPromise::all()
Behavioural changes
Backwards compatibility
Not for the moment
Follow-up
n/a
Tests