Fix vanilla packet splitter remote detection and add advancement packet splitting (#7802)

src/main/java/net/minecraftforge/fml/network/NetworkHooks.java

@@ -52,6 +52,7 @@ import net.minecraft.util.math.BlockPos;
 import net.minecraftforge.common.MinecraftForge;
 import net.minecraftforge.event.entity.player.PlayerContainerEvent;
 import net.minecraftforge.fml.config.ConfigTracker;
+import net.minecraftforge.network.NetworkFilters;
 
 import javax.annotation.Nullable;
 
@@ -117,6 +118,7 @@ public class NetworkHooks
     }
 
     public synchronized static void sendMCRegistryPackets(NetworkManager manager, String direction) {
+        NetworkFilters.injectIfNecessary(manager);
         final Set<ResourceLocation> resourceLocations = NetworkRegistry.buildChannelVersions().keySet().stream().
                 filter(rl -> !Objects.equals(rl.getNamespace(), "minecraft")).
                 collect(Collectors.toSet());

src/main/java/net/minecraftforge/fml/server/ServerLifecycleHooks.java

@@ -68,7 +68,6 @@ import net.minecraftforge.fml.loading.moddiscovery.ModFile;
 import net.minecraftforge.fml.packs.ModFileResourcePack;
 import net.minecraftforge.fml.packs.ResourcePackLoader;
 import net.minecraftforge.forgespi.language.IModInfo;
-import net.minecraftforge.network.NetworkFilters;
 import net.minecraftforge.registries.GameData;
 
 public class ServerLifecycleHooks
@@ -169,7 +168,6 @@ public class ServerLifecycleHooks
         if (packet.getIntention() == ProtocolType.STATUS) return true;
 
         NetworkHooks.registerServerLoginChannel(manager, packet);
-        NetworkFilters.injectIfNecessary(manager);
         return true;
 
     }

src/main/java/net/minecraftforge/network/ForgeConnectionNetworkFilter.java

@@ -20,15 +20,19 @@
 package net.minecraftforge.network;
 
 import java.util.List;
+import java.util.Map;
+import java.util.function.BiConsumer;
+
+import javax.annotation.Nullable;
 
 import io.netty.channel.ChannelHandler;
 import net.minecraft.network.IPacket;
 import net.minecraft.network.NetworkManager;
 import net.minecraft.network.PacketDirection;
 import net.minecraft.network.ProtocolType;
+import net.minecraft.network.play.server.SAdvancementInfoPacket;
 import net.minecraft.network.play.server.STagsListPacket;
 import net.minecraft.network.play.server.SUpdateRecipesPacket;
-import net.minecraftforge.fml.network.NetworkHooks;
 
 import com.google.common.collect.ImmutableMap;
 
