Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

increased supported blockids to 65792 #25

Merged
merged 4 commits into from
Jan 18, 2025

Conversation

Fabboz
Copy link

@Fabboz Fabboz commented Dec 7, 2024

I found a little trick to overcome the 4069 block id limit.
All previous schematics must be erased because the data is stored in a completely different way.

I found a little trick to overcome the 4069 block id limit.
All previous schematics must be erased because the data is stored in a completely different way.
@chochem chochem added the ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version label Dec 7, 2024
@Dream-Master Dream-Master requested a review from a team December 7, 2024 12:15
Copy link

@mist475 mist475 left a comment

Choose a reason for hiding this comment

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

Gtnh schematica is also used outside of gtnh (passive support).
If I understand what you said correctly this would break any existing schematics right?
If so, please make it able to still read old schematics and make it possible to differentiate between the old and the new format. You can achieve this by adding an extra nbt tag to schematics that use the new format and checking if that tag is present before trying to parse the schematic.
This would also break compatibility with world edit and any other tools that also use the .schematic format. Maybe this should be a config option (default can be true, as long as it can be turned off so people that rely on external tools can still use this version)

@Fabboz
Copy link
Author

Fabboz commented Dec 7, 2024

Gtnh schematica is also used outside of gtnh (passive support). If I understand what you said correctly this would break any existing schematics right? If so, please make it able to still read old schematics and make it possible to differentiate between the old and the new format. You can achieve this by adding an extra nbt tag to schematics that use the new format and checking if that tag is present before trying to parse the schematic. This would also break compatibility with world edit and any other tools that also use the .schematic format. Maybe this should be a config option (default can be true, as long as it can be turned off so people that rely on external tools can still use this version)

Yes it breaks all the schematics and from what you tell me they no longer represent their standard.
They can only be read by mods that make these changes.
However, to convert the schematics of the previous version you need to create a specific mod, and the new ones created can only be read by the mods that make these changes.
If there is a piece of code that updates ids using GameRegistry.findBlock I don't know where it is.
Also a config is not the best solution to resolve the issue with id block limit.
It might be better to import the schematic into the world with worldedit and create one specifically with schematica.
Maybe a change of the file extesion to better storing the schematics.

@mist475 mist475 requested a review from Cleptomania December 7, 2024 14:42
@Fabboz
Copy link
Author

Fabboz commented Dec 8, 2024

concluding the discussion I can create a config that allows you to choose before startup whether to use the old or new system combined with the creation of a different file extension. I'll do this.

@mist475
Copy link

mist475 commented Dec 8, 2024

@Fabboz
Copy link
Author

Fabboz commented Dec 8, 2024

I made the changes.
I have no idea how neid works.

@PlayfulPiano
Copy link

Is this the reason why some schematics are not recognized (e.g. eio conduits, lp pipes)?

@Fabboz
Copy link
Author

Fabboz commented Dec 10, 2024

Only vanilla blocks have the same id in every world. For modded blocks they can change depending on the sequence in which the mods are loaded. So it may be that in one modpack the EIO conduits have an id of 7000 and in another 4000. Besides this, in the classic schematics if the id exceeds a certain limit a wrong one is stored.

@PlayfulPiano
Copy link

Only vanilla blocks have the same id in every world. For modded blocks they can change depending on the sequence in which the mods are loaded. So it may be that in one modpack the EIO conduits have an id of 7000 and in another 4000. Besides this, in the classic schematics if the id exceeds a certain limit a wrong one is stored.

Do you happen to have a built jar with these commits so I can test? I tried doing a build but it failed on spotless.

@RecursivePineapple
Copy link

@PlayfulPiano I just allowed the workflow to run and it generated a few jars in a zip file. I'm not sure if these'll work, but you can try them.

https://github.com/GTNewHorizons/Schematica/actions/runs/12225450497?pr=25

@PlayfulPiano
Copy link

@PlayfulPiano I just allowed the workflow to run and it generated a few jars in a zip file. I'm not sure if these'll work, but you can try them.

https://github.com/GTNewHorizons/Schematica/actions/runs/12225450497?pr=25

damn, looks like the issue is separate then. oh well, was hoping.
image

@PlayfulPiano
Copy link

PlayfulPiano commented Dec 11, 2024

Also, my recommendation would be to include a toggle between the two formatting types within the load schematic gui (plus an easy toggle in the save schematic gui), and maybe include a message popup if it comes up blank for the current format but there exists the other format in the folder. That way, if people want to start switching over to the new formatting, they both will see visibly that it exists and if the default format changes, also warn why old schematics aren't showing up.

@Fabboz
Copy link
Author

Fabboz commented Dec 11, 2024

@PlayfulPiano check the material list in manipolation gui.

@Fabboz
Copy link
Author

Fabboz commented Dec 11, 2024

Also, my recommendation would be to include a toggle between the two formatting types within the load schematic gui (plus an easy toggle in the save schematic gui), and maybe include a message popup if it comes up blank for the current format but there exists the other format in the folder. That way, if people want to start switching over to the new formatting, they both will see visibly that it exists and if the default format changes, also warn why old schematics aren't showing up.

I dont know how i can do that.

@PlayfulPiano
Copy link

@PlayfulPiano check the material list in manipolation gui.

unsure what you mean by manipolation but if you're referring to the materials list, it doesn't show there either.

Also, my recommendation would be to include a toggle between the two formatting types within the load schematic gui (plus an easy toggle in the save schematic gui), and maybe include a message popup if it comes up blank for the current format but there exists the other format in the folder. That way, if people want to start switching over to the new formatting, they both will see visibly that it exists and if the default format changes, also warn why old schematics aren't showing up.

I dont know how i can do that.

Ah, I see, in that case maybe someone could try to PR after this one for that implementation.

@Fabboz
Copy link
Author

Fabboz commented Dec 11, 2024

@PlayfulPiano check the material list in manipolation gui.

unsure what you mean by manipolation but if you're referring to the materials list, it doesn't show there either.

Could be a tile entity.

@Dream-Master Dream-Master removed the ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version label Jan 15, 2025
@Dream-Master Dream-Master requested a review from a team January 15, 2025 11:30
@Dream-Master Dream-Master requested a review from mist475 January 15, 2025 11:30
@Dream-Master Dream-Master added the 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta label Jan 15, 2025
@mist475
Copy link

mist475 commented Jan 18, 2025

assuming this has been well tested, no objections by me anymore. Using the neid format is probably a better idea but in the meantime this will probably be fine

@Dream-Master Dream-Master merged commit f49af4d into GTNewHorizons:master Jan 18, 2025
1 check passed
@serenibyss serenibyss removed the 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta label Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants