From 2019e53bad109d79077b544cd9d9ec15e8f69472 Mon Sep 17 00:00:00 2001 From: Alex <40795980+AlexProgrammerDE@users.noreply.github.com> Date: Thu, 5 Dec 2024 11:35:03 +0100 Subject: [PATCH] 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 --- .../platform/spigot/GeyserSpigotInjector.java | 2 +- .../java/org/geysermc/geyser/GeyserImpl.java | 3 -- .../geyser/network/UpstreamPacketHandler.java | 2 +- .../geyser/network/netty/LocalSession.java | 7 +-- .../registry/PacketTranslatorRegistry.java | 6 +-- .../geyser/session/GeyserSession.java | 49 +++++++++---------- .../entity/SkullBlockEntityTranslator.java | 7 +-- .../translator/protocol/PacketTranslator.java | 1 + .../entity/player/BedrockEmoteTranslator.java | 7 +-- .../java/JavaCustomPayloadTranslator.java | 6 --- .../java/JavaDisconnectTranslator.java | 5 -- .../java/JavaKeepAliveTranslator.java | 5 -- .../java/JavaLoginDisconnectTranslator.java | 5 -- .../java/JavaSelectKnownPacksTranslator.java | 6 --- .../JavaStartConfigurationTranslator.java | 6 --- .../player/JavaCookieRequestTranslator.java | 4 +- gradle/libs.versions.toml | 2 +- 17 files changed, 41 insertions(+), 82 deletions(-) diff --git a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotInjector.java b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotInjector.java index a3402a752b6..9f7e21579d6 100644 --- a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotInjector.java +++ b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotInjector.java @@ -178,7 +178,7 @@ private void workAroundWeirdBug(GeyserBootstrap bootstrap) { 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(); } diff --git a/core/src/main/java/org/geysermc/geyser/GeyserImpl.java b/core/src/main/java/org/geysermc/geyser/GeyserImpl.java index 0a8222f8d72..065c1f0cc63 100644 --- a/core/src/main/java/org/geysermc/geyser/GeyserImpl.java +++ b/core/src/main/java/org/geysermc/geyser/GeyserImpl.java @@ -414,9 +414,6 @@ private void startInstance() { } } - // 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()); diff --git a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java b/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java index 0cf161c6aac..dfebb93dc75 100644 --- a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java +++ b/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java @@ -96,7 +96,7 @@ public UpstreamPacketHandler(GeyserImpl geyser, GeyserSession session) { } 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 } diff --git a/core/src/main/java/org/geysermc/geyser/network/netty/LocalSession.java b/core/src/main/java/org/geysermc/geyser/network/netty/LocalSession.java index 739c1c25ec6..3b86a0bf9f0 100644 --- a/core/src/main/java/org/geysermc/geyser/network/netty/LocalSession.java +++ b/core/src/main/java/org/geysermc/geyser/network/netty/LocalSession.java @@ -59,6 +59,7 @@ 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) { - super(host, port, protocol); + public LocalSession(String host, int port, SocketAddress targetAddress, String clientIp, PacketProtocol protocol, Executor packetHandlerExecutor) { + super(host, port, protocol, packetHandlerExecutor); this.targetAddress = targetAddress; this.clientIp = clientIp; - this.codecHelper = codecHelper; + this.codecHelper = protocol.createHelper(); } @Override diff --git a/core/src/main/java/org/geysermc/geyser/registry/PacketTranslatorRegistry.java b/core/src/main/java/org/geysermc/geyser/registry/PacketTranslatorRegistry.java index b31f2b4f000..e81935edfaf 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/PacketTranslatorRegistry.java +++ b/core/src/main/java/org/geysermc/geyser/registry/PacketTranslatorRegistry.java @@ -56,15 +56,15 @@ protected PacketTranslatorRegistry() { } @SuppressWarnings("unchecked") - public

