From a6f3ccfaf89eaee95a55ed0ec8f05b375229809d Mon Sep 17 00:00:00 2001 From: apple502j <33279053+apple502j@users.noreply.github.com> Date: Tue, 4 Apr 2023 12:55:12 +0100 Subject: [PATCH] Networking: introduce packet-object based API Signed-off-by: modmuss50 --- .../networking/v1/ClientPlayNetworking.java | 203 ++++++++++++++- .../api/networking/v1/FabricPacket.java | 78 ++++++ .../api/networking/v1/PacketSender.java | 34 +++ .../fabric/api/networking/v1/PacketType.java | 71 +++++ .../networking/v1/ServerPlayNetworking.java | 242 +++++++++++++++++- .../NetworkingKeybindPacketTest.java | 9 +- .../play/NetworkingPlayPacketClientTest.java | 18 +- .../play/NetworkingPlayPacketTest.java | 24 +- 8 files changed, 647 insertions(+), 32 deletions(-) create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/FabricPacket.java create mode 100644 fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketType.java 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 e3f81ab23..c95700577 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 @@ -23,12 +23,17 @@ import org.jetbrains.annotations.Nullable; import net.minecraft.client.MinecraftClient; import net.minecraft.client.network.ClientPlayNetworkHandler; -import net.minecraft.network.packet.Packet; +import net.minecraft.client.network.ClientPlayerEntity; import net.minecraft.network.PacketByteBuf; import net.minecraft.network.listener.ServerPlayPacketListener; +import net.minecraft.network.packet.Packet; import net.minecraft.util.Identifier; +import net.minecraft.util.thread.ThreadExecutor; +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.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; @@ -41,6 +46,9 @@ import net.fabricmc.fabric.impl.networking.client.ClientPlayNetworkAddon; * *

This class should be only used on the physical client and for the logical client. * + *

