owengage / fastnbt

Fast serde serializer and deserializer for Minecraft's NBT and Anvil formats
MIT License
195 stars 36 forks source link

Rough support for pre 1.13 worlds #65

Closed Badel2 closed 2 years ago

Badel2 commented 2 years ago

This was easier than expected, but there are some missing things. However it is enough for my use case of "finding blocks by name", so it may be useful. I tested the Chunk::block API in every version from 1.2 to 1.13 and it seems to work.

owengage commented 2 years ago

<=1.12 support was a daunting task to me, so it getting started like this is great, thank you :)

This is marked as draft, but I'd be happy to merge it without personal testing given it only touches fastanvil.

owengage commented 2 years ago

Managed to test this out and got a 1.12 world rendering just fine, except all the old block names and stuff not having a palette colour which makes sense.

It's pretty awkward to translate all of this to work with the Chunk trait, the main issue seems to be Block, since that's where the text ID assumption is baked in. I think this approach works well, especially given that <1.12 is no longer worked on. Doesn't make sense to me to muddy the current Chunk trait with 1.12 concepts.

Looks like a bit of the legwork is filling in that giant match in block_name. I'm happy to do that if you'd like?

Badel2 commented 2 years ago

So I have tested this a bit, and it turns out that with the current approach the generated .wasm files use 2MB more of disk space. Which makes sense because of the giant BLOCK_LIST static array, and I assume that it is all zeros because when compressed the .wasm file has the same size as the previous version without 1.12 support. So this may not be a huge issue, but I will try to make it smaller, because now it supports all the ids from 0 to 4095, but minecraft only actually uses ids from 0 to 255 for blocks, I guess the other ids are only used in mods or something like that. So my plan is to implement something like this:

if block_id < 256 {
    BLOCK_LIST[index]
} else {
    custom_block_callback(block_id, data_value)
}

Where custom_block_callback can be changed by the user, just in case they need to support some id greater than 255. With that change the extra disk space should be around 128KB, a bit more acceptable.

Looks like a bit of the legwork is filling in that giant match in block_name. I'm happy to do that if you'd like?

Sure, as you wish. I got the existing ids from this list.

The hard part about that is to implement data value conversion, which is needed to differentiate the different wool colors for example, now they all map to white. Since I don't need that feature for now, I added the init_block function which can be used to initialize these blocks from outside of this crate.

So let me remove some of the TODOs and refactor a bit, and then you can merge this.

owengage commented 2 years ago

Looks good! Thanks. I'll sort out the relevant GitHub issues soon.