Support for schematics file format by MrlnHi · Pull Request #263 · valence-rs/valence

MrlnHi · 2023-03-02T17:30:01Z

Description

Added auxiliary crate valence_schem that supports copying and pasting schematics to an Instance and also allows for parsing them from a Compound and serializing it back to one

Test Plan

The examples schem_loading and schem_saving are sufficient for testing. Also, as for a third party created schematic, I used SimpleSpongeSchematics to do so.

To test, either:
try the example schematic by running cargo r --example schem_loading -- assets/example_schem.schem

OR:

Create a Schematic file using the sponge mod above
Start the schem_loading with the path of your schematic, like so: cargo r --example schem_loading -- "my/path/to/something_cool.schem"

+    let mut instance = world
+        .resource::<Server>()
+        .new_instance(DimensionId::default());
+
+    match Schematic::load(path) {
+        Ok(schem) => {
+            schem.paste(&mut instance, SPAWN_POS, |_| BiomeId::default());
+        }
+        Err(err) => {
+            eprintln!("Error loading schematic: {err}");
+            world.send_event(AppExit);
+        }
+    }


It would be nice to include this kind of thing in doc comments so it's visible to people looking at the docs.

What exactly do you think should be in doc comments?

A short explanation of what schematics are, examples for basic usage with a little context. You can look at the doc comments in the inventory module for reference.

MrlnHi · 2023-03-03T05:31:15Z

Question: do we want to support all spec versions? (1-3)
I don't think we should for serialization, but for deserialization probably. The deserialization would then check the version and act accordingly.
So basically by deserializing an older schematic and serializing it again, we would upgrade it to version 3. I don't think we should support DataVersion (meaning knowing how to handle 1.8 schematics with blocks like minecraft:planks) but probably the Version property. What do you think?

rj00a · 2023-03-04T11:29:03Z

I don't think we should support DataVersion (meaning knowing how to handle 1.8 schematics with blocks like minecraft:planks) but probably the Version property. What do you think?

That seems like the most reasonable thing to do now.

MrlnHi · 2023-04-23T10:25:06Z

@rj00a ready for review

rj00a · 2023-04-28T10:36:53Z

Currently working on a redesign of the Instance API to fix a number of issues. Once that's mostly finished I can take a closer look at this and see what impact it has.

rj00a · 2023-07-01T23:25:48Z

I've (finally) finished the redesign in #402, so we can move forward with this.

MrlnHi · 2023-07-29T22:26:36Z

I've (finally) finished the redesign in #402, so we can move forward with this.

Ok, I updated the branch to main

dyc3

I gave it a quick look over again. This might need to be updated to reference chunk layers instead of instances.

rj00a · 2023-08-15T12:47:13Z

I think this code would be a bit easier to follow if it was split into "save" and "load" modules.

rj00a · 2023-08-15T12:48:03Z

Some documentation on the public items here would be appreciated.

rj00a · 2023-08-15T12:52:45Z

+# use valence_schem::Schematic;
+use flate2::Compression;


Suggested change

# use valence_schem::Schematic;

use flate2::Compression;

use valence_schem::Schematic;

rj00a · 2023-08-15T12:53:16Z

+# use valence_schem::Schematic;
+use valence_nbt::Compound;


Suggested change

# use valence_schem::Schematic;

use valence_nbt::Compound;

use valence_schem::Schematic;

use valence_nbt::Compound;

rj00a · 2023-08-15T12:54:07Z

