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.2k stars 241 forks source link

1.20.3 / 1.20.4 support #1275

Closed wgaylord closed 4 months ago

wgaylord commented 6 months ago

Updated minecraft-data, added 1.20.3 and 1.20.4 to the versions lists.

Added particle type into the packetTest.js.

Current Tests for 1.20.3/4 fail due to chat issues. Next step its to handle everywhere string is now an NBT object.

wgaylord commented 6 months ago

Honestly not very happy with what I came up with for fixing the chat for the client but it does work. Maybe we need to expand this out to pchat and have it handle it since it the server side of nmp will need to handle this nbt stuff as well and its not as simple.

extremeheat commented 6 months ago

This is not handling all the chat packets. We don't have tests for chat which is not good, so this should be manually tested against the examples in online mode also.

wgaylord commented 6 months ago

This is not handling all the chat packets. We don't have tests for chat which is not good, so this should be manually tested against the examples in online mode also.

Which ones is it missing? It covers all 3 chat packets.

rom1504 commented 6 months ago

please update the list of supported versions in the readme

wgaylord commented 6 months ago

When running the chat example it seems like we have an issue in the declare_commands packet somewhere. Also I do agree, should good behind a featureFlag. Since that prevents Mojang changing something in the future and breaking that type check hack.

wgaylord commented 6 months ago

Personally I am not happy with were this PR is going, the way I handle the NBT chat needs to be re-considered totally, with new info I learned from digging thru the change logs for the snapshots and we also have to dig thru the protocol.json to find what is breaking the declare command packet.

rom1504 commented 6 months ago

Let's maybe let it open for now and close if a new PR gets open? it does provide pieces of info even if not everything is good yet

rom1504 commented 6 months ago

so I looked at this PR again and actually I don't see what's wrong with it?

we just need to expose handleNBTStrings and do some manual testing and it should be fine?

what did you have in mind @wgaylord ?

wgaylord commented 6 months ago

It should be fine just doing that, but I am not that happy with how the handleNBTString works at all, but it could be good to get support for 1.20.3/4 then write a small library to properly handle the NBT String stuff. Plus we still need a function to go the other way for the server part of nmp.

rom1504 commented 5 months ago

would be nice to finish this we now have 1.20.2 support in minecraft ; so if this gets finished then we can have 1.20.4 in mineflayer and be up to date

boredcoder411 commented 4 months ago

I don't get what needs to be done for 1.20.4 compatibility. I have a week off starting Tuesday, so what can I do to help?

rom1504 commented 4 months ago

Maybe the best way to understand would be to open a PR on mineflayer pointing to this branch for nmp and get it to run

boredcoder411 commented 4 months ago

I would love to, but what do I actually need to get to work?

rom1504 commented 4 months ago

@boredcoder411 https://github.com/PrismarineJS/mineflayer/pull/3310 check the failing tests, read our comments above, then fix it

TerminalCalamitas commented 4 months ago

Is there a reason that this PR isn't getting merged even though it is passing all the checks?

rom1504 commented 4 months ago

Yes, please read above comments

rom1504 commented 4 months ago

code lgtm but I wonder if we need to update any example / doc

rom1504 commented 4 months ago

ok last thing to do here is updating the chat handling in the server example we could also decide to push that post this PR; but I think it'd be better to do it here

rom1504 commented 4 months ago

/fixlint

rom1504bot commented 4 months ago

I ran npm run fix, but there are errors still left that must be manually resolved:

> minecraft-protocol@1.45.0 fix
> standard --fix

  /home/runner/work/node-minecraft-protocol/node-minecraft-protocol/examples/server/server.js:15:7: 'nbt' is not defined. (no-undef)
  /home/runner/work/node-minecraft-protocol/node-minecraft-protocol/examples/server/server.js:15:24: 'nbt' is not defined. (no-undef)
standard: Use JavaScript Standard Style (https://standardjs.com)
rom1504 commented 4 months ago

ok updated examples

rom1504 commented 4 months ago

/makerelease