Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol only works on major verions #65

Closed
dariusc93 opened this issue Oct 9, 2013 · 18 comments · Fixed by #257
Closed

Protocol only works on major verions #65

dariusc93 opened this issue Oct 9, 2013 · 18 comments · Fixed by #257

Comments

@dariusc93
Copy link

I notice that the protocol only works on major version of Minecraft. This isnt a major bug or issue, but as a suggestion was to make it where the protocols are stored in different libs and defined in the code with a version so it would look like

var mc = require('minecraft-protocol');
var client = mc.createClient({
host: "localhost", // optional
port: 25565, // optional
username: "[email protected]",
password: "12345678",
version: "1.6.2",
});

Which could help alot.

I may submit a PR to implement this

@Gjum
Copy link
Member

Gjum commented Oct 9, 2013

Very useful feature, I would be happy to see it implemented.

@andrewrk
Copy link
Member

andrewrk commented Oct 9, 2013

You could implement this pretty creatively using npm's version locking. For example you could create a node project called minecraft-protocol-x.y.z which depends on the correct version of minecraft-protocol for that xyz. You'd also have minecraft-protocol-x'.y'.z' which depends on a different version, etc. One package for each minecraft protocol version to support. Then you'd have a package minecraft-multi-protocol or something which depends on all your different xyz's. In that package you have a simple dispatch function which figures out which library to use based on the version argument.

I'm not necessarily saying it's the best way to do it but 1) might be fun to try and 2) would allow minecraft-protocol project to ignore previous protocol versions safely.

@dariusc93
Copy link
Author

Correct, but many people who uses this project may wish to stick with the latest and be able to use it without having to rollback to a previous version. For me, I use it as a project for creating a terminal base client to access my server, but because of the static protocol, it is not possible to access other versions without changing the protocol manually. I just feel it would be more easier than to change to a different version. I am use to using C++ with a similar feature that this project have made, so implementing the features shouldnt be as bad as one would think. When I finish, I may have it support 1.4.7 and above. Before I do perform any PR's are there any rules other than having the code being clean from errors?

@mappum
Copy link
Contributor

mappum commented Oct 10, 2013

I agree with @superjoe30, I think version management should happen outside of the package. It would increase the maintenance overhead to add multi-version support, and you can already easily do it (manually or programatically) via npm. You could always make a wrapper plugin that works as you described in the original post by getting the correct version from npm.

@andrewrk
Copy link
Member

@dariusc93 I think you may have missed my point - my suggestion would solve your problem as you have described it. You would end up depending on the latest version of a different project though - in my example "minecraft-protocol-multi". If there was a bugfix on an older version, minecraft-protocol could fix that in a branch and release a patch version bump.

Essentially it boils down to maintaining support for older versions via branches instead of if statements.

@roblabla
Copy link
Member

I don't think it's the right way to do it. Right now, if you want an old version of the protocol, you can just change in your package.json the version of node-minecraft-protocol. It's simple, it works, and it's easy to maintain.

The main use-case I see for having the proto version as part of option is (I think) to allow us to write stuff like "protocol downgraders", or servers/client that accept servers/clients of multiple protocol versions. I may be wrong, but the way I understand, what you suggest would still make us stuck with one specific version, and thus wouldn't be that useful.

Again, I'm not sure of how NPM works, so I may be entirely wrong.

@andrewrk
Copy link
Member

My suggestion would not make us stuck with one specific version. It would adhere to the example API that @dariusc93 originally proposed in this issue.

@roblabla
Copy link
Member

And there you go, made a tiny mistake in protocol 1.7.5, and now I have to fix it for 1.7.7, and backport it for 1.7.5, thus having to create two versions. Bleigh.

Using npm version locking would still have the same backporting issue : we have X versions of the protocol.js to fix, X being the amount of versions affected.

I think I'm going to take a stab at fixing this issue by changing protocol definition into an incremental thing. We would have a protocol-1.7.5, and then a protocol-1.7.7 that deep-copies the 1.7.5 export, and then applies some modifications.

Pros :

  • Not that much maintenance overhead
  • Easier to do protocol updates
  • Backporting issues is not a problem, just fix the problematic protocol-X.js, and all affected versions will immediately get the fix.

Cons :

  • Protocol broken into a bunch of files, making documentation more complicated.
  • Slower startup, since it has to instantiate all the protocol-X and copy them.

@misiek08
Copy link

What about just requiring specified version of protocol?
Let's say, that I'm going to use 1.7.9 only, because I have game servers on that version. So I specify in client/server or globally someting like:

var mc = require('minecraft-protocol');
mc.proto_version = '1.7.9';

and in the lib there should be code something like:

require('client.js') ;......; require('lib/protocol-'+this.proto_version+'.js');

In big shortcut.
It will make many additional files, but they could be in subdir without any problem. And states or anything else will not confilct, because protocol itself it's in his file.
In my opinion it's easy to do and it will work. Even finding now files for each protocol in commits and adding them to subdir will take 15-20minutes and we will have support for all version accesible by one variable.

@roblabla
Copy link
Member

Except that won't work. The idea is good, but the old protocol files don't support named packets, so it's not going to work that easily. If we want to be truly compatible with all versions, we need to implement named packets in every protocol version.