See {@link ServerPlayNetworking} for information on how to use the packet + * object-based API. + * * @see ClientLoginNetworking * @see ServerPlayNetworking */ @@ -49,9 +57,15 @@ public final class ClientPlayNetworking { * Registers a handler to a channel. * A global receiver is registered to all connections, in the present and future. * + *

The handler runs on the network thread. After reading the buffer there, access to game state + * must be performed in the render thread by calling {@link ThreadExecutor#execute(Runnable)}. + * *

If a handler is already registered to the {@code channel}, this method will return {@code false}, and no change will be made. * Use {@link #unregisterGlobalReceiver(Identifier)} to unregister the existing handler. * + *

For new code, {@link #registerGlobalReceiver(PacketType, PlayPacketHandler)} + * is preferred, as it is designed in a way that prevents thread safety issues. + * * @param channelName the id of the channel * @param channelHandler the handler * @return false if a handler is already registered to the channel @@ -62,6 +76,45 @@ public final class ClientPlayNetworking { return ClientNetworkingImpl.PLAY.registerGlobalReceiver(channelName, channelHandler); } + /** + * Registers a handler for a packet type. + * A global receiver is registered to all connections, in the present and future. + * + *

If a handler is already registered for the {@code type}, this method will return {@code false}, and no change will be made. + * Use {@link #unregisterGlobalReceiver(PacketType)} to unregister the existing handler. + * + * @param type the packet type + * @param handler the handler + * @return false if a handler is already registered to the channel + * @see ClientPlayNetworking#unregisterGlobalReceiver(PacketType) + * @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); + }); + } + } + }); + } + /** * Removes the handler of a channel. * A global receiver is registered to all connections, in the present and future. @@ -78,6 +131,25 @@ public final class ClientPlayNetworking { return ClientNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName); } + /** + * Removes the handler for a packet type. + * A global receiver is registered to all connections, in the present and future. + * + *

The {@code type} is guaranteed not to have an associated handler after this call. + * + * @param type the packet type + * @return the previous handler, or {@code null} if no handler was bound to the channel, + * or it was not registered using {@link #registerGlobalReceiver(PacketType, PlayPacketHandler)} + * @see ClientPlayNetworking#registerGlobalReceiver(PacketType, PlayPacketHandler) + * @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; + } + /** * Gets all channel names which global receivers are registered for. * A global receiver is registered to all connections, in the present and future. @@ -97,6 +169,9 @@ public final class ClientPlayNetworking { *

For example, if you only register a receiver using this method when a {@linkplain ClientLoginNetworking#registerGlobalReceiver(Identifier, ClientLoginNetworking.LoginQueryRequestHandler)} * login query has been received, you should use {@link ClientPlayConnectionEvents#INIT} to register the channel handler. * + *

For new code, {@link #registerReceiver(PacketType, PlayPacketHandler)} + * is preferred, as it is designed in a way that prevents thread safety issues. + * * @param channelName the id of the channel * @return false if a handler is already registered to the channel * @throws IllegalStateException if the client is not connected to a server @@ -112,6 +187,47 @@ public final class ClientPlayNetworking { throw new IllegalStateException("Cannot register receiver while not in game!"); } + /** + * Registers a handler for a packet type. + * + *

If a handler is already registered for the {@code type}, this method will return {@code false}, and no change will be made. + * Use {@link #unregisterReceiver(PacketType)} to unregister the existing handler. + * + *

For example, if you only register a receiver using this method when a {@linkplain ClientLoginNetworking#registerGlobalReceiver(Identifier, ClientLoginNetworking.LoginQueryRequestHandler)} + * login query has been received, you should use {@link ClientPlayConnectionEvents#INIT} to register the channel handler. + * + * @param type the packet type + * @param handler the handler + * @return {@code false} if a handler is already registered for the type + * @throws IllegalStateException if the client is not connected to a server + * @see ClientPlayConnectionEvents#INIT + */ + public static boolean registerReceiver(PacketType type, PlayPacketHandler handler) { + return registerReceiver(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); + }); + } + } + }); + } + /** * Removes the handler of a channel. * @@ -132,6 +248,23 @@ public final class ClientPlayNetworking { throw new IllegalStateException("Cannot unregister receiver while not in game!"); } + /** + * Removes the handler for a packet type. + * + *

The {@code type} is guaranteed not to have an associated handler after this call. + * + * @param type the packet type + * @return the previous handler, or {@code null} if no handler was bound to the channel, + * or it was not registered using {@link #registerReceiver(PacketType, PlayPacketHandler)} + * @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; + } + /** * Gets all the channel names that the client can receive packets on. * @@ -168,7 +301,7 @@ public final class ClientPlayNetworking { * Checks if the connected server declared the ability to receive a packet on a specified channel name. * * @param channelName the channel name - * @return True if the connected server has declared the ability to receive a packet on the specified channel. + * @return {@code true} if the connected server has declared the ability to receive a packet on the specified channel. * False if the client is not in game. */ public static boolean canSend(Identifier channelName) throws IllegalArgumentException { @@ -180,6 +313,17 @@ public final class ClientPlayNetworking { return false; } + /** + * Checks if the connected server declared the ability to receive a packet on a specified channel name. + * This returns {@code false} if the client is not in game. + * + * @param type the packet type + * @return {@code true} if the connected server has declared the ability to receive a packet on the specified channel + */ + public static boolean canSend(PacketType type) { + return canSend(type.getId()); + } + /** * Creates a packet which may be sent to the connected server. * @@ -226,6 +370,21 @@ public final class ClientPlayNetworking { throw new IllegalStateException("Cannot send packets when not in game!"); } + /** + * Sends a packet to the connected server. + * + * @param packet the packet + * @throws IllegalStateException if the client is not connected to a server + */ + public static void send(T packet) { + Objects.requireNonNull(packet, "Packet cannot be null"); + Objects.requireNonNull(packet.getType(), "Packet#getType cannot return null"); + + PacketByteBuf buf = PacketByteBufs.create(); + packet.write(buf); + send(packet.getType().getId(), buf); + } + private ClientPlayNetworking() { } @@ -239,11 +398,11 @@ public final class ClientPlayNetworking { * *

An example usage of this is to display an overlay message: *

{@code
-		 * ClientPlayNetworking.registerReceiver(new Identifier("mymod", "overlay"), (client, handler, buf, responseSender) -&rt; {
+		 * ClientPlayNetworking.registerReceiver(new Identifier("mymod", "overlay"), (client, handler, buf, responseSender) -> {
 		 * 	String message = buf.readString(32767);
 		 *
 		 * 	// All operations on the server or world must be executed on the server thread
-		 * 	client.execute(() -&rt; {
+		 * 	client.execute(() -> {
 		 * 		client.inGameHud.setOverlayMessage(message, true);
 		 * 	});
 		 * });
@@ -255,4 +414,40 @@ 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
+	 */
+	@FunctionalInterface
+	public interface PlayPacketHandler {
+		/**
+		 * Handles the incoming packet. This is called on the render thread, and can safely
+		 * call client methods.
+		 *
+		 * 

An example usage of this is to display an overlay message: + *

{@code
+		 * // See FabricPacket for creating the packet
+		 * ClientPlayNetworking.registerReceiver(OVERLAY_PACKET_TYPE, (player, packet, responseSender) -> {
+		 * 	MinecraftClient.getInstance().inGameHud.setOverlayMessage(packet.message(), true);
+		 * });
+		 * }
+ * + *

The network handler can be accessed via {@link ClientPlayerEntity#networkHandler}. + * + * @param packet the packet + * @param player the player that received the packet + * @param responseSender the packet sender + * @see FabricPacket + */ + void receive(T packet, ClientPlayerEntity player, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/FabricPacket.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/FabricPacket.java new file mode 100644 index 000000000..7d3ec0d7c --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/FabricPacket.java @@ -0,0 +1,78 @@ +/* + * 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.api.networking.v1; + +import net.minecraft.network.PacketByteBuf; +import net.minecraft.server.network.ServerPlayerEntity; + +/** + * A packet to be sent using Networking API. An instance of this class is created + * each time the packet is sent. This can be used on both the client and the server. + * + *

Implementations should have fields of values sent over the network. + * For example, a packet consisting of two integers should have two {@code int} + * fields with appropriate name. This is written to the buffer in {@link #write}. + * The packet should have two constructors: one that creates a packet on the sender, + * which initializes the fields to be written, and one that takes a {@link PacketByteBuf} + * and reads the packet. + * + *

For each packet class, a corresponding {@link PacketType} instance should be created. + * The type should be stored in a {@code static final} field, and {@link #getType} should + * return that type. + * + *

Example of a packet: + *

{@code
+ * public record BoomPacket(boolean fire) implements FabricPacket {
+ * 	public static final PacketType TYPE = PacketType.create(new Identifier("example:boom"), BoomPacket::new);
+ *
+ * 	public BoomPacket(PacketByteBuf buf) {
+ * 		this(buf.readBoolean());
+ * 	}
+ *
+ * 	@Override
+ * 	public void write(PacketByteBuf buf) {
+ * 		buf.writeBoolean(this.fire);
+ * 	}
+ *
+ * 	@Override
+ * 	public PacketType getType() {
+ * 		return TYPE;
+ * 	}
+ * }
+ * }
+ * + * @see ServerPlayNetworking#registerGlobalReceiver(PacketType, ServerPlayNetworking.PlayPacketHandler) + * @see ServerPlayNetworking#send(ServerPlayerEntity, PacketType, FabricPacket) + * @see PacketSender#sendPacket(PacketType, FabricPacket) + */ +public interface FabricPacket { + /** + * Writes the contents of this packet to the buffer. + * @param buf the output buffer + */ + void write(PacketByteBuf buf); + + /** + * Returns the packet type of this packet. + * + *

Implementations should store the packet type instance in a {@code static final} + * field and return that here, instead of creating a new instance. + * + * @return the type of this packet + */ + PacketType getType(); +} diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketSender.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketSender.java index 6966e0aba..32564afef 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketSender.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketSender.java @@ -50,6 +50,16 @@ public interface PacketSender { */ void sendPacket(Packet packet); + /** + * Sends a packet. + * @param packet the packet + */ + default void sendPacket(T packet) { + PacketByteBuf buf = PacketByteBufs.create(); + packet.write(buf); + sendPacket(packet.getType().getId(), buf); + } + /** * Sends a packet. * @@ -58,6 +68,18 @@ public interface PacketSender { */ void sendPacket(Packet packet, @Nullable GenericFutureListener> callback); + /** + * Sends a packet. + * + * @param packet the packet + * @param callback an optional callback to execute after the packet is sent, may be {@code null}. The callback may also accept a {@link ChannelFutureListener}. + */ + default void sendPacket(T packet, @Nullable GenericFutureListener> callback) { + PacketByteBuf buf = PacketByteBufs.create(); + packet.write(buf); + sendPacket(packet.getType().getId(), buf, callback); + } + /** * Sends a packet. * @@ -66,6 +88,18 @@ public interface PacketSender { */ void sendPacket(Packet packet, @Nullable PacketCallbacks callback); + /** + * Sends a packet. + * + * @param packet the packet + * @param callback an optional callback to execute after the packet is sent, may be {@code null}. The callback may also accept a {@link ChannelFutureListener}. + */ + default void sendPacket(T packet, @Nullable PacketCallbacks callback) { + PacketByteBuf buf = PacketByteBufs.create(); + packet.write(buf); + sendPacket(packet.getType().getId(), buf, callback); + } + /** * Sends a packet to a channel. * diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketType.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketType.java new file mode 100644 index 000000000..25f6a1f5e --- /dev/null +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/PacketType.java @@ -0,0 +1,71 @@ +/* + * 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.api.networking.v1; + +import java.util.function.Function; + +import net.minecraft.network.PacketByteBuf; +import net.minecraft.util.Identifier; + +/** + * A type of packet. An instance of this should be created per a {@link FabricPacket} implementation. + * This holds the channel ID used for the packet. + * + * @param the type of the packet + * @see FabricPacket + */ +public final class PacketType { + private final Identifier id; + private final Function constructor; + + private PacketType(Identifier id, Function constructor) { + this.id = id; + this.constructor = constructor; + } + + /** + * Creates a new packet type. + * @param id the channel ID used for the packets + * @param constructor the reader that reads the received buffer + * @param

the type of the packet + * @return the newly created type + */ + public static

PacketType

create(Identifier id, Function constructor) { + return new PacketType<>(id, constructor); + } + + /** + * Returns the identifier of the channel used to send the packet. + * @return the identifier of the associated channel. + */ + public Identifier getId() { + return id; + } + + /** + * Reads the packet from the buffer. + * @param buf the buffer + * @return the packet + */ + public T read(PacketByteBuf buf) { + try { + return this.constructor.apply(buf); + } catch (RuntimeException e) { + throw new RuntimeException("Error while handling packet \"%s\": %s".formatted(this.id, e.getMessage()), e); + } + } +} 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 e03766316..9d3c08298 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 @@ -28,6 +28,7 @@ import net.minecraft.server.MinecraftServer; import net.minecraft.server.network.ServerPlayNetworkHandler; import net.minecraft.server.network.ServerPlayerEntity; import net.minecraft.util.Identifier; +import net.minecraft.util.thread.ThreadExecutor; import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; @@ -38,6 +39,26 @@ import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; * *

This class should be only used for the logical server. * + *

Packet object-based API

+ * + *

This class provides a classic registration method, {@link #registerGlobalReceiver(Identifier, PlayChannelHandler)}, + * and a newer method utilizing packet objects, {@link #registerGlobalReceiver(PacketType, PlayPacketHandler)}. + * For most mods, using the newer method will improve the readability of the code by separating packet + * reading/writing code to a separate class. Additionally, the newer method executes the callback in the + * server thread, ensuring thread safety. For this reason using the newer method is highly recommended. + * The two methods are network-compatible with each other, so long as the buffer contents are read and written + * in the same order. + * + *

The newer, packet object-based API involves three classes: + * + *

    + *
  • A class implementing {@link FabricPacket} that is "sent" over the network
  • + *
  • {@link PacketType} instance, which represents the packet's type (and its channel)
  • + *
  • {@link PlayPacketHandler}, which handles the packet (usually implemented as a functional interface)
  • + *
+ * + *

See the documentation on each class for more information. + * * @see ServerLoginNetworking */ public final class ServerPlayNetworking { @@ -45,9 +66,15 @@ public final class ServerPlayNetworking { * Registers a handler to a channel. * A global receiver is registered to all connections, in the present and future. * + *

The handler runs on the network thread. After reading the buffer there, the world + * must be modified in the server thread by calling {@link ThreadExecutor#execute(Runnable)}. + * *

If a handler is already registered to the {@code channel}, this method will return {@code false}, and no change will be made. * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, Identifier)} to unregister the existing handler. * + *

For new code, {@link #registerGlobalReceiver(PacketType, PlayPacketHandler)} + * is preferred, as it is designed in a way that prevents thread safety issues. + * * @param channelName the id of the channel * @param channelHandler the handler * @return false if a handler is already registered to the channel @@ -58,6 +85,45 @@ public final class ServerPlayNetworking { return ServerNetworkingImpl.PLAY.registerGlobalReceiver(channelName, channelHandler); } + /** + * Registers a handler for a packet type. + * A global receiver is registered to all connections, in the present and future. + * + *

If a handler is already registered for the {@code type}, this method will return {@code false}, and no change will be made. + * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, PacketType)} to unregister the existing handler. + * + * @param type the packet type + * @param handler the handler + * @return {@code false} if a handler is already registered to the channel + * @see ServerPlayNetworking#unregisterGlobalReceiver(PacketType) + * @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); + }); + } + } + }); + } + /** * Removes the handler of a channel. * A global receiver is registered to all connections, in the present and future. @@ -74,6 +140,25 @@ public final class ServerPlayNetworking { return ServerNetworkingImpl.PLAY.unregisterGlobalReceiver(channelName); } + /** + * Removes the handler for a packet type. + * A global receiver is registered to all connections, in the present and future. + * + *

The {@code type} is guaranteed not to have an associated handler after this call. + * + * @param type the packet type + * @return the previous handler, or {@code null} if no handler was bound to the channel, + * or it was not registered using {@link #registerGlobalReceiver(PacketType, PlayPacketHandler)} + * @see ServerPlayNetworking#registerGlobalReceiver(PacketType, PlayPacketHandler) + * @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; + } + /** * Gets all channel names which global receivers are registered for. * A global receiver is registered to all connections, in the present and future. @@ -89,12 +174,18 @@ public final class ServerPlayNetworking { * This method differs from {@link ServerPlayNetworking#registerGlobalReceiver(Identifier, PlayChannelHandler)} since * the channel handler will only be applied to the player represented by the {@link ServerPlayNetworkHandler}. * + *

The handler runs on the network thread. After reading the buffer there, the world + * must be modified in the server thread by calling {@link ThreadExecutor#execute(Runnable)}. + * *

For example, if you only register a receiver using this method when a {@linkplain ServerLoginNetworking#registerGlobalReceiver(Identifier, ServerLoginNetworking.LoginQueryResponseHandler)} * login response has been received, you should use {@link ServerPlayConnectionEvents#INIT} to register the channel handler. * *

If a handler is already registered to the {@code channelName}, this method will return {@code false}, and no change will be made. * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, Identifier)} to unregister the existing handler. * + *

For new code, {@link #registerReceiver(ServerPlayNetworkHandler, PacketType, PlayPacketHandler)} + * is preferred, as it is designed in a way that prevents thread safety issues. + * * @param networkHandler the handler * @param channelName the id of the channel * @param channelHandler the handler @@ -107,6 +198,49 @@ public final class ServerPlayNetworking { return ServerNetworkingImpl.getAddon(networkHandler).registerChannel(channelName, channelHandler); } + /** + * Registers a handler for a packet type. + * This method differs from {@link ServerPlayNetworking#registerGlobalReceiver(PacketType, PlayPacketHandler)} since + * the channel handler will only be applied to the player represented by the {@link ServerPlayNetworkHandler}. + * + *

For example, if you only register a receiver using this method when a {@linkplain ServerLoginNetworking#registerGlobalReceiver(Identifier, ServerLoginNetworking.LoginQueryResponseHandler)} + * login response has been received, you should use {@link ServerPlayConnectionEvents#INIT} to register the channel handler. + * + *

If a handler is already registered for the {@code type}, this method will return {@code false}, and no change will be made. + * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, PacketType)} to unregister the existing handler. + * + * @param networkHandler the network handler + * @param type the packet type + * @param handler the handler + * @return {@code false} if a handler is already registered to the channel name + * @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); + }); + } + } + }); + } + /** * Removes the handler of a channel. * @@ -122,6 +256,22 @@ public final class ServerPlayNetworking { return ServerNetworkingImpl.getAddon(networkHandler).unregisterChannel(channelName); } + /** + * Removes the handler for a packet type. + * + *

The {@code type} is guaranteed not to have an associated handler after this call. + * + * @param type the type of the packet + * @return the previous handler, or {@code null} if no handler was bound to the channel, + * 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; + } + /** * Gets all the channel names that the server can receive packets on. * @@ -162,7 +312,7 @@ public final class ServerPlayNetworking { * Gets all channel names that a connected client declared the ability to receive a packets on. * * @param handler the network handler - * @return True if the connected client has declared the ability to receive a packet on the specified channel + * @return {@code true} if the connected client has declared the ability to receive a packet on the specified channel */ public static Set getSendable(ServerPlayNetworkHandler handler) { Objects.requireNonNull(handler, "Server play network handler cannot be null"); @@ -175,7 +325,7 @@ public final class ServerPlayNetworking { * * @param player the player * @param channelName the channel name - * @return True if the connected client has declared the ability to receive a packet on the specified channel + * @return {@code true} if the connected client has declared the ability to receive a packet on the specified channel */ public static boolean canSend(ServerPlayerEntity player, Identifier channelName) { Objects.requireNonNull(player, "Server player entity cannot be null"); @@ -183,12 +333,25 @@ public final class ServerPlayNetworking { return canSend(player.networkHandler, channelName); } + /** + * Checks if the connected client declared the ability to receive a specific type of packet. + * + * @param player the player + * @param type the packet type + * @return {@code true} if the connected client has declared the ability to receive a specific type of packet + */ + public static boolean canSend(ServerPlayerEntity player, PacketType type) { + Objects.requireNonNull(player, "Server player entity cannot be null"); + + return canSend(player.networkHandler, type.getId()); + } + /** * Checks if the connected client declared the ability to receive a packet on a specified channel name. * * @param handler the network handler * @param channelName the channel name - * @return True if the connected client has declared the ability to receive a packet on the specified channel + * @return {@code true} if the connected client has declared the ability to receive a packet on the specified channel */ public static boolean canSend(ServerPlayNetworkHandler handler, Identifier channelName) { Objects.requireNonNull(handler, "Server play network handler cannot be null"); @@ -197,6 +360,20 @@ public final class ServerPlayNetworking { return ServerNetworkingImpl.getAddon(handler).getSendableChannels().contains(channelName); } + /** + * Checks if the connected client declared the ability to receive a specific type of packet. + * + * @param handler the network handler + * @param type the packet type + * @return {@code true} if the connected client has declared the ability to receive a specific type of packet + */ + public static boolean canSend(ServerPlayNetworkHandler handler, PacketType type) { + Objects.requireNonNull(handler, "Server play network handler cannot be null"); + Objects.requireNonNull(type, "Packet type cannot be null"); + + return ServerNetworkingImpl.getAddon(handler).getSendableChannels().contains(type.getId()); + } + /** * Creates a packet which may be sent to a connected client. * @@ -250,6 +427,22 @@ public final class ServerPlayNetworking { player.networkHandler.sendPacket(createS2CPacket(channelName, buf)); } + /** + * Sends a packet to a player. + * + * @param player the player to send the packet to + * @param packet the packet + */ + public static void send(ServerPlayerEntity player, T packet) { + Objects.requireNonNull(player, "Server player entity cannot be null"); + Objects.requireNonNull(packet, "Packet cannot be null"); + Objects.requireNonNull(packet.getType(), "Packet#getType cannot return null"); + + PacketByteBuf buf = PacketByteBufs.create(); + packet.write(buf); + player.networkHandler.sendPacket(createS2CPacket(packet.getType().getId(), buf)); + } + // Helper methods /** @@ -272,15 +465,15 @@ public final class ServerPlayNetworking { * Handles an incoming packet. * *

This method is executed on {@linkplain io.netty.channel.EventLoop netty's event loops}. - * Modification to the game should be {@linkplain net.minecraft.util.thread.ThreadExecutor#submit(Runnable) scheduled} using the provided Minecraft server instance. + * Modification to the game should be {@linkplain ThreadExecutor#submit(Runnable) scheduled} using the provided Minecraft server instance. * *

An example usage of this is to create an explosion where the player is looking: *

{@code
-		 * ServerPlayNetworking.registerReceiver(new Identifier("mymod", "boom"), (server, player, handler, buf, responseSender) -&rt; {
+		 * ServerPlayNetworking.registerReceiver(new Identifier("mymod", "boom"), (server, player, handler, buf, responseSender) -> {
 		 * 	boolean fire = buf.readBoolean();
 		 *
 		 * 	// All operations on the server or world must be executed on the server thread
-		 * 	server.execute(() -&rt; {
+		 * 	server.execute(() -> {
 		 * 		ModPacketHandler.createExplosion(player, fire);
 		 * 	});
 		 * });
@@ -293,4 +486,41 @@ 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
+	 */
+	@FunctionalInterface
+	public interface PlayPacketHandler {
+		/**
+		 * Handles the incoming packet. This is called on the server thread, and can safely
+		 * manipulate the world.
+		 *
+		 * 

An example usage of this is to create an explosion where the player is looking: + *

{@code
+		 * // See FabricPacket for creating the packet
+		 * ServerPlayNetworking.registerReceiver(BOOM_PACKET_TYPE, (player, packet, responseSender) -> {
+		 * 	ModPacketHandler.createExplosion(player, packet.fire());
+		 * });
+		 * }
+ * + *

The server and the network handler can be accessed via {@link ServerPlayerEntity#server} + * and {@link ServerPlayerEntity#networkHandler}, respectively. + * + * @param packet the packet + * @param player the player that received the packet + * @param responseSender the packet sender + * @see FabricPacket + */ + void receive(T packet, ServerPlayerEntity player, PacketSender responseSender); + } } diff --git a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/keybindreciever/NetworkingKeybindPacketTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/keybindreciever/NetworkingKeybindPacketTest.java index 353a42d56..64fc63acf 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/keybindreciever/NetworkingKeybindPacketTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/keybindreciever/NetworkingKeybindPacketTest.java @@ -36,16 +36,11 @@ public final class NetworkingKeybindPacketTest implements ModInitializer { public static final Identifier KEYBINDING_PACKET_ID = NetworkingTestmods.id("keybind_press_test"); private static void receive(MinecraftServer server, ServerPlayerEntity player, ServerPlayNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender) { - // TODO: Can we send chat off the server thread? - server.execute(() -> { - player.sendMessage(Text.literal("So you pressed ").append(Text.keybind("fabric-networking-api-v1-testmod-keybind").styled(style -> style.withFormatting(Formatting.BLUE))), false); - }); + server.execute(() -> player.sendMessage(Text.literal("So you pressed ").append(Text.keybind("fabric-networking-api-v1-testmod-keybind").styled(style -> style.withFormatting(Formatting.BLUE))), false)); } @Override public void onInitialize() { - ServerPlayConnectionEvents.INIT.register((handler, server) -> { - ServerPlayNetworking.registerReceiver(handler, KEYBINDING_PACKET_ID, NetworkingKeybindPacketTest::receive); - }); + ServerPlayConnectionEvents.INIT.register((handler, server) -> ServerPlayNetworking.registerReceiver(handler, KEYBINDING_PACKET_ID, NetworkingKeybindPacketTest::receive)); } } diff --git a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketClientTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketClientTest.java index 576b6c004..61e115f8d 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketClientTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketClientTest.java @@ -17,27 +17,21 @@ package net.fabricmc.fabric.test.networking.play; import net.minecraft.client.MinecraftClient; -import net.minecraft.client.network.ClientPlayNetworkHandler; -import net.minecraft.network.PacketByteBuf; -import net.minecraft.text.Text; +import net.minecraft.client.network.ClientPlayerEntity; import net.fabricmc.api.ClientModInitializer; 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.PacketSender; -public final class NetworkingPlayPacketClientTest implements ClientModInitializer { +public final class NetworkingPlayPacketClientTest implements ClientModInitializer, ClientPlayNetworking.PlayPacketHandler { @Override public void onInitializeClient() { - //ClientPlayNetworking.registerGlobalReceiver(NetworkingPlayPacketTest.TEST_CHANNEL, this::receive); - - ClientPlayConnectionEvents.INIT.register((handler, client) -> { - ClientPlayNetworking.registerReceiver(NetworkingPlayPacketTest.TEST_CHANNEL, (client1, handler1, buf, sender1) -> receive(handler1, sender1, client1, buf)); - }); + ClientPlayConnectionEvents.INIT.register((handler, client) -> ClientPlayNetworking.registerReceiver(NetworkingPlayPacketTest.OverlayPacket.PACKET_TYPE, this)); } - private void receive(ClientPlayNetworkHandler handler, PacketSender sender, MinecraftClient client, PacketByteBuf buf) { - Text text = buf.readText(); - client.execute(() -> client.inGameHud.setOverlayMessage(text, true)); + @Override + public void receive(NetworkingPlayPacketTest.OverlayPacket packet, ClientPlayerEntity player, PacketSender sender) { + MinecraftClient.getInstance().inGameHud.setOverlayMessage(packet.message(), true); } } diff --git a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java index 8d5112010..52da7a1db 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java @@ -35,7 +35,9 @@ import net.minecraft.util.Identifier; import net.fabricmc.api.ModInitializer; import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback; +import net.fabricmc.fabric.api.networking.v1.FabricPacket; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; +import net.fabricmc.fabric.api.networking.v1.PacketType; import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.fabric.test.networking.NetworkingTestmods; @@ -43,9 +45,7 @@ public final class NetworkingPlayPacketTest implements ModInitializer { public static final Identifier TEST_CHANNEL = NetworkingTestmods.id("test_channel"); public static void sendToTestChannel(ServerPlayerEntity player, String stuff) { - PacketByteBuf buf = PacketByteBufs.create(); - buf.writeText(Text.literal(stuff)); - ServerPlayNetworking.send(player, TEST_CHANNEL, buf); + ServerPlayNetworking.send(player, new OverlayPacket(Text.literal(stuff))); NetworkingTestmods.LOGGER.info("Sent custom payload packet in {}", TEST_CHANNEL); } @@ -80,4 +80,22 @@ public final class NetworkingPlayPacketTest implements ModInitializer { NetworkingPlayPacketTest.registerCommand(dispatcher); }); } + + public record OverlayPacket(Text message) implements FabricPacket { + public static final PacketType PACKET_TYPE = PacketType.create(TEST_CHANNEL, OverlayPacket::new); + + public OverlayPacket(PacketByteBuf buf) { + this(buf.readText()); + } + + @Override + public void write(PacketByteBuf buf) { + buf.writeText(this.message); + } + + @Override + public PacketType getType() { + return PACKET_TYPE; + } + } }