Improve 1.12 keep_alive handling

Fixes handling of batches of keep alives and the technically possible collisions

Author: kennytv

common/src/main/java/com/viaversion/viabackwards/protocol/v1_12_2to1_12_1/Protocol1_12_2To1_12_1.java

@@ -21,7 +21,6 @@
 import com.viaversion.viabackwards.api.BackwardsProtocol;
 import com.viaversion.viabackwards.protocol.v1_12_2to1_12_1.storage.KeepAliveTracker;
 import com.viaversion.viaversion.api.connection.UserConnection;
-import com.viaversion.viaversion.api.protocol.remapper.PacketHandlers;
 import com.viaversion.viaversion.api.type.Types;
 import com.viaversion.viaversion.protocols.v1_12to1_12_1.packet.ClientboundPackets1_12_1;
 import com.viaversion.viaversion.protocols.v1_12to1_12_1.packet.ServerboundPackets1_12_1;
@@ -34,32 +33,20 @@ public Protocol1_12_2To1_12_1() {
 
     @Override
     protected void registerPackets() {
-        registerClientbound(ClientboundPackets1_12_1.KEEP_ALIVE, new PacketHandlers() {
-            @Override
-            public void register() {
-                handler(packetWrapper -> {
-                    Long keepAlive = packetWrapper.read(Types.LONG);
-                    packetWrapper.user().get(KeepAliveTracker.class).setKeepAlive(keepAlive);
-                    packetWrapper.write(Types.VAR_INT, keepAlive.hashCode());
-                });
-            }
+        registerClientbound(ClientboundPackets1_12_1.KEEP_ALIVE, wrapper -> {
+            long keepAlive = wrapper.read(Types.LONG);
+            int id = wrapper.user().get(KeepAliveTracker.class).track(keepAlive);
+            wrapper.write(Types.VAR_INT, id);
         });
 
-        registerServerbound(ServerboundPackets1_12_1.KEEP_ALIVE, new PacketHandlers() {
-            @Override
-            public void register() {
-                handler(packetWrapper -> {
-                    int keepAlive = packetWrapper.read(Types.VAR_INT);
-                    long realKeepAlive = packetWrapper.user().get(KeepAliveTracker.class).getKeepAlive();
-                    if (keepAlive != Long.hashCode(realKeepAlive)) {
-                        packetWrapper.cancel(); // Wrong data, cancel packet
-                        return;
-                    }
-                    packetWrapper.write(Types.LONG, realKeepAlive);
-                    // Reset KeepAliveTracker (to prevent sending same valid value in a row causing a timeout)
-                    packetWrapper.user().get(KeepAliveTracker.class).setKeepAlive(Integer.MAX_VALUE);
-                });
+        registerServerbound(ServerboundPackets1_12_1.KEEP_ALIVE, wrapper -> {
+            int keepAlive = wrapper.read(Types.VAR_INT);
+            Long realKeepAlive = wrapper.user().get(KeepAliveTracker.class).consume(keepAlive);
+            if (realKeepAlive == null) {
+                wrapper.cancel(); // stale/bad
+                return;
             }
+            wrapper.write(Types.LONG, realKeepAlive);
         });
     }
 

common/src/main/java/com/viaversion/viabackwards/protocol/v1_12_2to1_12_1/storage/KeepAliveTracker.java

@@ -19,20 +19,22 @@
 package com.viaversion.viabackwards.protocol.v1_12_2to1_12_1.storage;
 
 import com.viaversion.viaversion.api.connection.StorableObject;
+import java.util.HashMap;
+import java.util.Map;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class KeepAliveTracker implements StorableObject {
-    private long keepAlive = Integer.MAX_VALUE;
 
-    public long getKeepAlive() {
-        return keepAlive;
-    }
+    private final Map<Integer, Long> pending = new HashMap<>();
+    private int nextId;
 
-    public void setKeepAlive(long keepAlive) {
-        this.keepAlive = keepAlive;
+    public int track(long original) {
+        int id = nextId++;
+        pending.put(id, original);
+        return id;
     }
 
-    @Override
-    public String toString() {
-        return "KeepAliveTracker{" + "keepAlive=" + keepAlive + '}';
+    public @Nullable Long consume(int id) {
+        return pending.remove(id);
     }
 }