Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start adding 1.16.2 changes #323

Closed
rom1504 opened this issue Aug 6, 2020 · 10 comments
Closed

Start adding 1.16.2 changes #323

rom1504 opened this issue Aug 6, 2020 · 10 comments

Comments

@rom1504
Copy link
Member

rom1504 commented Aug 6, 2020

Seems that it's almost there and there are a few protocol changes https://wiki.vg/index.php?title=Pre-release_protocol&action=history
Could be nice to have 1.16.2-pre3 protocol

@nickelpro
Copy link
Member

I'm working on this and I'm fairly certain we need a new native type.

1.16.2 encodes block changes as varint based bit fields, where the 12 LSBs are always the chunk section offsets but the MSBs are based on the block id, and thus variable. We need a type that decodes as a varint, shifts the offsets out, and uses the whatever is left as a value.

@rom1504
Copy link
Member Author

rom1504 commented Aug 13, 2020

what packet ?

@nickelpro
Copy link
Member

@rom1504
Copy link
Member Author

rom1504 commented Aug 13, 2020

ok I would encode that as an array of varint and let the user do appropriate bit operations.

@nickelpro
Copy link
Member

nickelpro commented Aug 13, 2020

I've done it and intend to PR, but I don't really like that solution. Functions that worked perfectly fine with MultiBlock packet before 1.6.2, and conceivably could still work, are now broken. We decode all other bitfields and weirdness (topbitsetarray) at the packet level, why not this one?

@rom1504
Copy link
Member Author

rom1504 commented Aug 13, 2020

topbitsetarray was necessary, here it's not really necessary.
However if you have an idea of a reasonable native type (like not overly specific) we could add, please say.
What could we add except some kind of varint bitfield ?

@nickelpro
Copy link
Member

nickelpro commented Aug 13, 2020

In my original version I just had a native type called "varbitfield". It worked identically to a bitfield except it had a "remainder" field that would be left over after the shifts.

It's really no more necessary than topbitsetarray. If you wanted to you could encode/decode topbitsetarray as a rest buffer and leave the rest up to the user. At some level of granularity, all packets are just rest buffers that we special case from. I think maintaining a consistent interface to a given packet is more important than the difficulty of initially implementing the native types.

@rom1504
Copy link
Member Author

rom1504 commented Aug 13, 2020

varbitfield might be ok
The cost in "initially implementing the native types" is important to take into account not for the current implementation of protodefs but for future ones.
If we have few native types, that makes it easier to implement new and better protodef implementation (or in other languages)

in this case it could make sense to introduce it. Could you have a look how it would look like in js?

@nickelpro
Copy link
Member

nickelpro commented Aug 13, 2020

I took a good long look at node-protodef and node-minecraft and decided I have no idea what I'm doing. If you understand how bitfields and varints work, then this type is trivial though. Typical generated code for a bitfield right now looks like this:

"example": [
  "bitfield",
  [
    {
      "name": "unused",
      "size": 4,
      "signed": false
    },
    {
      "name": "x",
      "size": 4,
      "signed": false
    },
    {
      "name": "y",
      "size": 4,
      "signed": false
    },
    {
      "name": "z",
      "size": 4,
      "signed": false
    }
  ]
]

In pseudo-code

short temp_variable = decode_big_endian_short(buffer)
example.x = (temp_variable >> 8) & 0xF
example.y = (temp_variable >> 4) & 0xF
example.z = temp_variable & 0xF

A varbitfield is nearly identical, except is uses a varint for the decode and has a single extra field that you get by shifting for the full length of the bitfield. The remainder cannot be signed.

"example": [
  "varbitfield",
  [
    {
      "name": "extra_data",
      "size": "remainder",
    },
    {
      "name": "x",
      "size": 4,
      "signed": false
    },
    {
      "name": "y",
      "size": 4,
      "signed": false
    },
    {
      "name": "z",
      "size": 4,
      "signed": false
    }
  ]
]
varint temp_variable = decode_varint(buffer)
example.extra_data = temp_variable >> 12
example.x = (temp_variable >> 8) & 0xF
example.y = (temp_variable >> 4) & 0xF
example.z = temp_variable & 0xF

Actually looking at this now, I don't think varbitfield is really a necessary type. I think that bitfields should just be expanded to be varbitfields whenever they contain a "remainder" size.

@rom1504
Copy link
Member Author

rom1504 commented Dec 22, 2020

this is long done

@rom1504 rom1504 closed this as completed Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants