Feature: Accurate Java packet ticking (#5121)

* Use proposed mcpl ticking PR

* Remove more not needed overrides

* Bump mcpl

* Fix missing import

* Bump mcpl

* Switch to official version

---------

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

bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotInjector.java

@@ -178,7 +178,7 @@ public class GeyserSpigotInjector extends GeyserInjector {
         MinecraftProtocol protocol = new MinecraftProtocol();
         LocalSession session = new LocalSession(bootstrap.getGeyserConfig().getRemote().address(),
                 bootstrap.getGeyserConfig().getRemote().port(), this.serverSocketAddress,
-                InetAddress.getLoopbackAddress().getHostAddress(), protocol, protocol.createHelper());
+                InetAddress.getLoopbackAddress().getHostAddress(), protocol, Runnable::run);
         session.connect();
     }
 

core/src/main/java/org/geysermc/geyser/GeyserImpl.java

@@ -414,9 +414,6 @@ public class GeyserImpl implements GeyserApi, EventRegistrar {
             }
         }
 
-        // Ensure that PacketLib does not create an event loop for handling packets; we'll do that ourselves
-        TcpSession.USE_EVENT_LOOP_FOR_PACKETS = false;
-
         pendingMicrosoftAuthentication = new PendingMicrosoftAuthentication(config.getPendingAuthenticationTimeout());
 
         this.newsHandler = new NewsHandler(BRANCH, this.buildNumber());

core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java

@@ -96,7 +96,7 @@ public class UpstreamPacketHandler extends LoggingPacketHandler {
     }
 
     private PacketSignal translateAndDefault(BedrockPacket packet) {
-        Registries.BEDROCK_PACKET_TRANSLATORS.translate(packet.getClass(), packet, session);
+        Registries.BEDROCK_PACKET_TRANSLATORS.translate(packet.getClass(), packet, session, false);
         return PacketSignal.HANDLED; // PacketSignal.UNHANDLED will log a WARN publicly
     }
 

core/src/main/java/org/geysermc/geyser/network/netty/LocalSession.java

