fix an oopsie in chunk caches with dynamic world height #3375

wagyourtail · 2022-04-04T05:18:39Z

fixes #mine chest and some stuff not working properly

wagyourtail · 2022-04-04T05:19:16Z

src/main/java/baritone/cache/CachedRegion.java

@@ -43,7 +43,7 @@
    /**
     * Magic value to detect invalid cache files, or incompatible cache files saved in an old version of Baritone
     */
-    private static final int CACHED_REGION_MAGIC = 456022910;
+    private static final int CACHED_REGION_MAGIC = 456022911;


am I supposed to just bump this, or roll some dice and pick new numbers, or what

wagyourtail · 2022-04-04T05:19:40Z

src/main/java/baritone/cache/CachedRegion.java

@@ -166,7 +166,7 @@ public synchronized final void save(String directory) {
                                out.writeShort(entry.getValue().size());
                                for (BlockPos pos : entry.getValue()) {
                                    out.writeByte((byte) (pos.getZ() << 4 | pos.getX()));
-                                    out.writeByte((byte) (pos.getY()));
+                                    out.writeInt(pos.getY()-dimension.minY());


it needs updating due to expanding the range of this

Rorschach3533 · 2022-04-04T13:28:57Z

You guys gonna update this or are we going to change something ?

leijurv · 2022-04-05T21:38:07Z

I explained how it should be done here 05cc63f

I'd like to see something like LEGACY_REGION_MAGIC = 456022910; and CACHED_REGION_MAGIC = 456022911;. When loading, switch on that value and do something like boolean yCoordinateIsAByte = magic == LEGACY_REGION_MAGIC;, then int Y = yCoordinateIsAByte ? in.readByte() & 0xff : in.readInt() + dimension.minY();

Also I don't see why we need to add and subtract out dimension.minY(). It seems simpler to just directly write the actual value?

That should only happen on read. Writing a chunk should always do it the new way with the new magic.

wagyourtail · 2022-04-05T21:56:17Z

Also I don't see why we need to add and subtract out dimension.minY()

yeah, I suppose that doesn't need to be there as it'll be signed when read/writing, it will match the y value of the chunk data tho, since those are -minY to be able to shove into a long value

wagyourtail · 2022-04-05T21:59:48Z

I'd like to see something like LEGACY_REGION_MAGIC ...

I'm not sure if this is a good idea, this might cause issues with people who have old cache data that is wrong (incorrect y value currently from chunks parsed with worlds with -ymin values).
it's good I guess for loading cachedata from 1.17/older worlds that didn't previously have the minY, but iirc we're now saving the cache data in a folder name based on the worldheight, so it'll always have been wrong

leijurv · 2022-04-05T22:26:48Z

we're now saving the cache data in a folder name based on the worldheight

mmmmmmmmmmmm I don't like that, with a patch to saving the Y coord I don't think that's a good idea anymore, want to undo that afterwards

this might cause issues with people who have old cache data that is wrong (incorrect y value currently from chunks parsed with worlds with -ymin values).

I actually don't think this would cause any issues. Imagining you do #goto chest or some such, and it's out of your render distance, the Y coordinate doesn't matter in practice (and if simplifyUnloadedYCoord is left at default, it doesn't matter at all). Once it comes into your render distance, incorrect Y coordinate data in the cache is overwritten with the true data from the actual chunks, so there is no issue.

Really, thinking on it now, the XYZ coordinate within the chunk of the interesting block doesn't matter lol. Saving it in RAM is good, but saving it on disk is a little pointless. I think we could get away just as well with the on-disk format just containing the quantity of blocks in the chunk, not their locations?

leijurv · 2022-04-05T22:32:04Z

tldr: given that the XYZ doesn't matter so much, my opinion is that it's much better to scrape together as much data as we can from older or incompatible caches, than to ignore it because its Y could be incorrect.

wagyourtail · 2022-04-06T00:07:01Z

Really, thinking on it now, the XYZ coordinate within the chunk of the interesting block doesn't matter lol. Saving it in RAM is good, but saving it on disk is a little pointless. I think we could get away just as well with the on-disk format just containing the quantity of blocks in the chunk, not their locations?

goto spawner/chest isn't working properly without cache, so might need to fix a few things to do that...

leijurv · 2022-04-06T07:52:07Z

goto spawner/chest isn't working properly without cache, so might need to fix a few things to do that...

Reread what I said - "Saving it in RAM is good, but saving it on disk is a little pointless."

If it's in the CachedChunk.specialBlockLocations, which it will be for a chunk in render distance that just came out of the ChunkPacker, then it will work just fine for goto spawner. If it's out of render distance, the Y coordinate will be unknown, as well as the X and Z within the chunk, but that's fine since if it's out of render distance we don't need to know where it is more precisely than what chunk it's in.

So I'm saying that actually I'm pretty sure goto spawner/chest would work perfectly without saving the XYZ of chest/spawner to disk. Just whether they're in the chunk or not (and maybe a quantity).

leijurv · 2022-06-07T18:21:04Z

Update: to be clear, packed CachedChunks SHOULD still store the XYZ list while in RAM. XYZ should only be discarded when written to disk, it should NOT be discarded when packed. The reason is scenarios like 2b2t spawn

wagyourtail · 2022-06-20T20:59:17Z

2b2t spawn is probably big enough it shouldnt be discarded on disk either...
not like it takes that much space...

fix an oopsie in chunk caches with dynamic world height

9c91107

wagyourtail commented Apr 4, 2022

View reviewed changes

wagyourtail requested a review from leijurv April 5, 2022 02:16

allow loading older data

65f5d9e

scorbett123 mentioned this pull request Apr 12, 2022

Can't path to chests, furnaces, beds (potentially other items as well) #3271

Open

4 tasks

scorbett123 mentioned this pull request Apr 24, 2022

(prefix)goto chest doesn't work properly in overworld #3416

Closed

4 tasks

This was referenced Jun 6, 2022

Cannot mine spawners #3143

Closed

Caching performance upgrade #3479

Merged

leijurv mentioned this pull request Jul 15, 2022

Nasty parts of Baritone #3565

Open

wagyourtail mentioned this pull request Mar 15, 2023

find command gives a wrong Y coordinate. #3815

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix an oopsie in chunk caches with dynamic world height #3375

fix an oopsie in chunk caches with dynamic world height #3375

wagyourtail commented Apr 4, 2022

wagyourtail Apr 4, 2022

wagyourtail Apr 4, 2022

Rorschach3533 commented Apr 4, 2022

leijurv commented Apr 5, 2022

wagyourtail commented Apr 5, 2022

wagyourtail commented Apr 5, 2022

leijurv commented Apr 5, 2022

leijurv commented Apr 5, 2022

wagyourtail commented Apr 6, 2022

leijurv commented Apr 6, 2022 •

edited

Loading

leijurv commented Jun 7, 2022

wagyourtail commented Jun 20, 2022 •

edited

Loading

fix an oopsie in chunk caches with dynamic world height #3375

Are you sure you want to change the base?

fix an oopsie in chunk caches with dynamic world height #3375

Conversation

wagyourtail commented Apr 4, 2022

wagyourtail Apr 4, 2022

Choose a reason for hiding this comment

wagyourtail Apr 4, 2022

Choose a reason for hiding this comment

Rorschach3533 commented Apr 4, 2022

leijurv commented Apr 5, 2022

wagyourtail commented Apr 5, 2022

wagyourtail commented Apr 5, 2022

leijurv commented Apr 5, 2022

leijurv commented Apr 5, 2022

wagyourtail commented Apr 6, 2022

leijurv commented Apr 6, 2022 • edited Loading

leijurv commented Jun 7, 2022

wagyourtail commented Jun 20, 2022 • edited Loading

leijurv commented Apr 6, 2022 •

edited

Loading

wagyourtail commented Jun 20, 2022 •

edited

Loading