Better fix for when packets of type HANDSHAKING are not found in the registry.

[nickuc]

This PR finally fixes the problem that was reported in #2601 introduced by commit af33a2a and replaces the workaround that was previously used.

Based on this code comment:

ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java

Lines 208 to 210 in b0c4b7f

	// since 1.20.5 the encoder is renamed to outbound_config only in the handshake phase
	String encoderName = pipeline.get("outbound_config") != null
	? "outbound_config" : "encoder";

[derklaro]

I'm wondering: if the key is not present in the handshaking phase, how are packets encoded during that phase? They do need to get the information from somewhere, can this be detected more dynamically than just assuming that it must be handshaking if the key is not present (especially if at some point another protocol is not using the key anymore: the decoding process works but fails with a misleading error + we would have to implement such a detection anyway)

[Ingrim4]

I believe a better solution would be to iterate over the entire pipeline. Since ChannelPipeline implements Iterable<Map.Entry<String, ChannelHandler>>, we can directly check each entry. If we encounter a PacketEncoder or PacketDecoder, we can use them as handler instances. This approach allows us to avoid the need for handler name dependent code.

Edit: this is a partial duplicate of #2933 but I would still prefer my suggested solution since it's more dynamic and less dependent on names not changing.

Maybe something along those lines:

@Override
public Object apply(Channel channel, PacketType.Sender sender) {
    Class<? extends ChannelHandler> handlerClass = this.getHandlerClass(sender)
    		.asSubclass(ChannelHandler.class);
    
    ChannelHandlerContext handlerContext = channel.pipeline().context(handlerClass);
    if (handlerContext == null) {
        return null;
    }

    Function<Object, Object> protocolAccessor = this.getProtocolAccessor(handlerClass, sender);
    return protocolAccessor.apply(handlerContext.handler());
}

private Class<?> getHandlerClass(PacketType.Sender sender) {
	switch (sender) {
		case SERVER:
			return MinecraftReflection.getMinecraftClass("network.PacketEncoder");
		case CLIENT:
			return MinecraftReflection.getMinecraftClass("network.PacketDecoder");
		default:
			throw new IllegalArgumentException("Illegal packet sender " + sender.name());
	}
}

[nickuc]

I believe a better solution would be to iterate over the entire pipeline. Since ChannelPipeline implements Iterable<Map.Entry<String, ChannelHandler>>, we can directly check each entry. If we encounter a PacketEncoder or PacketDecoder, we can use them as handler instances. This approach allows us to avoid the need for handler name dependent code.

Edit: this is a partial duplicate of #2933 but I would still prefer my suggested solution since it's more dynamic and less dependent on names not changing.

Maybe something along those lines:

@Override
public Object apply(Channel channel, PacketType.Sender sender) {
    Class<? extends ChannelHandler> handlerClass = this.getHandlerClass(sender)
    		.asSubclass(ChannelHandler.class);
    
    ChannelHandlerContext handlerContext = channel.pipeline().context(handlerClass);
    if (handlerContext == null) {
        return null;
    }

    Function<Object, Object> protocolAccessor = this.getProtocolAccessor(handlerClass, sender);
    return protocolAccessor.apply(handlerContext.handler());
}

private Class<?> getHandlerClass(PacketType.Sender sender) {
	switch (sender) {
		case SERVER:
			return MinecraftReflection.getMinecraftClass("network.PacketEncoder");
		case CLIENT:
			return MinecraftReflection.getMinecraftClass("network.PacketDecoder");
		default:
			throw new IllegalArgumentException("Illegal packet sender " + sender.name());
	}
}

I liked your solution. But I'm still having trouble identifying the protocol when the HANDSHAKE packet is received.

I did some tests, and I think I found something:

• If you call the NettyChannelInjector#getCurrentProtocol(PacketType.Sender.CLIENT) method before the PacketDecoder handler, it correctly returns the HANDSHAKING protocol enum.
• However, if called after the PacketDecoder handler (where the processInboundPacket is executed), it will return null. Something going on in the internal classes is influencing the behavior of the HANDSHAKING phase, but I haven't figured it out yet.

Oh, I also enabled PR editing, I didn't realize it was disabled.

[nickuc]

Closed in favor of #2951