prismarine-chunk icon indicating copy to clipboard operation
prismarine-chunk copied to clipboard

networkDecodeNoCache super call passes Stream instead of Buffer

Open lowlines opened this issue 2 years ago • 10 comments

The super call this function makes parses the Stream instead of Buffer, which throws an error since new Stream(stream) doesn't type check that it is a buffer and handle accordingly.

https://github.com/PrismarineJS/prismarine-chunk/blob/0f4ec64bd0ef8a84f04d825358dd1443e4310aa6/src/bedrock/1.18/ChunkColumn.js#L149

lowlines avatar May 23 '22 05:05 lowlines

It's also worth noting that this line should return as errors thrown inside this super call cannot be caught and the app will crash.

return super.networkDecodeNoCache(buffer, sectionCount)

lowlines avatar May 23 '22 06:05 lowlines

This isn't a condition that should generally be hit at all. Feel free to open a PR to fix.

extremeheat avatar May 23 '22 18:05 extremeheat

Really? I'm seeing that condition hit 100% of the time with 1.18.30 level_chunk packet data. Also the presence of border blocks.

lowlines avatar May 24 '22 02:05 lowlines

Bedrock 1.18.30 chunks are not currently supported, 1.18.0 chunks are.

extremeheat avatar May 24 '22 02:05 extremeheat

I implemented experimental support for newer chunks and fixed 2 errors I found. I'm not sure how to go about doing a PR though as I don't have write access. Should I create a fork and link to that?

lowlines avatar May 24 '22 03:05 lowlines

Yes you can open a PR. You need to fork, push changes to your branch and make a PR to merge your branch into PrismarineJs master

extremeheat avatar May 24 '22 04:05 extremeheat

Only change with bedrock 1.18.10 over .0 I believe was an extra byte in the stream header. Will address this in a PR tomorrow, hopefully Mojang doesn't ship 1.19 before then 😅.

extremeheat avatar Jun 04 '22 06:06 extremeheat

So what I ended up doing was implement my own ChunkColumn and SubChunk classes because updating prismarine was creating a bit of a blocker for me. I mostly just added a condition for when the sub_chunk_count is -2 to just loadBiomes and ignore the rest of the data. It's still a few days before 1.19 ships!

lowlines avatar Jun 04 '22 07:06 lowlines

mode -2 (added in 1.18.30) appears to just add an extra field to the LevelChunk packet excluding the change with the RequestSubChunk now taking arrays. Could be more changes, needs more investigation. Will address tomorrow if it's just that, no other changes with 1.19 it seems like

extremeheat avatar Jul 23 '22 20:07 extremeheat

Turns out there were more changes with chunk request procedure. This needs to be handled in bedrock provider tests

extremeheat avatar Aug 24 '22 17:08 extremeheat