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

Add support for multiple palettes #149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

almic
Copy link
Contributor

@almic almic commented Feb 20, 2020

Minecraft shipwrecks use multiple palettes, allowing a single structure file to contain various schemes depending on the location the structure is placed.

All of the code supporting this exists, except for the actual loading of multiple palettes. Structure.setSeed() is called using the chunk coords, and Structure.getObject() as well, prior to rendering the final object into the world.

Attempting to load an NBT file with multiple palettes causes a NullPointerException and provides no information about the error. Now, loading an NBT with multiple palettes is supported and works as expected.

Unfortunately I was not able to test this as I couldn't build this myself. However, since the changes are so few and the code is clear, I'm confident this will work perfectly with all existing worlds, objects, etc.

Minecraft shipwrecks use multiple palettes, allowing a single structure file to contain various schemes depending on the location the structure is placed.

All of the code supporting this exists, except for the actual loading of multiple palettes. Structure.setSeed() is called using the chunk coords, and Structure.getObject() as well, prior to rendering the final object into the world.

Attempting to load an NBT file with multiple palettes causes a NullPointerException and provides no information about the error. Now, loading an NBT with multiple palettes is supported and works as expected.
@almic almic requested a review from Captain-Chaos February 20, 2020 14:52
@Captain-Chaos
Copy link
Owner

Cool, my first pull request! 😊 I'll take a look.

@almic
Copy link
Contributor Author

almic commented Feb 25, 2020

I've been working with some other code and discovered a major issue with this PR. Due to the way that .layer files are saved, it will become impossible to open older .layer files made for custom objects. WorldPainter simply fails to load the file and errors out, since the signature of old layer files with Structure objects is different now. I'm not familiar enough with how saving/ loading Java objects on the disk actually works.

My first idea is to add a catch clause to the loading function that detects this specific problem, and just loads the older version of the class and then updates it to the new version. I'd like to get your thoughts on this first before I try anything!

@Captain-Chaos
Copy link
Owner

@almic Yeah, good point. Shouldn't be too hard to fix I don't think. I have to deal with that issue all the time. Fortunately Java serialization allows you to evolve file formats in some moderately elegant ways. I'll keep the issue in mind when I look at this.

@almic
Copy link
Contributor Author

almic commented Aug 18, 2021

Sorry I kinda vanished, coming back to this now!

Fix issues when deserializing existing objects.
Drop support for the "blocks" field in favor of the pallet ready field.
@almic almic requested a review from Captain-Chaos August 18, 2021 17:22
dont look at my spelling
@almic
Copy link
Contributor Author

almic commented Apr 23, 2022

Hello @Captain-Chaos , it's been a long time and I see you've been working quite a lot. Is this pull request still valid?

@Captain-Chaos Captain-Chaos force-pushed the master branch 2 times, most recently from bd6d96d to ee498ad Compare May 6, 2022 11:47
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.

2 participants