Cross version

[rom1504]

There are 2 kinds of cross version support:

• static : for one instance of a program using nmp, only one version is supported
• dynamic : for one instance of a program using nmp, several versions are supported

Currently we almost got static support (see #212 about that).

You'd think dynamic support would be much harder but actually all it would be about is getting the protocol information from the server/client you are connecting to, and using the correct protocol.json to parse and serialize the packets for that server/client.

Of course, that wouldn't solve the multi-version support for users of nmp which would still have to deal with the packets difference between version, one way to deal with that might be #231

Dynamic support would allow createClient and createServer to work for both 1.8 and 1.9 without change to the api, nmp would just need to expose the version of the server/client it connected to.

This idea might be extendable to multi-platform support (with mcpe) but that would be a bit harder (the protocol architecture of mcpe is different from mcpc)

Related with #65

[rom1504]

About dynamic support:
It would be better to add a versions:["1.8", "1.9"] option to createClient so that the bot will only connect to versions supported by the nmp user.

And the default versions might be just ["1.8"] in nmp 0.15.0 . In 0.16.0 it might become ["1.9"] and the nmp user would anyway would have to manually update to 0.16.0 so we wouldn't "pull the rug under the user's feets".

Or we could just say that "versions" is not optional.

Dynamic just mean multiple versions support at the same time, we don't force the user to support everything if he doesn't care to.

(the same thing goes for createServer)

[rom1504]

We need to be able to convert the protocolVersion (https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/src/createServer.js#L143) (=version.version) to the corresponding majorVersion/mcDataVersion. node-minecraft-data should be able to do that in https://github.com/PrismarineJS/node-minecraft-data/blob/master/index.js#L18 (just check if one of the mcData submodule is that version)

Then we basically just need to do require('minecraft-data') when we know the version. To have cross version support dynamically I think we'd need to assume the other states than play are the same cross version (is this true ?) in order to go far enough for the client/server to give us the version he wants.

the other states than play are the same between 1.8 and 1.9, so that would work. 1.7 would be something else though (among other problems related to 1.7)

[roblabla]

@rom1504 it would work for anything >= 1.7. For < 1.6, there would only be one state (LEGACY, or whatever we end up calling it). I think we should keep the require('minecraft-data')('version'). It's easier and more flexible.

[rom1504]

Ah yeah sure, I forgot the version. I meant putting require ('minecraft-data')(protocolVersion) when the client (or server) sends us the protocol version

[rom1504]

Places that should use a dynamically stored version :

file	places	method
createServer.js	ping and set_protocol	store the version the client is sending us in the client object so the correct protocol packets can be used for that client, use that version for the ping
ping.js	set_protocol	used the version stored for that client, or a default version (1.8 for example)
createClient.js	set_protocol	store the version the server is using it, to the correct protocol packets can be used
client.js	packets loading	preload a default protocol packets version, then when we know the correct version change it (the change can be done in a function that will be called by createServer.js or createClient.js)
serializer.js	packets loading	same thing than in client.js
test.js	version checking	execute the test for each supported version (see #240)
circle-ci	server download	same than test.js
index.js	packets exposing	expose all the versions or nothing
browser.js	packets exposing	same than index.js

createClient and createServer should also have a new option "versions" to prevent client connecting to a server with a version the user doesn't want to support, and client connecting to a server version the user doesn't want to support. That "versions" info should be used in createClient.js and createServer.js to restrict the supported versions (a bit like #213 )

[rom1504]

I think I'll start by implementing static cross version (the API proposed in #65) before dynamic, it's easier.

[rom1504]

Once #257 is merged, adding dynamic cross version will mainly be about changing the serializer and deserializer dynamically in client.js when we know the version we want.

[rom1504]

hmm hmm, so it seems the server never send the version to the client, so it doesn't seem possible to implement dynamic cross version for the client.
Ah actually, the client can ask the server its version with the ping packet, and get it back with the pong packet http://wiki.vg/Protocol#Pong
So it is possible to do that to figure out the version.

[rom1504]

Once dynamic cross version is implemented, I think a good API for createClient and createServer would be:

• no version option is provided : default to enabledVersions = [defaultVersion]
• version:"1.9" : enabledVersions = ["1.9"]
• version:["1.8","1.9"] : enabledVersions = ["1.8","1.9"]
• version:"any" : enabledVersions = supportedVersions

Then NMP would refuse to connect to any version not in enabledVersions.

That way:

• we don't break users than don't specify the version (it won't automatically connect to 1.9 while their code only handle 1.8 either)
• we let people that want to handle any versions "at the same time" do it. It might be useful for users that don't rely too much on the specific packets fields, for example a simple proxy. (and for people that have a compatibility layer like I want to do in High level abstractions above nmp #231 )

[deathcap]

Ah actually, the client can ask the server its version with the ping packet, and get it back with the pong packet

Implemented exactly this in #327 / https://github.com/deathcap/node-minecraft-protocol-auto:). It works pretty well in my testing, though as @roblabla mentions the ping exchange adds additional latency to the connection time and incompatibility with servers that do not allow ping, so it is probably a good idea to make it optional. The client also has to wait for the ping response from the server before it creates the client and sets a version, so the current synchronous client = createClient() API is troublesome.

How about keeping createClient() as is — only accepting one version — but add a wrapper for dynamic cross protocol support? (this is what I'm doing in node-minecraft-protocol-auto). This way, existing clients won't break nor get the ping overhead, and the API signature can change (createClient is sync, but this new ?createClientAsync? API could be async, accepting a callback function to return the client object once it is created).

Maybe FML/Forge (#326) could also be considered as "version" for the cross-version support? Not in the same enabledVersions list, but a separate boolean option. idk if we actually need enabledVersions, if the current API remains for specifying exactly one version, and there is another API for automatically getting and using the appropriate version, unless filtering the supported versions is desired for some reason.

[deathcap]

In #331 (ready to pull/review), left the default version option to createClient() as it is (defaults to 1.8 currently), but now also allow version: false to enable the autoVersion() client plugin.

This works alright but there are some larger worthwhile improvements imho. These may require changes to multiple projects.

Naming consistency: the term version is variously in the code used to refer to the version object, the major version (version parameter to createClient), the release version, and the protocol version (as in version.version). It would be clearer if the protocol version number code (47) was always protocolVersion, and maybe version objects (from minecraft-data) as versionInfo or something.

Major version / dataset dichotomy: minecraft-data is built around one set of data per major version. Not necessarily an unreasonable assumption, but it breaks down when considering protocols. The 1.7 major version encompasses both protocol version 4 (1.7.1 to 1.7.5) and 5 (1.7.6 to 1.7.10) for example, and 1.9 (prerelease) just had a recent major protocol change making 15w40b (76) in minecraft-data/1.9 incompatible with newer 1.9 prereleases (renumbering the packets). I think we can tackle this problem by abstracting out the "dataset" from the "version", so for example, you would ask minecraft-data "give me the protocol for version X" (where X = 76, or '15w40b', etc.,), and it would return the appropriate set of data, if available. The data could be stored in 1.8/, 1.7/ directories internally, but not exposed to users of the module.

Major versioning in createClient/client object: the constructor takes a majorVersion as the second argument, should be changed to use the release version, and let minecraft-data handle getting the right data. This might already be possible in minecraft-data 0.16.0 since it supports require('minecraft-data')('15w40b') for example, but should be explicit in node-minecraft-protocol, and the createClient() factory method should use the version from the versions object array, instead of mcData.version (which probably could be removed? since version:data is a n:n mapping)

Remove version in constructors?: I suppose it makes sense for the Server object, but for Client, we want to be able to create the object and return it without immediately knowing its version. Currently, the version is set, and then overridden after the object is created.

Version validation: src/createClient.js validates the supported version, but it is not validated when set afterwards (client.version = majorVersion; client.state = states.HANDSHAKE). It is partially validated in that the version must be known to minecraft-data protocolVersions, but not that there is any protocol data available for it. Maybe make version property with a setter to validate, and also have it setup the serializers for the right version when set.

[rom1504]

Client cross-version is now implemented (thanks @deathcap)
Missing here is at least server cross-version. That requires less change since we already get the version from the client there https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/src/createServer.js#L187

[deathcap]

Found this protocol recognizer in Cuberite, might be useful for reference: https://github.com/cuberite/cuberite/blob/master/src/Protocol/ProtocolRecognizer.cpp#L948

[rom1504]

@kobiluvesmemes is interested by dynamic cross version in the server. I might do it.
and modularize the server in the process (related #210)

[ghost]

How long until this is on npm?

[rom1504]

@kobiluvesmemes released as 1.4.0