@@ -39,21 +43,41 @@ import com.google.common.collect.ImmutableMap;
 public class ForgeConnectionNetworkFilter extends VanillaPacketFilter
 {
 
+    @Deprecated
     public ForgeConnectionNetworkFilter()
     {
-        super(
-                ImmutableMap.of(
-                        SUpdateRecipesPacket.class, ForgeConnectionNetworkFilter::splitPacket,
-                        STagsListPacket.class, ForgeConnectionNetworkFilter::splitPacket
-                )
-        );
+        this(null);
+    }
+
+    public ForgeConnectionNetworkFilter(@Nullable NetworkManager manager)
+    {
+        super(buildHandlers(manager));
+    }
+
+    private static Map<Class<? extends IPacket<?>>, BiConsumer<IPacket<?>, List<? super IPacket<?>>>> buildHandlers(@Nullable NetworkManager manager)
+    {
+
+        VanillaPacketSplitter.RemoteCompatibility compatibility = manager == null ? VanillaPacketSplitter.RemoteCompatibility.ABSENT : VanillaPacketSplitter.getRemoteCompatibility(manager);
+        if (compatibility == VanillaPacketSplitter.RemoteCompatibility.ABSENT)
+        {
+            return ImmutableMap.of();
+        }
+        ImmutableMap.Builder<Class<? extends IPacket<?>>, BiConsumer<IPacket<?>, List<? super IPacket<?>>>> builder = ImmutableMap.<Class<? extends IPacket<?>>, BiConsumer<IPacket<?>, List<? super IPacket<?>>>>builder()
+                .put(SUpdateRecipesPacket.class, ForgeConnectionNetworkFilter::splitPacket)
+                .put(STagsListPacket.class, ForgeConnectionNetworkFilter::splitPacket);
+
+        if (compatibility == VanillaPacketSplitter.RemoteCompatibility.V11)
+        {
+            builder.put(SAdvancementInfoPacket.class, ForgeConnectionNetworkFilter::splitPacket);
+        }
+        return builder.build();
     }
 
     @Override
     protected boolean isNecessary(NetworkManager manager)
     {
         // not needed on local connections, because packets are not encoded to bytes there
-        return !manager.isMemoryConnection() && !VanillaPacketSplitter.isRemoteCompatible(manager);
+        return !manager.isMemoryConnection() && VanillaPacketSplitter.isRemoteCompatible(manager);
     }
 
     private static void splitPacket(IPacket<?> packet, List<? super IPacket<?>> out)

src/main/java/net/minecraftforge/network/NetworkFilters.java

@@ -20,6 +20,7 @@
 package net.minecraftforge.network;
 
 import java.util.Map;
+import java.util.function.Function;
 
 import net.minecraft.network.NetworkManager;
 
@@ -33,14 +34,15 @@ public class NetworkFilters
 
     private static final Logger LOGGER = LogManager.getLogger();
 
-    private static final Map<String, VanillaPacketFilter> instances = ImmutableMap.of(
-            "forge:vanilla_filter", new VanillaConnectionNetworkFilter(),
-            "forge:forge_fixes", new ForgeConnectionNetworkFilter()
+    private static final Map<String, Function<NetworkManager, VanillaPacketFilter>> instances = ImmutableMap.of(
+            "forge:vanilla_filter", manager -> new VanillaConnectionNetworkFilter(),
+            "forge:forge_fixes", ForgeConnectionNetworkFilter::new
     );
 
     public static void injectIfNecessary(NetworkManager manager)
     {
-        instances.forEach((key, filter) -> {
+        instances.forEach((key, filterFactory) -> {
+            VanillaPacketFilter filter = filterFactory.apply(manager);
             if (filter.isNecessary(manager))
             {
                 manager.channel().pipeline().addBefore("packet_handler", key, filter);

src/main/java/net/minecraftforge/network/VanillaPacketSplitter.java

@@ -32,9 +32,8 @@ import net.minecraft.client.Minecraft;
 import net.minecraft.client.network.play.ClientPlayNetHandler;
 import net.minecraft.network.*;
 import net.minecraft.network.play.server.SCustomPayloadPlayPacket;
-import net.minecraft.network.play.server.STagsListPacket;
-import net.minecraft.network.play.server.SUpdateRecipesPacket;
 import net.minecraft.util.ResourceLocation;
+import net.minecraftforge.fml.network.FMLConnectionData;
 import net.minecraftforge.fml.network.NetworkDirection;
 import net.minecraftforge.fml.network.NetworkEvent;
 import net.minecraftforge.fml.network.NetworkHooks;
@@ -56,12 +55,13 @@ public class VanillaPacketSplitter
     private static final Logger LOGGER = LogManager.getLogger();
 
     private static final ResourceLocation CHANNEL = new ResourceLocation("forge", "split");
+    // TODO 1.17: bump this to version 1.1 and remove dummy channel
+    // Version 1.1 removed client-side whitelist
+    // we don't bump actual channel version because that would prevent old clients from connecting
+    // but we still need to detect new clients, so we add a dummy channel just for that.
+    private static final ResourceLocation V11_DUMMY_CHANNEL = new ResourceLocation("forge", "split_11");
     private static final String VERSION = "1.0";
-
-    private static final Set<Class<? extends IPacket<?>>> ALLOWED_PACKETS = ImmutableSet.of(
-            SUpdateRecipesPacket.class,
-            STagsListPacket.class
-    );
+    private static final String VERSION_11 = "1.1";
 
     private static final int PROTOCOL_MAX = 2097152;
 
@@ -72,13 +72,14 @@ public class VanillaPacketSplitter
     private static final byte STATE_FIRST = 1;
     private static final byte STATE_LAST = 2;
 
-    private static EventNetworkChannel channel;
-
     public static void register()
     {
         Predicate<String> versionCheck = NetworkRegistry.acceptMissingOr(VERSION);
-        channel = NetworkRegistry.newEventChannel(CHANNEL, () -> VERSION, versionCheck, versionCheck);
+        EventNetworkChannel channel = NetworkRegistry.newEventChannel(CHANNEL, () -> VERSION, versionCheck, versionCheck);
         channel.addListener(VanillaPacketSplitter::onClientPacket);
+
+        Predicate<String> version11Check = NetworkRegistry.acceptMissingOr(VERSION_11);
+        NetworkRegistry.newEventChannel(V11_DUMMY_CHANNEL, () -> VERSION_11, version11Check, version11Check);
     }
 
     /**
@@ -184,10 +185,6 @@ public class VanillaPacketSplitter
             {
                 LOGGER.error("Received invalid packet ID {} in forge:split", packetId);
             }
-            else if (!ALLOWED_PACKETS.contains(packet.getClass()))
-            {
-                LOGGER.error("Received not allowed packet type {} in forge:split", packet);
-            }
             else
             {
                 try
@@ -206,8 +203,36 @@ public class VanillaPacketSplitter
         }
     }
 
+    public enum RemoteCompatibility
+    {
+        ABSENT,
+        V10_LEGACY,
+        V11
+    }
+
+    public static RemoteCompatibility getRemoteCompatibility(NetworkManager manager)
+    {
+        FMLConnectionData connectionData = NetworkHooks.getConnectionData(manager);
+        if (connectionData == null)
+        {
+            return RemoteCompatibility.ABSENT;
+        }
+        else if (connectionData.getChannels().containsKey(V11_DUMMY_CHANNEL))
+        {
+            return RemoteCompatibility.V11;
+        }
+        else if (connectionData.getChannels().containsKey(CHANNEL))
+        {
+            return RemoteCompatibility.V10_LEGACY;
+        }
+        else
+        {
+            return RemoteCompatibility.ABSENT;
+        }
+    }
+
     public static boolean isRemoteCompatible(NetworkManager manager)
     {
-        return !NetworkHooks.isVanillaConnection(manager) && channel.isRemotePresent(manager);
+        return getRemoteCompatibility(manager) != RemoteCompatibility.ABSENT;
     }
 }