boolean translate(Class clazz, P packet, GeyserSession session) { + public

boolean translate(Class clazz, P packet, GeyserSession session, boolean canRunImmediately) { if (session.getUpstream().isClosed() || session.isClosed()) { return false; } PacketTranslator

translator = (PacketTranslator

) this.mappings.get(clazz); if (translator != null) { - EventLoop eventLoop = session.getEventLoop(); - if (!translator.shouldExecuteInEventLoop() || eventLoop.inEventLoop()) { + EventLoop eventLoop = session.getTickEventLoop(); + if (canRunImmediately || !translator.shouldExecuteInEventLoop() || eventLoop.inEventLoop()) { translate0(session, translator, packet); } else { eventLoop.execute(() -> translate0(session, translator, packet)); diff --git a/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java b/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java index cfb981b7d17..b4a8e6203f8 100644 --- a/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java +++ b/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 @@ private void connectDownstream() { 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 void disconnected(DisconnectedEvent event) { @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 void disconnect(String reason) { * 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()) { - runnable.run(); + if (tickEventLoop.inEventLoop()) { + executeRunnable(runnable); return; } + executeInEventLoop(runnable); } @@ -1224,15 +1225,7 @@ public void ensureInEventLoop(Runnable runnable) { * Executes a task and prints a stack trace if an error occurs. */ public void executeInEventLoop(Runnable runnable) { - eventLoop.execute(() -> { - 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); - } - }); + tickEventLoop.execute(() -> executeRunnable(runnable)); } /** @@ -1241,19 +1234,25 @@ public void executeInEventLoop(Runnable runnable) { * The task will not run if the session is closed. */ public ScheduledFuture scheduleInEventLoop(Runnable runnable, long duration, TimeUnit timeUnit) { - return eventLoop.schedule(() -> { - try { + return tickEventLoop.schedule(() -> { + 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. */ diff --git a/core/src/main/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java index be0f8560faf..10d45658ea8 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java @@ -131,11 +131,8 @@ private static UUID getUUID(NbtMap profile) { session.getGeyser().getLogger().debug("Custom skull with invalid profile tag: " + blockPosition + " " + javaNbt); return; } - if (session.getEventLoop().inEventLoop()) { - putSkull(session, blockPosition, uuid, texturesProperty, blockState); - } else { - session.executeInEventLoop(() -> putSkull(session, blockPosition, uuid, texturesProperty, blockState)); - } + + session.ensureInEventLoop(() -> 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 diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/PacketTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/PacketTranslator.java index d49cdd6d08d..da5cd5cb0a0 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/PacketTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/PacketTranslator.java @@ -34,6 +34,7 @@ public abstract class PacketTranslator { /** * 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; diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockEmoteTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockEmoteTranslator.java index 7a37aa72e4a..07af4ddc4af 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockEmoteTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockEmoteTranslator.java @@ -61,11 +61,8 @@ public void translate(GeyserSession session, EmotePacket packet) { for (GeyserSession otherSession : session.getGeyser().getSessionManager().getSessions().values()) { if (otherSession != session) { if (otherSession.isClosed()) continue; - if (otherSession.getEventLoop().inEventLoop()) { - playEmote(otherSession, javaId, xuid, emote); - } else { - otherSession.executeInEventLoop(() -> playEmote(otherSession, javaId, xuid, emote)); - } + + otherSession.ensureInEventLoop(() -> playEmote(otherSession, javaId, xuid, emote)); } } } diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaCustomPayloadTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaCustomPayloadTranslator.java index 811449a99b1..c3108167b2f 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaCustomPayloadTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaCustomPayloadTranslator.java @@ -140,10 +140,4 @@ public void translate(GeyserSession session, ClientboundCustomPayloadPacket pack }); } } - - @Override - public boolean shouldExecuteInEventLoop() { - // For Erosion packets - return false; - } } diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaDisconnectTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaDisconnectTranslator.java index 2eb08fb92ae..0012390cbe9 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaDisconnectTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaDisconnectTranslator.java @@ -38,9 +38,4 @@ public class JavaDisconnectTranslator extends PacketTranslator { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 029e444a578..ebd378d572e 100644 --- a/gradle/libs.versions.toml +++ b/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"