Add support for non-full packets

[Pandapip1]

CC #811, #1094

Currently, this project seems extraordinarily dead, in that it isn't being updated.

Admittedly, this PR is a band-aid and not a fix. But I'd rather have the option for hacky non-0% support than forced 0% support, when it comes to any recent version of the game.

[extremeheat]

What does this actually do?

[Pandapip1]

What does this actually do?

It adds a client / server option to have createDeserializer return a Parser object instead of a FullPacketParser:

https://github.com/PrismarineJS/node-minecraft-protocol/pull/1278/files#diff-7e2074fe137467401d88e0861ae91fede6ef856efbdea692c4656c68ae243d8fR49-R52

[extremeheat]

What does that do?

[Pandapip1]

What does that do?

FullPacketParser performs packet validity checks. New minecraft versions extend the existing protocol in ways that break those validitity checks, but where existing code can read 90% of the useful information. node-minecraft-protocol does need to be updated to read that remaining 10% that gets cut off, but that doesn't have to break compatibility if those features aren't yet needed.

[extremeheat]

extend the existing protocol in ways that break those validitity checks

Where in the code are you seeing this and for what version? Do you have a link to code somewhere?

[Pandapip1]

This conversation is already well-documented at #811, except oddly I can't find MC version numbers. IIRC, >1.19.0

https://github.com/ProtoDef-io/node-protodef/blob/e008e688d52478d82926888a59d85d79704ff462/src/serializer.js#L76

[extremeheat]

There is no such thing AFAIK of a partial packet. #811 is unrelated, that's caused by error in minecraft-data protocol, or invalid server data.

[Pandapip1]

There is no such thing AFAIK of a partial packet. #811 is unrelated, that's caused by error in minecraft-data protocol, or invalid server data.

Well then, this allows clients to handle invalid server data, and vice versa.

I'll fully admit: I'm not particularly experienced with regards to the MC protocol or protobuf in general. But what I do know is that there was a server that vanilla MC clients were able to connect to and that node-minecraft-protocol (and by extension, mineflayer) clients were not able to connect to, and it was because of that particular check. When decoding the extra data, it didn't look like invalid data. In fact, it looked like extra data that belonged in the packet.

At the very least, this config option allows us users to connect to such servers and submit more upstream PRs to node-minecraft-protocol to fix issues associated with these unusual configurations. The point of node-minecraft-protocol is to have a programmable client / server that can connect to the same servers / clients that vanilla MC can, right?

[extremeheat]

I think you misunderstood my last comment. This PR implies there is a thing as a partial packet, like one that has variable length. Or something to do with packet batching. As far as I am aware, this does not exist. Actually I think this seems like a dupe PR of #1094, what's changed here ?

As for #811, you are looking at two different errors. The deserializer (in node-minecraft-protocol) is failing to read something (not fatal) and the serializer is incorrectly encoding something that is being sent to a vanilla client, causing it to crash (which is fatal). These are two independent errors, and like I mention, caused by legitimate error in data from outside or a wrong definition in minecraft-data

[Pandapip1]

I think you misunderstood my last comment. This PR implies there is a thing as a partial packet, like one that has variable length. Or something to do with packet batching. As far as I am aware, this does not exist. Actually I think this seems like a dupe PR of #1094, what's changed here ?

#1094 and this PR share a git branch, which is why their files are synced. #1094 was closed when 7da26b4 was the latest commit. It fully disabled the FullPacketParser checks all the time.

This PR adds an option to disable them instead of having it disabled 100% of the time.

I'll look at #811 again. It's been half a year since I last touched this stuff.

[Pandapip1]

Never mind. I can't find the code that reproduces the error. Closing unless I can repro again.

[CrowSt]

I'm currently facing this issue:
Chunk size is 75 but only 1 was read ; partial packet : {"name":123}

mineflayer -> ZenithProxy -> 2b2t (but I guess the destination server doesn't matter)

I just check some blocks:
bot.findBlocks({ matching: bot.registry.blocksByName['dirt'].id, maxDistance: 5, count: 2 })

[extremeheat]

Those errors are caused by bad/missing packet definitions in minecraft-data, or illegal packets being sent by the server. This PR wanted to just remove sanity checks which is not good (does not fix anything)

[Pandapip1]

This PR wanted to just remove sanity checks which is not good (does not fix anything)

Correction: this PR added an option that would manually disable the sanity checks for debugging purposes. I'm not suggesting this option should ever be used in a user-facing application, but for debugging it'd be helpful for the client to at least try to parse the data.

[rom1504]

The current code https://github.com/Pandapip1/node-minecraft-protocol/blob/master/src/transforms/serializer.js#L5 is always using FullPacketParser and hence always parses partial packets if possible

So the only only thing this PR would have done is add an option to not parse partial packets and instead crash.