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

Async message processing. #91

Open
BlitzOffline opened this issue Jul 18, 2022 · 4 comments
Open

Async message processing. #91

BlitzOffline opened this issue Jul 18, 2022 · 4 comments
Labels
Issue Something isn't working

Comments

@BlitzOffline
Copy link
Member

There are some instances where the AsyncPlayerChatEvent isn't actually async which means all the message processing is done on the main thread. I believe this happens when the player's message is sent using the spigot api but possible in other instances as well. We need to detect those instances and execute the code on another thread.

@BlitzOffline BlitzOffline added the Issue Something isn't working label Jul 18, 2022
@Jaimss
Copy link
Contributor

Jaimss commented Jul 21, 2022

All you should need to do is check Event#isAsynchronous(). No special detection or anything should be necessary

I can PR this if you want

@BlitzOffline
Copy link
Member Author

All you should need to do is check Event#isAsynchronous(). No special detection or anything should be necessary

I can PR this if you want

Yes, that's true, but there's a pr that changes some stuff (the one that cancels AsyncChatEvent when ChatChatEvent is cancelled) so idk. I really can't be bothered to think rn. If you think you can make it work with that PR, then sure.

@Jaimss
Copy link
Contributor

Jaimss commented Jul 22, 2022

So I've done some work on this and as far as I can tell it's not going to be possible to make it work the way it needs to.

If AsyncPlayerChatEvent is fired async, everything works fine.

If AsyncPlayerChatEvent is fired sync, there are some issues. You can't perform async computations or processing and then use those results to update the event. So #setMessage(), #setFormat(), #setCancelled(), etc. don't work. The way things are set up currently, those things depend on processing, which should be done asynchronously, but you can't update them with that async value unless you block the main thread and wait for the async processing to complete, which defeats the purpose.

I believe the only way to ensure that the processing is always completed async is to cancel the AsyncPlayerChatEvent entirely and handle the console format, sending the message, etc. entirely on your own, although I may be overlooking a simple solution lol.

I believe AsyncPlayerChatEvent only gets called synchronously when a plugin uses Player#chat(), so it isn't something that will happen very often.

@BlitzOffline
Copy link
Member Author

We can't cancel the APCEvent. That's because plugins like DiscordSRV, listen to that event with the MONITOR priority. I knew this will be an issue that's why I didn't take on this task yet. Also the addition of bungee support will require us to completely redo the MessageProcessor class since right now it is a mess.

Btw we only take the result from the process method which tells us if the event should continue running or if it should be cancelled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants