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.

Sign up for GitHub

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

Jump to bottom

Fix ChunkDataVersion issues with third party applications #1466

Merged
merged 1 commit into from
Sep 29, 2024
Merged

Fix ChunkDataVersion issues with third party applications #1466

merged 1 commit into from
Sep 29, 2024
+40 −13

Conversation

NotStirred
Copy link
Member

@NotStirred NotStirred commented Oct 10, 2022 •
edited
Loading

Some WorldPainter worlds have their ChunkDataVersion set to a version prior to minecraft's support for worlds with >=256 height.
This PR changes how we create ChunkData objects to be based on the actual block range found in a chunk, rather than its version

@NotStirred NotStirred force-pushed the feature/force_genericchunkdata branch from 125e095 to 9f8d3a0 Compare October 10, 2022 21:52
@NotStirred NotStirred changed the title Add a button to force the use of GenericChunkData Fix ChunkDataVersion issues with third party applications Oct 10, 2022
@leMaik leMaik added bug minecraft Block updates, world format changes, and anything related to Minecraft updates hacktoberfest-accepted labels Oct 10, 2022
ThatRedox
ThatRedox reviewed Oct 10, 2022
View reviewed changes
chunky/src/java/se/llbit/chunky/world/Chunk.java Outdated
@@ -168,7 +169,8 @@ public synchronized boolean loadChunk(@NotNull Mutable<ChunkData> chunkData, int

surfaceTimestamp = dataTimestamp;
version = chunkVersion(data);
chunkData.set(this.world.createChunkData(chunkData.get(), data.get(DATAVERSION).intValue()));
IntIntImmutablePair chunkBounds = inclusiveChunkBounds(data);
chunkData.set(this.world.createChunkData(chunkData.get(), chunkBounds.leftInt(), chunkBounds.rightInt()));
Copy link
Member

Choose a reason for hiding this comment

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

Could we use yMin and yMax instead of parsing the chunk bounds on every chunk? Although we would probably need to test to see which one is faster.

@jackjt8
Copy link
Member

jackjt8 commented Oct 15, 2022

This PR is somewhat difficult to test. We still have #1175 which may or may not be related which could be causing issues. There are certain moments when I can sometimes load 1 chunk or 20k chunks to the correct 0 to 320 height. Other times I can do 0 to 256.

Retyping the Scene Y clip does not fix this inconsistent behaviour. I have tried both loading/reloading with it set to 256 and 320.

Retyping the Map Y clip does not fix things either.

The Reload chunks button can sometimes give us the correct height... and sometimes not.. even without changing the selection.

Loading 10k chunks
image

Map view (max Map Y clip 320) gives an indication as to where terrain should be
image

Map view with max Map Y clip at 256 shows that terrain in the preview is still cut at 256.
image

Loading just 1024 chunks
image

@leMaik
Copy link
Member

leMaik commented Oct 28, 2023

The screenshots in the previous comment worried me so I didn't merge this PR yet. Are these issues caused by this PR or could this be merged?

If this PR is fine, could you please rebase it, @NotStirred? 🙏

@leMaik leMaik added this to the 2.5.0 milestone Sep 17, 2024
@leMaik leMaik linked an issue Sep 24, 2024 that may be closed by this pull request
Y min/max clip (re)loading inconsistencies #1175
Closed
@NotStirred @leMaik
ChunkData creation now checks against chunk bounds, rather than chunk…
611bd42 
… version.

This fixes annoying ChunkDataVersion issues with third-party apps like WorldPainter
@leMaik leMaik force-pushed the feature/force_genericchunkdata branch from 69d167d to 611bd42 Compare September 24, 2024 09:42
@leMaik leMaik mentioned this pull request Sep 24, 2024
Closed
@leMaik leMaik merged commit 61a184a into chunky-dev:master Sep 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThatRedox ThatRedox ThatRedox left review comments

@leMaik leMaik leMaik approved these changes

Assignees
No one assigned
Labels
bug hacktoberfest-accepted minecraft Block updates, world format changes, and anything related to Minecraft updates
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.

Y min/max clip (re)loading inconsistencies
4 participants
@NotStirred @jackjt8 @leMaik @ThatRedox