feather-rs / feather

A Minecraft server implementation in Rust
Apache License 2.0
2.61k stars 142 forks source link

Can't load vanilla world #265

Closed Schuwi closed 4 years ago

Schuwi commented 4 years ago

According to the README, loading a vanilla save should work but it doesn't.

2020-06-10 23:53:53,133 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: 3, z: -3 }: Region file contains invalid NBT: invalid type: map, expected a sequence

This is the error feather produces for basically every chunk the server wants to load. Investigating a bit further it seems that the NBT chunk format implementation is wrong. feather-anvil::region::ChunkLevel.heightmaps is a Vec but inspecting the vanilla save in an NBT viewer Level/Heightmaps is actually a struct of multiple [u64; 36] (basically [u9; 256] packed into u64 array) fields. (And also a bit different field names than those listed on the Minecraft Wiki.)

When temporarily circumventing this error I also get a plethora of other errors:

2020-06-11 01:33:02,281 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -9, z: -15 }: Chunk contains invalid block minecraft:vine
2020-06-11 01:33:02,281 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -9, z: -14 }: Chunk contains invalid block minecraft:lava
2020-06-11 01:33:02,330 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -9, z: -7 }: Chunk contains invalid block minecraft:water
2020-06-11 01:33:02,330 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -8, z: -13 }: Region file contains invalid NBT: invalid type: integer `5`, expected f32
2020-06-11 01:33:02,380 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -8, z: -7 }: Chunk contains invalid block minecraft:oak_log
2020-06-11 01:33:02,380 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -7, z: -5 }: Chunk contains invalid block minecraft:birch_log
2020-06-11 01:33:02,480 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -2, z: -14 }: Chunk contains invalid block minecraft:oak_fence
2020-06-11 01:33:02,480 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: -2, z: -11 }: Chunk contains invalid block minecraft:tall_grass
2020-06-11 01:33:02,530 WARN  [feather_server_chunk::chunk_manager] Failed to load chunk at ChunkPosition { x: 3, z: -15 }: Chunk contains invalid block minecraft:rail

(There were many of each of those errors but I picked only one of each kind for the issue.) I haven't investigated those further yet.

I'm working on fixing the heightmaps implementation. Shouldn't be that hard, just need to implement a de/serializer for those packed unsigned 9-bit arrays.

caelunshun commented 4 years ago

Thanks for the report! Sorry about that, I'll get to fixing this. Haven't tried loading vanilla worlds in a while :)

Schuwi commented 4 years ago

So I've looked into it a bit more. I am currently fixing the heightmaps (Should basically work already, but I need to test it. Also I took this opportunity to try to implement LongArray serailization in hermatite-nbt, so I'll try to wait for that (coming soon) PR to be merged).

Regarding the other errors:

Region file contains invalid NBT: invalid type: integer `5`, expected f32

is caused by the Health field in feather-anvil::entity::BaseEntityData which turns out to not be a common field for all entities. Only mobs apparently have it (https://minecraft.gamepedia.com/Chunk_format#Entity_format) and the parsing error is created because the Item entity also has a field named Health but it is of type i16.

Chunk contains invalid block minecraft:xxxx

seem to be caused by invalid block_state property names in feather-blocks/generated/block_fns.rs. I haven't looked into how this file is generated yet.

Region file contains invalid NBT: unknown variant `minecraft:mob_spawner`, expected one of `minecraft:beacon`, `minecraft:bed`, [...]

is caused by a lot of block entity kinds missing from feather-anvil::block_entity::BlockEntityKind.

Region file contains invalid NBT: missing field `Items`

The chest block entity apparently has no Items NBT tag when it is a generated loot chest (like a village blacksmith chest) that hasn't been opened yet. Maybe just use #[serde(default)] on this field as it's a vec?

caelunshun commented 4 years ago

Interesting, thanks for investigating this!

Chunk contains invalid block minecraft:xxxx

seem to be caused by invalid block_state property names in feather-blocks/generated/block_fns.rs. I haven't looked into how this file is generated yet.

I think this is caused by us renaming properties to account for the same property name being used for different values in vanilla. For example, axis in vanilla is renamed to axis_xyz, axis_xz, etc. depending on its meaning. The problem is that we use these renamed strings in the serializer functions as well, meaning we can't interoperate properly with vanilla. I've pushed a commit which I hope fixes this, though I haven't had time to test it thoroughly yet.

Schuwi commented 4 years ago

Yes! This fixed it! Though your commit message and the name of the generate_block_serializing_fns function are a bit ambiguous because the method also generates functions used for deserialization (e.g. block_fns::xxxxxx_from_identifier_and_properties). Otherwise this would also have had no effect on this issue, as I have only tested world loading for now ^^

The only message I get now related to block_fns.rs is

Chunk contains invalid block minecraft:stone_slab

Looking at the NBT data minecraft:stone_slab has a block property named type that is apparently an enum of values double, top or bottom (https://minecraft.gamepedia.com/Slab#Block_states). In block_fns.rs the functions apparently look for the property name kind instead. I have yet to figure out what causes this.

Defman commented 4 years ago

Kind is a feather synonym for type, due to type being a keyword in rust.

Schuwi commented 4 years ago

The only message I get now related to block_fns.rs is

Chunk contains invalid block minecraft:stone_slab

Looking at the NBT data minecraft:stone_slab has a block property named type that is apparently an enum of values double, top or bottom (https://minecraft.gamepedia.com/Slab#Block_states). In block_fns.rs the functions apparently look for the property name kind instead. I have yet to figure out what causes this.

feather-blocks-generator::load::fix_keywords seems to be the culprit here. This modifies the name even before you make the distinction of real_name and new_name. Is this function even necessary? One renaming function should be enough right? Removing fix_keywords and changing NAME_OVERRIDES to

[...]
"type_slr" => "chest_kind",
"type_tbd" => "slab_kind",
"type_ns" => "piston_kind",
[...]

seems to work for me, but I'm not sure whether there are any side-effects to this solution.

Schuwi commented 4 years ago

Kind is a feather synonym for type, due to type being a keyword in rust.

Ahhh, I see, that makes sense. Okay then this will have to be incorparated into real_name and new_name somehow.

caelunshun commented 4 years ago
[...]
"type_slr" => "chest_kind",
"type_tbd" => "slab_kind",
"type_ns" => "piston_kind",
[...]

seems to work for me, but I'm not sure whether there are any side-effects to this solution.

Thanks, that did the trick—I've pushed a commit which should fix the kind => type issue. (I just realized what a mess feather-blocks-generator is... I'm going to have to clean it up at some point :) )