From bd733d6b9fae6c855666426884ec6a9b20176bdb Mon Sep 17 00:00:00 2001 From: deirn Date: Sun, 26 Nov 2023 20:08:12 +0700 Subject: [PATCH] Avoid serializing object-based payload into `PacketByteBuf` on the main thread (#3407) * channeled network addon refactor * checkstyle * fix junit tests * convert TypedPayload <-> UntypedPayload if necessary * assert payload size * add vm arg to force serialization * change log level to info and make it single line (cherry picked from commit 6225d43a705d1c1cf5589e25b8e127fe3f241c91) --- .../v1/ClientConfigurationNetworking.java | 126 +++++++++--------- .../networking/v1/ClientPlayNetworking.java | 123 +++++++++-------- .../ClientConfigurationNetworkAddon.java | 29 ++-- .../client/ClientNetworkingImpl.java | 20 +-- .../client/ClientPlayNetworkAddon.java | 23 ++-- .../ClientCommonNetworkHandlerMixin.java | 12 +- .../v1/ServerConfigurationNetworking.java | 83 ++++++------ .../networking/v1/ServerPlayNetworking.java | 116 ++++++++-------- .../AbstractChanneledNetworkAddon.java | 28 ++-- .../networking/GlobalReceiverRegistry.java | 1 + .../impl/networking/NetworkingImpl.java | 19 +++ .../networking/payload/ResolvablePayload.java | 40 ++++++ ...teBufPayload.java => ResolvedPayload.java} | 10 +- .../networking/payload/RetainedPayload.java | 54 ++++++++ .../impl/networking/payload/TypedPayload.java | 49 +++++++ .../networking/payload/UntypedPayload.java | 47 +++++++ .../ServerConfigurationNetworkAddon.java | 29 ++-- .../server/ServerNetworkingImpl.java | 22 +-- .../server/ServerPlayNetworkAddon.java | 24 ++-- .../CustomPayloadC2SPacketMixin.java | 13 +- .../CustomPayloadS2CPacketMixin.java | 13 +- .../ServerCommonNetworkHandlerMixin.java | 10 +- .../networking/unit/CommonPacketTests.java | 36 ++--- 23 files changed, 565 insertions(+), 362 deletions(-) create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvablePayload.java rename fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/{PacketByteBufPayload.java => ResolvedPayload.java} (67%) create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/RetainedPayload.java create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/TypedPayload.java create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/UntypedPayload.java diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientConfigurationNetworking.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientConfigurationNetworking.java index 455771367..fa78e9ff3 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientConfigurationNetworking.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientConfigurationNetworking.java @@ -32,9 +32,11 @@ import net.minecraft.util.thread.ThreadExecutor; import net.fabricmc.fabric.api.networking.v1.FabricPacket; import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.api.networking.v1.PacketType; -import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking; import net.fabricmc.fabric.impl.networking.client.ClientConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.client.ClientNetworkingImpl; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; import net.fabricmc.fabric.mixin.networking.client.accessor.ClientCommonNetworkHandlerAccessor; /** @@ -71,7 +73,7 @@ public final class ClientConfigurationNetworking { * @see ClientConfigurationNetworking#registerReceiver(Identifier, ConfigurationChannelHandler) */ public static boolean registerGlobalReceiver(Identifier channelName, ConfigurationChannelHandler channelHandler) { - return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, channelHandler); + return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, wrapUntyped(channelHandler)); } /** @@ -88,29 +90,7 @@ public final class ClientConfigurationNetworking { * @see ClientConfigurationNetworking#registerReceiver(PacketType, ConfigurationPacketHandler) */ public static boolean registerGlobalReceiver(PacketType type, ConfigurationPacketHandler handler) { - return registerGlobalReceiver(type.getId(), new ConfigurationChannelHandlerProxy() { - @Override - public ConfigurationPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftClient client, ClientConfigurationNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - - if (client.isOnThread()) { - // Do not submit to the render thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, sender); - } else { - client.execute(() -> { - if (((ClientCommonNetworkHandlerAccessor) networkHandler).getConnection().isOpen()) handler.receive(packet, sender); - }); - } - } - }); + return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(type.getId(), wrapTyped(type, handler)); } /** @@ -126,7 +106,7 @@ public final class ClientConfigurationNetworking { */ @Nullable public static ClientConfigurationNetworking.ConfigurationChannelHandler unregisterGlobalReceiver(Identifier channelName) { - return ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName); + return unwrapUntyped(ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName)); } /** @@ -142,10 +122,8 @@ public final class ClientConfigurationNetworking { * @see ClientConfigurationNetworking#unregisterReceiver(PacketType) */ @Nullable - @SuppressWarnings("unchecked") public static ClientConfigurationNetworking.ConfigurationPacketHandler unregisterGlobalReceiver(PacketType type) { - ConfigurationChannelHandler handler = ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId()); - return handler instanceof ConfigurationChannelHandlerProxy proxy ? (ConfigurationPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId())); } /** @@ -179,7 +157,7 @@ public final class ClientConfigurationNetworking { final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon(); if (addon != null) { - return addon.registerChannel(channelName, channelHandler); + return addon.registerChannel(channelName, wrapUntyped(channelHandler)); } throw new IllegalStateException("Cannot register receiver while not configuring!"); @@ -201,29 +179,13 @@ public final class ClientConfigurationNetworking { * @see ClientPlayConnectionEvents#INIT */ public static boolean registerReceiver(PacketType type, ConfigurationPacketHandler handler) { - return registerReceiver(type.getId(), new ConfigurationChannelHandlerProxy() { - @Override - public ConfigurationPacketHandler getOriginalHandler() { - return handler; - } + final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon(); - @Override - public void receive(MinecraftClient client, ClientConfigurationNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); + if (addon != null) { + return addon.registerChannel(type.getId(), wrapTyped(type, handler)); + } - if (client.isOnThread()) { - // Do not submit to the render thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, sender); - } else { - client.execute(() -> { - if (((ClientCommonNetworkHandlerAccessor) networkHandler).getConnection().isOpen()) handler.receive(packet, sender); - }); - } - } - }); + throw new IllegalStateException("Cannot register receiver while not configuring!"); } /** @@ -240,7 +202,7 @@ public final class ClientConfigurationNetworking { final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon(); if (addon != null) { - return addon.unregisterChannel(channelName); + return unwrapUntyped(addon.unregisterChannel(channelName)); } throw new IllegalStateException("Cannot unregister receiver while not configuring!"); @@ -257,10 +219,14 @@ public final class ClientConfigurationNetworking { * @throws IllegalStateException if the client is not connected to a server */ @Nullable - @SuppressWarnings("unchecked") public static ClientConfigurationNetworking.ConfigurationPacketHandler unregisterReceiver(PacketType type) { - ConfigurationChannelHandler handler = unregisterReceiver(type.getId()); - return handler instanceof ConfigurationChannelHandlerProxy proxy ? (ConfigurationPacketHandler) proxy.getOriginalHandler() : null; + final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon(); + + if (addon != null) { + return unwrapTyped(addon.unregisterChannel(type.getId())); + } + + throw new IllegalStateException("Cannot unregister receiver while not configuring!"); } /** @@ -394,6 +360,48 @@ public final class ClientConfigurationNetworking { private ClientConfigurationNetworking() { } + private static ResolvablePayload.Handler wrapUntyped(ConfigurationChannelHandler actualHandler) { + return new ResolvablePayload.Handler<>(null, actualHandler, (client, handler, payload, responseSender) -> { + actualHandler.receive(client, handler, ((UntypedPayload) payload).buffer(), responseSender); + }); + } + + @SuppressWarnings("unchecked") + private static ResolvablePayload.Handler wrapTyped(PacketType type, ConfigurationPacketHandler actualHandler) { + return new ResolvablePayload.Handler<>(type, actualHandler, (client, handler, payload, responseSender) -> { + T packet = (T) ((TypedPayload) payload).packet(); + + if (client.isOnThread()) { + // Do not submit to the render thread if we're already running there. + // Normally, packets are handled on the network IO thread - though it is + // not guaranteed (for example, with 1.19.4 S2C packet bundling) + // Since we're handling it right now, connection check is redundant. + actualHandler.receive(packet, responseSender); + } else { + client.execute(() -> { + if (((ClientCommonNetworkHandlerAccessor) handler).getConnection().isOpen()) { + actualHandler.receive(packet, responseSender); + } + }); + } + }); + } + + @Nullable + private static ConfigurationChannelHandler unwrapUntyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof ConfigurationChannelHandler actual) return actual; + return null; + } + + @Nullable + @SuppressWarnings({"rawtypes", "unchecked"}) + private static ConfigurationPacketHandler unwrapTyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof ConfigurationPacketHandler actual) return actual; + return null; + } + @FunctionalInterface public interface ConfigurationChannelHandler { /** @@ -421,14 +429,6 @@ public final class ClientConfigurationNetworking { void receive(MinecraftClient client, ClientConfigurationNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender); } - /** - * An internal packet handler that works as a proxy between old and new API. - * @param the type of the packet - */ - private interface ConfigurationChannelHandlerProxy extends ConfigurationChannelHandler { - ConfigurationPacketHandler getOriginalHandler(); - } - /** * A thread-safe packet handler utilizing {@link FabricPacket}. * @param the type of the packet diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java index e0493d125..cc056b2dd 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java @@ -36,6 +36,9 @@ import net.fabricmc.fabric.api.networking.v1.PacketType; import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.fabric.impl.networking.client.ClientNetworkingImpl; import net.fabricmc.fabric.impl.networking.client.ClientPlayNetworkAddon; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; /** * Offers access to play stage client-side networking functionalities. @@ -73,7 +76,7 @@ public final class ClientPlayNetworking { * @see ClientPlayNetworking#registerReceiver(Identifier, PlayChannelHandler) */ public static boolean registerGlobalReceiver(Identifier channelName, PlayChannelHandler channelHandler) { - return ClientNetworkingImpl.PLAY.registerGlobalReceiver(channelName, channelHandler); + return ClientNetworkingImpl.PLAY.registerGlobalReceiver(channelName, wrapUntyped(channelHandler)); } /** @@ -90,29 +93,7 @@ public final class ClientPlayNetworking { * @see ClientPlayNetworking#registerReceiver(PacketType, PlayPacketHandler) */ public static boolean registerGlobalReceiver(PacketType type, PlayPacketHandler handler) { - return registerGlobalReceiver(type.getId(), new PlayChannelHandlerProxy() { - @Override - public PlayPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftClient client, ClientPlayNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - - if (client.isOnThread()) { - // Do not submit to the render thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, client.player, sender); - } else { - client.execute(() -> { - if (networkHandler.getConnection().isOpen()) handler.receive(packet, client.player, sender); - }); - } - } - }); + return ClientNetworkingImpl.PLAY.registerGlobalReceiver(type.getId(), wrapTyped(type, handler)); } /** @@ -128,7 +109,7 @@ public final class ClientPlayNetworking { */ @Nullable public static PlayChannelHandler unregisterGlobalReceiver(Identifier channelName) { - return ClientNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName); + return unwrapUntyped(ClientNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName)); } /** @@ -144,10 +125,8 @@ public final class ClientPlayNetworking { * @see ClientPlayNetworking#unregisterReceiver(PacketType) */ @Nullable - @SuppressWarnings("unchecked") public static PlayPacketHandler unregisterGlobalReceiver(PacketType type) { - PlayChannelHandler handler = ClientNetworkingImpl.PLAY.unregisterGlobalReceiver(type.getId()); - return handler instanceof PlayChannelHandlerProxy proxy ? (PlayPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ClientNetworkingImpl.PLAY.unregisterGlobalReceiver(type.getId())); } /** @@ -181,7 +160,7 @@ public final class ClientPlayNetworking { final ClientPlayNetworkAddon addon = ClientNetworkingImpl.getClientPlayAddon(); if (addon != null) { - return addon.registerChannel(channelName, channelHandler); + return addon.registerChannel(channelName, wrapUntyped(channelHandler)); } throw new IllegalStateException("Cannot register receiver while not in game!"); @@ -203,29 +182,13 @@ public final class ClientPlayNetworking { * @see ClientPlayConnectionEvents#INIT */ public static boolean registerReceiver(PacketType type, PlayPacketHandler handler) { - return registerReceiver(type.getId(), new PlayChannelHandlerProxy() { - @Override - public PlayPacketHandler getOriginalHandler() { - return handler; - } + final ClientPlayNetworkAddon addon = ClientNetworkingImpl.getClientPlayAddon(); - @Override - public void receive(MinecraftClient client, ClientPlayNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); + if (addon != null) { + return addon.registerChannel(type.getId(), wrapTyped(type, handler)); + } - if (client.isOnThread()) { - // Do not submit to the render thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, client.player, sender); - } else { - client.execute(() -> { - if (networkHandler.getConnection().isOpen()) handler.receive(packet, client.player, sender); - }); - } - } - }); + throw new IllegalStateException("Cannot register receiver while not in game!"); } /** @@ -242,7 +205,7 @@ public final class ClientPlayNetworking { final ClientPlayNetworkAddon addon = ClientNetworkingImpl.getClientPlayAddon(); if (addon != null) { - return addon.unregisterChannel(channelName); + return unwrapUntyped(addon.unregisterChannel(channelName)); } throw new IllegalStateException("Cannot unregister receiver while not in game!"); @@ -259,10 +222,14 @@ public final class ClientPlayNetworking { * @throws IllegalStateException if the client is not connected to a server */ @Nullable - @SuppressWarnings("unchecked") public static PlayPacketHandler unregisterReceiver(PacketType type) { - PlayChannelHandler handler = unregisterReceiver(type.getId()); - return handler instanceof PlayChannelHandlerProxy proxy ? (PlayPacketHandler) proxy.getOriginalHandler() : null; + final ClientPlayNetworkAddon addon = ClientNetworkingImpl.getClientPlayAddon(); + + if (addon != null) { + return unwrapTyped(addon.unregisterChannel(type.getId())); + } + + throw new IllegalStateException("Cannot unregister receiver while not in game!"); } /** @@ -402,6 +369,46 @@ public final class ClientPlayNetworking { private ClientPlayNetworking() { } + private static ResolvablePayload.Handler wrapUntyped(PlayChannelHandler actualHandler) { + return new ResolvablePayload.Handler<>(null, actualHandler, (client, handler, payload, responseSender) -> { + actualHandler.receive(client, handler, ((UntypedPayload) payload).buffer(), responseSender); + }); + } + + @SuppressWarnings("unchecked") + private static ResolvablePayload.Handler wrapTyped(PacketType type, PlayPacketHandler actualHandler) { + return new ResolvablePayload.Handler<>(type, actualHandler, (client, handler, payload, responseSender) -> { + T packet = (T) ((TypedPayload) payload).packet(); + + if (client.isOnThread()) { + // Do not submit to the render thread if we're already running there. + // Normally, packets are handled on the network IO thread - though it is + // not guaranteed (for example, with 1.19.4 S2C packet bundling) + // Since we're handling it right now, connection check is redundant. + actualHandler.receive(packet, client.player, responseSender); + } else { + client.execute(() -> { + if (handler.getConnection().isOpen()) actualHandler.receive(packet, client.player, responseSender); + }); + } + }); + } + + @Nullable + private static PlayChannelHandler unwrapUntyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof PlayChannelHandler actual) return actual; + return null; + } + + @Nullable + @SuppressWarnings({"rawtypes", "unchecked"}) + private static PlayPacketHandler unwrapTyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof PlayPacketHandler actual) return actual; + return null; + } + @FunctionalInterface public interface PlayChannelHandler { /** @@ -429,14 +436,6 @@ public final class ClientPlayNetworking { void receive(MinecraftClient client, ClientPlayNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender); } - /** - * An internal packet handler that works as a proxy between old and new API. - * @param the type of the packet - */ - private interface PlayChannelHandlerProxy extends PlayChannelHandler { - PlayPacketHandler getOriginalHandler(); - } - /** * A thread-safe packet handler utilizing {@link FabricPacket}. * @param the type of the packet diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java index 17ca09e3d..9b598d018 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java @@ -28,17 +28,18 @@ import net.minecraft.util.Identifier; import net.fabricmc.fabric.api.client.networking.v1.C2SConfigurationChannelEvents; import net.fabricmc.fabric.api.client.networking.v1.ClientConfigurationConnectionEvents; -import net.fabricmc.fabric.api.client.networking.v1.ClientConfigurationNetworking; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.fabricmc.fabric.api.networking.v1.FabricPacket; +import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.impl.networking.AbstractChanneledNetworkAddon; import net.fabricmc.fabric.impl.networking.ChannelInfoHolder; import net.fabricmc.fabric.impl.networking.NetworkingImpl; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; import net.fabricmc.fabric.mixin.networking.client.accessor.ClientCommonNetworkHandlerAccessor; import net.fabricmc.fabric.mixin.networking.client.accessor.ClientConfigurationNetworkHandlerAccessor; -public final class ClientConfigurationNetworkAddon extends AbstractChanneledNetworkAddon { +public final class ClientConfigurationNetworkAddon extends AbstractChanneledNetworkAddon { private final ClientConfigurationNetworkHandler handler; private final MinecraftClient client; private boolean sentInitialRegisterPacket; @@ -62,8 +63,8 @@ public final class ClientConfigurationNetworkAddon extends AbstractChanneledNetw } @Override - protected void receiveRegistration(boolean register, PacketByteBuf buf) { - super.receiveRegistration(register, buf); + protected void receiveRegistration(boolean register, ResolvablePayload resolvable) { + super.receiveRegistration(register, resolvable); if (register && !this.sentInitialRegisterPacket) { this.sendInitialChannelRegistrationPacket(); @@ -71,19 +72,9 @@ public final class ClientConfigurationNetworkAddon extends AbstractChanneledNetw } } - /** - * Handles an incoming packet. - * - * @param payload the payload to handle - * @return true if the packet has been handled - */ - public boolean handle(PacketByteBufPayload payload) { - return this.handle(payload.id(), payload.data()); - } - @Override - protected void receive(ClientConfigurationNetworking.ConfigurationChannelHandler handler, PacketByteBuf buf) { - handler.receive(this.client, this.handler, buf, this); + protected void receive(Handler handler, ResolvedPayload payload) { + handler.receive(this.client, this.handler, payload, this); } // impl details @@ -155,4 +146,8 @@ public final class ClientConfigurationNetworkAddon extends AbstractChanneledNetw public ChannelInfoHolder getChannelInfoHolder() { return (ChannelInfoHolder) ((ClientCommonNetworkHandlerAccessor) handler).getConnection(); } + + public interface Handler { + void receive(MinecraftClient client, ClientConfigurationNetworkHandler handler, ResolvedPayload payload, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientNetworkingImpl.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientNetworkingImpl.java index 269d90057..af4a8c23a 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientNetworkingImpl.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientNetworkingImpl.java @@ -39,7 +39,6 @@ import net.fabricmc.fabric.api.client.networking.v1.ClientLoginNetworking; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayConnectionEvents; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.fabricmc.fabric.api.networking.v1.FabricPacket; -import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.impl.networking.CommonPacketsImpl; import net.fabricmc.fabric.impl.networking.CommonRegisterPayload; @@ -47,14 +46,18 @@ import net.fabricmc.fabric.impl.networking.CommonVersionPayload; import net.fabricmc.fabric.impl.networking.GlobalReceiverRegistry; import net.fabricmc.fabric.impl.networking.NetworkHandlerExtensions; import net.fabricmc.fabric.impl.networking.NetworkingImpl; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; import net.fabricmc.fabric.mixin.networking.client.accessor.ConnectScreenAccessor; import net.fabricmc.fabric.mixin.networking.client.accessor.MinecraftClientAccessor; public final class ClientNetworkingImpl { public static final GlobalReceiverRegistry LOGIN = new GlobalReceiverRegistry<>(NetworkState.LOGIN); - public static final GlobalReceiverRegistry CONFIGURATION = new GlobalReceiverRegistry<>(NetworkState.CONFIGURATION); - public static final GlobalReceiverRegistry PLAY = new GlobalReceiverRegistry<>(NetworkState.PLAY); + public static final GlobalReceiverRegistry> CONFIGURATION = new GlobalReceiverRegistry<>(NetworkState.CONFIGURATION); + public static final GlobalReceiverRegistry> PLAY = new GlobalReceiverRegistry<>(NetworkState.PLAY); + private static ClientPlayNetworkAddon currentPlayAddon; private static ClientConfigurationNetworkAddon currentConfigurationAddon; @@ -71,16 +74,17 @@ public final class ClientNetworkingImpl { } public static Packet createC2SPacket(Identifier channelName, PacketByteBuf buf) { - return new CustomPayloadC2SPacket(new PacketByteBufPayload(channelName, buf)); + return new CustomPayloadC2SPacket(new UntypedPayload(channelName, buf)); } public static Packet createC2SPacket(FabricPacket packet) { Objects.requireNonNull(packet, "Packet cannot be null"); Objects.requireNonNull(packet.getType(), "Packet#getType cannot return null"); - PacketByteBuf buf = PacketByteBufs.create(); - packet.write(buf); - return createC2SPacket(packet.getType().getId(), buf); + ResolvedPayload payload = new TypedPayload(packet); + if (NetworkingImpl.FORCE_PACKET_SERIALIZATION) payload = payload.resolve(null); + + return new CustomPayloadC2SPacket(payload); } /** diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java index 9b7b94b13..cfe34baae 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java @@ -33,12 +33,13 @@ import net.fabricmc.fabric.api.client.networking.v1.C2SPlayChannelEvents; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayConnectionEvents; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.fabricmc.fabric.api.networking.v1.FabricPacket; +import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.impl.networking.AbstractChanneledNetworkAddon; import net.fabricmc.fabric.impl.networking.ChannelInfoHolder; import net.fabricmc.fabric.impl.networking.NetworkingImpl; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; -public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon { +public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon { private final ClientPlayNetworkHandler handler; private final MinecraftClient client; private boolean sentInitialRegisterPacket; @@ -71,19 +72,9 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< this.sentInitialRegisterPacket = true; } - /** - * Handles an incoming packet. - * - * @param payload the payload to handle - * @return true if the packet has been handled - */ - public boolean handle(PacketByteBufPayload payload) { - return this.handle(payload.id(), payload.data()); - } - @Override - protected void receive(ClientPlayNetworking.PlayChannelHandler handler, PacketByteBuf buf) { - handler.receive(this.client, this.handler, buf, this); + protected void receive(Handler handler, ResolvedPayload payload) { + handler.receive(this.client, this.handler, payload, this); } // impl details @@ -146,4 +137,8 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< protected boolean isReservedChannel(Identifier channelName) { return NetworkingImpl.isReservedCommonChannel(channelName); } + + public interface Handler { + void receive(MinecraftClient client, ClientPlayNetworkHandler handler, ResolvedPayload payload, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/mixin/networking/client/ClientCommonNetworkHandlerMixin.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/mixin/networking/client/ClientCommonNetworkHandlerMixin.java index 494811ee7..5fc88f42e 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/mixin/networking/client/ClientCommonNetworkHandlerMixin.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/mixin/networking/client/ClientCommonNetworkHandlerMixin.java @@ -27,13 +27,14 @@ import net.minecraft.network.packet.s2c.common.CustomPayloadS2CPacket; import net.fabricmc.fabric.impl.networking.NetworkHandlerExtensions; import net.fabricmc.fabric.impl.networking.client.ClientConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.client.ClientPlayNetworkAddon; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.RetainedPayload; @Mixin(ClientCommonNetworkHandler.class) public abstract class ClientCommonNetworkHandlerMixin implements NetworkHandlerExtensions { @Inject(method = "onCustomPayload(Lnet/minecraft/network/packet/s2c/common/CustomPayloadS2CPacket;)V", at = @At("HEAD"), cancellable = true) public void onCustomPayload(CustomPayloadS2CPacket packet, CallbackInfo ci) { - if (packet.payload() instanceof PacketByteBufPayload payload) { + if (packet.payload() instanceof ResolvablePayload payload) { boolean handled; if (this.getAddon() instanceof ClientPlayNetworkAddon addon) { @@ -46,8 +47,11 @@ public abstract class ClientCommonNetworkHandlerMixin implements NetworkHandlerE if (handled) { ci.cancel(); - } else { - payload.data().skipBytes(payload.data().readableBytes()); + } else if (payload instanceof RetainedPayload retained && retained.buf().refCnt() > 0) { + // Vanilla forces to use the render thread for its payloads, + // that means this method can get called multiple times. + retained.buf().skipBytes(retained.buf().readableBytes()); + retained.buf().release(); } } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java index 2261d5817..218dec454 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerConfigurationNetworking.java @@ -29,6 +29,10 @@ import net.minecraft.server.network.ServerConfigurationNetworkHandler; import net.minecraft.util.Identifier; import net.minecraft.util.thread.ThreadExecutor; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; +import net.fabricmc.fabric.impl.networking.server.ServerConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; import net.fabricmc.fabric.mixin.networking.accessor.ServerCommonNetworkHandlerAccessor; @@ -68,7 +72,7 @@ public final class ServerConfigurationNetworking { * @see ServerConfigurationNetworking#registerReceiver(ServerConfigurationNetworkHandler, Identifier, ConfigurationChannelHandler) */ public static boolean registerGlobalReceiver(Identifier channelName, ConfigurationChannelHandler channelHandler) { - return ServerNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, channelHandler); + return ServerNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, wrapUntyped(channelHandler)); } /** @@ -85,18 +89,7 @@ public final class ServerConfigurationNetworking { * @see ServerConfigurationNetworking#registerReceiver(ServerConfigurationNetworkHandler, PacketType, ConfigurationPacketHandler) */ public static boolean registerGlobalReceiver(PacketType type, ConfigurationPacketHandler handler) { - return registerGlobalReceiver(type.getId(), new ConfigurationChannelHandlerProxy() { - @Override - public ConfigurationPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftServer server, ServerConfigurationNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - handler.receive(packet, networkHandler, sender); - } - }); + return ServerNetworkingImpl.CONFIGURATION.registerGlobalReceiver(type.getId(), wrapTyped(type, handler)); } /** @@ -112,7 +105,7 @@ public final class ServerConfigurationNetworking { */ @Nullable public static ServerConfigurationNetworking.ConfigurationChannelHandler unregisterGlobalReceiver(Identifier channelName) { - return ServerNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName); + return unwrapUntyped(ServerNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName)); } /** @@ -128,10 +121,8 @@ public final class ServerConfigurationNetworking { * @see ServerConfigurationNetworking#unregisterReceiver(ServerConfigurationNetworkHandler, PacketType) */ @Nullable - @SuppressWarnings("unchecked") public static ServerConfigurationNetworking.ConfigurationPacketHandler unregisterGlobalReceiver(PacketType type) { - ConfigurationChannelHandler handler = ServerNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId()); - return handler instanceof ConfigurationChannelHandlerProxy proxy ? (ConfigurationPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ServerNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId())); } /** @@ -170,7 +161,7 @@ public final class ServerConfigurationNetworking { public static boolean registerReceiver(ServerConfigurationNetworkHandler networkHandler, Identifier channelName, ConfigurationChannelHandler channelHandler) { Objects.requireNonNull(networkHandler, "Network handler cannot be null"); - return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(channelName, channelHandler); + return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(channelName, wrapUntyped(channelHandler)); } /** @@ -191,18 +182,7 @@ public final class ServerConfigurationNetworking { * @see ServerPlayConnectionEvents#INIT */ public static boolean registerReceiver(ServerConfigurationNetworkHandler networkHandler, PacketType type, ConfigurationPacketHandler handler) { - return registerReceiver(networkHandler, type.getId(), new ConfigurationChannelHandlerProxy() { - @Override - public ConfigurationPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftServer server, ServerConfigurationNetworkHandler networkHandler2, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - handler.receive(packet, networkHandler2, sender); - } - }); + return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(type.getId(), wrapTyped(type, handler)); } /** @@ -217,7 +197,7 @@ public final class ServerConfigurationNetworking { public static ServerConfigurationNetworking.ConfigurationChannelHandler unregisterReceiver(ServerConfigurationNetworkHandler networkHandler, Identifier channelName) { Objects.requireNonNull(networkHandler, "Network handler cannot be null"); - return ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(channelName); + return unwrapUntyped(ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(channelName)); } /** @@ -230,10 +210,8 @@ public final class ServerConfigurationNetworking { * or it was not registered using {@link #registerReceiver(ServerConfigurationNetworkHandler, PacketType, ConfigurationPacketHandler)} */ @Nullable - @SuppressWarnings("unchecked") public static ServerConfigurationNetworking.ConfigurationPacketHandler unregisterReceiver(ServerConfigurationNetworkHandler networkHandler, PacketType type) { - ConfigurationChannelHandler handler = unregisterReceiver(networkHandler, type.getId()); - return handler instanceof ConfigurationChannelHandlerProxy proxy ? (ConfigurationPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(type.getId())); } /** @@ -372,6 +350,35 @@ public final class ServerConfigurationNetworking { private ServerConfigurationNetworking() { } + private static ResolvablePayload.Handler wrapUntyped(ConfigurationChannelHandler actualHandler) { + return new ResolvablePayload.Handler<>(null, actualHandler, (server, handler, payload, responseSender) -> { + actualHandler.receive(server, handler, ((UntypedPayload) payload).buffer(), responseSender); + }); + } + + @SuppressWarnings("unchecked") + private static ResolvablePayload.Handler wrapTyped(PacketType type, ConfigurationPacketHandler actualHandler) { + return new ResolvablePayload.Handler<>(type, actualHandler, (server, handler, payload, responseSender) -> { + T packet = (T) ((TypedPayload) payload).packet(); + actualHandler.receive(packet, handler, responseSender); + }); + } + + @Nullable + private static ConfigurationChannelHandler unwrapUntyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof ConfigurationChannelHandler actual) return actual; + return null; + } + + @Nullable + @SuppressWarnings({"rawtypes", "unchecked"}) + private static ConfigurationPacketHandler unwrapTyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof ConfigurationPacketHandler actual) return actual; + return null; + } + @FunctionalInterface public interface ConfigurationChannelHandler { /** @@ -399,14 +406,6 @@ public final class ServerConfigurationNetworking { void receive(MinecraftServer server, ServerConfigurationNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender); } - /** - * An internal packet handler that works as a proxy between old and new API. - * @param the type of the packet - */ - private interface ConfigurationChannelHandlerProxy extends ConfigurationChannelHandler { - ConfigurationPacketHandler getOriginalHandler(); - } - /** * A thread-safe packet handler utilizing {@link FabricPacket}. * @param the type of the packet diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java index 9ac7ce58d..4df25147e 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java @@ -30,7 +30,11 @@ import net.minecraft.server.network.ServerPlayerEntity; import net.minecraft.util.Identifier; import net.minecraft.util.thread.ThreadExecutor; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; +import net.fabricmc.fabric.impl.networking.server.ServerPlayNetworkAddon; /** * Offers access to play stage server-side networking functionalities. @@ -83,7 +87,7 @@ public final class ServerPlayNetworking { * @see ServerPlayNetworking#registerReceiver(ServerPlayNetworkHandler, Identifier, PlayChannelHandler) */ public static boolean registerGlobalReceiver(Identifier channelName, PlayChannelHandler channelHandler) { - return ServerNetworkingImpl.PLAY.registerGlobalReceiver(channelName, channelHandler); + return ServerNetworkingImpl.PLAY.registerGlobalReceiver(channelName, wrapUntyped(channelHandler)); } /** @@ -100,29 +104,7 @@ public final class ServerPlayNetworking { * @see ServerPlayNetworking#registerReceiver(ServerPlayNetworkHandler, PacketType, PlayPacketHandler) */ public static boolean registerGlobalReceiver(PacketType type, PlayPacketHandler handler) { - return registerGlobalReceiver(type.getId(), new PlayChannelHandlerProxy() { - @Override - public PlayPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftServer server, ServerPlayerEntity player, ServerPlayNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - - if (server.isOnThread()) { - // Do not submit to the server thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, player, sender); - } else { - server.execute(() -> { - if (networkHandler.isConnectionOpen()) handler.receive(packet, player, sender); - }); - } - } - }); + return ServerNetworkingImpl.PLAY.registerGlobalReceiver(type.getId(), wrapTyped(type, handler)); } /** @@ -138,7 +120,7 @@ public final class ServerPlayNetworking { */ @Nullable public static PlayChannelHandler unregisterGlobalReceiver(Identifier channelName) { - return ServerNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName); + return unwrapUntyped(ServerNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName)); } /** @@ -154,10 +136,8 @@ public final class ServerPlayNetworking { * @see ServerPlayNetworking#unregisterReceiver(ServerPlayNetworkHandler, PacketType) */ @Nullable - @SuppressWarnings("unchecked") public static PlayPacketHandler unregisterGlobalReceiver(PacketType type) { - PlayChannelHandler handler = ServerNetworkingImpl.PLAY.unregisterGlobalReceiver(type.getId()); - return handler instanceof PlayChannelHandlerProxy proxy ? (PlayPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ServerNetworkingImpl.PLAY.unregisterGlobalReceiver(type.getId())); } /** @@ -196,7 +176,7 @@ public final class ServerPlayNetworking { public static boolean registerReceiver(ServerPlayNetworkHandler networkHandler, Identifier channelName, PlayChannelHandler channelHandler) { Objects.requireNonNull(networkHandler, "Network handler cannot be null"); - return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(channelName, channelHandler); + return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(channelName, wrapUntyped(channelHandler)); } /** @@ -217,29 +197,7 @@ public final class ServerPlayNetworking { * @see ServerPlayConnectionEvents#INIT */ public static boolean registerReceiver(ServerPlayNetworkHandler networkHandler, PacketType type, PlayPacketHandler handler) { - return registerReceiver(networkHandler, type.getId(), new PlayChannelHandlerProxy() { - @Override - public PlayPacketHandler getOriginalHandler() { - return handler; - } - - @Override - public void receive(MinecraftServer server, ServerPlayerEntity player, ServerPlayNetworkHandler networkHandler2, PacketByteBuf buf, PacketSender sender) { - T packet = type.read(buf); - - if (server.isOnThread()) { - // Do not submit to the server thread if we're already running there. - // Normally, packets are handled on the network IO thread - though it is - // not guaranteed (for example, with 1.19.4 S2C packet bundling) - // Since we're handling it right now, connection check is redundant. - handler.receive(packet, player, sender); - } else { - server.execute(() -> { - if (networkHandler2.isConnectionOpen()) handler.receive(packet, player, sender); - }); - } - } - }); + return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(type.getId(), wrapTyped(type, handler)); } /** @@ -254,7 +212,7 @@ public final class ServerPlayNetworking { public static PlayChannelHandler unregisterReceiver(ServerPlayNetworkHandler networkHandler, Identifier channelName) { Objects.requireNonNull(networkHandler, "Network handler cannot be null"); - return ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(channelName); + return unwrapUntyped(ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(channelName)); } /** @@ -267,10 +225,8 @@ public final class ServerPlayNetworking { * or it was not registered using {@link #registerReceiver(ServerPlayNetworkHandler, PacketType, PlayPacketHandler)} */ @Nullable - @SuppressWarnings("unchecked") public static PlayPacketHandler unregisterReceiver(ServerPlayNetworkHandler networkHandler, PacketType type) { - PlayChannelHandler handler = unregisterReceiver(networkHandler, type.getId()); - return handler instanceof PlayChannelHandlerProxy proxy ? (PlayPacketHandler) proxy.getOriginalHandler() : null; + return unwrapTyped(ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(type.getId())); } /** @@ -468,6 +424,46 @@ public final class ServerPlayNetworking { private ServerPlayNetworking() { } + private static ResolvablePayload.Handler wrapUntyped(PlayChannelHandler actualHandler) { + return new ResolvablePayload.Handler<>(null, actualHandler, (server, player, handler, payload, responseSender) -> { + actualHandler.receive(server, player, handler, ((UntypedPayload) payload).buffer(), responseSender); + }); + } + + @SuppressWarnings("unchecked") + private static ResolvablePayload.Handler wrapTyped(PacketType type, PlayPacketHandler actualHandler) { + return new ResolvablePayload.Handler<>(type, actualHandler, (server, player, handler, payload, responseSender) -> { + T packet = (T) ((TypedPayload) payload).packet(); + + if (server.isOnThread()) { + // Do not submit to the server thread if we're already running there. + // Normally, packets are handled on the network IO thread - though it is + // not guaranteed (for example, with 1.19.4 S2C packet bundling) + // Since we're handling it right now, connection check is redundant. + actualHandler.receive(packet, player, responseSender); + } else { + server.execute(() -> { + if (handler.isConnectionOpen()) actualHandler.receive(packet, player, responseSender); + }); + } + }); + } + + @Nullable + private static PlayChannelHandler unwrapUntyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof PlayChannelHandler actual) return actual; + return null; + } + + @Nullable + @SuppressWarnings({"rawtypes", "unchecked"}) + private static PlayPacketHandler unwrapTyped(@Nullable ResolvablePayload.Handler handler) { + if (handler == null) return null; + if (handler.actual() instanceof PlayPacketHandler actual) return actual; + return null; + } + @FunctionalInterface public interface PlayChannelHandler { /** @@ -496,14 +492,6 @@ public final class ServerPlayNetworking { void receive(MinecraftServer server, ServerPlayerEntity player, ServerPlayNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender); } - /** - * An internal packet handler that works as a proxy between old and new API. - * @param the type of the packet - */ - private interface PlayChannelHandlerProxy extends PlayChannelHandler { - PlayPacketHandler getOriginalHandler(); - } - /** * A thread-safe packet handler utilizing {@link FabricPacket}. * @param the type of the packet diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java index 203d4dbff..4605a0e52 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java @@ -40,20 +40,23 @@ import net.minecraft.util.InvalidIdentifierException; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.PacketSender; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; /** * A network addon which is aware of the channels the other side may receive. * * @param the channel handler type */ -public abstract class AbstractChanneledNetworkAddon extends AbstractNetworkAddon implements PacketSender, CommonPacketHandler { +public abstract class AbstractChanneledNetworkAddon extends AbstractNetworkAddon> implements PacketSender, CommonPacketHandler { protected final ClientConnection connection; - protected final GlobalReceiverRegistry receiver; + protected final GlobalReceiverRegistry> receiver; protected final Set sendableChannels; protected int commonVersion = -1; - protected AbstractChanneledNetworkAddon(GlobalReceiverRegistry receiver, ClientConnection connection, String description) { + protected AbstractChanneledNetworkAddon(GlobalReceiverRegistry> receiver, ClientConnection connection, String description) { super(receiver, description); this.connection = connection; this.receiver = receiver; @@ -70,28 +73,30 @@ public abstract class AbstractChanneledNetworkAddon extends AbstractNetworkAd } // always supposed to handle async! - protected boolean handle(Identifier channelName, PacketByteBuf buf) { + public boolean handle(ResolvablePayload resolvable) { + Identifier channelName = resolvable.id(); this.logger.debug("Handling inbound packet from channel with name \"{}\"", channelName); // Handle reserved packets if (NetworkingImpl.REGISTER_CHANNEL.equals(channelName)) { - this.receiveRegistration(true, buf); + this.receiveRegistration(true, resolvable); return true; } if (NetworkingImpl.UNREGISTER_CHANNEL.equals(channelName)) { - this.receiveRegistration(false, buf); + this.receiveRegistration(false, resolvable); return true; } - @Nullable H handler = this.getHandler(channelName); + @Nullable ResolvablePayload.Handler handler = this.getHandler(channelName); if (handler == null) { return false; } try { - this.receive(handler, buf); + ResolvedPayload resolved = resolvable.resolve(handler.type()); + this.receive(handler.internal(), resolved); } catch (Throwable ex) { this.logger.error("Encountered exception while handling in channel with name \"{}\"", channelName, ex); throw ex; @@ -100,7 +105,7 @@ public abstract class AbstractChanneledNetworkAddon extends AbstractNetworkAd return true; } - protected abstract void receive(H handler, PacketByteBuf buf); + protected abstract void receive(H handler, ResolvedPayload payload); protected void sendInitialChannelRegistrationPacket() { final PacketByteBuf buf = this.createRegistrationPacket(this.getReceivableChannels()); @@ -133,7 +138,10 @@ public abstract class AbstractChanneledNetworkAddon extends AbstractNetworkAd } // wrap in try with res (buf) - protected void receiveRegistration(boolean register, PacketByteBuf buf) { + protected void receiveRegistration(boolean register, ResolvablePayload resolvable) { + UntypedPayload payload = (UntypedPayload) resolvable.resolve(null); + PacketByteBuf buf = payload.buffer(); + List ids = new ArrayList<>(); StringBuilder active = new StringBuilder(); diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/GlobalReceiverRegistry.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/GlobalReceiverRegistry.java index 452944539..8f1cf1de7 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/GlobalReceiverRegistry.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/GlobalReceiverRegistry.java @@ -86,6 +86,7 @@ public final class GlobalReceiverRegistry { } } + @Nullable public H unregisterGlobalReceiver(Identifier channelName) { Objects.requireNonNull(channelName, "Channel name cannot be null"); diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/NetworkingImpl.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/NetworkingImpl.java index d9c9d9760..fb9dc7aa1 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/NetworkingImpl.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/NetworkingImpl.java @@ -21,10 +21,23 @@ import org.slf4j.LoggerFactory; import net.minecraft.util.Identifier; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; +import net.fabricmc.loader.api.FabricLoader; + public final class NetworkingImpl { public static final String MOD_ID = "fabric-networking-api-v1"; public static final Logger LOGGER = LoggerFactory.getLogger(MOD_ID); + /** + * Force {@link TypedPayload} to be serialized into {@link UntypedPayload}, mimicking remote connection. + * + *

Defaults to {@code true} in dev env and {@code false} in production. + */ + public static final boolean FORCE_PACKET_SERIALIZATION = Boolean.parseBoolean(System.getProperty( + "fabric-api.networking.force-packet-serialization", + Boolean.toString(FabricLoader.getInstance().isDevelopmentEnvironment()))); + /** * Id of packet used to register supported channels. */ @@ -38,4 +51,10 @@ public final class NetworkingImpl { public static boolean isReservedCommonChannel(Identifier channelName) { return channelName.equals(REGISTER_CHANNEL) || channelName.equals(UNREGISTER_CHANNEL); } + + static { + if (FORCE_PACKET_SERIALIZATION) { + LOGGER.info("Force Packet Serialization is enabled to mimic remote connection on single player, this is the default behaviour on dev env. Add -Dfabric-api.networking.force-packet-serialization=false JVM arg to disable it."); + } + } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvablePayload.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvablePayload.java new file mode 100644 index 000000000..2f766ec35 --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvablePayload.java @@ -0,0 +1,40 @@ +/* + * 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.impl.networking.payload; + +import org.jetbrains.annotations.Nullable; + +import net.minecraft.network.packet.CustomPayload; + +import net.fabricmc.fabric.api.networking.v1.PacketType; + +public sealed interface ResolvablePayload extends CustomPayload permits ResolvedPayload, RetainedPayload { + /** + * Resolve the payload to one of the resolved types. + * + * @return {@link UntypedPayload} if type is {@code null}, {@link TypedPayload} if otherwise. + */ + ResolvedPayload resolve(@Nullable PacketType type); + + /** + * @param type the packet type, if it has any + * @param actual the public handler that exposed to API consumer + * @param internal the internal handler + */ + record Handler(@Nullable PacketType type, Object actual, H internal) { + } +} diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/PacketByteBufPayload.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvedPayload.java similarity index 67% rename from fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/PacketByteBufPayload.java rename to fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvedPayload.java index 78ac1cc39..04b5e7d33 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/PacketByteBufPayload.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/ResolvedPayload.java @@ -16,13 +16,5 @@ package net.fabricmc.fabric.impl.networking.payload; -import net.minecraft.network.PacketByteBuf; -import net.minecraft.network.packet.CustomPayload; -import net.minecraft.util.Identifier; - -public record PacketByteBufPayload(Identifier id, PacketByteBuf data) implements CustomPayload { - @Override - public void write(PacketByteBuf buf) { - PayloadHelper.write(buf, data()); - } +public sealed interface ResolvedPayload extends ResolvablePayload permits TypedPayload, UntypedPayload { } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/RetainedPayload.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/RetainedPayload.java new file mode 100644 index 000000000..8e4f0113d --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/RetainedPayload.java @@ -0,0 +1,54 @@ +/* + * 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.impl.networking.payload; + +import org.jetbrains.annotations.Nullable; + +import net.minecraft.network.PacketByteBuf; +import net.minecraft.util.Identifier; + +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; +import net.fabricmc.fabric.api.networking.v1.PacketType; + +public record RetainedPayload(Identifier id, PacketByteBuf buf) implements ResolvablePayload { + @Override + public ResolvedPayload resolve(@Nullable PacketType type) { + try { + if (type == null) { + PacketByteBuf copy = PacketByteBufs.create(); + copy.writeBytes(buf); + return new UntypedPayload(this.id, copy); + } else { + TypedPayload typed = new TypedPayload(type.read(buf)); + int dangling = buf.readableBytes(); + + if (dangling > 0) { + throw new IllegalStateException("Found " + dangling + " extra bytes when reading packet " + id); + } + + return typed; + } + } finally { + buf.release(); + } + } + + @Override + public void write(PacketByteBuf buf) { + throw new UnsupportedOperationException("RetainedPayload shouldn't be used to send"); + } +} diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/TypedPayload.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/TypedPayload.java new file mode 100644 index 000000000..4240ee991 --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/TypedPayload.java @@ -0,0 +1,49 @@ +/* + * 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.impl.networking.payload; + +import org.jetbrains.annotations.Nullable; + +import net.minecraft.network.PacketByteBuf; +import net.minecraft.util.Identifier; + +import net.fabricmc.fabric.api.networking.v1.FabricPacket; +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; +import net.fabricmc.fabric.api.networking.v1.PacketType; + +public record TypedPayload(FabricPacket packet) implements ResolvedPayload { + @Override + public ResolvedPayload resolve(@Nullable PacketType type) { + if (type == null) { + PacketByteBuf buf = PacketByteBufs.create(); + write(buf); + return new UntypedPayload(packet.getType().getId(), buf); + } else { + return this; + } + } + + @Override + public void write(PacketByteBuf buf) { + packet.write(buf); + } + + @Override + public Identifier id() { + return packet.getType().getId(); + } +} diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/UntypedPayload.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/UntypedPayload.java new file mode 100644 index 000000000..028734a4b --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/UntypedPayload.java @@ -0,0 +1,47 @@ +/* + * 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.impl.networking.payload; + +import org.jetbrains.annotations.Nullable; + +import net.minecraft.network.PacketByteBuf; +import net.minecraft.util.Identifier; + +import net.fabricmc.fabric.api.networking.v1.PacketType; + +public record UntypedPayload(Identifier id, PacketByteBuf buffer) implements ResolvedPayload { + @Override + public ResolvedPayload resolve(@Nullable PacketType type) { + if (type == null) { + return this; + } else { + TypedPayload typed = new TypedPayload(type.read(buffer)); + int dangling = buffer.readableBytes(); + + if (dangling > 0) { + throw new IllegalStateException("Found " + dangling + " extra bytes when reading packet " + id); + } + + return typed; + } + } + + @Override + public void write(PacketByteBuf buf) { + buf.writeBytes(buffer); + } +} diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java index 93357b596..338a2d621 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java @@ -29,17 +29,18 @@ import net.minecraft.server.network.ServerConfigurationNetworkHandler; import net.minecraft.util.Identifier; import net.fabricmc.fabric.api.networking.v1.FabricPacket; +import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.api.networking.v1.S2CConfigurationChannelEvents; import net.fabricmc.fabric.api.networking.v1.ServerConfigurationConnectionEvents; -import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking; import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.fabric.impl.networking.AbstractChanneledNetworkAddon; import net.fabricmc.fabric.impl.networking.ChannelInfoHolder; import net.fabricmc.fabric.impl.networking.NetworkingImpl; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; import net.fabricmc.fabric.mixin.networking.accessor.ServerCommonNetworkHandlerAccessor; -public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetworkAddon { +public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetworkAddon { private final ServerConfigurationNetworkHandler handler; private final MinecraftServer server; private RegisterState registerState = RegisterState.NOT_SENT; @@ -83,8 +84,8 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw } @Override - protected void receiveRegistration(boolean register, PacketByteBuf buf) { - super.receiveRegistration(register, buf); + protected void receiveRegistration(boolean register, ResolvablePayload resolvable) { + super.receiveRegistration(register, resolvable); if (register && registerState == RegisterState.SENT) { // We received the registration packet, thus we know this is a modded client, continue with configuration. @@ -101,19 +102,9 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw } } - /** - * Handles an incoming packet. - * - * @param payload the payload to handle - * @return true if the packet has been handled - */ - public boolean handle(PacketByteBufPayload payload) { - return this.handle(payload.id(), payload.data()); - } - @Override - protected void receive(ServerConfigurationNetworking.ConfigurationChannelHandler handler, PacketByteBuf buf) { - handler.receive(this.server, this.handler, buf, this); + protected void receive(Handler handler, ResolvedPayload payload) { + handler.receive(this.server, this.handler, payload, this); } // impl details @@ -192,4 +183,8 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw public ChannelInfoHolder getChannelInfoHolder() { return (ChannelInfoHolder) ((ServerCommonNetworkHandlerAccessor) handler).getConnection(); } + + public interface Handler { + void receive(MinecraftServer server, ServerConfigurationNetworkHandler handler, ResolvedPayload payload, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerNetworkingImpl.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerNetworkingImpl.java index 2a3c88e59..04c219d1a 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerNetworkingImpl.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerNetworkingImpl.java @@ -29,18 +29,19 @@ import net.minecraft.server.network.ServerPlayNetworkHandler; import net.minecraft.util.Identifier; import net.fabricmc.fabric.api.networking.v1.FabricPacket; -import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; -import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking; import net.fabricmc.fabric.api.networking.v1.ServerLoginNetworking; -import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.fabric.impl.networking.GlobalReceiverRegistry; import net.fabricmc.fabric.impl.networking.NetworkHandlerExtensions; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.NetworkingImpl; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; +import net.fabricmc.fabric.impl.networking.payload.TypedPayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; public final class ServerNetworkingImpl { public static final GlobalReceiverRegistry LOGIN = new GlobalReceiverRegistry<>(NetworkState.LOGIN); - public static final GlobalReceiverRegistry CONFIGURATION = new GlobalReceiverRegistry<>(NetworkState.CONFIGURATION); - public static final GlobalReceiverRegistry PLAY = new GlobalReceiverRegistry<>(NetworkState.PLAY); + public static final GlobalReceiverRegistry> CONFIGURATION = new GlobalReceiverRegistry<>(NetworkState.CONFIGURATION); + public static final GlobalReceiverRegistry> PLAY = new GlobalReceiverRegistry<>(NetworkState.PLAY); public static ServerPlayNetworkAddon getAddon(ServerPlayNetworkHandler handler) { return (ServerPlayNetworkAddon) ((NetworkHandlerExtensions) handler).getAddon(); @@ -55,15 +56,16 @@ public final class ServerNetworkingImpl { } public static Packet createS2CPacket(Identifier channel, PacketByteBuf buf) { - return new CustomPayloadS2CPacket(new PacketByteBufPayload(channel, buf)); + return new CustomPayloadS2CPacket(new UntypedPayload(channel, buf)); } public static Packet createS2CPacket(FabricPacket packet) { Objects.requireNonNull(packet, "Packet cannot be null"); Objects.requireNonNull(packet.getType(), "Packet#getType cannot return null"); - PacketByteBuf buf = PacketByteBufs.create(); - packet.write(buf); - return createS2CPacket(packet.getType().getId(), buf); + ResolvedPayload payload = new TypedPayload(packet); + if (NetworkingImpl.FORCE_PACKET_SERIALIZATION) payload = payload.resolve(null); + + return new CustomPayloadS2CPacket(payload); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerPlayNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerPlayNetworkAddon.java index 23adf62b1..c055c7b28 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerPlayNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerPlayNetworkAddon.java @@ -25,18 +25,20 @@ import net.minecraft.network.PacketByteBuf; import net.minecraft.network.packet.Packet; import net.minecraft.server.MinecraftServer; import net.minecraft.server.network.ServerPlayNetworkHandler; +import net.minecraft.server.network.ServerPlayerEntity; import net.minecraft.util.Identifier; import net.fabricmc.fabric.api.networking.v1.FabricPacket; +import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.fabricmc.fabric.api.networking.v1.S2CPlayChannelEvents; import net.fabricmc.fabric.api.networking.v1.ServerPlayConnectionEvents; import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.fabric.impl.networking.AbstractChanneledNetworkAddon; import net.fabricmc.fabric.impl.networking.ChannelInfoHolder; import net.fabricmc.fabric.impl.networking.NetworkingImpl; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvedPayload; -public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon { +public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon { private final ServerPlayNetworkHandler handler; private final MinecraftServer server; private boolean sentInitialRegisterPacket; @@ -62,19 +64,9 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< this.sentInitialRegisterPacket = true; } - /** - * Handles an incoming packet. - * - * @param payload the payload to handle - * @return true if the packet has been handled - */ - public boolean handle(PacketByteBufPayload payload) { - return this.handle(payload.id(), payload.data()); - } - @Override - protected void receive(ServerPlayNetworking.PlayChannelHandler handler, PacketByteBuf buf) { - handler.receive(this.server, this.handler.player, this.handler, buf, this); + protected void receive(Handler handler, ResolvedPayload payload) { + handler.receive(this.server, this.handler.player, this.handler, payload, this); } // impl details @@ -137,4 +129,8 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< protected boolean isReservedChannel(Identifier channelName) { return NetworkingImpl.isReservedCommonChannel(channelName); } + + public interface Handler { + void receive(MinecraftServer server, ServerPlayerEntity player, ServerPlayNetworkHandler handler, ResolvedPayload payload, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadC2SPacketMixin.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadC2SPacketMixin.java index bb1966cc2..d9e653e03 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadC2SPacketMixin.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadC2SPacketMixin.java @@ -28,8 +28,8 @@ import net.minecraft.network.packet.CustomPayload; import net.minecraft.network.packet.c2s.common.CustomPayloadC2SPacket; import net.minecraft.util.Identifier; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; -import net.fabricmc.fabric.impl.networking.payload.PayloadHelper; +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; +import net.fabricmc.fabric.impl.networking.payload.RetainedPayload; @Mixin(CustomPayloadC2SPacket.class) public class CustomPayloadC2SPacketMixin { @@ -43,6 +43,13 @@ public class CustomPayloadC2SPacketMixin { cancellable = true ) private static void readPayload(Identifier id, PacketByteBuf buf, CallbackInfoReturnable cir) { - cir.setReturnValue(new PacketByteBufPayload(id, PayloadHelper.read(buf, MAX_PAYLOAD_SIZE))); + int size = buf.readableBytes(); + + if (size < 0 || size > MAX_PAYLOAD_SIZE) { + throw new IllegalArgumentException("Payload may not be larger than " + MAX_PAYLOAD_SIZE + " bytes"); + } + + cir.setReturnValue(new RetainedPayload(id, PacketByteBufs.retainedSlice(buf))); + buf.skipBytes(size); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadS2CPacketMixin.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadS2CPacketMixin.java index 59e905f97..a56a63c36 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadS2CPacketMixin.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/CustomPayloadS2CPacketMixin.java @@ -28,8 +28,8 @@ import net.minecraft.network.packet.CustomPayload; import net.minecraft.network.packet.s2c.common.CustomPayloadS2CPacket; import net.minecraft.util.Identifier; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; -import net.fabricmc.fabric.impl.networking.payload.PayloadHelper; +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; +import net.fabricmc.fabric.impl.networking.payload.RetainedPayload; @Mixin(CustomPayloadS2CPacket.class) public class CustomPayloadS2CPacketMixin { @@ -43,6 +43,13 @@ public class CustomPayloadS2CPacketMixin { cancellable = true ) private static void readPayload(Identifier id, PacketByteBuf buf, CallbackInfoReturnable cir) { - cir.setReturnValue(new PacketByteBufPayload(id, PayloadHelper.read(buf, MAX_PAYLOAD_SIZE))); + int size = buf.readableBytes(); + + if (size < 0 || size > MAX_PAYLOAD_SIZE) { + throw new IllegalArgumentException("Payload may not be larger than " + MAX_PAYLOAD_SIZE + " bytes"); + } + + cir.setReturnValue(new RetainedPayload(id, PacketByteBufs.retainedSlice(buf))); + buf.skipBytes(size); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java index 22e31cf1a..2d90a2e21 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/mixin/networking/ServerCommonNetworkHandlerMixin.java @@ -26,7 +26,8 @@ import net.minecraft.network.packet.c2s.common.CustomPayloadC2SPacket; import net.minecraft.server.network.ServerCommonNetworkHandler; import net.fabricmc.fabric.impl.networking.NetworkHandlerExtensions; -import net.fabricmc.fabric.impl.networking.payload.PacketByteBufPayload; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.RetainedPayload; import net.fabricmc.fabric.impl.networking.server.ServerConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.server.ServerPlayNetworkAddon; @@ -34,7 +35,7 @@ import net.fabricmc.fabric.impl.networking.server.ServerPlayNetworkAddon; public abstract class ServerCommonNetworkHandlerMixin implements NetworkHandlerExtensions { @Inject(method = "onCustomPayload", at = @At("HEAD"), cancellable = true) private void handleCustomPayloadReceivedAsync(CustomPayloadC2SPacket packet, CallbackInfo ci) { - if (packet.payload() instanceof PacketByteBufPayload payload) { + if (packet.payload() instanceof ResolvablePayload payload) { boolean handled; if (getAddon() instanceof ServerPlayNetworkAddon addon) { @@ -47,8 +48,9 @@ public abstract class ServerCommonNetworkHandlerMixin implements NetworkHandlerE if (handled) { ci.cancel(); - } else { - payload.data().skipBytes(payload.data().readableBytes()); + } else if (payload instanceof RetainedPayload retained) { + retained.buf().skipBytes(retained.buf().readableBytes()); + retained.buf().release(); } } } diff --git a/fabric-networking-api-v1/src/test/java/net/fabricmc/fabric/test/networking/unit/CommonPacketTests.java b/fabric-networking-api-v1/src/test/java/net/fabricmc/fabric/test/networking/unit/CommonPacketTests.java index d3868a642..f735f7721 100644 --- a/fabric-networking-api-v1/src/test/java/net/fabricmc/fabric/test/networking/unit/CommonPacketTests.java +++ b/fabric-networking-api-v1/src/test/java/net/fabricmc/fabric/test/networking/unit/CommonPacketTests.java @@ -47,11 +47,9 @@ import net.minecraft.network.packet.CustomPayload; import net.minecraft.server.network.ServerConfigurationNetworkHandler; import net.minecraft.util.Identifier; -import net.fabricmc.fabric.api.client.networking.v1.ClientConfigurationNetworking; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.PacketSender; -import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking; import net.fabricmc.fabric.impl.networking.ChannelInfoHolder; import net.fabricmc.fabric.impl.networking.CommonPacketHandler; import net.fabricmc.fabric.impl.networking.CommonPacketsImpl; @@ -59,6 +57,8 @@ import net.fabricmc.fabric.impl.networking.CommonRegisterPayload; import net.fabricmc.fabric.impl.networking.CommonVersionPayload; import net.fabricmc.fabric.impl.networking.client.ClientConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.client.ClientNetworkingImpl; +import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload; +import net.fabricmc.fabric.impl.networking.payload.UntypedPayload; import net.fabricmc.fabric.impl.networking.server.ServerConfigurationNetworkAddon; import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; @@ -101,14 +101,14 @@ public class CommonPacketTests { // Test handling the version packet on the client @Test void handleVersionPacketClient() { - ClientConfigurationNetworking.ConfigurationChannelHandler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); assertNotNull(packetHandler); // Receive a packet from the server PacketByteBuf buf = PacketByteBufs.create(); buf.writeIntArray(new int[]{1, 2, 3}); - packetHandler.receive(null, clientNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, clientNetworkHandler, new UntypedPayload(null, buf), packetSender); // Assert the entire packet was read assertEquals(0, buf.readableBytes()); @@ -124,7 +124,7 @@ public class CommonPacketTests { // Test handling the version packet on the client, when the server sends unsupported versions @Test void handleVersionPacketClientUnsupported() { - ClientConfigurationNetworking.ConfigurationChannelHandler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); assertNotNull(packetHandler); // Receive a packet from the server @@ -132,7 +132,7 @@ public class CommonPacketTests { buf.writeIntArray(new int[]{2, 3}); // We only support version 1 assertThrows(UnsupportedOperationException.class, () -> { - packetHandler.receive(null, clientNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, clientNetworkHandler, new UntypedPayload(null, buf), packetSender); }); // Assert the entire packet was read @@ -142,14 +142,14 @@ public class CommonPacketTests { // Test handling the version packet on the server @Test void handleVersionPacketServer() { - ServerConfigurationNetworking.ConfigurationChannelHandler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); assertNotNull(packetHandler); // Receive a packet from the client PacketByteBuf buf = PacketByteBufs.create(); buf.writeIntArray(new int[]{1, 2, 3}); - packetHandler.receive(null, serverNetworkHandler, buf, null); + packetHandler.internal().receive(null, serverNetworkHandler, new UntypedPayload(null, buf), null); // Assert the entire packet was read assertEquals(0, buf.readableBytes()); @@ -159,7 +159,7 @@ public class CommonPacketTests { // Test handling the version packet on the server unsupported version @Test void handleVersionPacketServerUnsupported() { - ServerConfigurationNetworking.ConfigurationChannelHandler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonVersionPayload.PACKET_ID); assertNotNull(packetHandler); // Receive a packet from the client @@ -167,7 +167,7 @@ public class CommonPacketTests { buf.writeIntArray(new int[]{3}); // Server only supports version 1 assertThrows(UnsupportedOperationException.class, () -> { - packetHandler.receive(null, serverNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, serverNetworkHandler, new UntypedPayload(null, buf), packetSender); }); // Assert the entire packet was read @@ -177,7 +177,7 @@ public class CommonPacketTests { // Test handing the play registry packet on the client configuration handler @Test void handlePlayRegistryClient() { - ClientConfigurationNetworking.ConfigurationChannelHandler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); assertNotNull(packetHandler); when(clientAddon.getNegotiatedVersion()).thenReturn(1); @@ -188,7 +188,7 @@ public class CommonPacketTests { buf.writeString("play"); // Target phase buf.writeCollection(List.of(new Identifier("fabric", "test")), PacketByteBuf::writeIdentifier); - packetHandler.receive(null, clientNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, clientNetworkHandler, new UntypedPayload(null, buf), packetSender); // Assert the entire packet was read assertEquals(0, buf.readableBytes()); @@ -205,7 +205,7 @@ public class CommonPacketTests { // Test handling the configuration registry packet on the client configuration handler @Test void handleConfigurationRegistryClient() { - ClientConfigurationNetworking.ConfigurationChannelHandler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ClientNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); assertNotNull(packetHandler); when(clientAddon.getNegotiatedVersion()).thenReturn(1); @@ -217,7 +217,7 @@ public class CommonPacketTests { buf.writeString("configuration"); // Target phase buf.writeCollection(List.of(new Identifier("fabric", "test")), PacketByteBuf::writeIdentifier); - packetHandler.receive(null, clientNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, clientNetworkHandler, new UntypedPayload(null, buf), packetSender); // Assert the entire packet was read assertEquals(0, buf.readableBytes()); @@ -234,7 +234,7 @@ public class CommonPacketTests { // Test handing the play registry packet on the server configuration handler @Test void handlePlayRegistryServer() { - ServerConfigurationNetworking.ConfigurationChannelHandler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); assertNotNull(packetHandler); when(serverAddon.getNegotiatedVersion()).thenReturn(1); @@ -245,7 +245,7 @@ public class CommonPacketTests { buf.writeString("play"); // Target phase buf.writeCollection(List.of(new Identifier("fabric", "test")), PacketByteBuf::writeIdentifier); - packetHandler.receive(null, serverNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, serverNetworkHandler, new UntypedPayload(null, buf), packetSender); // Assert the entire packet was read assertEquals(0, buf.readableBytes()); @@ -255,7 +255,7 @@ public class CommonPacketTests { // Test handing the configuration registry packet on the server configuration handler @Test void handleConfigurationRegistryServer() { - ServerConfigurationNetworking.ConfigurationChannelHandler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); + ResolvablePayload.Handler packetHandler = ServerNetworkingImpl.CONFIGURATION.getHandler(CommonRegisterPayload.PACKET_ID); assertNotNull(packetHandler); when(serverAddon.getNegotiatedVersion()).thenReturn(1); @@ -266,7 +266,7 @@ public class CommonPacketTests { buf.writeString("configuration"); // Target phase buf.writeCollection(List.of(new Identifier("fabric", "test")), PacketByteBuf::writeIdentifier); - packetHandler.receive(null, serverNetworkHandler, buf, packetSender); + packetHandler.internal().receive(null, serverNetworkHandler, new UntypedPayload(null, buf), packetSender); // Assert the entire packet was read assertEquals(0, buf.readableBytes());