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

Added support for partial chunk/region load #49

Merged
merged 7 commits into from
Jun 8, 2020

Conversation

prydin
Copy link
Contributor

@prydin prydin commented Jun 2, 2020

Implemented partial load of chunks and regions as described in #48.

@coveralls
Copy link

coveralls commented Jun 2, 2020

Coverage Status

Coverage increased (+0.6%) to 89.906% when pulling 49e613b on prydin:prydin-partial-load into 0755fe5 on Querz:master.

public static long TILE_ENTITIES = 0x0010;
public static long CARVING_MARKS = 0x0020;
public static long TILE_TICKS = 0x0040;
public static long LIQUID_TILE_TICKS = 0x0040;
Copy link
Owner

Choose a reason for hiding this comment

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

LIQUID_TILE_TICKS should not be the same as TILE_TICKS

public static long BLOCK_STATES = 0x0800;
public static long SKY_LIGHT = 0x1000;
public static long LIGHTS = 0x2000;
public static long LIQUIDS_TO_BE_TICKED = 0x2000;
Copy link
Owner

Choose a reason for hiding this comment

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

LIQUIDS_TO_BE_TICKED should not be the same as LIGHTS

@Querz
Copy link
Owner

Querz commented Jun 3, 2020

Right now when trying to save a chunk that only has been partially loaded it will throw an NPE (see Chunk.java#L600 which is called in Chunk.java#L133). I think it would be better to throw an UnsupportedOperationException with an appropriate message when trying to save a chunk with only partially loaded data.

@prydin
Copy link
Contributor Author

prydin commented Jun 3, 2020

Thanks! I also realized that I have a lot of OR when it should be AND. I have NO idea what I was thinking. :)

@prydin
Copy link
Contributor Author

prydin commented Jun 6, 2020

Made a lot of changes:

Fixed all my bugs described above.
Added test cases.
Fixed typo on Heightmaps in serialization code.
Upgraded to gradle 6.5. The older version didn't support modern JDKs.
Added check for partial chunk on serialization.

@Querz
Copy link
Owner

Querz commented Jun 8, 2020

  • removed Intellij module file
  • added .iml files to .gitignore
  • gradle-wrapper.jar had the checksum of gradle 6.0/6.0.1 (see https://gradle.org/release-checksums/), upgraded everything to actual gradle 6.5

@Querz
Copy link
Owner

Querz commented Jun 8, 2020

  • adjusted formatting to match the rest of the code
  • removed deletion of tmp file from unit test because NBTTestCase#tearDown() takes care of that
  • removed code duplication in MCAUtil
  • removed unused import
  • renamed CARVINGMASKS to CARVING_MASKS for consistency

@prydin
Copy link
Contributor Author

prydin commented Jun 8, 2020

Thanks! Is this ready to be merged now, or are there more changes you'd like?

@Querz
Copy link
Owner

Querz commented Jun 8, 2020

lgtm

@Querz Querz merged commit 795f1f2 into Querz:master Jun 8, 2020
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.

3 participants