PrismarineJS / node-minecraft-protocol

Parse and serialize minecraft packets, plus authentication and encryption.
https://prismarinejs.github.io/node-minecraft-protocol/
BSD 3-Clause "New" or "Revised" License
1.22k stars 239 forks source link

Pc1.20.2 #1265

Closed extremeheat closed 9 months ago

extremeheat commented 9 months ago

Fix https://github.com/PrismarineJS/node-minecraft-protocol/issues/1258

The configuration state introduces lots of small problems in server/client expectations so there remain some errors in the test due to state desync. Will need to be investigated further

Needs audit of protocol.json for 1.20.2 against vanilla source code diff. Also, node-protodef-validator is not working.

After https://github.com/PrismarineJS/prismarine-nbt/pull/81 and https://github.com/PrismarineJS/minecraft-data/pull/810

extremeheat commented 9 months ago

ok, tests are now passing, but looks like there may be an issue with packet_chunk_batch_start

[05:40:13] [Server thread/INFO]: [Not Secure] <Player> hello everyone; I have logged in.
PartialReadError: Read error for undefined : undefined
    at new ExtendableError (/workspace/node-minecraft-protocol/node_modules/protodef/src/utils.js:63:13)
    at new PartialReadError (/workspace/node-minecraft-protocol/node_modules/protodef/src/utils.js:70:5)
    at Object.readVarInt [as varint] (/workspace/node-minecraft-protocol/node_modules/protodef/src/datatypes/utils.js:69:45)
    at Object.packet_chunk_batch_start (/workspace/node-minecraft-protocol/node_modules/protodef/src/g-ReadCompiler104533269d.js:956:67)
    at /workspace/node-minecraft-protocol/node_modules/protodef/src/g-ReadCompiler104533269d.js:3002:74
    at packet (/workspace/node-minecraft-protocol/node_modules/protodef/src/g-ReadCompiler104533269d.js:3104:9)
    at CompiledProtodef.read (/workspace/node-minecraft-protocol/node_modules/protodef/src/compiler.js:70:12)
    at e.message (/workspace/node-minecraft-protocol/node_modules/protodef/src/compiler.js:111:49)
    at tryCatch (/workspace/node-minecraft-protocol/node_modules/protodef/src/utils.js:50:16)
    at CompiledProtodef.parsePacketBuffer (/workspace/node-minecraft-protocol/node_modules/protodef/src/compiler.js:111:29)
    at FullPacketParser.parsePacketBuffer (/workspace/node-minecraft-protocol/node_modules/protodef/src/serializer.js:68:23)
    at FullPacketParser._transform (/workspace/node-minecraft-protocol/node_modules/protodef/src/serializer.js:74:21)
  ...
    at Socket.emit (node:events:514:28)
    ...
      ✔ does not crash for 10000ms (10489ms)

Currently chunk batch start looks like

        "packet_chunk_batch_start": [
          "container",
          [
            {
              "name": "batchSize",
              "type": "varint"
            }
          ]
        ],

But it seems chunk batch start in 1.20.2 has no fields https://github.com/extremeheat/extracted_minecraft_data/blob/client1.20.2/client/net/minecraft/network/protocol/game/ClientboundChunkBatchStartPacket.java

but finish retains the varint https://github.com/extremeheat/extracted_minecraft_data/blob/client1.20.2/client/net/minecraft/network/protocol/game/ClientboundChunkBatchFinishedPacket.java

extremeheat commented 9 months ago

Actually, packet_chunk_batch_start is a new packet in 1.20.2, it's incorrect in mc-data. Would be a good idea to check the other types also against the source code, there are probably more unreviewed errors/missing things

wgaylord commented 9 months ago

Actually, packet_chunk_batch_start is a new packet in 1.20.2, it's incorrect in mc-data. Would be a good idea to check the other types also against the source code, there are probably more unreviewed errors/missing things

Looks like that may have been a dumb copy paste error on my part when doing all the packet changes in 1.20.2. As all my work was based on info from wiki.vg (Probably should have sanity checked it from source at some point)

extremeheat commented 9 months ago

Ok, tests are passing now. Validator issue is fixed, just need to check over the protocol.json now. But anyway I think this can be merged before that, would just need to be checking before working on mineflayer

rom1504 commented 9 months ago

looks good, I'm glad we could solve the nbt change with no hack and actually even improving the code, nice work @extremeheat

I think it would be good to fix PartialReadError: Read error for undefined : undefined before merging as this is definitely going to break usage of the lib for this version

@wgaylord do you want to check the packets you added against the source? that way you can learn to do it for 1.20.2 and then apply the same method for 1.20.3

wgaylord commented 9 months ago

Yeah, I am going to go thru all the packets I touched and double check them some time tomorrow.

rom1504 commented 9 months ago

No value for type soundSource , packedChunkedPos in packet tests

rom1504 commented 9 months ago

does seem to work fully except for this one, no error anymore for 1.20.2

rom1504 commented 9 months ago

I think it's good now, merging

rom1504 commented 9 months ago

/makerelease