-
Notifications
You must be signed in to change notification settings - Fork 59
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
chunk .dump()
doesn't match section mask for all versions <=1.17
#205
Comments
So two things:
So The problem is how we serialize in if (section !== null && !section.isEmpty()) {
section.write(smartBuffer)
} Ostensibly as a space-saving measure. And this is wrong, or at least a tricky corner. I think we need to rethink this. It might be we need do a naive compression pass (eliminating empty chunks and recalculating the section mask) whenever |
.dump()
doesn't match section mask for versions all versions <=1.17
I'm not looking at any docs here, is it invalid to serialize a chunk with zero solid blocks? I don't think so. The bug fix might be that we remove the Ping @extremeheat |
I was thinking of adding a return value to .dump() that returns the actual mask for the dumped chunk. But removing isEmpty() and having a compress function which removes empty sections and recalculates the section mask makes more sense to me. |
.dump()
doesn't match section mask for versions all versions <=1.17.dump()
doesn't match section mask for all versions <=1.17
I tried implementing your suggestion, and indeed there are other areas which subsequently fail if you update the section mask. So I took the quick and easy route and added a new function to get the actual mask for the dumped buffer:
When I'm using dump and load in my fs plugin, instead of calling getMask I call dumpMask and get the correct value, and the rest of the code works as before. |
Yeah, on the bedrock implementation we have some handling for this: prismarine-chunk/src/bedrock/1.3/SubChunk.js Lines 282 to 297 in 23ae199
And compact() on serializatized export: prismarine-chunk/src/bedrock/1.3/SubChunk.js Line 166 in 23ae199
Works by tracking the count of the items in the palette and rebuilding the storage at the end (if needed) when something like dump() would be called. |
In 1.15.2 PC version if you build up a chunk, then delete the blocks (replacing them with air), the chunk.getMask value returned is that for the previous tall chunk, i.e. it doesn't recompute the mask to only count upward to the last non-air block.
This means that if you build a tall tower, delete it with mining, save the chunk using chunk.dump, save the bitmask value using chunk.getMask, and then try to load the saved chunk using chunk.load with the stored getMask value, you get an error that you're reading past the memory bound.
This is using flying-squid and prismarine-web-client
The text was updated successfully, but these errors were encountered: