superjoe30/node-minecraft-protocol — pull request #70 by deathcap (2014-01-07 05:03:46+00:00)
Title: Allow sending and listening for packets by "name" instead of number

Adds descriptive names for each packet and changes to allow using the name as an alternative to state/number. Not complete yet (can squash later, etc.) but I wanted to get your feedback for developing further, specifically:

- is this a worthwhile feature? I think so, makes it easier to use the API without knowing intricate specifics of the protocol, for example from mineflayer:

`bot.client.on(0x38, function(packet) {`

would have to change to "0x26" for the 1.7 update, trading one "magic number" for another. But with this PR, you can simply write:

bot.client.on('map_chunk_bulk', function(packet) {

and the code is much more descriptive and maintainable. node-minecraft-protocol already decodes field names, so I'd say decoding packet names is well within the scope of this module. Also it mitigates the complexity of having to listen on a stringified array "[state, id]", since the names are unique across all protocol states — the name infers both the state and id.

- Are the names themselves alright? They are almost straight from http://wiki.vg/Protocol (with a few changes for uniqueness), but maybe could use some refinement

- lib/client.js setSocket() now emits up to 4 events per packet: packetName, [state,id], id, 'packet' - is this too much?

- Is it a good idea to pass the name as a string literal, having the function translate to an id? Could instead have an object with keys of names, values state and id, but then mistyping a packet name would pass 'undefined' to the function, so I'm not sure that's a better approach.

- How about the data structures in lib/protocol.js? I added packetIDs, packetStates, packetNames, in addition to the existing 'packets' object which maps to the fields. Could be worthwhile to merge packetIDs and packets so the field names and packet names are defined together, but I wasn't sure of the best way to do this.

- Listening/sending by state/id still works as before

- Updated the examples to demonstrate this new API (plus a few minor unrelated changes: client_chat.js offline mode, fix the server.js example to send the given message, and remove unsupported '§e' styling).