Skip to content

fix: read directory entries in name order so packs are reproducible - #93

Merged
FixedDev merged 1 commit into
unnamed:mainfrom
duranaaron:fix/deterministic-directory-read-order
Jul 31, 2026
Merged

FixedDev merged 1 commit into
unnamed:mainfrom
duranaaron:fix/deterministic-directory-read-order

Conversation

@duranaaron

Copy link
Copy Markdown
Contributor

DirectoryFileTreeReader walks a folder with File.listFiles(), which returns entries in whatever order the file system stores them. That order is a property of the file system rather than of the files, so two machines holding the same folder can read it in two different orders.

ResourceContainerImpl keeps its resources in LinkedHashMaps and MinecraftResourcePackWriter writes them in insertion order, so the read order becomes the zip entry order. Two servers with identical files then build packs that have identical content but different bytes, and so different hashes.

I hit this on a Minecraft network where two backends held byte-identical resource files and produced a different pack hash each. The two servers had separate ext4 file systems, and ext4 orders directory entries by a hash of the file name seeded with a random value written at mkfs time, so the two orders never matched. It does not reproduce on a single machine, because both directories are then on the same file system.

This sorts each folder's entries by name before reading them, so the read order depends only on the file names.

The added test writes files in a non-alphabetical order and checks that the reader returns them sorted. Without the change it fails with:

expected: [a.txt, m.txt, z.txt, sub/b.txt, sub/y.txt]
but was:  [z.txt, m.txt, a.txt, sub/y.txt, sub/b.txt]

All 41 tests in creative-serializer-minecraft pass with the change.

One unrelated note: creative-api does not compile on main right now, because SpecialRender.playerHead() is declared twice, at lines 167 and 277. I patched that locally so I could run the tests, and left it out of this branch.

@FixedDev

Copy link
Copy Markdown
Member

lgtm

@FixedDev
FixedDev merged commit 0fa6ff1 into unnamed:main Jul 31, 2026
1 check failed
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