From 3997ea70f71e9d983a6ebf626d5a06f644dfce65 Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Mon, 25 Sep 2023 18:59:15 +1000 Subject: [PATCH] Fixup state splitting --- .../api/protocol/ProtocolPipeline.java | 7 +- .../protocol/ProtocolPipelineImpl.java | 81 +++++++------------ .../protocol/packet/PacketWrapperImpl.java | 23 ++++-- .../protocols/base/BaseProtocol1_7.java | 7 +- .../Protocol1_20_2To1_20.java | 2 +- 5 files changed, 55 insertions(+), 65 deletions(-) diff --git a/api/src/main/java/com/viaversion/viaversion/api/protocol/ProtocolPipeline.java b/api/src/main/java/com/viaversion/viaversion/api/protocol/ProtocolPipeline.java index bd43a9fcc..922dc8f8c 100644 --- a/api/src/main/java/com/viaversion/viaversion/api/protocol/ProtocolPipeline.java +++ b/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 pipes(); + /** + * Returns the list of protocols this pipeline contains in reversed order. + * + * @return immutable list of protocols in reversed direction + */ List reversedPipes(); /** diff --git a/common/src/main/java/com/viaversion/viaversion/protocol/ProtocolPipelineImpl.java b/common/src/main/java/com/viaversion/viaversion/protocol/ProtocolPipelineImpl.java index de4884920..91fedc515 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocol/ProtocolPipelineImpl.java +++ b/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 protocolList; - private List reversedProtocolList; - private Set> protocolSet; + private final List protocolList = new CopyOnWriteArrayList<>(); + private final Set> protocolSet = new HashSet<>(); + private List 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) { - protocolList.add(protocol); + public synchronized void add(final Protocol 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 reversedProtocolList = new ArrayList<>(protocolList); - Collections.reverse(this.reversedProtocolList); - moveBaseProtocolsToTail(reversedProtocolList); - this.reversedProtocolList = new CopyOnWriteArrayList<>(reversedProtocolList); } @Override - public void add(Collection protocols) { + public synchronized void add(final Collection 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); - setReversedProtocolList(); + refreshReversedList(); } - private List filterBaseProtocols(final List protocols) { - final List baseProtocols = new ArrayList<>(); - for (final Protocol protocol : protocolList) { - if (protocol.isBaseProtocol()) { - baseProtocols.add(protocol); - } - } - return baseProtocols; - } - - private void moveBaseProtocolsToTail(final List protocols) { - // Move base Protocols to the end, so the login packets can be modified by other protocols - final List baseProtocols = filterBaseProtocols(protocols); - if (!baseProtocols.isEmpty()) { - protocols.removeAll(baseProtocols); - protocols.addAll(baseProtocols); - } + private synchronized void refreshReversedList() { + final List protocols = new ArrayList<>(protocolList.subList(0, this.baseProtocols)); + final List additionalProtocols = new ArrayList<>(protocolList.subList(this.baseProtocols, protocolList.size())); + Collections.reverse(additionalProtocols); + protocols.addAll(additionalProtocols); + reversedProtocolList = new CopyOnWriteArrayList<>(protocols); } @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 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 pipes() { - return protocolList; + return Collections.unmodifiableList(protocolList); } @Override public List reversedPipes() { - return reversedProtocolList; + return Collections.unmodifiableList(reversedProtocolList); } @Override diff --git a/common/src/main/java/com/viaversion/viaversion/protocol/packet/PacketWrapperImpl.java b/common/src/main/java/com/viaversion/viaversion/protocol/packet/PacketWrapperImpl.java index 7af754c8f..6437b3fa4 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocol/packet/PacketWrapperImpl.java +++ b/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 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 pipes = reverse ? protocolInfo.getPipeline().reversedPipes() : protocolInfo.getPipeline().pipes(); - final Protocol[] protocols = pipes.toArray(PROTOCOL_ARRAY); + final List pipes = direction == Direction.SERVERBOUND ? protocolInfo.getPipeline().pipes() : protocolInfo.getPipeline().reversedPipes(); + final List protocols = new ArrayList<>(); int index = -1; - for (int i = 0; i < protocols.length; i++) { - if (protocols[i].getClass() == packetProtocol) { + for (int i = 0; i < pipes.size(); i++) { + // 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); diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/base/BaseProtocol1_7.java b/common/src/main/java/com/viaversion/viaversion/protocols/base/BaseProtocol1_7.java index a689c89e9..7c3cde8bd 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/base/BaseProtocol1_7.java +++ b/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()) { // 1.20.2+ clients will send a login ack first - info.setClientState(State.PLAY); + if (info.getProtocolVersion() < ProtocolVersion.v1_20_2.getVersion()) { // On 1.20.2+, wait for the login ack + info.setState(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); }); } diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/protocol1_20_2to1_20/Protocol1_20_2To1_20.java b/common/src/main/java/com/viaversion/viaversion/protocols/protocol1_20_2to1_20/Protocol1_20_2To1_20.java index c5d7c04e2..3641af07b 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/protocol1_20_2to1_20/Protocol1_20_2To1_20.java +++ b/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