PrismarineJS / prismarine-schematic

Read and write schematic of any minecraft version, provide them in a convenient and stable API
https://prismarine.js.org/prismarine-schematic/
MIT License
21 stars 8 forks source link

improve handling of legacy schematics #14

Closed rom1504 closed 3 years ago

rom1504 commented 3 years ago

https://github.com/PrismarineJS/prismarine-schematic/blob/a15b4c8003ad7ce9d77111efaf52cc99c1afa860/lib/mceditSchematic.js

why are we using legacy.json ? why not simply minecraft-data ? this doesn't work for many schematics, as can be reproduced by https://github.com/rom1504/minecraft-schematics-dataset/pull/7/files

Karang commented 3 years ago

We dont have this mapping in mcdata. Its the mapping between id:data from pre-1.13 to the string names of 1.13

Karang commented 3 years ago

One hypothesis: maybe the data is swap with the next block. Leading to invalide id/data couples

Karang commented 3 years ago

Also the field material in the nbt should be alpha. Classic and pocket are not supported

rom1504 commented 3 years ago

That legacy.json file contains a very small part of all the blocks. That seems the most likely problem to me.

We do have all ids for pre 1.13 version.

Why do we need to map to post 1.13 ?

rom1504 commented 3 years ago

https://github.com/1b8/schematic/blob/master/index.js#L40 the previous implementation was indeed doing something with block entities

rom1504 commented 3 years ago

Mostly it was just using the block and data directly with prismarine-block without converting to anything else though

Karang commented 3 years ago

The mapping in legacy.json seems to be complete. The schematics that fails have invalid id data, we need to understand why.

The mapping is useful to read an old schematic in a new version

rom1504 commented 3 years ago

so https://github.com/PrismarineJS/prismarine-schematic/pull/16 will improve a bit the situation

However I think it would be better if we could first load it as it is (that means with a new LegacySchematic class), and only afterwards have a export function that will be lossy That's the only way to have a cycle test that passes, and to handle old schematics (even if metadata is invalid)

Not doing it right now as it's not really needed for my use case

rom1504 commented 3 years ago

Working fine enough