From 33708c72c955f5dd2914ec7ea1c586cb1d1d517a Mon Sep 17 00:00:00 2001 From: i509VCB <git@i509.me> Date: Tue, 8 Dec 2020 11:55:19 -0600 Subject: [PATCH] Fix networking v1 (#1205) * Handle registration of global handlers as early as possible. This may not be correct? * Do not handle packets on game thread * Add PLAY_READY event * READY -> JOIN, some javadoc clarifications and impl oversight fixes * Omit redundant PLAY and LOGIN prefixes in events * Checkstyle go brr --- .../v1/ClientLoginConnectionEvents.java | 29 ++++++++------- .../networking/v1/ClientLoginNetworking.java | 8 ++-- .../v1/ClientPlayConnectionEvents.java | 37 ++++++++++++++----- .../networking/v1/ClientPlayNetworking.java | 18 +++++---- .../v1/ServerLoginConnectionEvents.java | 30 +++++++-------- .../networking/v1/ServerLoginNetworking.java | 4 +- .../v1/ServerPlayConnectionEvents.java | 37 ++++++++++++++----- .../networking/v1/ServerPlayNetworking.java | 14 +++++-- .../AbstractChanneledNetworkAddon.java | 2 +- .../impl/networking/NetworkingImpl.java | 2 +- .../client/ClientLoginNetworkAddon.java | 6 +-- .../client/ClientPlayNetworkAddon.java | 33 ++++++++++------- .../server/ServerLoginNetworkAddon.java | 6 +-- .../server/ServerPlayNetworkAddon.java | 32 ++++++++++------ .../NetworkingChannelClientTest.java | 4 +- .../NetworkingKeybindPacketTest.java | 2 +- .../login/NetworkingLoginQueryTest.java | 4 +- .../play/NetworkingPlayPacketClientTest.java | 2 +- 18 files changed, 165 insertions(+), 105 deletions(-) diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginConnectionEvents.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginConnectionEvents.java index 55de31160..af5ccee64 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginConnectionEvents.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginConnectionEvents.java @@ -18,6 +18,7 @@ package net.fabricmc.fabric.api.client.networking.v1; import net.minecraft.client.MinecraftClient; import net.minecraft.client.network.ClientLoginNetworkHandler; +import net.minecraft.util.Identifier; import net.fabricmc.api.EnvType; import net.fabricmc.api.Environment; @@ -30,14 +31,14 @@ import net.fabricmc.fabric.api.event.EventFactory; @Environment(EnvType.CLIENT) public final class ClientLoginConnectionEvents { /** - * An event for when the client's login process has begun. + * Event indicating a connection entered the LOGIN state, ready for registering query request handlers. * This event may be used by mods to prepare their client side state. * This event does not guarantee that a login attempt will be successful. * - * <p>No packets should be sent when this event is invoked. + * @see ClientLoginNetworking#registerReceiver(Identifier, ClientLoginNetworking.LoginQueryRequestHandler) */ - public static final Event<LoginInit> LOGIN_INIT = EventFactory.createArrayBacked(LoginInit.class, callbacks -> (handler, client) -> { - for (LoginInit callback : callbacks) { + public static final Event<Init> INIT = EventFactory.createArrayBacked(Init.class, callbacks -> (handler, client) -> { + for (Init callback : callbacks) { callback.onLoginStart(handler, client); } }); @@ -56,8 +57,8 @@ public final class ClientLoginConnectionEvents { * * <p>No packets should be sent when this event is invoked. */ - public static final Event<LoginQueryStart> LOGIN_QUERY_START = EventFactory.createArrayBacked(LoginQueryStart.class, callbacks -> (handler, client) -> { - for (LoginQueryStart callback : callbacks) { + public static final Event<QueryStart> QUERY_START = EventFactory.createArrayBacked(QueryStart.class, callbacks -> (handler, client) -> { + for (QueryStart callback : callbacks) { callback.onLoginQueryStart(handler, client); } }); @@ -67,8 +68,8 @@ public final class ClientLoginConnectionEvents { * * <p>No packets should be sent when this event is invoked. */ - public static final Event<LoginDisconnect> LOGIN_DISCONNECT = EventFactory.createArrayBacked(LoginDisconnect.class, callbacks -> (handler, client) -> { - for (LoginDisconnect callback : callbacks) { + public static final Event<Disconnect> DISCONNECT = EventFactory.createArrayBacked(Disconnect.class, callbacks -> (handler, client) -> { + for (Disconnect callback : callbacks) { callback.onLoginDisconnect(handler, client); } }); @@ -77,29 +78,29 @@ public final class ClientLoginConnectionEvents { } /** - * @see ClientLoginConnectionEvents#LOGIN_INIT + * @see ClientLoginConnectionEvents#INIT */ @Environment(EnvType.CLIENT) @FunctionalInterface - public interface LoginInit { + public interface Init { void onLoginStart(ClientLoginNetworkHandler handler, MinecraftClient client); } /** - * @see ClientLoginConnectionEvents#LOGIN_QUERY_START + * @see ClientLoginConnectionEvents#QUERY_START */ @Environment(EnvType.CLIENT) @FunctionalInterface - public interface LoginQueryStart { + public interface QueryStart { void onLoginQueryStart(ClientLoginNetworkHandler handler, MinecraftClient client); } /** - * @see ClientLoginConnectionEvents#LOGIN_DISCONNECT + * @see ClientLoginConnectionEvents#DISCONNECT */ @Environment(EnvType.CLIENT) @FunctionalInterface - public interface LoginDisconnect { + public interface Disconnect { void onLoginDisconnect(ClientLoginNetworkHandler handler, MinecraftClient client); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginNetworking.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginNetworking.java index e336583ae..c076a478b 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginNetworking.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientLoginNetworking.java @@ -51,7 +51,7 @@ public final class ClientLoginNetworking { * A global receiver is registered to all connections, in the present and future. * * <p>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.</p> + * Use {@link #unregisterGlobalReceiver(Identifier)} to unregister the existing handler. * * @param channelName the id of the channel * @param queryHandler the handler @@ -67,7 +67,7 @@ public final class ClientLoginNetworking { * Removes the handler of a query request channel. * A global receiver is registered to all connections, in the present and future. * - * <p>The {@code channel} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channel} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel @@ -93,7 +93,7 @@ public final class ClientLoginNetworking { * Registers a handler to a query request channel. * * <p>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(Identifier)} to unregister the existing handler.</p> + * Use {@link #unregisterReceiver(Identifier)} to unregister the existing handler. * * @param channelName the id of the channel * @param queryHandler the handler @@ -117,7 +117,7 @@ public final class ClientLoginNetworking { /** * Removes the handler of a query request channel. * - * <p>The {@code channelName} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channelName} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel name diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayConnectionEvents.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayConnectionEvents.java index 1afc79c09..55cfe1f1e 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayConnectionEvents.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayConnectionEvents.java @@ -18,6 +18,7 @@ package net.fabricmc.fabric.api.client.networking.v1; import net.minecraft.client.MinecraftClient; import net.minecraft.client.network.ClientPlayNetworkHandler; +import net.minecraft.util.Identifier; import net.fabricmc.api.EnvType; import net.fabricmc.api.Environment; @@ -31,13 +32,25 @@ import net.fabricmc.fabric.api.networking.v1.PacketSender; @Environment(EnvType.CLIENT) public final class ClientPlayConnectionEvents { /** - * An event for the initialization of the client play network handler. + * Event indicating a connection entered the PLAY state, ready for registering channel handlers. + * + * @see ClientPlayNetworking#registerReceiver(Identifier, ClientPlayNetworking.PlayChannelHandler) + */ + public static final Event<Init> INIT = EventFactory.createArrayBacked(Init.class, callbacks -> (handler, client) -> { + for (Init callback : callbacks) { + callback.onPlayInit(handler, client); + } + }); + + /** + * An event for notification when the client play network handler is ready to send packets to the server. * * <p>At this stage, the network handler is ready to send packets to the server. + * Since the client's local state has been setup. */ - public static final Event<PlayInit> PLAY_INIT = EventFactory.createArrayBacked(PlayInit.class, callbacks -> (handler, sender, client) -> { - for (PlayInit callback : callbacks) { - callback.onPlayInit(handler, sender, client); + public static final Event<Join> JOIN = EventFactory.createArrayBacked(Join.class, callbacks -> (handler, sender, client) -> { + for (Join callback : callbacks) { + callback.onPlayReady(handler, sender, client); } }); @@ -46,8 +59,8 @@ public final class ClientPlayConnectionEvents { * * <p>No packets should be sent when this event is invoked. */ - public static final Event<PlayDisconnect> PLAY_DISCONNECT = EventFactory.createArrayBacked(PlayDisconnect.class, callbacks -> (handler, client) -> { - for (PlayDisconnect callback : callbacks) { + public static final Event<Disconnect> DISCONNECT = EventFactory.createArrayBacked(Disconnect.class, callbacks -> (handler, client) -> { + for (Disconnect callback : callbacks) { callback.onPlayDisconnect(handler, client); } }); @@ -57,13 +70,19 @@ public final class ClientPlayConnectionEvents { @Environment(EnvType.CLIENT) @FunctionalInterface - public interface PlayInit { - void onPlayInit(ClientPlayNetworkHandler handler, PacketSender sender, MinecraftClient client); + public interface Init { + void onPlayInit(ClientPlayNetworkHandler handler, MinecraftClient client); } @Environment(EnvType.CLIENT) @FunctionalInterface - public interface PlayDisconnect { + public interface Join { + void onPlayReady(ClientPlayNetworkHandler handler, PacketSender sender, MinecraftClient client); + } + + @Environment(EnvType.CLIENT) + @FunctionalInterface + public interface Disconnect { void onPlayDisconnect(ClientPlayNetworkHandler handler, MinecraftClient client); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java index d21818f68..c078fb5c0 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/client/networking/v1/ClientPlayNetworking.java @@ -51,7 +51,7 @@ public final class ClientPlayNetworking { * A global receiver is registered to all connections, in the present and future. * * <p>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.</p> + * Use {@link #unregisterGlobalReceiver(Identifier)} to unregister the existing handler. * * @param channelName the id of the channel * @param channelHandler the handler @@ -67,7 +67,7 @@ public final class ClientPlayNetworking { * Removes the handler of a channel. * A global receiver is registered to all connections, in the present and future. * - * <p>The {@code channel} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channel} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel @@ -93,11 +93,15 @@ public final class ClientPlayNetworking { * Registers a handler to a channel. * * <p>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(Identifier)} to unregister the existing handler.</p> + * Use {@link #unregisterReceiver(Identifier)} to unregister the existing handler. + * + * <p>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 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 + * @see ClientPlayConnectionEvents#INIT */ public static boolean registerReceiver(Identifier channelName, PlayChannelHandler channelHandler) { if (MinecraftClient.getInstance().getNetworkHandler() != null) { @@ -110,7 +114,7 @@ public final class ClientPlayNetworking { /** * Removes the handler of a channel. * - * <p>The {@code channel} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channelName} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel @@ -157,15 +161,15 @@ 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 - * @throws IllegalStateException if the client is not connected to a server + * @return 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 { if (MinecraftClient.getInstance().getNetworkHandler() != null) { return ClientNetworkingImpl.getAddon(MinecraftClient.getInstance().getNetworkHandler()).getSendableChannels().contains(channelName); } - throw new IllegalStateException("Cannot check whether the server can receive a packet while not in game!"); + return false; } /** diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginConnectionEvents.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginConnectionEvents.java index 260ef8e03..bee731ae2 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginConnectionEvents.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginConnectionEvents.java @@ -28,14 +28,12 @@ import net.fabricmc.fabric.api.event.EventFactory; */ public final class ServerLoginConnectionEvents { /** - * An event for the initialization of the server login network handler. - * This event may be used to register {@link ServerLoginNetworking.LoginQueryResponseHandler login query response handlers} - * using {@link ServerLoginNetworking#registerReceiver(ServerLoginNetworkHandler, Identifier, ServerLoginNetworking.LoginQueryResponseHandler)}. + * Event indicating a connection entered the LOGIN state, ready for registering query response handlers. * - * <p>No packets should be sent when this event is invoked. + * @see ServerLoginNetworking#registerReceiver(ServerLoginNetworkHandler, Identifier, ServerLoginNetworking.LoginQueryResponseHandler) */ - public static final Event<LoginInit> LOGIN_INIT = EventFactory.createArrayBacked(LoginInit.class, callbacks -> (handler, server) -> { - for (LoginInit callback : callbacks) { + public static final Event<Init> INIT = EventFactory.createArrayBacked(Init.class, callbacks -> (handler, server) -> { + for (Init callback : callbacks) { callback.onLoginInit(handler, server); } }); @@ -48,8 +46,8 @@ public final class ServerLoginConnectionEvents { * * <p>You may send login queries to the connected client using the provided {@link PacketSender}. */ - public static final Event<LoginQueryStart> LOGIN_QUERY_START = EventFactory.createArrayBacked(LoginQueryStart.class, callbacks -> (handler, server, sender, synchronizer) -> { - for (LoginQueryStart callback : callbacks) { + public static final Event<QueryStart> QUERY_START = EventFactory.createArrayBacked(QueryStart.class, callbacks -> (handler, server, sender, synchronizer) -> { + for (QueryStart callback : callbacks) { callback.onLoginStart(handler, server, sender, synchronizer); } }); @@ -59,8 +57,8 @@ public final class ServerLoginConnectionEvents { * * <p>No packets should be sent when this event is invoked. */ - public static final Event<LoginDisconnect> LOGIN_DISCONNECT = EventFactory.createArrayBacked(LoginDisconnect.class, callbacks -> (handler, server) -> { - for (LoginDisconnect callback : callbacks) { + public static final Event<Disconnect> DISCONNECT = EventFactory.createArrayBacked(Disconnect.class, callbacks -> (handler, server) -> { + for (Disconnect callback : callbacks) { callback.onLoginDisconnect(handler, server); } }); @@ -69,26 +67,26 @@ public final class ServerLoginConnectionEvents { } /** - * @see ServerLoginConnectionEvents#LOGIN_INIT + * @see ServerLoginConnectionEvents#INIT */ @FunctionalInterface - public interface LoginInit { + public interface Init { void onLoginInit(ServerLoginNetworkHandler handler, MinecraftServer server); } /** - * @see ServerLoginConnectionEvents#LOGIN_QUERY_START + * @see ServerLoginConnectionEvents#QUERY_START */ @FunctionalInterface - public interface LoginQueryStart { + public interface QueryStart { void onLoginStart(ServerLoginNetworkHandler handler, MinecraftServer server, PacketSender sender, ServerLoginNetworking.LoginSynchronizer synchronizer); } /** - * @see ServerLoginConnectionEvents#LOGIN_DISCONNECT + * @see ServerLoginConnectionEvents#DISCONNECT */ @FunctionalInterface - public interface LoginDisconnect { + public interface Disconnect { void onLoginDisconnect(ServerLoginNetworkHandler handler, MinecraftServer server); } } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginNetworking.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginNetworking.java index 8c4391e8d..2a4b37fde 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginNetworking.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerLoginNetworking.java @@ -88,7 +88,7 @@ public final class ServerLoginNetworking { * Registers a handler to a query response channel. * * <p>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(ServerLoginNetworkHandler, Identifier)} to unregister the existing handler.</p> + * Use {@link #unregisterReceiver(ServerLoginNetworkHandler, Identifier)} to unregister the existing handler. * * @param networkHandler the handler * @param channelName the id of the channel @@ -104,7 +104,7 @@ public final class ServerLoginNetworking { /** * Removes the handler of a query response channel. * - * <p>The {@code channelName} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channelName} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel name diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayConnectionEvents.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayConnectionEvents.java index 3901cbcc7..1957126c9 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayConnectionEvents.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayConnectionEvents.java @@ -18,6 +18,7 @@ package net.fabricmc.fabric.api.networking.v1; import net.minecraft.server.MinecraftServer; import net.minecraft.server.network.ServerPlayNetworkHandler; +import net.minecraft.util.Identifier; import net.fabricmc.fabric.api.event.Event; import net.fabricmc.fabric.api.event.EventFactory; @@ -27,23 +28,34 @@ import net.fabricmc.fabric.api.event.EventFactory; */ public final class ServerPlayConnectionEvents { /** - * An event for the initialization of the server play network handler. + * Event indicating a connection entered the PLAY state, ready for registering channel handlers. + * + * @see ServerPlayNetworking#registerReceiver(ServerPlayNetworkHandler, Identifier, ServerPlayNetworking.PlayChannelHandler) + */ + public static final Event<Init> INIT = EventFactory.createArrayBacked(Init.class, callbacks -> (handler, server) -> { + for (Init callback : callbacks) { + callback.onPlayInit(handler, server); + } + }); + + /** + * An event for notification when the server play network handler is ready to send packets to the client. * * <p>At this stage, the network handler is ready to send packets to the client. */ - public static final Event<PlayInit> PLAY_INIT = EventFactory.createArrayBacked(PlayInit.class, callbacks -> (handler, sender, server) -> { - for (PlayInit callback : callbacks) { - callback.onPlayInit(handler, sender, server); + public static final Event<Join> JOIN = EventFactory.createArrayBacked(Join.class, callbacks -> (handler, sender, server) -> { + for (Join callback : callbacks) { + callback.onPlayReady(handler, sender, server); } }); /** * An event for the disconnection of the server play network handler. * - * <p>No packets should be sent when this event is invoked.</p> + * <p>No packets should be sent when this event is invoked. */ - public static final Event<PlayDisconnect> PLAY_DISCONNECT = EventFactory.createArrayBacked(PlayDisconnect.class, callbacks -> (handler, server) -> { - for (PlayDisconnect callback : callbacks) { + public static final Event<Disconnect> DISCONNECT = EventFactory.createArrayBacked(Disconnect.class, callbacks -> (handler, server) -> { + for (Disconnect callback : callbacks) { callback.onPlayDisconnect(handler, server); } }); @@ -52,12 +64,17 @@ public final class ServerPlayConnectionEvents { } @FunctionalInterface - public interface PlayInit { - void onPlayInit(ServerPlayNetworkHandler handler, PacketSender sender, MinecraftServer server); + public interface Init { + void onPlayInit(ServerPlayNetworkHandler handler, MinecraftServer server); } @FunctionalInterface - public interface PlayDisconnect { + public interface Join { + void onPlayReady(ServerPlayNetworkHandler handler, PacketSender sender, MinecraftServer server); + } + + @FunctionalInterface + public interface Disconnect { void onPlayDisconnect(ServerPlayNetworkHandler handler, MinecraftServer server); } } 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 42552871d..f58967a49 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 @@ -48,7 +48,7 @@ public final class ServerPlayNetworking { * A global receiver is registered to all connections, in the present and future. * * <p>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.</p> + * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, Identifier)} to unregister the existing handler. * * @param channelName the id of the channel * @param channelHandler the handler @@ -64,7 +64,7 @@ public final class ServerPlayNetworking { * Removes the handler of a channel. * A global receiver is registered to all connections, in the present and future. * - * <p>The {@code channel} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channel} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel @@ -88,14 +88,20 @@ public final class ServerPlayNetworking { /** * Registers a handler to a channel. + * 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}. + * + * <p>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. * * <p>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.</p> + * Use {@link #unregisterReceiver(ServerPlayNetworkHandler, Identifier)} to unregister the existing handler. * * @param networkHandler the handler * @param channelName the id of the channel * @param channelHandler the handler * @return false if a handler is already registered to the channel name + * @see ServerPlayConnectionEvents#INIT */ public static boolean registerReceiver(ServerPlayNetworkHandler networkHandler, Identifier channelName, PlayChannelHandler channelHandler) { Objects.requireNonNull(networkHandler, "Network handler cannot be null"); @@ -106,7 +112,7 @@ public final class ServerPlayNetworking { /** * Removes the handler of a channel. * - * <p>The {@code channelName} is guaranteed not to have a handler after this call.</p> + * <p>The {@code channelName} is guaranteed not to have a handler after this call. * * @param channelName the id of the channel * @return the previous handler, or {@code null} if no handler was bound to the channel name 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 3af4a5796..30b733e69 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 @@ -106,7 +106,7 @@ public abstract class AbstractChanneledNetworkAddon<H> extends AbstractNetworkAd protected abstract void receive(H handler, PacketByteBuf buf); - public void sendChannelRegistrationPacket() { + protected void sendInitialChannelRegistrationPacket() { final PacketByteBuf buf = this.createRegistrationPacket(this.receiver.getChannels()); if (buf != 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 36980f279..37d6d017a 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 @@ -50,7 +50,7 @@ public final class NetworkingImpl { public static void init() { // Login setup - ServerLoginConnectionEvents.LOGIN_QUERY_START.register((handler, server, sender, synchronizer) -> { + ServerLoginConnectionEvents.QUERY_START.register((handler, server, sender, synchronizer) -> { // Send early registration packet PacketByteBuf buf = PacketByteBufs.create(); Collection<Identifier> channelsNames = ServerPlayNetworking.getGlobalReceivers(); diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientLoginNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientLoginNetworkAddon.java index e8fca6255..b1e5cfe77 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientLoginNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientLoginNetworkAddon.java @@ -52,7 +52,7 @@ public final class ClientLoginNetworkAddon extends AbstractNetworkAddon<ClientLo this.handler = handler; this.client = client; - ClientLoginConnectionEvents.LOGIN_INIT.invoker().onLoginStart(this.handler, this.client); + ClientLoginConnectionEvents.INIT.invoker().onLoginStart(this.handler, this.client); this.receiver.startSession(this); } @@ -70,7 +70,7 @@ public final class ClientLoginNetworkAddon extends AbstractNetworkAddon<ClientLo ClientLoginNetworking.registerReceiver(entry.getKey(), entry.getValue()); } - ClientLoginConnectionEvents.LOGIN_QUERY_START.invoker().onLoginQueryStart(this.handler, this.client); + ClientLoginConnectionEvents.QUERY_START.invoker().onLoginQueryStart(this.handler, this.client); this.firstResponse = false; } @@ -113,7 +113,7 @@ public final class ClientLoginNetworkAddon extends AbstractNetworkAddon<ClientLo @Override public void invokeDisconnectEvent() { - ClientLoginConnectionEvents.LOGIN_DISCONNECT.invoker().onLoginDisconnect(this.handler, this.client); + ClientLoginConnectionEvents.DISCONNECT.invoker().onLoginDisconnect(this.handler, this.client); this.receiver.endSession(this); } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java index e0dd1bdb1..6f93f07e0 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java @@ -40,7 +40,7 @@ import net.fabricmc.fabric.impl.networking.NetworkingImpl; public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon<ClientPlayNetworking.PlayChannelHandler> { private final ClientPlayNetworkHandler handler; private final MinecraftClient client; - private boolean canSendPackets; + private boolean sentInitialRegisterPacket; public ClientPlayNetworkAddon(ClientPlayNetworkHandler handler, MinecraftClient client) { super(ClientNetworkingImpl.PLAY, handler.getConnection(), "ClientPlayNetworkAddon for " + handler.getProfile().getName()); @@ -49,21 +49,23 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< // Must register pending channels via lateinit this.registerPendingChannels((ChannelInfoHolder) this.connection); - } - // also expose sendRegistration + // Register global receivers and attach to session + this.receiver.startSession(this); - public void onServerReady() { - // Register global receivers - for (Map.Entry<Identifier, ClientPlayNetworking.PlayChannelHandler> entry : ClientNetworkingImpl.PLAY.getHandlers().entrySet()) { + for (Map.Entry<Identifier, ClientPlayNetworking.PlayChannelHandler> entry : this.receiver.getHandlers().entrySet()) { this.registerChannel(entry.getKey(), entry.getValue()); } - this.sendChannelRegistrationPacket(); - this.canSendPackets = true; + ClientPlayConnectionEvents.INIT.invoker().onPlayInit(handler, this.client); + } - ClientPlayConnectionEvents.PLAY_INIT.invoker().onPlayInit(this.handler, this, this.client); - this.receiver.startSession(this); + public void onServerReady() { + ClientPlayConnectionEvents.JOIN.invoker().onPlayReady(this.handler, this, this.client); + + // The client cannot send any packets, including `minecraft:register` until after GameJoinS2CPacket is received. + this.sendInitialChannelRegistrationPacket(); + this.sentInitialRegisterPacket = true; } /** @@ -73,6 +75,11 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< * @return true if the packet has been handled */ public boolean handle(CustomPayloadS2CPacket packet) { + // Do not handle the packet on game thread + if (this.client.isOnThread()) { + return false; + } + PacketByteBuf buf = packet.getData(); try { @@ -112,7 +119,7 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override protected void handleRegistration(Identifier channelName) { // If we can already send packets, immediately send the register packet for this channel - if (this.canSendPackets) { + if (this.sentInitialRegisterPacket) { final PacketByteBuf buf = this.createRegistrationPacket(Collections.singleton(channelName)); if (buf != null) { @@ -124,7 +131,7 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override protected void handleUnregistration(Identifier channelName) { // If we can already send packets, immediately send the unregister packet for this channel - if (this.canSendPackets) { + if (this.sentInitialRegisterPacket) { final PacketByteBuf buf = this.createRegistrationPacket(Collections.singleton(channelName)); if (buf != null) { @@ -135,7 +142,7 @@ public final class ClientPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override public void invokeDisconnectEvent() { - ClientPlayConnectionEvents.PLAY_DISCONNECT.invoker().onPlayDisconnect(this.handler, this.client); + ClientPlayConnectionEvents.DISCONNECT.invoker().onPlayDisconnect(this.handler, this.client); this.receiver.endSession(this); } diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerLoginNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerLoginNetworkAddon.java index b77d64559..7ab0df371 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerLoginNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerLoginNetworkAddon.java @@ -64,7 +64,7 @@ public final class ServerLoginNetworkAddon extends AbstractNetworkAddon<ServerLo this.server = ((ServerLoginNetworkHandlerAccessor) handler).getServer(); this.queryIdFactory = QueryIdFactory.create(); - ServerLoginConnectionEvents.LOGIN_INIT.invoker().onLoginInit(handler, this.server); + ServerLoginConnectionEvents.INIT.invoker().onLoginInit(handler, this.server); this.receiver.startSession(this); } @@ -79,7 +79,7 @@ public final class ServerLoginNetworkAddon extends AbstractNetworkAddon<ServerLo ServerLoginNetworking.registerReceiver(this.handler, entry.getKey(), entry.getValue()); } - ServerLoginConnectionEvents.LOGIN_QUERY_START.invoker().onLoginStart(this.handler, this.server, this, this.waits::add); + ServerLoginConnectionEvents.QUERY_START.invoker().onLoginStart(this.handler, this.server, this, this.waits::add); this.firstQueryTick = false; } @@ -200,7 +200,7 @@ public final class ServerLoginNetworkAddon extends AbstractNetworkAddon<ServerLo @Override public void invokeDisconnectEvent() { - ServerLoginConnectionEvents.LOGIN_DISCONNECT.invoker().onLoginDisconnect(this.handler, this.server); + ServerLoginConnectionEvents.DISCONNECT.invoker().onLoginDisconnect(this.handler, this.server); this.receiver.endSession(this); } 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 695cb208f..5f5a1368e 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 @@ -38,7 +38,7 @@ import net.fabricmc.fabric.mixin.networking.accessor.CustomPayloadC2SPacketAcces public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon<ServerPlayNetworking.PlayChannelHandler> { private final ServerPlayNetworkHandler handler; private final MinecraftServer server; - private boolean canSendPackets = false; + private boolean sentInitialRegisterPacket; public ServerPlayNetworkAddon(ServerPlayNetworkHandler handler, MinecraftServer server) { super(ServerNetworkingImpl.PLAY, handler.getConnection(), "ServerPlayNetworkAddon for " + handler.player.getEntityName()); @@ -47,19 +47,22 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< // Must register pending channels via lateinit this.registerPendingChannels((ChannelInfoHolder) this.connection); - } - public void onClientReady() { - // Register global receivers - for (Map.Entry<Identifier, ServerPlayNetworking.PlayChannelHandler> entry : ServerNetworkingImpl.PLAY.getHandlers().entrySet()) { + // Register global receivers and attach to session + this.receiver.startSession(this); + + for (Map.Entry<Identifier, ServerPlayNetworking.PlayChannelHandler> entry : this.receiver.getHandlers().entrySet()) { this.registerChannel(entry.getKey(), entry.getValue()); } - this.sendChannelRegistrationPacket(); - this.canSendPackets = true; + ServerPlayConnectionEvents.INIT.invoker().onPlayInit(this.handler, this.server); + } - ServerPlayConnectionEvents.PLAY_INIT.invoker().onPlayInit(this.handler, this, this.server); - this.receiver.startSession(this); + public void onClientReady() { + ServerPlayConnectionEvents.JOIN.invoker().onPlayReady(this.handler, this, this.server); + + this.sendInitialChannelRegistrationPacket(); + this.sentInitialRegisterPacket = true; } /** @@ -69,6 +72,11 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< * @return true if the packet has been handled */ public boolean handle(CustomPayloadC2SPacket packet) { + // Do not handle the packet on game thread + if (this.server.isOnThread()) { + return false; + } + CustomPayloadC2SPacketAccessor access = (CustomPayloadC2SPacketAccessor) packet; return this.handle(access.getChannel(), access.getData()); } @@ -103,7 +111,7 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override protected void handleRegistration(Identifier channelName) { // If we can already send packets, immediately send the register packet for this channel - if (this.canSendPackets) { + if (this.sentInitialRegisterPacket) { final PacketByteBuf buf = this.createRegistrationPacket(Collections.singleton(channelName)); if (buf != null) { @@ -115,7 +123,7 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override protected void handleUnregistration(Identifier channelName) { // If we can already send packets, immediately send the unregister packet for this channel - if (this.canSendPackets) { + if (this.sentInitialRegisterPacket) { final PacketByteBuf buf = this.createRegistrationPacket(Collections.singleton(channelName)); if (buf != null) { @@ -126,7 +134,7 @@ public final class ServerPlayNetworkAddon extends AbstractChanneledNetworkAddon< @Override public void invokeDisconnectEvent() { - ServerPlayConnectionEvents.PLAY_DISCONNECT.invoker().onPlayDisconnect(this.handler, this.server); + ServerPlayConnectionEvents.DISCONNECT.invoker().onPlayDisconnect(this.handler, this.server); this.receiver.endSession(this); } diff --git a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/channeltest/NetworkingChannelClientTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/channeltest/NetworkingChannelClientTest.java index 811f1b5d8..5aa279a00 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/channeltest/NetworkingChannelClientTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/channeltest/NetworkingChannelClientTest.java @@ -64,11 +64,11 @@ public final class NetworkingChannelClientTest implements ClientModInitializer { }); // State destruction on disconnection: - ClientLoginConnectionEvents.LOGIN_DISCONNECT.register((handler, client) -> { + ClientLoginConnectionEvents.DISCONNECT.register((handler, client) -> { SUPPORTED_C2S_CHANNELS.clear(); }); - ClientPlayConnectionEvents.PLAY_DISCONNECT.register((handler, client) -> { + ClientPlayConnectionEvents.DISCONNECT.register((handler, client) -> { SUPPORTED_C2S_CHANNELS.clear(); }); } 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 6f706b3ab..0ac0b51ab 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 @@ -45,7 +45,7 @@ public final class NetworkingKeybindPacketTest implements ModInitializer { @Override public void onInitialize() { - ServerPlayConnectionEvents.PLAY_INIT.register((handler, sender, server) -> { + 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/login/NetworkingLoginQueryTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/login/NetworkingLoginQueryTest.java index 424b2c65a..2c8d6ded5 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/login/NetworkingLoginQueryTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/login/NetworkingLoginQueryTest.java @@ -36,8 +36,8 @@ public final class NetworkingLoginQueryTest implements ModInitializer { @Override public void onInitialize() { - ServerLoginConnectionEvents.LOGIN_QUERY_START.register(this::onLoginStart); - ServerLoginConnectionEvents.LOGIN_QUERY_START.register(this::delaySimply); + ServerLoginConnectionEvents.QUERY_START.register(this::onLoginStart); + ServerLoginConnectionEvents.QUERY_START.register(this::delaySimply); // login delaying example ServerLoginNetworking.registerGlobalReceiver(NetworkingPlayPacketTest.TEST_CHANNEL, (server, handler, understood, buf, synchronizer, sender) -> { 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 4d865ecc0..576b6c004 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 @@ -31,7 +31,7 @@ public final class NetworkingPlayPacketClientTest implements ClientModInitialize public void onInitializeClient() { //ClientPlayNetworking.registerGlobalReceiver(NetworkingPlayPacketTest.TEST_CHANNEL, this::receive); - ClientPlayConnectionEvents.PLAY_INIT.register((handler, sender, client) -> { + ClientPlayConnectionEvents.INIT.register((handler, client) -> { ClientPlayNetworking.registerReceiver(NetworkingPlayPacketTest.TEST_CHANNEL, (client1, handler1, buf, sender1) -> receive(handler1, sender1, client1, buf)); }); }