Fix server play networking (#3648)

fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java

@@ -28,7 +28,6 @@ import net.minecraft.server.network.ServerCommonNetworkHandler;
 
 import net.fabricmc.fabric.impl.networking.NetworkHandlerExtensions;
 import net.fabricmc.fabric.impl.networking.server.ServerConfigurationNetworkAddon;
-import net.fabricmc.fabric.impl.networking.server.ServerPlayNetworkAddon;
 
 @Mixin(ServerCommonNetworkHandler.class)
 public abstract class ServerCommonNetworkHandlerMixin implements NetworkHandlerExtensions {
@@ -38,11 +37,10 @@ public abstract class ServerCommonNetworkHandlerMixin implements NetworkHandlerE
 
 		boolean handled;
 
-		if (getAddon() instanceof ServerPlayNetworkAddon addon) {
-			handled = addon.handle(payload);
-		} else if (getAddon() instanceof ServerConfigurationNetworkAddon addon) {
+		if (getAddon() instanceof ServerConfigurationNetworkAddon addon) {
 			handled = addon.handle(payload);
 		} else {
+			// Play should be handled in ServerPlayNetworkHandlerMixin
 			throw new IllegalStateException("Unknown addon");
 		}
 

fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerPlayNetworkHandlerMixin.java

@@ -24,6 +24,7 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 
 import net.minecraft.network.ClientConnection;
 import net.minecraft.network.packet.Packet;
+import net.minecraft.network.packet.c2s.common.CustomPayloadC2SPacket;
 import net.minecraft.network.packet.s2c.common.DisconnectS2CPacket;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.network.ConnectedClientData;
@@ -52,6 +53,13 @@ abstract class ServerPlayNetworkHandlerMixin extends ServerCommonNetworkHandler
 		this.addon.lateInit();
 	}
 
+	@Inject(method = "onCustomPayload", at = @At("HEAD"), cancellable = true)
+	private void handleCustomPayloadReceivedAsync(CustomPayloadC2SPacket packet, CallbackInfo ci) {
+		if (getAddon().handle(packet.payload())) {
+			ci.cancel();
+		}
+	}
+
 	@Override
 	public ServerPlayNetworkAddon getAddon() {
 		return this.addon;

fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/common/NetworkingCommonTest.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.test.networking.common;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.codec.PacketCodec;
+import net.minecraft.network.codec.PacketCodecs;
+import net.minecraft.network.packet.CustomPayload;
+import net.minecraft.server.MinecraftServer;
+import net.minecraft.server.network.ServerPlayerEntity;
+
+import net.fabricmc.api.ModInitializer;
+import net.fabricmc.fabric.api.event.lifecycle.v1.ServerEntityEvents;
+import net.fabricmc.fabric.api.networking.v1.PayloadTypeRegistry;
+import net.fabricmc.fabric.api.networking.v1.ServerConfigurationConnectionEvents;
+import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking;
+import net.fabricmc.fabric.api.networking.v1.ServerPlayConnectionEvents;
+import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking;
+import net.fabricmc.fabric.test.networking.NetworkingTestmods;
+
+public class NetworkingCommonTest implements ModInitializer {
+	private List<String> recievedPlay = new ArrayList<>();
+	private List<String> recievedConfig = new ArrayList<>();
+
+	@Override
+	public void onInitialize() {
+		// Register the payload on both sides for play and configuration
+		PayloadTypeRegistry.playS2C().register(CommonPayload.ID, CommonPayload.CODEC);
+		PayloadTypeRegistry.playC2S().register(CommonPayload.ID, CommonPayload.CODEC);
+		PayloadTypeRegistry.configurationS2C().register(CommonPayload.ID, CommonPayload.CODEC);
+		PayloadTypeRegistry.configurationC2S().register(CommonPayload.ID, CommonPayload.CODEC);
+
+		// When the client joins, send a packet expecting it to be echoed back
+		ServerPlayConnectionEvents.JOIN.register((handler, sender, server) -> sender.sendPacket(new CommonPayload("play")));
+		ServerConfigurationConnectionEvents.CONFIGURE.register((handler, server) -> ServerConfigurationNetworking.send(handler, new CommonPayload("configuration")));
+
+		// Store the player uuid once received from the client
+		ServerPlayNetworking.registerGlobalReceiver(CommonPayload.ID, (payload, context) -> recievedPlay.add(context.player().getUuidAsString()));
+		ServerConfigurationNetworking.registerGlobalReceiver(CommonPayload.ID, (payload, context) -> recievedConfig.add(context.networkHandler().getDebugProfile().getId().toString()));
+
+		// Ensure that the packets were received on the server
+		ServerEntityEvents.ENTITY_LOAD.register((entity, world) -> {
+			if (entity instanceof ServerPlayerEntity player) {
+				final String uuid = player.getUuidAsString();
+
+				// Allow a few ticks for the packets to be received
+				executeIn(world.getServer(), 50, () -> {
+					if (!recievedPlay.remove(uuid)) {
+						throw new IllegalStateException("Did not receive play response");
+					}
+
+					if (!recievedConfig.remove(uuid)) {
+						throw new IllegalStateException("Did not receive configuration response");
+					}
+				});;
+			}
+		});
+	}
+
+	// A payload registered on both sides, for play and configuration
+	// This tests that the server can send a packet to the client, and then recieve a response from the client
+	public record CommonPayload(String data) implements CustomPayload {
+		public static final CustomPayload.Id<CommonPayload> ID = new Id<>(NetworkingTestmods.id("common_payload"));
+		public static final PacketCodec<PacketByteBuf, CommonPayload> CODEC = PacketCodecs.STRING.xmap(CommonPayload::new, CommonPayload::data).cast();
+
+		@Override
+		public Id<? extends CustomPayload> getId() {
+			return ID;
+		}
+	}
+
+	private static void executeIn(MinecraftServer server, int ticks, Runnable runnable) {
+		int targetTime = server.getTicks() + ticks;
+		server.execute(new Runnable() {
+			@Override
+			public void run() {
+				if (server.getTicks() >= targetTime) {
+					runnable.run();
+					return;
+				}
+
+				server.execute(this);
+			}
+		});
+	}
+}

fabric-networking-api-v1/src/testmod/resources/fabric.mod.json

@@ -11,6 +11,7 @@
   "entrypoints": {
     "main": [
       "net.fabricmc.fabric.test.networking.channeltest.NetworkingChannelTest",
+      "net.fabricmc.fabric.test.networking.common.NetworkingCommonTest",
       "net.fabricmc.fabric.test.networking.configuration.NetworkingConfigurationTest",
       "net.fabricmc.fabric.test.networking.keybindreciever.NetworkingKeybindPacketTest",
       "net.fabricmc.fabric.test.networking.login.NetworkingLoginQueryTest",
@@ -18,6 +19,7 @@
     ],
     "client": [
       "net.fabricmc.fabric.test.networking.client.channeltest.NetworkingChannelClientTest",
+      "net.fabricmc.fabric.test.networking.client.common.NetworkingCommonClientTest",
       "net.fabricmc.fabric.test.networking.client.configuration.NetworkingConfigurationClientTest",
       "net.fabricmc.fabric.test.networking.client.DisconnectScreenTest",
       "net.fabricmc.fabric.test.networking.client.keybindreciever.NetworkingKeybindClientPacketTest",

fabric-networking-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/networking/client/common/NetworkingCommonClientTest.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.test.networking.client.common;
+
+import net.fabricmc.api.ClientModInitializer;
+import net.fabricmc.fabric.api.client.networking.v1.ClientConfigurationNetworking;
+import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking;
+import net.fabricmc.fabric.test.networking.common.NetworkingCommonTest;
+
+public class NetworkingCommonClientTest implements ClientModInitializer {
+	@Override
+	public void onInitializeClient() {
+		ClientPlayNetworking.registerGlobalReceiver(NetworkingCommonTest.CommonPayload.ID, (payload, context) -> {
+			// Echo the payload back
+			context.responseSender().sendPacket(payload);
+		});
+
+		ClientConfigurationNetworking.registerGlobalReceiver(NetworkingCommonTest.CommonPayload.ID, (payload, context) -> {
+			// Echo the payload back
+			context.responseSender().sendPacket(payload);
+		});
+	}
+}