To make things worse, the pipeline changed widely accross versiosn. Let me explain :

  • pre-1.3, the packets were in the format [byte packetId, byte[] packetData].
  • 1.3-1.7, the packets were in that format, but encrypted.
  • 1.7, the protocol was overhauled to be stateful. They also added varints everywhere, and added a length prefix. Packets effectively became [varint length, varint packetId, byte[] packetData].
  • 1.8, the protocol is changed to add compression.

Oh and as if that wasn't enough, the way login works changed as well, bringing another layer of disparity.

Because of these disparities, making everything work together is a huge pain in the ass.

I'm currently working on making a huge change to this project, by splitting it into three smaller chunks that will be easier to work with.

  • The first is 'protocols', which is just a binary protocol (de)serializer. I hope this will allow me to bring a bunch of improvement to the binary parser, by splitting it from the minecraft world. Besides, it's also a handy tool in general ^^.
  • The second is 'minecraft-proto-def', which is both a minecraft protocol definition file for protocols, and an abstraction layer over the different minecraft functions. To put it simply, every protocol file will have a couple functions, such as "pipeline", "login", and of course the protocol definition. This way, less problems overall, and each minecraft protocol version will be easily self-documented.
  • The final project, 'node-minecraft-protocol', is what we currently know, but will contain protocol detection for the client, and version selection for the server. It will also be hopefully a bit faster, and a lot easier to maintain, since all the work will mostly be happening in minecraft-proto-def.

Doing this gives us another benefit : it allows us to support projects like deathcap 's websocket minecraft port.

The problem is, this is taking a lot of time. I'm currently working on the biggest chunk of work, protocols. I've got most of the 'reading' part done, so what's mostly left is implementing the 'writing' part. Once that's done, I'll move on to creating the protocol files for 1.7.10 and 1.8. I can't give an exact time-frame, but I'll def keep this issue up-to-date with my progress.

@misiek08
Copy link

God. In every project there are code with comments like "Mojang why?!??!" :)

Keep going, this module is great and many users like it.

I think about writing someting like BungeeCord basing on your library. Simple example for one server is ready, but writing stuff for multiserver with handling all the stuff it's a little bit complicated. I'm doing that for 1.7.9 and if I will be not able to optimize bungeecord in way I need in few weeks - I will write proxy using node-minecraft-protocol.

@roblabla
Copy link
Member

Haha, I've started doing that a while ago, but never got around finishing it. Code for it is in MyLittleProxy repository.

@misiek08
Copy link

Your works only for 1 backend server, but that's what I propably will do. For many servers ofc.

@Corgano
Copy link

Corgano commented Oct 20, 2014

Something I just wanted to to note here after reading the first post, I don't mind this protocol being behind the "latest" vanillia builds. 90% (est) servers are bukkit based, so there's always a month or two before bukkit catches up (PROPERLY! bukkit likes to fuck with the protocol versions to let multiple versions connect) so spending the time to work on other issues instead of rushing a version update is always nice.

@mrfrase3
Copy link
Contributor

mrfrase3 commented Nov 6, 2014

I run a FTB Unleashed server, with a vanilla lobby server connected to it. The server versions are 1.5.2 and I'm trying to connect to the vanilla lobby server to send and receive chat events, but the 0.10.1 version is basically useless at this point, with the depreciated login system and other connection errors.

adding protocol selection would be muchly appreciated.

Edit: I managed to get it to work by simply spoofing the old login system on my own web server, by taking in the values, authenticating with the new system, then returning in the old format

@roblabla
Copy link
Member

roblabla commented Nov 7, 2014

1.5 is not in the pipeline anyway. The goal for me right now is to support everything from 1.7 onward. Anything pre-1.7 will probably not be included by default unless someone else provides the definition file.

As for how far I am with this issue : It needs roblabla/protocols to be finished before I can move on to write the different protocol definition files for each versions. This may take a little while, but progress is being made.

@roblabla
Copy link
Member

Nearly a year later, mc-proto-def exists (though it is called minecraft-data). node-minecrat-data, the library we use for it, exposes the multiple versions minecraft-data supports (currently only 1.8 and a pre-1.9, but we can add the older versions as we go). Protocols is slowly approaching completion too, as seen by #162 . We're making progress !

@wtfaremyinitials
Copy link
Member

😄

@rom1504 rom1504 mentioned this issue Sep 12, 2015
rom1504 added a commit to rom1504/node-minecraft-protocol that referenced this issue Sep 29, 2015
* put parsePacketData in deserializer and createPacketBuffer in serializer
* remove packets from the index and expose readPacket instead
* load packets when needed in various files
* make tests test every supported version
static cross version of PrismarineJS#234, fix PrismarineJS#65, fix PrismarineJS#240
rom1504 added a commit to rom1504/node-minecraft-protocol that referenced this issue Sep 29, 2015
* put parsePacketData in deserializer and createPacketBuffer in serializer
* remove packets from the index and expose readPacket instead
* load packets when needed in various files
* make tests test every supported version
static cross version of PrismarineJS#234, fix PrismarineJS#65, fix PrismarineJS#240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants