Support for schematics file format #263

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.

crates/valence_schem/examples/schem_loading.rs

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.

crates/valence_generated/src/block.rs

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

crates/valence_schem/src/lib.rs

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

crates/valence_schem/src/lib.rs

Some documentation on the public items here would be appreciated.

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

crates/valence_schem/README.md

+# 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

crates/valence_schem/README.md

+# 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

crates/valence_schem/README.md

+
+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

crates/valence_schem/src/lib.rs

+        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

crates/valence_schem/src/lib.rs

+                        .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

crates/valence_schem/src/lib.rs

+                        (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

crates/valence_schem/src/lib.rs

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

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

crates/valence_schem/src/lib.rs

+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

examples/schem_loading.rs

+        .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

crates/valence_generated/src/block.rs

+                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

crates/valence_schem/examples/schem_loading.rs Outdated Show resolved Hide resolved

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 rj00a and dyc3 and removed request for rj00a and dyc3 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

crates/valence_generated/src/block.rs Show resolved Hide resolved

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

examples/schem_loading.rs

.run();

}

fn setup(

Copy link

Member

rj00a Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

rj00a reviewed Aug 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for schematics file format #263

Support for schematics file format #263

MrlnHi commented Mar 2, 2023 •

edited

Loading

qualterz commented Mar 2, 2023

MrlnHi commented Mar 2, 2023

dyc3 left a comment

dyc3 Mar 2, 2023

MrlnHi Mar 4, 2023

dyc3 Mar 5, 2023 •

edited

Loading

MrlnHi commented Mar 3, 2023

rj00a commented Mar 4, 2023

MrlnHi commented Apr 23, 2023

rj00a commented Apr 28, 2023

rj00a commented Jul 1, 2023

MrlnHi commented Jul 29, 2023

dyc3 left a comment

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023 •

edited

Loading

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023

rj00a Aug 15, 2023 •

edited

Loading


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

		This crate implements [Sponge schematics]

Support for schematics file format #263

Are you sure you want to change the base?

Support for schematics file format #263

Conversation

MrlnHi commented Mar 2, 2023 • edited Loading

Description

Test Plan

Related

qualterz commented Mar 2, 2023

MrlnHi commented Mar 2, 2023

dyc3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dyc3 Mar 5, 2023 • edited Loading

Choose a reason for hiding this comment

MrlnHi commented Mar 3, 2023

rj00a commented Mar 4, 2023

MrlnHi commented Apr 23, 2023

rj00a commented Apr 28, 2023

rj00a commented Jul 1, 2023

MrlnHi commented Jul 29, 2023

dyc3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rj00a Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rj00a Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

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