+
+Support for the [Sponge schematic file format](https://github.com/SpongePowered/Schematic-Specification).
+
+This crate implements [Sponge schematics]


This line is redundant.

rj00a · 2023-08-15T13:41:02Z

+        let width = u16::try_from(max.x - min.x + 1).expect("width too large");
+        let height = u16::try_from(max.y - min.y + 1).expect("height too large");
+        let length = u16::try_from(max.z - min.z + 1).expect("length too large");
+        let offset = IVec3::new(min.x - origin.x, min.y - origin.y, min.z - origin.z);
+        let blocks: Vec<_> = (min.y..=max.y)
+            .flat_map(|y| {
+                (min.z..=max.z).flat_map(move |z| {
+                    (min.x..=max.x).map(move |x| {
+                        let Some(block) = layer.block([x, y, z]) else {
+                            panic!("coordinates ({x} {y} {z}) are out of bounds");


We should return an error instead of panicking in these situations.

rj00a · 2023-08-15T13:41:38Z

+                        .map(map_biome)
+                        .enumerate()
+                        .flat_map(|(idx, biome)| {
+                            let idx = u16::try_from(idx).unwrap();


Can these unwraps fail?

rj00a · 2023-08-15T13:52:34Z

+                        (min.y..=max.y).map(move |y| {
+                            layer
+                                .chunk(ChunkPos::from_block_pos(BlockPos::new(x, y, z)))
+                                .unwrap()


This looks like a bad unwrap.

rj00a · 2023-08-15T13:53:36Z

I'm seeing a lot of unwraps that are making me nervous. Consider if the unwrapping can panic.

rj00a · 2023-08-15T14:06:07Z

+struct VarIntReader<I: ExactSizeIterator<Item = u8>>(I);
+impl<I: ExactSizeIterator<Item = u8>> Iterator for VarIntReader<I> {
+    type Item = Result<i32, VarIntDecodeError>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        struct ReadWrapper<I: ExactSizeIterator<Item = u8>>(I);
+        impl<I: ExactSizeIterator<Item = u8>> Read for ReadWrapper<I> {
+            fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+                for (idx, byte) in buf.iter_mut().enumerate() {
+                    let Some(val) = self.0.next() else {
+                        return Ok(idx);
+                    };
+                    *byte = val;
+                }
+                Ok(buf.len())
+            }
+        }
+
+        if self.0.len() == 0 {
+            None
+        } else {
+            Some(VarInt::decode_partial(ReadWrapper(&mut self.0)))
+        }
+    }
+}
+
+struct VarIntWriteWrapper<'a>(&'a mut Vec<i8>);
+impl<'a> Write for VarIntWriteWrapper<'a> {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        self.0.extend(buf.iter().map(|byte| *byte as i8));
+        Ok(buf.len())
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        Ok(())
+    }
+}


I'm going to expose some functions in valence_nbt that should make these wrappers unnecessary.

Ok, I've done that in #465.

rj00a · 2023-08-15T15:11:50Z

+        .run();
+}
+
+fn setup(


Chunks aren't being initialized here so the example isn't quite working.

# Objective When using valence_nbt, it's often necessary to convert to and from slices and vecs of `i8` and `u8`. But this requires unsafe code if you want to avoid copying things. # Solution Expose the following functions in valence_nbt: - `u8_vec_into_i8_vec(vec: Vec<u8>) -> Vec<i8>` - `i8_vec_into_u8_vec(vec: Vec<i8>) -> Vec<u8>` - `u8_slice_as_i8_slice(slice: &[u8]) -> &[i8]` - `i8_slice_as_u8_slice(slice: &[i8]) -> &[u8]` We've also made use of these functions ourselves to reduce the total amount of unsafe code. Should also help in #263

rj00a · 2023-08-15T15:17:41Z

+                let Some(kind) = BlockKind::from_str(kind) else {
+                    return Err(ParseBlockStateError::UnknownBlockKind(kind.to_string()));
+                };
+                props[..props.len() - 1]


This will panic if props is empty. Also it doesn't check if the last char is actually a ].

dyc3 reviewed Mar 2, 2023

View reviewed changes

rj00a reviewed Mar 3, 2023

View reviewed changes

Comment thread crates/valence_schem/examples/schem_loading.rs Outdated

MrlnHi force-pushed the feat/sponge_schem branch 3 times, most recently from 8555e08 to dddc967 Compare March 5, 2023 08:18

MrlnHi marked this pull request as ready for review March 8, 2023 12:56

MrlnHi requested review from dyc3 and rj00a and removed request for dyc3 and rj00a March 8, 2023 12:56

MrlnHi force-pushed the feat/sponge_schem branch from c5e8674 to 12cc6b7 Compare April 22, 2023 23:24

MrlnHi added 14 commits April 23, 2023 11:51

initial sponge schematic version 2 implementation

bc6d058

upgrade to sponge schematic version 3

dbe79ab

add entities to schematics

8ee6c3f

Implement schem saving

b7d671a

Make setup not exclusive systems

86f7b9c

Make select pos1 not break block

2a96178

Improve saving

4ae80ba

Fix BlockEntityKind::from_ident

314eb0c

Add example schematic to assets

4c37f60

Fixed test

ada8da4

fix rebase

a2e32be

Implement version 1 and 2

8f474ee

add module documentation

a076977

Fix unit tests

8c0054c

MrlnHi added 3 commits April 23, 2023 11:51

Fix formatting

953a851

Fix schem_loading example

58001c9

Allow type complexity in schem_saving example because of bevy queries

ae387ff

MrlnHi force-pushed the feat/sponge_schem branch from 0679801 to ae387ff Compare April 23, 2023 09:51

MrlnHi force-pushed the feat/sponge_schem branch from 8d04394 to b69399d Compare June 11, 2023 13:48

MrlnHi added 2 commits June 11, 2023 15:54

Merge branch 'main' into HEAD

9bf5af0

Fix after merge

13e0594

MrlnHi force-pushed the feat/sponge_schem branch from b69399d to 13e0594 Compare June 11, 2023 13:59

MrlnHi added 2 commits July 29, 2023 23:41

Merge branch 'main' into feat/sponge_schem

1c5c8fa

Fix unit test

d4eb42d

Merge branch 'main' into feat/sponge_schem

32f20ca

dyc3 reviewed Aug 3, 2023

View reviewed changes

MrlnHi added 4 commits August 14, 2023 21:49

Use chunk layers instead of instances

e4c20b9

Merge branch 'main' into feat/sponge_schem

3c7a2b2

Fix after merge

e646c63

Fix unit test and formatting

b24fb5a

dyc3 reviewed Aug 14, 2023

View reviewed changes

Comment thread crates/valence_generated/src/block.rs

rj00a requested changes Aug 15, 2023

View reviewed changes

rj00a mentioned this pull request Aug 15, 2023

Consolidate unsafe byte conversions in valence_nbt #465

Merged

rj00a reviewed Aug 15, 2023

View reviewed changes


		Support for the [Sponge schematic file format](https://github.com/SpongePowered/Schematic-Specification).

		This crate implements [Sponge schematics]

Uh oh!

Uh oh!

Conversation

MrlnHi commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Plan

Related

Uh oh!

qualterz commented Mar 2, 2023

Uh oh!

MrlnHi commented Mar 2, 2023

Uh oh!

dyc3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dyc3 Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MrlnHi commented Mar 3, 2023

Uh oh!

rj00a commented Mar 4, 2023

Uh oh!

MrlnHi commented Apr 23, 2023

Uh oh!

rj00a commented Apr 28, 2023

Uh oh!

rj00a commented Jul 1, 2023

Uh oh!

MrlnHi commented Jul 29, 2023

Uh oh!

dyc3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rj00a Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rj00a Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MrlnHi commented Mar 2, 2023 •

edited

Loading

dyc3 Mar 5, 2023 •

edited

Loading

rj00a Aug 15, 2023 •

edited

Loading

rj00a Aug 15, 2023 •

edited

Loading