AsyncPlayerSendCommandsEvent is fired twice, once async, then sync. The javadocs suggest using the logic if (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff } to only react to the async event.
However, this check will currently pass during both times the event fires. As seen using this simple plugin:
public final class BukkitTestPlugin extends JavaPlugin implements Listener {
@Override
public void onEnable() {
Bukkit.getServer().getPluginManager().registerEvents(this, this);
}
@EventHandler
public void onCommandsSentToPlayer(AsyncPlayerSendCommandsEvent<?> event) {
getLogger().info("AsyncPlayerSendCommandsEvent{" +
"isAsynchronous=" + event.isAsynchronous() +
", hasFiredAsync=" + event.hasFiredAsync() + "}"
);
}
}
When a player logs in, the event is fired like so:
[07:51:27] [Server thread/INFO]: willkroboth joined the game
[07:51:27] [Server thread/INFO]: willkroboth[/127.0.0.1:41644] logged in with entity id 15 at ([world]-276.5, 72.0, 27.5)
[07:51:27] [Paper Async Command Builder Thread Pool - 0/INFO]: [BukkitTestPlugin] AsyncPlayerSendCommandsEvent{isAsynchronous=true, hasFiredAsync=false}
[07:51:27] [Server thread/INFO]: [BukkitTestPlugin] AsyncPlayerSendCommandsEvent{isAsynchronous=false, hasFiredAsync=false}
[07:52:04] [Server thread/INFO]: Checking version, please wait...
[07:52:05] [Thread-5/INFO]: This server is running Paper version 1.20.6-145-ver/1.20.6@fe7043e (2024-06-15T21:16:04Z) (Implementing API version 1.20.6-R0.1-SNAPSHOT)
You are running the latest version
Both times the event is called, the hasFiredAsync value is false. Therefore !event.hasFiredAsync() is always true, and // do stuff would always run.
This PR simply makes it so the second time the event is called, hasFiredAsync is true:
[08:27:10] [Server thread/INFO]: willkroboth joined the game
[08:27:10] [Server thread/INFO]: willkroboth[/127.0.0.1:52662] logged in with entity id 126 at ([world]-276.5, 72.0, 27.5)
[08:27:10] [Paper Async Command Builder Thread Pool - 0/INFO]: [BukkitTestPlugin] AsyncPlayerSendCommandsEvent{isAsynchronous=true, hasFiredAsync=false}
[08:27:10] [Server thread/INFO]: [BukkitTestPlugin] AsyncPlayerSendCommandsEvent{isAsynchronous=false, hasFiredAsync=true}
[08:27:19] [Server thread/INFO]: Checking version, please wait...
[08:27:19] [Thread-5/INFO]: This server is running Paper version 1.20.6-DEV-fix-CommandSend-hasFiredAsync@79e2cb6 (2024-06-17T12:19:25Z) (Implementing API version 1.20.6-R0.1-SNAPSHOT)
Unknown version
Previous version: 1.20.6-145-fe7043e (MC: 1.20.6)
With this, if (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff } will only do stuff during the async event, as the javadocs suggest.
The javadocs mention If for some reason we are unable to send this asynchronously in the future, only the sync method will fire. If the async call is eventually removed, I imagine the sync event call should have hasFiredAsync set to false (which would then be a correct statement). I believe that is the intended purpose of suggesting the if (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff } logic, so that // do stuff is still run if the async call is ever removed.
AsyncPlayerSendCommandsEventis fired twice, once async, then sync. The javadocs suggest using the logicif (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff }to only react to the async event.However, this check will currently pass during both times the event fires. As seen using this simple plugin:
When a player logs in, the event is fired like so:
Both times the event is called, the
hasFiredAsyncvalue isfalse. Therefore!event.hasFiredAsync()is alwaystrue, and// do stuffwould always run.This PR simply makes it so the second time the event is called,
hasFiredAsyncistrue:With this,
if (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff }will only do stuff during the async event, as the javadocs suggest.I imagine this is a copy-paste error back from when the event was first added in this commit: https://github.com/PaperMC/Paper/commit/fdf41b742d5e78e7e97659a829f43803c61efb8f#diff-1b1388b7c287d608bf170cd9f7f3ac9477793ac427c444b8968f86e9f4477468R20-R28.
The javadocs mention
If for some reason we are unable to send this asynchronously in the future, only the sync method will fire. If the async call is eventually removed, I imagine the sync event call should havehasFiredAsyncset tofalse(which would then be a correct statement). I believe that is the intended purpose of suggesting theif (event.isAsynchronous() || !event.hasFiredAsync()) { // do stuff }logic, so that// do stuffis still run if the async call is ever removed.