Closed remplerus closed 8 months ago
Digging into this, it turns out this is caused by a problematic extension of a vanilla packet, specifically the ClientIntentionPacket
. The issue is that we cannot, ever, add additional elements to vanilla packets. Adding additional stuff to existing elements (i.e. what the 1.20.1 and earlier patch to this packet did with the FML version appended to the host name) is fine as long as vanilla can ignore it on receipt. Adding additional elements however will break network compatibility with vanilla completely for two reasons:
It is worth noting that there is an additional issue with NF client -> vanilla server connections: NetworkHooks.handleClientLoginSuccess()
is currently never called, causing server configs to not be initialized to defaults and then subsequently blowing up with "config not loaded".
This could technically be resolved with a band-aid fix by basically going back to the original approach of appending the FML version to the host name (this is possible due to a small trick with records that allows re-assigning values) and then calling NetworkHooks.handleClientLoginSuccess()
from ClientConfigurationPacketListenerImpl#handleConfigurationFinished()
. This works in my cursory test in both directions. The proper way to fix this however is to go through with the planned networking rework to properly support the configuration phase (which should provide better support for proxies and lobby servers) and implement the handshake spec hashed out with and already implemented by Fabric in https://github.com/FabricMC/fabric/pull/3244.
For sake of completeness, this is what the relevant part of the ClientIntentionPacket
patch would look like.
public ClientIntentionPacket {
if (fmlVersion == null) {
fmlVersion = net.neoforged.neoforge.network.NetworkHooks.getFMLVersion(hostName);
hostName = hostName.split("\0")[0];
}
}
@Deprecated
public ClientIntentionPacket(int protocolVersion, String hostName, int port, ClientIntent intention) {
this(protocolVersion, hostName, port, intention, net.neoforged.neoforge.network.NetworkConstants.NETVERSION);
}
public ClientIntentionPacket(FriendlyByteBuf p_179801_) {
this(p_179801_.readVarInt(), p_179801_.readUtf(255), p_179801_.readUnsignedShort(), ClientIntent.byId(p_179801_.readVarInt()), null);
}
@Override
public void write(FriendlyByteBuf p_134737_) {
p_134737_.writeVarInt(this.protocolVersion);
p_134737_.writeUtf(this.hostName + "\0" + this.fmlVersion + "\0");
p_134737_.writeShort(this.port);
p_134737_.writeVarInt(this.intention.id());
}
Minecraft Version: 1.20.2
NeoForge Version: 20.2.4-beta (probably with 20.2.3 and 20.2.5 too, haven't tested yet)
Logs: Client's debug log when connecting to a vanilla server: neoforge client's debug.log Server's debug log when a vanilla client tries to connect to the server neoforge server's debug.log
Steps to Reproduce:
Description of issue: When trying to connect to a Neoforge server with a vanilla client you'll get an "Incompatible Version" Error (as seen in the next image)
You'll get the same error when running a vanilla server and try to connect with a neoforge client (as seen in the next image)
![image](https://github.com/neoforged/NeoForge/assets/44590574/55b669b3-a697-4db4-b54e-d7907dc5ae7e)