From 00df4c26ba13fc8ddf0c92500b414061b592dae4 Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Tue, 3 Jan 2023 15:51:58 -0500 Subject: [PATCH] Don't send more than one ServerboundSwingPacket per tick Should address #2875 --- .../geyser/session/GeyserSession.java | 1 - .../bedrock/BedrockAnimateTranslator.java | 21 ++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) 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 72697a85c..a95706307 100644 --- a/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java +++ b/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java @@ -457,7 +457,6 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource { * Counts how many ticks have occurred since an arm animation started. * -1 means there is no active arm swing. */ - @Getter(AccessLevel.NONE) private int armAnimationTicks = -1; /** diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockAnimateTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockAnimateTranslator.java index 670e55785..4905b5647 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockAnimateTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockAnimateTranslator.java @@ -49,11 +49,26 @@ public class BedrockAnimateTranslator extends PacketTranslator { case SWING_ARM -> // Delay so entity damage can be processed first session.scheduleInEventLoop(() -> { + if (session.getArmAnimationTicks() != 0) { + // So, generally, a Java player can only do one *thing* at a time. + // If a player right-clicks, for example, then there's probably only one action associated with + // that right-click that will send a swing. + // The only exception I can think of to this, *maybe*, is a player dropping items + // Bedrock is a little funkier than this - it can send several arm animation packets in the + // same tick, notably with high levels of haste applied. + // Packet limiters do not like this and can crash the player. + // If arm animation ticks is 0, then we just sent an arm swing packet this tick. + // See https://github.com/GeyserMC/Geyser/issues/2875 + // This behavior was last touched on with ViaVersion 4.5.1 (with its packet limiter), Java 1.16.5, + // and Bedrock 1.19.51. + // Note for the future: we should probably largely ignore this packet and instead replicate + // all actions on our end, and send swings where needed. session.sendDownstreamPacket(new ServerboundSwingPacket(Hand.MAIN_HAND)); session.activateArmAnimationTicking(); - }, - 25, - TimeUnit.MILLISECONDS + } + }, + 25, + TimeUnit.MILLISECONDS ); // These two might need to be flipped, but my recommendation is getting moving working first case ROW_LEFT -> {