use the minecraft-protocol npm module

[andrewrk]

https://github.com/superjoe30/node-minecraft-protocol

If we all depend on the same protocol module, the maintenance burden for every individual goes down. @mappum had already written a very similar module, but we worked out the differences and came up with something that we were both happy with, and that resulted in this minecraft-protocol npm module.

This module is well documented, has a robust test suite, a clean API, and 2 maintainers already.

Save us all time and energy, and use this module!

Look at all these automated tests that could be covering your code:

  client
    ✓ pings the server (4189ms)
    ✓ connects successfully - online mode (2425ms)
    ✓ connects successfully - offline mode (1730ms)
    ✓ gets kicked when no credentials supplied in online mode (3722ms)
    ✓ does not crash for 10000ms (11645ms)
  mc-server
    ✓ starts listening and shuts down cleanly 
    ✓ kicks clients that do not log in (102ms)
    ✓ kicks clients that do not send keepalive packets (103ms)
    ✓ responds to ping requests 
    ✓ clients can log in and chat (41ms)
    ✓ gives correct reason for kicking clients when shutting down 

  11 tests complete (36 seconds)

[nickpoorman]

Your protocol doesn't seem to distinguish between parsers and serializers ie. client -> server & server -> client.

[andrewrk]

Can you give an example of how the API is insufficient?

[deoxxa]

I really hate doing this because it makes me a massive hypocrite about reusing code, but right now I want to say no. Mostly because there's no proper streaming interface (i.e. I can't socket.pipe(parser) or serialiser.pipe(socket)), which means the actual data-pushing code gets a lot more complicated (handling backpressure manually, etc).

The differences Nick is talking about (hi Nick!) are probably the switched fields in one of the movement packets. I can't recall off the top of my head which one it is, but one has the stance and the y values swapped around depending on whether it's a client stream or a server stream. It's not being handled in the jsmc parser because we're never parsing server streams, but minecraft-protocol will probably need to be correct in that area.

Would it be possible for you to add a (well behaved) streaming API to minecraft-protocol? Take a look at steez if you like, I use it fairly extensively at work so I'm confident that it's correct, but even if you just want to steal ideas from it that's cool.

[andrewrk]

As for the point @nickpoorman brought up, these lines should explain: https://github.com/superjoe30/node-minecraft-protocol/blob/master/lib/protocol.js#L84

As for streams, I'm certainly willing to compromise - my goal is to reduce maintenance burden for everyone. In order to do that in this case, I need to better understand the API that you desire.

After spending quite some time trying to grok your code, I still don't understand what streams buy you (but I would like to understand). You're piping:

from client socket -> parser -> client (which does not emit 'data' ...?) -> serialiser -> to client socket.

So if you get some backpressure from sending to the client socket, does that mean that you want to pause receiving packets from the client socket? It seems like you would want to handle packets as soon as you get them, and let data buffer as much as necessary when writing to the client socket.

But anyway, I don't think we have to have the same solution in this regard - we can still benefit from code sharing.

Based on everything so far, I think something that will work for all of us is that minecraft-protocol can expose createPacketBuffer and parsePacket. These functions can be integrated into serialiser and parser respectively. That way you still have your stream stuff set up exactly the same way, but we're all depending on the same code for the actual packet parsing and serialisation.

I'd be happy to make a PR to demonstrate this.

Thoughts?

[andrewrk]

Another possibility is ripping out protocol.js from minecraft-protocol and making that its own npm package.

[mappum]

@whiskers75 Are you asking us to code your project for you? That's not how it works...

[SomeoneWeird]

I'm just going to say this: do your own goddamn work and stop asking other people todo it for you.

[whiskers75]

@SomeoneWeird @mappum Here: tried to implement this (https://github.com/whiskers75/jsmc/tree/minecraft_protocol) but I get Error: read ECONNRESET when the client connects.