1.6.4 Protocol

[brenoxavier]

This is my attempt to continue the work started by this PR #443.

I've written the specifications for protocol 78 (pre Netty) in version 1.6.4, but I have some doubts about whether this is correct because some packages are quite different from the post-Netty versions.

By setting the strings to UTF-16, I found that the protodef-validator is failing to validate the latest version of ProtoDef. Should I try to fix the test?

Used documentation:

Protocol
Data Types
Metadata
Slot Data
Object Data

[wgaylord]

It would be best to fix the tests so that they pass.

[wgaylord]

Looks like ProtoDef-io/node-protodef-validator#11 will fix this.

[extremeheat]

Can you be specific here with 1.6.4 instead of 1.6? Or were there no changes in protocol between then?

[brenoxavier]

Can you be specific here with 1.6.4 instead of 1.6? Or were there no changes in protocol between then?

I couldn't identify major differences in the protocol. I believe the protocol from 1.6.4 should work for all versions of 1.6.

https://wiki.vg/index.php?title=Protocol&type=revision&diff=4657&oldid=1096

[extremeheat]

There should not be any ambiguity there, if there is a protocol version change then there likely is a difference.

[extremeheat]

This needs to be added to the protocolVersions.json file

[brenoxavier]

The version is already present in the file, does it need any other adjustments?

[brenoxavier]

There should not be any ambiguity there, if there is a protocol version change then there likely is a difference.

Ok, I'll specify it as 1.6.4.

[extremeheat]

What version did you use as a base to writing this?

[brenoxavier]

What version did you use as a base to writing this?

I used 1.7

[extremeheat]

Yeah, I'm diffing 1.6.4 protocol.json to 1.7 and it seems there are alot of differences. Did the protocol really change that much or did you re-organize things? That would make review much harder as would need to go through all the packets individually

[wgaylord]

Yeah, I'm diffing 1.6.4 protocol.json to 1.7 and it seems there are alot of differences. Did the protocol really change that much or did you re-organize things? That would make review much harder as would need to go through all the packets individually

Yes, because 1.6 4 is pre-netty.

[brenoxavier]

Yeah, I'm diffing 1.6.4 protocol.json to 1.7 and it seems there are alot of differences. Did the protocol really change that much or did you re-organize things? That would make review much harder as would need to go through all the packets individually

Many things changed from 1.6 to 1.7, I tried to keep them as close as possible.

[extremeheat]

I see. So yes this would require a more thorough review. Since there is no concept of states or clientbound/serverbound packets, it would probably be cleaner to not use the 1.7 protocol as reference (as its structure assumes those things, like toServer/toClient/play/etc).

1.7.6-1.7.10 protocol - https://wiki.vg/index.php?title=Protocol&oldid=6003
1.6.4 protocol - https://wiki.vg/index.php?title=Protocol&oldid=4899

[brenoxavier]

I see. So yes this would require a more thorough review. Since there is no concept of states or clientbound/serverbound packets, it would probably be cleaner to not use the 1.7 protocol as reference (as its structure assumes those things, like toServer/toClient/play/etc).

1.7.6-1.7.10 protocol - https://wiki.vg/index.php?title=Protocol&oldid=6003
1.6.4 protocol - https://wiki.vg/index.php?title=Protocol&oldid=4899

So is it better to specify packages in just a single structure?

[extremeheat]

The point of that separation is so node-Minecraft-protocol can swap serializers when the state changes. That's not necessary for 1.6, so there should be one mapper for switching between packets (in the single state). It technically could be separate in the data here but it doesn't make sense because the implementation in nmp would have to dedupe the switches.

[brenoxavier]

The point of that separation is so node-Minecraft-protocol can swap serializers when the state changes. That's not necessary for 1.6, so there should be one mapper for switching between packets (in the single state). It technically could be separate in the data here but it doesn't make sense because the implementation in nmp would have to dedupe the switches.

Would a structure like this be a good idea? Segregate only into client packages, server packages, and packages used by both.

{
  "types": {

  },
  "toClient": {

  },
  "toServer": {

  },
  "twoWay": {

  }
}

[extremeheat]

