Is the code in registerGlobalReceiver correct?

[FieryRobot]

I've been trying to debug an issue trying to use the networking API where I want to dynamically register for a message. But it doesn't work. registerGlobalReceiver seems to only work in the initializer for the mod and not any time after. When I look at the code, it looks backwards to me:

final boolean replaced = this.handlers.putIfAbsent(channelName, handler) == null;

if (!replaced) {
  this.handleRegistration(channelName, handler);
}

Should that not be if (replaced) ? I haven't tried it yet as I am not 100% sure how to build my mod AND fabric and run them at the same time, but it seems like it should fix my issue, no?

For reference, registerChannel seems to do the correct thing:

final boolean replaced = this.handlers.putIfAbsent(channelName, handler) == null;

if (replaced) {
  this.handleRegistration(channelName);
}

[warjort]

The javadoc says you are meant to unregister your previous handler before registering a new one.
https://github.com/FabricMC/fabric/blob/1.19/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/api/networking/v1/ServerPlayNetworking.java

	/**
	 * Registers a handler to a channel.
	 * 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.

That !replaced does look weird. But it is not the actual registration of the handler, which is the putIfAbsent().
Instead it is for something called NetworkAddons, I don't claim to know what they are. :-)

fabric/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/GlobalReceiverRegistry.java

Line 150 in f11b62a

private void handleRegistration(Identifier channelName, H handler) {

The network addon registration has similar code but there it doesn't negate the replaced.

fabric/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractNetworkAddon.java

Line 81 in f11b62a

if (replaced) {

[sfPlayer1]

FYI a "NetworkAddon" is just an object we attach to each network handler to hold some extra state like the subscriber list (and some methods to mess with this state)

[FieryRobot]

Sure, but it's that addon that is the thing calling the server with the channel names. Specifically in ClientPlayNetworkAddon::handleRegistration. That's the thing that needs to happen so the server knows that something was registered. Without it, my listener on the server side registered via S2CPlayChannelEvents.REGISTER is never called.

[FieryRobot]

I did verify that registerReceiver works as expected. My callback on the server is invoked.

[warjort]

I think I agree this is a bug.

If you register your handler during mod initialisation, it doesn't matter that the !replaced is wrong logic because there are no NetworkAddons yet that need to updated.
But, if you do it after some addons have been created, the !replaced means the registration doesn't happen.

I bet if you do the registerGlobalReceiver() twice, it works as you expect?
On first invocation it will register the handler but not update the addons (replaced = true).
On the second it won't register the handler again, but it will update the addons (replaced = false).

[FieryRobot]

I bet if you do the registerGlobalReceiver() twice, it works as you expect?

Sure enough, it does. I think in my case the registerReceiver is the more appropriate call. I still don't completely understand the difference between the two...

[sfPlayer1]

I think this is indeed a bug, usually hidden by global registrations happening before any players connect.

[warjort]

I created this bug report: #2359