Guarantee the order of AsyncEvent execution when using intentions

[BoomEaro]

This pull requester modifies the behavior of AsyncEvent to ensure that all handlers are executed according to their order or priority when using intents.

There is a race condition problem where multiple plugins could register an intent with asynchronous tasks and all tasks would just execute in the background, not guaranteeing any order of execution at all. Event prioritization for asynchronous events doesn't really have any effect.

The current implementation fixes this, by now registering an intent will suspend processing of all listeners until the intent is called to complete.

I've tried to maintain backwards compatibility with plugins, so let me know if I've missed something

[md-5]

I'm not sure this is what was intended by the API. The API was really designed to offload background loading and similar. This serialises the Async intents so they are all dependent on each other. If this is something you need I think it should be a separate intent method

[BoomEaro]

I'm not sure this is what was intended by the API. The API was really designed to offload background loading and similar. This serialises the Async intents so they are all dependent on each other. If this is something you need I think it should be a separate intent method

Creating a separate intent method doesn't seem like a good idea, because then there would be no way to precisely override the result of the event, not to mention that many plugins would continue to use the old intent method.

The current implementation already allows background tasks, except that there can't be multiple background tasks at the same time in the context of a single event.
I think this should be correct, because imagine a situation where multiple plugins register an intent. One plugin decides to change the event asynchronously and the second plugin does the same, the result of the event depends solely on which intent task finishes last, overriding everything the previous plugins have done and calculated for it.

This is a behavior that is not documented anywhere and it would be nice to have control over it.

[md-5]

The current implementation already allows background tasks, except that there can't be multiple background tasks at the same time in the context of a single event.

Well there can be when all they are doing is loading data, which was the original purpose of this API.

Separate from the question of which is the default, I think at the very least there needs to be a way to have the current behaviour for plugins that use intents to load data. Because previously 5 plugins all loading data from a slow remote source (say 200ms) would take 200ms, now it takes a second.

[BoomEaro]

Well there can be when all they are doing is loading data, which was the original purpose of this API.

Separate from the question of which is the default, I think at the very least there needs to be a way to have the current behaviour for plugins that use intents to load data. Because previously 5 plugins all loading data from a slow remote source (say 200ms) would take 200ms, now it takes a second.

Yes, this is the only problem with this approach, potentially slowing down event processing, however my personal opinion is that the developer or server administrator should still take care that the event does not always take a very long time, using various database caching mechanisms if possible.

However, on the other hand, it will be possible for other plugins to determine whether they should load their background tasks when, for example, an event has been cancelled.
Of course, if no one canceled the event, the worst case scenario will happen, but if the plugin was asynchronously dependent on the outcome of the event, it will have the opportunity to at least get the correct result, rather than a random one.

It seems to me almost impossible to preserve the old behavior, since the use of the old behavior is only to prevent the event from completing execution before loading something asynchronous in the plugin. This works great in a scenario where plugins don't modify events, but simply load something.
However, plugins such as ban managers can change the event asynchronously, and problems begin when there are several such plugins.