Improve MCPL ticking data and behaviour (#865)

* Move packets to proper packages & implement game thread metadata for every packet

* Add new ticking info for 1.21.3

Notable info:
ServerboundSelectBundleItemPacket is not ticked, it's instant
ClientboundSetHeldSlotPacket is already ticked as it is just renamed ClientboundSetCarriedItemPacket

* Expose packet handler executor

* Do not tick delimiter packet

* Allow server to define packet handler executor factory

* Update protocol/src/main/java/org/geysermc/mcprotocollib/network/packet/Packet.java

Co-authored-by: chris <github@onechris.mozmail.com>

---------

Co-authored-by: chris <github@onechris.mozmail.com>

protocol/src/main/java/org/geysermc/mcprotocollib/network/packet/Packet.java

@@ -9,13 +9,14 @@
 public interface Packet {
 
     /**
-     * Gets whether the packet has handling priority.
-     * If the result is true, the packet will be handled immediately after being
-     * decoded.
+     * Gets whether the packet should run on an async game thread rather than blocking the network (Netty) thread.
+     * Packets that qualify for this are usually packets with an ensureRunningOnSameThread call at the top of their packet listener method in the Minecraft code.
+     * Packets which need extra attention because they aren't "fully" handled async are marked using
+     * // GAME THREAD DETAIL comments in the MCProtocolLib code.
      *
-     * @return Whether the packet has priority.
+     * @return Whether the packet be handled async from the Netty thread.
      */
-    default boolean isPriority() {
+    default boolean shouldRunOnGameThread() {
         return false;
     }
 

protocol/src/main/java/org/geysermc/mcprotocollib/network/tcp/DefaultPacketHandlerExecutor.java

@@ -0,0 +1,34 @@
+package org.geysermc.mcprotocollib.network.tcp;
+
+import io.netty.channel.DefaultEventLoopGroup;
+import io.netty.channel.EventLoopGroup;
+import io.netty.util.concurrent.DefaultThreadFactory;
+
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+
+public class DefaultPacketHandlerExecutor {
+    /**
+     * Controls whether non-priority packets are handled in a separate event loop
+     */
+    public static boolean USE_EVENT_LOOP_FOR_PACKETS = true;
+    private static EventLoopGroup PACKET_EVENT_LOOP;
+    private static final int SHUTDOWN_QUIET_PERIOD_MS = 100;
+    private static final int SHUTDOWN_TIMEOUT_MS = 500;
+
+    public static Executor createExecutor() {
+        if (!USE_EVENT_LOOP_FOR_PACKETS) {
+            return Runnable::run;
+        }
+
+        if (PACKET_EVENT_LOOP == null) {
+            // See TcpClientSession.newThreadFactory() for details on
+            // daemon threads and their interaction with the runtime.
+            PACKET_EVENT_LOOP = new DefaultEventLoopGroup(new DefaultThreadFactory(DefaultPacketHandlerExecutor.class, true));
+            Runtime.getRuntime().addShutdownHook(new Thread(
+                () -> PACKET_EVENT_LOOP.shutdownGracefully(SHUTDOWN_QUIET_PERIOD_MS, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)));
+        }
+
+        return PACKET_EVENT_LOOP.next();
+    }
+}

protocol/src/main/java/org/geysermc/mcprotocollib/network/tcp/TcpClientSession.java

@@ -18,6 +18,7 @@
 import org.geysermc.mcprotocollib.network.packet.PacketProtocol;
 
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 
@@ -48,7 +49,11 @@ public TcpClientSession(String host, int port, String bindAddress, int bindPort,
     }
 
     public TcpClientSession(String host, int port, String bindAddress, int bindPort, PacketProtocol protocol, ProxyInfo proxy) {
-        super(host, port, protocol);
+        this(host, port, bindAddress, bindPort, protocol, proxy, DefaultPacketHandlerExecutor.createExecutor());
+    }
+
+    public TcpClientSession(String host, int port, String bindAddress, int bindPort, PacketProtocol protocol, ProxyInfo proxy, Executor packetHandlerExecutor) {
+        super(host, port, protocol, packetHandlerExecutor);
         this.bindAddress = bindAddress;
         this.bindPort = bindPort;
         this.proxy = proxy;

protocol/src/main/java/org/geysermc/mcprotocollib/network/tcp/TcpServer.java

@@ -19,16 +19,23 @@
 
 import java.net.InetSocketAddress;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
 import java.util.function.Supplier;
 
 public class TcpServer extends AbstractServer {
     private static final Logger log = LoggerFactory.getLogger(TcpServer.class);
 
+    private final Supplier<Executor> packetHandlerExecutorFactory;
     private EventLoopGroup group;
     private Channel channel;
 
     public TcpServer(String host, int port, Supplier<? extends PacketProtocol> protocol) {
+        this(host, port, protocol, DefaultPacketHandlerExecutor::createExecutor);
+    }
+
+    public TcpServer(String host, int port, Supplier<? extends PacketProtocol> protocol, Supplier<Executor> packetHandlerExecutorFactory) {
         super(host, port, protocol);
+        this.packetHandlerExecutorFactory = packetHandlerExecutorFactory;
     }
 
     @Override
@@ -56,7 +63,7 @@ public void initChannel(@NonNull Channel channel) {
                 InetSocketAddress address = (InetSocketAddress) channel.remoteAddress();
                 PacketProtocol protocol = createPacketProtocol();
 
-                TcpSession session = new TcpServerSession(address.getHostName(), address.getPort(), protocol, TcpServer.this);
+                TcpSession session = new TcpServerSession(address.getHostName(), address.getPort(), protocol, TcpServer.this, packetHandlerExecutorFactory.get());
                 session.getPacketProtocol().newServerSession(TcpServer.this, session);
 
                 ChannelPipeline pipeline = channel.pipeline();

protocol/src/main/java/org/geysermc/mcprotocollib/network/tcp/TcpServerSession.java

@@ -7,13 +7,14 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.Executor;
 
 public class TcpServerSession extends TcpSession {
     private final TcpServer server;
     private final PacketCodecHelper codecHelper;
 
-    public TcpServerSession(String host, int port, PacketProtocol protocol, TcpServer server) {
-        super(host, port, protocol);
+    public TcpServerSession(String host, int port, PacketProtocol protocol, TcpServer server, Executor packetHandlerExecutor) {
+        super(host, port, protocol, packetHandlerExecutor);
         this.server = server;
         this.codecHelper = protocol.createHelper();
     }

protocol/src/main/java/org/geysermc/mcprotocollib/network/tcp/TcpSession.java

@@ -3,11 +3,7 @@
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.channel.DefaultEventLoopGroup;
-import io.netty.channel.EventLoop;
-import io.netty.channel.EventLoopGroup;
 import io.netty.channel.SimpleChannelInboundHandler;
-import io.netty.util.concurrent.DefaultThreadFactory;
 import net.kyori.adventure.text.Component;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -33,35 +29,28 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeoutException;
 
 public abstract class TcpSession extends SimpleChannelInboundHandler<Packet> implements Session {
     private static final Logger log = LoggerFactory.getLogger(TcpSession.class);
 
-    /**
-     * Controls whether non-priority packets are handled in a separate event loop
-     */
-    public static boolean USE_EVENT_LOOP_FOR_PACKETS = true;
-    private static EventLoopGroup PACKET_EVENT_LOOP;
-    private static final int SHUTDOWN_QUIET_PERIOD_MS = 100;
-    private static final int SHUTDOWN_TIMEOUT_MS = 500;
-
     protected String host;
     protected int port;
     private final PacketProtocol protocol;
-    private final EventLoop eventLoop = createEventLoop();
+    private final Executor packetHandlerExecutor;
 
     private final Map<String, Object> flags = new HashMap<>();
     private final List<SessionListener> listeners = new CopyOnWriteArrayList<>();
 
     private Channel channel;
     protected boolean disconnected = false;
 
-    public TcpSession(String host, int port, PacketProtocol protocol) {
+    public TcpSession(String host, int port, PacketProtocol protocol, Executor packetHandlerExecutor) {
         this.host = host;
         this.port = port;
         this.protocol = protocol;
+        this.packetHandlerExecutor = packetHandlerExecutor;
     }
 
     @Override
@@ -269,21 +258,6 @@ public void setAutoRead(boolean autoRead) {
         }
     }
 
-    private @Nullable EventLoop createEventLoop() {
-        if (!USE_EVENT_LOOP_FOR_PACKETS) {
-            return null;
-        }
-
-        if (PACKET_EVENT_LOOP == null) {
-            // See TcpClientSession.newThreadFactory() for details on
-            // daemon threads and their interaction with the runtime.
-            PACKET_EVENT_LOOP = new DefaultEventLoopGroup(new DefaultThreadFactory(this.getClass(), true));
-            Runtime.getRuntime().addShutdownHook(new Thread(
-                () -> PACKET_EVENT_LOOP.shutdownGracefully(SHUTDOWN_QUIET_PERIOD_MS, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)));
-        }
-        return PACKET_EVENT_LOOP.next();
-    }
-
     @Override
     public Channel getChannel() {
         return this.channel;
@@ -322,8 +296,8 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
 
     @Override
     protected void channelRead0(ChannelHandlerContext ctx, Packet packet) {
-        if (!packet.isPriority() && eventLoop != null) {
-            eventLoop.execute(() -> this.callPacketReceived(packet));
+        if (packet.shouldRunOnGameThread()) {
+            packetHandlerExecutor.execute(() -> this.callPacketReceived(packet));
         } else {
             this.callPacketReceived(packet);
         }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/ClientListener.java

@@ -37,9 +37,9 @@
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;
 
 import javax.crypto.KeyGenerator;

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/ServerListener.java

@@ -35,9 +35,9 @@
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;
 
 import javax.crypto.SecretKey;

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/codec/MinecraftCodec.java

@@ -1,7 +1,7 @@
 package org.geysermc.mcprotocollib.protocol.codec;
 
 import org.geysermc.mcprotocollib.protocol.data.ProtocolState;
-import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCookieRequestPacket;
+import org.geysermc.mcprotocollib.protocol.packet.cookie.clientbound.ClientboundCookieRequestPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCustomPayloadPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCustomReportDetailsPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundDisconnectPacket;
@@ -13,7 +13,7 @@
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundStoreCookiePacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundTransferPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundUpdateTagsPacket;
-import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ServerboundCookieResponsePacket;
+import org.geysermc.mcprotocollib.protocol.packet.cookie.serverbound.ServerboundCookieResponsePacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundClientInformationPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundCustomPayloadPacket;
 import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundKeepAlivePacket;
@@ -207,9 +207,9 @@
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
 import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
-import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
+import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
 import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;
 
 public class MinecraftCodec {

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundCustomPayloadPacket.java

@@ -26,4 +26,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeResourceLocation(out, this.channel);
         out.writeBytes(this.data);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        // GAME THREAD DETAIL: Only non-discarded payloads are handled async.
+        return false; // False, you need to handle making it async yourself
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundCustomReportDetailsPacket.java

@@ -33,4 +33,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
             helper.writeString(out, entry.getValue());
         }
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundDisconnectPacket.java

@@ -28,9 +28,4 @@ public ClientboundDisconnectPacket(ByteBuf in, MinecraftCodecHelper helper) {
     public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeComponent(out, this.reason);
     }
-
-    @Override
-    public boolean isPriority() {
-        return true;
-    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundPingPacket.java

@@ -21,4 +21,9 @@ public ClientboundPingPacket(ByteBuf in, MinecraftCodecHelper helper) {
     public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         out.writeInt(this.id);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundResourcePackPopPacket.java

@@ -28,4 +28,9 @@ public ClientboundResourcePackPopPacket(ByteBuf in, MinecraftCodecHelper helper)
     public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeNullable(out, this.id, helper::writeUUID);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundResourcePackPushPacket.java

@@ -38,4 +38,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         out.writeBoolean(this.required);
         helper.writeNullable(out, this.prompt, helper::writeComponent);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundServerLinksPacket.java

@@ -52,4 +52,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
             helper.writeString(out, link.link());
         }
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundStoreCookiePacket.java

@@ -25,4 +25,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeResourceLocation(out, this.key);
         helper.writeByteArray(out, this.payload);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundTransferPacket.java

@@ -24,4 +24,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeString(out, this.host);
         helper.writeVarInt(out, this.port);
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        // GAME THREAD DETAIL: Code runs before packet is made async.
+        return false; // False, you need to handle making it async yourself
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/clientbound/ClientboundUpdateTagsPacket.java

@@ -53,4 +53,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
             }
         }
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/serverbound/ServerboundClientInformationPacket.java

@@ -72,4 +72,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         out.writeBoolean(allowsListing);
         helper.writeVarInt(out, this.particleStatus.ordinal());
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        // GAME THREAD DETAIL: Code is only async during GAME state.
+        return false; // False, you need to handle making it async yourself
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/common/serverbound/ServerboundResourcePackPacket.java

@@ -29,4 +29,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
         helper.writeUUID(out, id);
         helper.writeVarInt(out, this.status.ordinal());
     }
+
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
 }

protocol/src/main/java/org/geysermc/mcprotocollib/protocol/packet/configuration/clientbound/ClientboundFinishConfigurationPacket.java

@@ -16,6 +16,11 @@ public ClientboundFinishConfigurationPacket(ByteBuf in, MinecraftCodecHelper hel
     public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
     }
 
+    @Override
+    public boolean shouldRunOnGameThread() {
+        return true;
+    }
+
     @Override
     public boolean isTerminal() {
         return true;