Fix OldCombatMechanics integration

common/src/main/java/me/caseload/knockbacksync/listener/packetevents/PingReceiveListener.java

@@ -33,32 +33,41 @@ public class PingReceiveListener extends PacketListenerAbstract {
         PlayerData playerData = PlayerDataManager.getPlayerData(user);
         if (playerData == null) return;
 
-        if (playerData.pingStrategy == PingStrategy.KEEPALIVE && packetType == PacketType.Play.Client.KEEP_ALIVE) {
+        // Cancel echoes of injected packets before any strategy check: on 1.20.2+ an unexpected keepalive is an instant "Timed out" kick, so a strategy switch mid-flight must not leak one.
+        if (packetType == PacketType.Play.Client.KEEP_ALIVE) {
             WrapperPlayClientKeepAlive keepAlive = new WrapperPlayClientKeepAlive(event);
             long receivedId = keepAlive.getId();
 
-            handlePingCalculationPackets(event, playerData, receivedId, playerData.keepaliveMap);
-        } else if (playerData.pingStrategy == PingStrategy.TRANSACTION && packetType == PacketType.Play.Client.PONG) {
+            if (playerData.didWeSendThatPacket(receivedId))
+                event.setCancelled(true);
+
+            if (playerData.pingStrategy == PingStrategy.KEEPALIVE)
+                handlePingCalculationPackets(playerData, receivedId, playerData.keepaliveMap);
+        } else if (packetType == PacketType.Play.Client.PONG) {
             WrapperPlayClientPong pong = new WrapperPlayClientPong(event);
             int id = pong.getId();
 
-            handlePingCalculationPackets(event, playerData, id, playerData.transactionsSent);
-        } else if (playerData.pingStrategy == PingStrategy.TRANSACTION && packetType == PacketType.Play.Client.WINDOW_CONFIRMATION) {
+            if (playerData.didWeSendThatPacket(id))
+                event.setCancelled(true);
+
+            if (playerData.pingStrategy == PingStrategy.TRANSACTION)
+                handlePingCalculationPackets(playerData, id, playerData.transactionsSent);
+        } else if (packetType == PacketType.Play.Client.WINDOW_CONFIRMATION) {
             WrapperPlayClientWindowConfirmation windowConfirmation = new WrapperPlayClientWindowConfirmation(event);
             int id = windowConfirmation.getActionId();
 
-            handlePingCalculationPackets(event, playerData, id, playerData.transactionsSent);
+            if (playerData.didWeSendThatPacket(id))
+                event.setCancelled(true);
+
+            if (playerData.pingStrategy == PingStrategy.TRANSACTION)
+                handlePingCalculationPackets(playerData, id, playerData.transactionsSent);
         }
     }
 
-    private <T extends Number> void handlePingCalculationPackets(PacketReceiveEvent event, PlayerData playerData, long id, Queue<Pair<T, Long>> packetSentList) {
+    private <T extends Number> void handlePingCalculationPackets(PlayerData playerData, long id, Queue<Pair<T, Long>> packetSentList) {
 //        System.out.println("Received response ID: " + id + " Queue size before: " + packetSentList.size());
 //        System.out.println("Current queue contents: " + packetSentList.toString());
 
-        if (playerData.didWeSendThatPacket(id)) {
-            event.setCancelled(true);
-        }
-
         if (!Base.INSTANCE.getConfigManager().isToggled()) return;
 
         Pair<T, Long> data = null;

common/src/main/java/me/caseload/knockbacksync/manager/ConfigManager.java

@@ -19,7 +19,7 @@ import java.util.Map;
 @Setter
 public class ConfigManager {
 
-    public static final long CONFIG_VERSION = 8;
+    public static final long CONFIG_VERSION = 9;
 
     private boolean toggled;
     private boolean runnableEnabled;
@@ -103,11 +103,11 @@ public class ConfigManager {
 
         toggled = configWrapper.getBoolean("enabled", true);
 
-        // Checks to see if the runnable was enabled...
-        // and if we now want to disable it
         boolean newRunnableEnabled = configWrapper.getBoolean("runnable.enabled", true);
-        if (runnableEnabled && newRunnableEnabled && pingTask != null) { // null check for first startup
+        // Always stop the previous ping task on reload (reschedule below only if still enabled); gating the cancel on staying enabled leaked the task when disabling via reload.
+        if (pingTask != null) { // null on first startup
             pingTask.cancel();
+            pingTask = null;
         }
 
         runnableEnabled = newRunnableEnabled;
@@ -134,7 +134,7 @@ public class ConfigManager {
 
         notifyUpdate = configWrapper.getBoolean("notify_updates", true);
         autoUpdate = configWrapper.getBoolean("auto_update", true);
-        combatTimer = configWrapper.getLong("runnable.timer", 30L);
+        combatTimer = configWrapper.getLong("runnable.combat_timer", 30L);
         spikeThreshold = configWrapper.getLong("spike_threshold", 20L);
         enableMessage = configWrapper.getString("messages.toggle.global.enable", "&aSuccessfully enabled KnockbackSync.");
         disableMessage = configWrapper.getString("messages.toggle.global.disable", "&cSuccessfully disabled KnockbackSync.");

common/src/main/java/me/caseload/knockbacksync/player/PlayerData.java

@@ -221,12 +221,13 @@ public class PlayerData {
                    packet = new WrapperPlayServerWindowConfirmation((byte) 0, pingTransactionID, false);
                }
 
+               // sendPacket (write + flush) rather than writePacket: an unflushed write sits in the channel until the next flush and inflates the measured ping.
                if (async) {
                    ChannelHelper.runInEventLoop(user.getChannel(), () -> {
-                       user.writePacket(packet);
+                       user.sendPacket(packet);
                    });
                } else {
-                   user.writePacket(packet);
+                   user.sendPacket(packet);
                }
                break;
        }
@@ -416,7 +417,8 @@ public class PlayerData {
     }
 
     private PingStrategy loadPingStrategy(ConfigManager  configManager) {
-        String pingStrategy = configManager.getConfigWrapper().getString("ping_strategy", "KEEPALIVE");
+        // Default PING: vanilla ignores unintercepted pong echoes, while unintercepted keepalive echoes kick ("Timed out") on 1.20.2+.
+        String pingStrategy = configManager.getConfigWrapper().getString("ping_strategy", "PING");
         switch (pingStrategy) {
             case "KEEPALIVE":
                 return PingStrategy.KEEPALIVE;

common/src/main/resources/config.yml

@@ -3,7 +3,7 @@
 #########################################
 
 # Do not modify!
-config_version: 8
+config_version: 9
 
 # Plugin enabled state
 # Toggleable using /knockbacksync toggle
@@ -30,17 +30,8 @@ runnable:
 # the threshold, the previous ping value is used to avoid calculation inaccuracies.
 spike_threshold: 20
 
-# OPTIONS: KEEPALIVE, PING, TRANSACTION - PING and TRANSACTION are aliases for each other and are effectively the same
-# The packet type used to measure ping by the runnable.
-# KEEPALIVE is the most accurate however sending too many of them too often (such as using a runnable with a low interval)
-# - May disconnect players, especially laggy ones if sent too often
-# - The default interval of 20 ticks or 1 second
-#    means it is in theory possible for a player to have a ping spike in-between two sword attacks
-#    and have it immediately spike back down, slightly throwing off knockback synchronization
-#    in practice this is very rare and the extra ping accuracy and precision is more valued
-# PING/TRANSACTION can be sent more often (and therefore better detect ping spikes) but are less precise
-# ping measured in ms with PING/TRANSACTION is typically within ± 5ms of actual but can be up to 50ms off
-ping_strategy: "KEEPALIVE"
+# OPTIONS: PING, TRANSACTION (aliases), KEEPALIVE. PING is safe (vanilla ignores unexpected pongs, ViaVersion translates for 1.8-1.16, ± 5ms); KEEPALIVE is slightly more precise but any unintercepted echo kicks ("Timed out") on 1.20.2+ and anticheat NET checks may kick for the extra traffic.
+ping_strategy: "PING"
 
 # Whether to try to detect when player is offground and apply less kb
 enable_offground_synchronization: true