@@ -59,6 +59,7 @@ import java.net.Inet4Address;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -72,11 +73,11 @@ public final class LocalSession extends TcpSession {
     private final String clientIp;
     private final PacketCodecHelper codecHelper;
 
-    public LocalSession(String host, int port, SocketAddress targetAddress, String clientIp, PacketProtocol protocol, MinecraftCodecHelper codecHelper) {
+    public LocalSession(String host, int port, SocketAddress targetAddress, String clientIp, PacketProtocol protocol, Executor packetHandlerExecutor) {
-        super(host, port, protocol);
+        super(host, port, protocol, packetHandlerExecutor);
         this.targetAddress = targetAddress;
         this.clientIp = clientIp;
-        this.codecHelper = codecHelper;
+        this.codecHelper = protocol.createHelper();
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/registry/PacketTranslatorRegistry.java

@@ -56,15 +56,15 @@ public class PacketTranslatorRegistry<T> extends AbstractMappedRegistry<Class<?
     }
 
     @SuppressWarnings("unchecked")
-    public <P extends T> boolean translate(Class<? extends P> clazz, P packet, GeyserSession session) {
+    public <P extends T> boolean translate(Class<? extends P> clazz, P packet, GeyserSession session, boolean canRunImmediately) {
         if (session.getUpstream().isClosed() || session.isClosed()) {
             return false;
         }
 
         PacketTranslator<P> translator = (PacketTranslator<P>) this.mappings.get(clazz);
         if (translator != null) {
-            EventLoop eventLoop = session.getEventLoop();
+            EventLoop eventLoop = session.getTickEventLoop();
-            if (!translator.shouldExecuteInEventLoop() || eventLoop.inEventLoop()) {
+            if (canRunImmediately || !translator.shouldExecuteInEventLoop() || eventLoop.inEventLoop()) {
                 translate0(session, translator, packet);
             } else {
                 eventLoop.execute(() -> translate0(session, translator, packet));

core/src/main/java/org/geysermc/geyser/session/GeyserSession.java

@@ -253,7 +253,7 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
      * The loop where all packets and ticking is processed to prevent concurrency issues.
      * If this is manually called, ensure that any exceptions are properly handled.
      */
-    private final EventLoop eventLoop;
+    private final EventLoop tickEventLoop;
     @Setter
     private AuthData authData;
     private BedrockClientData clientData;
@@ -653,10 +653,10 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
     private MinecraftProtocol protocol;
 
-    public GeyserSession(GeyserImpl geyser, BedrockServerSession bedrockServerSession, EventLoop eventLoop) {
+    public GeyserSession(GeyserImpl geyser, BedrockServerSession bedrockServerSession, EventLoop tickEventLoop) {
         this.geyser = geyser;
         this.upstream = new UpstreamSession(bedrockServerSession);
-        this.eventLoop = eventLoop;
+        this.tickEventLoop = tickEventLoop;
 
         this.erosionHandler = new GeyserboundHandshakePacketHandler(this);
 
@@ -947,17 +947,17 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
         boolean floodgate = this.remoteServer.authType() == AuthType.FLOODGATE;
 
         // Start ticking
-        tickThread = eventLoop.scheduleAtFixedRate(this::tick, 50, 50, TimeUnit.MILLISECONDS);
+        tickThread = tickEventLoop.scheduleAtFixedRate(this::tick, 50, 50, TimeUnit.MILLISECONDS);
 
         TcpSession downstream;
         if (geyser.getBootstrap().getSocketAddress() != null) {
             // We're going to connect through the JVM and not through TCP
             downstream = new LocalSession(this.remoteServer.address(), this.remoteServer.port(),
                     geyser.getBootstrap().getSocketAddress(), upstream.getAddress().getAddress().getHostAddress(),
-                    this.protocol, this.protocol.createHelper());
+                    this.protocol, tickEventLoop);
             this.downstream = new DownstreamSession(downstream);
         } else {
-            downstream = new TcpClientSession(this.remoteServer.address(), this.remoteServer.port(), this.protocol);
+            downstream = new TcpClientSession(this.remoteServer.address(), this.remoteServer.port(), "0.0.0.0", 0, this.protocol, null, tickEventLoop);
             this.downstream = new DownstreamSession(downstream);
 
             boolean resolveSrv = false;
@@ -1143,7 +1143,7 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
             @Override
             public void packetReceived(Session session, Packet packet) {
-                Registries.JAVA_PACKET_TRANSLATORS.translate(packet.getClass(), packet, GeyserSession.this);
+                Registries.JAVA_PACKET_TRANSLATORS.translate(packet.getClass(), packet, GeyserSession.this, true);
             }
 
             @Override
@@ -1213,10 +1213,11 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
      * Moves task to the session event loop if already not in it. Otherwise, the task is automatically ran.
      */
     public void ensureInEventLoop(Runnable runnable) {
-        if (eventLoop.inEventLoop()) {
+        if (tickEventLoop.inEventLoop()) {
-            runnable.run();
+            executeRunnable(runnable);
             return;
         }
+
         executeInEventLoop(runnable);
     }
 
@@ -1224,15 +1225,7 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
      * Executes a task and prints a stack trace if an error occurs.
      */
     public void executeInEventLoop(Runnable runnable) {
-        eventLoop.execute(() -> {
+        tickEventLoop.execute(() -> executeRunnable(runnable));
-            try {
-                runnable.run();
-            } catch (ErosionCancellationException e) {
-                geyser.getLogger().debug("Caught ErosionCancellationException");
-            } catch (Throwable e) {
-                geyser.getLogger().error("Error thrown in " + this.bedrockUsername() + "'s event loop!", e);
-            }
-        });
     }
 
     /**
@@ -1241,19 +1234,25 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
      * The task will not run if the session is closed.
      */
     public ScheduledFuture<?> scheduleInEventLoop(Runnable runnable, long duration, TimeUnit timeUnit) {
-        return eventLoop.schedule(() -> {
+        return tickEventLoop.schedule(() -> {
-            try {
+            executeRunnable(() -> {
                 if (!closed) {
                     runnable.run();
                 }
-            } catch (ErosionCancellationException e) {
+            });
-                geyser.getLogger().debug("Caught ErosionCancellationException");
-            } catch (Throwable e) {
-                geyser.getLogger().error("Error thrown in " + this.bedrockUsername() + "'s event loop!", e);
-            }
         }, duration, timeUnit);
     }
 
+    private void executeRunnable(Runnable runnable) {
+        try {
+            runnable.run();
+        } catch (ErosionCancellationException e) {
+            geyser.getLogger().debug("Caught ErosionCancellationException");
+        } catch (Throwable e) {
+            geyser.getLogger().error("Error thrown in " + this.bedrockUsername() + "'s event loop!", e);
+        }
+    }
+
     /**
      * Called every 50 milliseconds - one Minecraft tick.
      */

core/src/main/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java

@@ -131,11 +131,8 @@ public class SkullBlockEntityTranslator extends BlockEntityTranslator implements
                 session.getGeyser().getLogger().debug("Custom skull with invalid profile tag: " + blockPosition + " " + javaNbt);
                 return;
             }
-            if (session.getEventLoop().inEventLoop()) {
+
-                putSkull(session, blockPosition, uuid, texturesProperty, blockState);
+            session.ensureInEventLoop(() -> putSkull(session, blockPosition, uuid, texturesProperty, blockState));
-            } else {
-                session.executeInEventLoop(() -> putSkull(session, blockPosition, uuid, texturesProperty, blockState));
-            }
         });
 
         // We don't have the textures yet, so we can't determine if a custom block was defined for this skull

core/src/main/java/org/geysermc/geyser/translator/protocol/PacketTranslator.java

@@ -34,6 +34,7 @@ public abstract class PacketTranslator<T> {
     /**
      * Determines if this packet should be handled in the session's event loop. This should generally be true -
      * only when the packet has to be executed immediately should it be false.
+     * This method is only used for bedrock packets, java packets have a more sophisticated system through MCProtocolLib.
      */
     public boolean shouldExecuteInEventLoop() {
         return true;

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockEmoteTranslator.java

@@ -61,11 +61,8 @@ public class BedrockEmoteTranslator extends PacketTranslator<EmotePacket> {
         for (GeyserSession otherSession : session.getGeyser().getSessionManager().getSessions().values()) {
             if (otherSession != session) {
                 if (otherSession.isClosed()) continue;
-                if (otherSession.getEventLoop().inEventLoop()) {
+
-                    playEmote(otherSession, javaId, xuid, emote);
+                otherSession.ensureInEventLoop(() -> playEmote(otherSession, javaId, xuid, emote));
-                } else {
-                    otherSession.executeInEventLoop(() -> playEmote(otherSession, javaId, xuid, emote));
-                }
             }
         }
     }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaCustomPayloadTranslator.java

@@ -140,10 +140,4 @@ public class JavaCustomPayloadTranslator extends PacketTranslator<ClientboundCus
             });
         }
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        // For Erosion packets
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaDisconnectTranslator.java

@@ -38,9 +38,4 @@ public class JavaDisconnectTranslator extends PacketTranslator<ClientboundDiscon
     public void translate(GeyserSession session, ClientboundDisconnectPacket packet) {
         session.disconnect(MessageTranslator.convertMessage(packet.getReason(), session.locale()));
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaKeepAliveTranslator.java

@@ -67,9 +67,4 @@ public class JavaKeepAliveTranslator extends PacketTranslator<ClientboundKeepAli
         latencyPacket.setTimestamp(timestamp);
         session.sendUpstreamPacketImmediately(latencyPacket);
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaLoginDisconnectTranslator.java

@@ -98,9 +98,4 @@ public class JavaLoginDisconnectTranslator extends PacketTranslator<ClientboundL
     private boolean testForMissingProfilePublicKey(Component disconnectReason) {
         return disconnectReason instanceof TranslatableComponent component && "multiplayer.disconnect.missing_public_key".equals(component.key());
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaSelectKnownPacksTranslator.java

@@ -59,10 +59,4 @@ public class JavaSelectKnownPacksTranslator extends PacketTranslator<Clientbound
         }
         session.sendDownstreamPacket(new ServerboundSelectKnownPacks(knownPacks));
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        // This technically isn't correct behavior, but it prevents race conditions between MCProtocolLib's packet handler and ours.
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaStartConfigurationTranslator.java

@@ -43,10 +43,4 @@ public class JavaStartConfigurationTranslator extends PacketTranslator<Clientbou
             erosionHandler.close();
         }
     }
-
-    @Override
-    public boolean shouldExecuteInEventLoop() {
-        // Execute outside of event loop to cancel any pending erosion futures
-        return false;
-    }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/entity/player/JavaCookieRequestTranslator.java

@@ -28,8 +28,8 @@ package org.geysermc.geyser.translator.protocol.java.entity.player;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
-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.ServerboundCookieResponsePacket;
+import org.geysermc.mcprotocollib.protocol.packet.cookie.serverbound.ServerboundCookieResponsePacket;
 
 @Translator(packet = ClientboundCookieRequestPacket.class)
 public class JavaCookieRequestTranslator extends PacketTranslator<ClientboundCookieRequestPacket> {

gradle/libs.versions.toml

@@ -15,7 +15,7 @@ protocol-common = "3.0.0.Beta5-20241203.200249-19"
 protocol-codec = "3.0.0.Beta5-20241203.200249-19"
 raknet = "1.0.0.CR3-20240416.144209-1"
 minecraftauth = "4.1.1"
-mcprotocollib = "1.21.2-20241107.110329-3"
+mcprotocollib = "1.21.2-20241127.160724-5"
 adventure = "4.14.0"
 adventure-platform = "4.3.0"
 junit = "5.9.2"