Fixup state splitting

api/src/main/java/com/viaversion/viaversion/api/protocol/ProtocolPipeline.java

@@ -69,10 +69,15 @@ public interface ProtocolPipeline extends SimpleProtocol {
     /**
      * Returns the list of protocols this pipeline contains.
      *
-     * @return list of protocols in this pipe
+     * @return immutable list of protocols in this pipe
      */
     List<Protocol> pipes();
 
+    /**
+     * Returns the list of protocols this pipeline contains in reversed order.
+     *
+     * @return immutable list of protocols in reversed direction
+     */
     List<Protocol> reversedPipes();
 
     /**

common/src/main/java/com/viaversion/viaversion/protocol/ProtocolPipelineImpl.java

@@ -17,7 +17,6 @@
  */
 package com.viaversion.viaversion.protocol;
 
-import com.google.common.collect.Sets;
 import com.viaversion.viaversion.api.Via;
 import com.viaversion.viaversion.api.connection.UserConnection;
 import com.viaversion.viaversion.api.debug.DebugHandler;
@@ -31,9 +30,9 @@ import com.viaversion.viaversion.api.protocol.packet.State;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.logging.Level;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -43,9 +42,10 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
     /**
      * Protocol list ordered from client to server transforation with the base protocols at the end.
      */
-    private List<Protocol> protocolList;
+    private final List<Protocol> protocolList = new CopyOnWriteArrayList<>();
-    private List<Protocol> reversedProtocolList;
+    private final Set<Class<? extends Protocol>> protocolSet = new HashSet<>();
-    private Set<Class<? extends Protocol>> protocolSet;
+    private List<Protocol> reversedProtocolList = new CopyOnWriteArrayList<>();
+    private int baseProtocols;
 
     public ProtocolPipelineImpl(UserConnection userConnection) {
         this.userConnection = userConnection;
@@ -55,16 +55,12 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
 
     @Override
     protected void registerPackets() {
-        protocolList = new CopyOnWriteArrayList<>();
-        reversedProtocolList = new CopyOnWriteArrayList<>();
-        // Create a backing set for faster contains calls with larger pipes
-        protocolSet = Sets.newSetFromMap(new ConcurrentHashMap<>());
-
         // This is a pipeline so we register basic pipes
         final Protocol<?, ?, ?, ?> baseProtocol = Via.getManager().getProtocolManager().getBaseProtocol();
         protocolList.add(baseProtocol);
         reversedProtocolList.add(baseProtocol);
         protocolSet.add(baseProtocol.getClass());
+        baseProtocols++;
     }
 
     @Override
@@ -73,55 +69,38 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
     }
 
     @Override
-    public void add(Protocol protocol) {
+    public synchronized void add(final Protocol protocol) {
-        protocolList.add(protocol);
+        if (protocol.isBaseProtocol()) {
+            // Add base protocol on top of previous ones
+            protocolList.add(baseProtocols, protocol);
+            reversedProtocolList.add(baseProtocols, protocol);
+            baseProtocols++;
+        } else {
+            protocolList.add(protocol);
+            reversedProtocolList.add(0, protocol);
+        }
 
         protocolSet.add(protocol.getClass());
         protocol.init(userConnection);
-
-        if (!protocol.isBaseProtocol()) {
-            moveBaseProtocolsToTail(protocolList);
-        }
-
-        setReversedProtocolList();
-    }
-
-    private void setReversedProtocolList() {
-        final List<Protocol> reversedProtocolList = new ArrayList<>(protocolList);
-        Collections.reverse(this.reversedProtocolList);
-        moveBaseProtocolsToTail(reversedProtocolList);
-        this.reversedProtocolList = new CopyOnWriteArrayList<>(reversedProtocolList);
     }
 
     @Override
-    public void add(Collection<Protocol> protocols) {
+    public synchronized void add(final Collection<Protocol> protocols) {
         protocolList.addAll(protocols);
-        for (Protocol protocol : protocols) {
+        for (final Protocol protocol : protocols) {
             protocol.init(userConnection);
-            this.protocolSet.add(protocol.getClass());
+            protocolSet.add(protocol.getClass());
         }
 
-        moveBaseProtocolsToTail(protocolList);
+        refreshReversedList();
-        setReversedProtocolList();
     }
 
-    private List<Protocol> filterBaseProtocols(final List<Protocol> protocols) {
+    private synchronized void refreshReversedList() {
-        final List<Protocol> baseProtocols = new ArrayList<>();
+        final List<Protocol> protocols = new ArrayList<>(protocolList.subList(0, this.baseProtocols));
-        for (final Protocol protocol : protocolList) {
+        final List<Protocol> additionalProtocols = new ArrayList<>(protocolList.subList(this.baseProtocols, protocolList.size()));
-            if (protocol.isBaseProtocol()) {
+        Collections.reverse(additionalProtocols);
-                baseProtocols.add(protocol);
+        protocols.addAll(additionalProtocols);
-            }
+        reversedProtocolList = new CopyOnWriteArrayList<>(protocols);
-        }
-        return baseProtocols;
-    }
-
-    private void moveBaseProtocolsToTail(final List<Protocol> protocols) {
-        // Move base Protocols to the end, so the login packets can be modified by other protocols
-        final List<Protocol> baseProtocols = filterBaseProtocols(protocols);
-        if (!baseProtocols.isEmpty()) {
-            protocols.removeAll(baseProtocols);
-            protocols.addAll(baseProtocols);
-        }
     }
 
     @Override
@@ -134,7 +113,7 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
         }
 
         // Apply protocols
-        packetWrapper.apply(direction, state, 0, protocolListFor(direction), true);
+        packetWrapper.apply(direction, state, 0, protocolListFor(direction));
         super.transform(direction, state, packetWrapper);
 
         if (debugHandler.enabled() && debugHandler.logPostPacketTransform() && debugHandler.shouldLog(packetWrapper, direction)) {
@@ -143,7 +122,7 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
     }
 
     private List<Protocol> protocolListFor(final Direction direction) {
-        return direction == Direction.CLIENTBOUND ? reversedProtocolList : protocolList;
+        return Collections.unmodifiableList(direction == Direction.SERVERBOUND ? protocolList : reversedProtocolList);
     }
 
     private void logPacket(Direction direction, State state, PacketWrapper packetWrapper, int originalID) {
@@ -185,12 +164,12 @@ public class ProtocolPipelineImpl extends AbstractSimpleProtocol implements Prot
 
     @Override
     public List<Protocol> pipes() {
-        return protocolList;
+        return Collections.unmodifiableList(protocolList);
     }
 
     @Override
     public List<Protocol> reversedPipes() {
-        return reversedProtocolList;
+        return Collections.unmodifiableList(reversedProtocolList);
     }
 
     @Override

common/src/main/java/com/viaversion/viaversion/protocol/packet/PacketWrapperImpl.java

@@ -309,14 +309,18 @@ public class PacketWrapperImpl implements PacketWrapper {
      * @throws Exception if it fails to write
      */
     private ByteBuf constructPacket(Class<? extends Protocol> packetProtocol, boolean skipCurrentPipeline, Direction direction) throws Exception {
-        // Apply current pipeline - for outgoing protocol, the collection will be reversed in the apply method
         final ProtocolInfo protocolInfo = user().getProtocolInfo();
-        final boolean reverse = direction == Direction.CLIENTBOUND;
+        final List<Protocol> pipes = direction == Direction.SERVERBOUND ? protocolInfo.getPipeline().pipes() : protocolInfo.getPipeline().reversedPipes();
-        final List<Protocol> pipes = reverse ? protocolInfo.getPipeline().reversedPipes() : protocolInfo.getPipeline().pipes();
+        final List<Protocol> protocols = new ArrayList<>();
-        final Protocol[] protocols = pipes.toArray(PROTOCOL_ARRAY);
         int index = -1;
-        for (int i = 0; i < protocols.length; i++) {
+        for (int i = 0; i < pipes.size(); i++) {
-            if (protocols[i].getClass() == packetProtocol) {
+            // Always add base protocols to the head
+            final Protocol protocol = pipes.get(i);
+            if (protocol.isBaseProtocol()) {
+                protocols.add(protocol);
+            }
+
+            if (protocol.getClass() == packetProtocol) {
                 index = i;
                 break;
             }
@@ -328,14 +332,17 @@ public class PacketWrapperImpl implements PacketWrapper {
         }
 
         if (skipCurrentPipeline) {
-            index = reverse ? index - 1 : index + 1;
+            index = Math.min(index + 1, pipes.size());
         }
 
+        // Add remaining protocols on top
+        protocols.addAll(pipes.subList(index, pipes.size()));
+
         // Reset reader before we start
         resetReader();
 
         // Apply other protocols
-        apply(direction, protocolInfo.getState(direction), index, protocols, true);
+        apply(direction, protocolInfo.getState(direction), 0, protocols);
         final ByteBuf output = inputBuffer == null ? user().getChannel().alloc().buffer() : inputBuffer.alloc().buffer();
         try {
             writeToBuffer(output);

common/src/main/java/com/viaversion/viaversion/protocols/base/BaseProtocol1_7.java

@@ -119,9 +119,8 @@ public class BaseProtocol1_7 extends AbstractProtocol {
         // Login Success Packet
         registerClientbound(ClientboundLoginPackets.GAME_PROFILE, wrapper -> {
             ProtocolInfo info = wrapper.user().getProtocolInfo();
-            info.setServerState(State.PLAY);
+            if (info.getProtocolVersion() < ProtocolVersion.v1_20_2.getVersion()) { // On 1.20.2+, wait for the login ack
-            if (info.getProtocolVersion() < ProtocolVersion.v1_20_2.getVersion()) { // 1.20.2+ clients will send a login ack first
+                info.setState(State.PLAY);
-                info.setClientState(State.PLAY);
             }
 
             UUID uuid = passthroughLoginUUID(wrapper);
@@ -166,7 +165,7 @@ public class BaseProtocol1_7 extends AbstractProtocol {
 
         registerServerbound(ServerboundLoginPackets.LOGIN_ACKNOWLEDGED, wrapper -> {
             final ProtocolInfo info = wrapper.user().getProtocolInfo();
-            info.setClientState(State.CONFIGURATION);
+            info.setState(State.CONFIGURATION);
         });
     }
 

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_20_2to1_20/Protocol1_20_2To1_20.java

@@ -209,7 +209,7 @@ public final class Protocol1_20_2To1_20 extends AbstractProtocol<ClientboundPack
             // Map some of them to their configuration state counterparts, but make sure to let join game through
             final int unmappedId = packetWrapper.getId();
             if (unmappedId == ClientboundPackets1_19_4.JOIN_GAME.getId()) {
-                super.transform(direction, state, packetWrapper);
+                super.transform(direction, State.PLAY, packetWrapper);
                 return;
             }