The point of protocol.json is something that protodef can read and use inside node-minecraft-protocol. It's not designed around documenting anything for sake of it (like what direction the packets are). It just needs to work inside node-minecraft-protocol. You can make an accompanying PR to node-minecraft-protocol if you think you can make it work that way. What I'm saying is I don't see how that will work without special handling for merging the main packet switch, at least not in nice way in the code, as it would require some preprocessing to make it work your way.

What I meant was you just need { types, packets: {types} } and in node-minecraft-protocol you would do .addTypes(protocol.types); .addTypes(protocol.packets.types) to utilize it. Since there would be no collisions there would be no problem that way. Or just put them all directly into the root types.

[brenoxavier]

The point of protocol.json is something that protodef can read and use inside node-minecraft-protocol. It's not designed around documenting anything for sake of it (like what direction the packets are). It just needs to work inside node-minecraft-protocol. You can make an accompanying PR to node-minecraft-protocol if you think you can make it work that way. What I'm saying is I don't see how that will work without special handling for merging the main packet switch, at least not in nice way in the code, as it would require some preprocessing to make it work your way.

What I meant was you just need { types, packets: {types} } and in node-minecraft-protocol you would do .addTypes(protocol.types); .addTypes(protocol.packets.types) to utilize it. Since there would be no collisions there would be no problem that way. Or just put them all directly into the root types.

Perfect, I'll simplify the packet structure and work to make node-minecraft-protocol compatible with it.

[wgaylord]

I see some large issues so far. Why are their varints anywhere in the protocol.json. 1.6.4 didn't use varints at all.

Multiple packets don't line up even with Wiki.vg for 1.6.4 Example the packet_encryption_key_response packet you have the handling of the byte arrays all incorrect where you read the length but then tell the array that it need to read the length as a byte itself. (I think this might be from now knowing how to use ProtoDef to do that more then just errors.) You also have this multiple times in the file?

packet_encryption_key_request - Doesn't match wiki.vg at all

We may want to change some field names on packet_player_position_and_look due to the order changing between if the packet is going from server to client or client to server

packet_player_digging - Missing the status byte as the first argument

packet_bed - Missing a byte in it.

And those are only some of the issues I found at first glance, let alone the organizational issue of 1.6.4 not having server vs client or the concepts of states.

Also we may want to rename some packets to be consistent with the other versions, provided they do the same type of thing.

[brenoxavier]

I see some large issues so far. Why are their varints anywhere in the protocol.json. 1.6.4 didn't use varints at all.

Multiple packets don't line up even with Wiki.vg for 1.6.4 Example the packet_encryption_key_response packet you have the handling of the byte arrays all incorrect where you read the length but then tell the array that it need to read the length as a byte itself. (I think this might be from now knowing how to use ProtoDef to do that more then just errors.) You also have this multiple times in the file?

packet_encryption_key_request - Doesn't match wiki.vg at all

We may want to change some field names on packet_player_position_and_look due to the order changing between if the packet is going from server to client or client to server

packet_player_digging - Missing the status byte as the first argument

packet_bed - Missing a byte in it.

And those are only some of the issues I found at first glance, let alone the organizational issue of 1.6.4 not having server vs client or the concepts of states.

Also we may want to rename some packets to be consistent with the other versions, provided they do the same type of thing.

I will review the packages and fix this. This is my first contact with ProtoDef.

[brenoxavier]

I made some changes to the protocol.json:

• Simplified the protocol specification by removing states and directions. (@extremeheat)
• Reviewed all packages and corrected those identified as incorrect (@wgaylord).
• Aligned the protocol to closely resemble version 1.7.10.

[wgaylord]

• Reviewed all packages and corrected those identified as incorrect (@wgaylord).

I will go thru all the packets sometime soon and compare to source. Hopefully everything will line up well.

[wgaylord]

There are still quite a few errors in the protocol.json

I have commented on all them that I found. They could be still a few I missed but I went thru each back and compared it to 1.6.4 source (mapped using Minecraft Forge's mappings)

[rom1504]

hi @brenoxavier would you like to try and open a PR on nmp using this ?
otherwise it's a bit hard to tell if this would work

[brenoxavier]

hi @brenoxavier would you like to try and open a PR on nmp using this ? otherwise it's a bit hard to tell if this would work

It won't work. I will analyze how to modify the nmp to make it work with this.