From 67e364ab6d92ad85e17519df3772e4642a1bed33 Mon Sep 17 00:00:00 2001
From: TheMeinerLP The engine has to be usable before anything boots: a world is upgraded on disk, offline, and
+ * only afterwards handed to a server that can load it. Nothing in the build enforces that today — a
+ * single {@code import net.minestom....} would compile happily, because the compiler cannot tell
+ * "runs on a server" from "runs on stored bytes" for us. This rule reads the bytecode instead and
+ * fails the moment that boundary is crossed.
+ */
+@AnalyzeClasses(
+ packages = "net.onelitefeather.falco",
+ importOptions = ImportOption.DoNotIncludeTests.class)
+class MigrationBoundaryTest {
+
+ private static final String MIGRATION = "net.onelitefeather.falco.migration..";
+
+ /**
+ * The engine converts stored NBT and must run without a server, which is what lets a world be
+ * converted before anything boots.
+ */
+ @ArchTest
+ static final ArchRule migrationKnowsNoMinestom = noClasses()
+ .that().resideInAPackage(MIGRATION)
+ .should().dependOnClassesThat().resideInAnyPackage("net.minestom..")
+ .because("the engine converts stored NBT and must run without a server, which is what "
+ + "lets a world be converted before anything boots");
+}
diff --git a/falco-migration/build.gradle.kts b/falco-migration/build.gradle.kts
new file mode 100644
index 0000000..99efbfb
--- /dev/null
+++ b/falco-migration/build.gradle.kts
@@ -0,0 +1,18 @@
+description = "Converts stored Anvil chunk data from Minecraft 1.13 upwards"
+
+dependencies {
+ implementation(platform(libs.mycelium.bom))
+ implementation(libs.slf4j.api)
+ implementation(project(":falco-anvil"))
+
+ compileOnly(platform(libs.adventure.bom))
+ compileOnly(libs.adventure.nbt)
+ compileOnly(libs.annotations)
+
+ testImplementation(platform(libs.adventure.bom))
+ testImplementation(libs.adventure.nbt)
+ testImplementation(libs.annotations)
+ testImplementation(libs.junit.jupiter)
+ testImplementation(libs.junit.platform.launcher)
+ testRuntimeOnly(libs.junit.jupiter.engine)
+}
diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockState.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockState.java
new file mode 100644
index 0000000..45fe0b7
--- /dev/null
+++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockState.java
@@ -0,0 +1,42 @@
+package net.onelitefeather.falco.migration;
+
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Unmodifiable;
+
+import java.util.Map;
+
+/**
+ * A block state as stored in Anvil chunk NBT since Minecraft 1.13: a namespaced block name together
+ * with its property map.
+ *
+ * @param name the namespaced block identifier, for example {@code "minecraft:oak_log"}
+ * @param properties the block's properties, copied into an unmodifiable map by the canonical
+ * constructor
+ * @since 2.1.0
+ */
+@ApiStatus.Experimental
+public record BlockState(String name, @Unmodifiable Map
+ * This package works on NBT read from disk and nothing else. It does not import Minestom, does not
+ * start a server and does not need one running: a world has to be convertible before anything boots,
+ * because migration is a step a server takes before it ever loads the chunks it is about to serve.
+ * {@code falco-archunit} enforces that boundary; see {@code MigrationBoundaryTest} there for the rule
+ * and the reason behind it.
+ *
+ * The floor is DataVersion 1519, the release version of Minecraft 1.13. That release rewrote the
+ * chunk format from numeric block IDs plus damage values to the palette-based block state format
+ * still in use today, which is the format this engine's types speak. Anything older would need a
+ * translation this package does not attempt.
+ *
+ * Every public type here is experimental and may still change in a minor release.
+ *
+ * Worlds before Minecraft 1.16 kept the overworld directly under the world root in {@code region},
+ * and put the nether and the end in the sibling directories {@code DIM-1} and {@code DIM1}
+ * respectively — names carried over from a numeric dimension id scheme that predates namespaced keys
+ * entirely. Since 1.16, every dimension, vanilla or added by a data pack, lives under
+ * {@code dimensions/
+ * {@code FalcoAnvilLoader.resolveRegionDirectory} is not reused here because it solves a narrower
+ * problem: given a single dimension a caller already named, it builds that dimension's modern path
+ * and falls back to {@code
+ * The two layouts are read independently and neither is skipped because the other already
+ * produced a match, so the same {@code dimensionKey} can appear twice in the result: once with
+ * {@code legacy = true} from {@code
+ * A rule is keyed on the whole state, never on a single property: {@link #matches(BlockState)} and
+ * {@link #apply(BlockState)} both see the complete name-plus-properties pair, because some changes
+ * cannot be expressed any narrower. A cauldron's new block name is decided by its {@code level}
+ * property, and a wall's new value for one direction would need the direction alone in a
+ * property-by-property model — the whole state is the smallest unit that stays correct.
+ *
+ * A rule also carries the {@link #since()} version it belongs to, because the same block name can
+ * mean different things on either side of a version boundary. {@code stone_slab} is the case that
+ * forces this: unversioned, a rename rule for it would corrupt a world that already has the
+ * newer meaning.
+ *
+ * This is the version a fix landed in, not the version the rule targets: a rule applies to a
+ * chunk's source version exactly when the source is older than this number, i.e. when
+ * {@code since() > sourceVersion}. A rule with {@code since() == 1802} therefore applies to a
+ * 1.13 world ({@code 1519 < 1802}) and leaves a 1.16 world ({@code 2566 > 1802}) alone, because
+ * by 1.16 the change already happened and the state already carries its later meaning.
+ *
+ * Called after every rule that ran before this one in version order, so {@code state} may
+ * already differ from the state a chunk originally stored.
+ *
+ * Only called for a state {@link #matches(BlockState)} accepted. May change the block name, the
+ * properties, or both — a rule is not required to keep the name stable.
+ *
+ * Every rule below carries a comment naming where it came from — a PaperMC/DataConverter fix
+ * version, or a diff computed directly over the vendored registry lists — because none of them is
+ * written from memory; see
+ * {@code docs/superpowers/specs/2026-08-04-blockstate-property-research.md} for the full
+ * measurement. One fact from that measurement, {@code redstone_wire}, is deliberately absent; see
+ * the note above {@link #RULES} for why.
+ *
+ * {@code redstone_wire} (V2531, 20w17a, Minecraft 1.16, 144 states) is not in this list.
+ * The research document establishes that a direction's new value depends on the other three
+ * directions of the same state (DataConverter's {@code connectedX}/{@code connectedZ} in V2531),
+ * that a per-property implementation is guaranteed wrong, and that 9 of the 81 direction
+ * combinations change (times 16 {@code power} values = 144 states). It does not say which 9
+ * combinations change or what they become — only the count. That is not enough to reproduce
+ * V2531's logic exactly, and a guessed whole-state table for redstone wiring is exactly the
+ * silent corruption this task was told to refuse: the chunk still loads, the wiring looks
+ * subtly different, and nobody notices. A state this module does not recognize — including every
+ * {@code redstone_wire} state — passes through {@link #translate(BlockState, int)} unchanged;
+ * see {@code BlockStateRulesTest.testAStateNoRuleKnowsAboutPassesThroughUnchanged}.
+ *
+ * A rule applies exactly when {@code rule.since() > sourceVersion}: {@link BlockStateRule#since()}
+ * names the {@code DataVersion} the change happened in, so a source strictly older than that
+ * version has not seen the change yet and needs it, while a source at or after that version
+ * already carries the change's meaning and must be left alone. A rule with
+ * {@code since() == 1802} therefore applies to a 1.13 world ({@code 1519 < 1802}) and not to a
+ * 1.16 world ({@code 2566 > 1802}).
+ *
+ * A state no rule recognizes — including every {@code redstone_wire} state, for which this
+ * module deliberately carries no rule — passes through unchanged.
+ *
* A state no rule recognizes — including every {@code redstone_wire} state, for which this
diff --git a/falco-migration/src/test/java/net/onelitefeather/falco/migration/BlockStateRulesTest.java b/falco-migration/src/test/java/net/onelitefeather/falco/migration/BlockStateRulesTest.java
index 6f87d8b..aa90f96 100644
--- a/falco-migration/src/test/java/net/onelitefeather/falco/migration/BlockStateRulesTest.java
+++ b/falco-migration/src/test/java/net/onelitefeather/falco/migration/BlockStateRulesTest.java
@@ -60,10 +60,13 @@ void testAMossyWallIsRewrittenByTheSameSharedTable() {
@Test
void testARuleAppliesExactlyBelowItsOwnVersionAndNotAtOrAboveIt() {
+ // stone_slab's rule fires below DataVersion 1901 (snapshot 18w43a, where the rename actually
+ // happened) and not at or above it. An earlier version of this test used 1801/1802, matching
+ // an earlier, wrong value for the rule itself; see BlockStateRules for the correction.
assertEquals("minecraft:smooth_stone_slab",
- BlockStateRules.translate(BlockState.of("minecraft:stone_slab"), 1801).name());
+ BlockStateRules.translate(BlockState.of("minecraft:stone_slab"), 1900).name());
assertEquals("minecraft:stone_slab",
- BlockStateRules.translate(BlockState.of("minecraft:stone_slab"), 1802).name());
+ BlockStateRules.translate(BlockState.of("minecraft:stone_slab"), 1901).name());
}
@Test
From f7cdd06f1babdf0371d830679d8dc83dc09f7763 Mon Sep 17 00:00:00 2001
From: TheMeinerLP
+ * {@link #migrate(CompoundBinaryTag, int)} reads {@code DataVersion} off the chunk, declines a chunk
+ * older than {@link #MINIMUM_SOURCE_VERSION} with a {@link MigrationException}, runs every
+ * {@link MigrationStep} whose {@link MigrationStep#appliesTo(int)} accepts the source version in the
+ * chain's declared order, and finally stamps the target version onto the result.
+ *
+ * This task wires only the three steps that move or delete data without needing any renaming
+ * knowledge: {@link UnfoldLevel}, {@link NamespaceStatus} and {@link DiscardHeightmapsAndLight}, in
+ * that order — the order the design's step table gives them (steps 3, 6 and 8). The remaining steps
+ * of that table (bit packing, entity counting, biome rebuilding, Y-range widening and block-state
+ * renaming) belong to later tasks and are not part of this chain yet.
+ *
+ * A step decides whether it runs at all from {@code sourceVersion} alone, through
+ * {@link MigrationStep#appliesTo(int)}. {@code targetVersion} is carried for the steps that have to
+ * know how far a chunk is going, not only where it started — none of the three structural steps built
+ * in this task need it, but a step that resolves a rename table for a specific target does.
+ *
+ * Unchecked, and deliberately so: an earlier draft of this module made this type checked, following
+ * {@code falco-anvil}'s {@code AnvilFormatException} hierarchy. That hierarchy is
+ * {@code sealed … permits ChunkDataException, RegionFormatException}, declared in another package, so
+ * it cannot be extended from here at all — and {@code falco-archunit}'s
+ * {@code ErrorHandlingTest.checkedFaultsStayInsideTheHierarchy} runs over the whole
+ * {@code net.onelitefeather.falco..} tree and requires every checked {@link Throwable} in it to be
+ * assignable to that sealed root. A second checked hierarchy is therefore not an option, and this
+ * module's own house style is unchecked anyway: {@code ErrorHandlingTest} asks every
+ * {@link RuntimeException} under {@code net.onelitefeather.falco..} to carry a public
+ * {@code (String, Throwable)} constructor, which this type has below.
+ *
+ * A step is attached to a {@code DataVersion} threshold through {@link #appliesTo(int)}, which
+ * {@link ChunkMigration#migrate(CompoundBinaryTag, int)} asks before calling {@link #apply}, so a step
+ * that only concerns pre-1.18 chunks never has to guard its own body against a chunk that already has
+ * the shape it would otherwise produce.
+ *
+ * Implementations do not mutate {@code chunk}; {@link CompoundBinaryTag} is immutable by construction,
+ * so {@link #apply} returns the chunk a step produced rather than changing the one it received.
+ *
+ * This is a deliberate deletion, not a shortcut taken for lack of time. A wrongly ported heightmap
+ * never announces itself: it is a plain long array the server trusts on faith, so a bug in a bit-width
+ * or coordinate conversion here would sit silently in every converted world until something built on
+ * top of it looked wrong for an unrelated reason. A missing heightmap has no such failure mode — the
+ * server rebuilds it the moment it needs one. Light is discarded for the same reason and because
+ * {@code falco-light} already computes it from scratch; recomputing was never this module's job to
+ * begin with. Runs at every version, because a chunk from any point in this module's range can carry
+ * either field.
+ *
+ * This step tests no {@code DataVersion} at all — {@link #appliesTo(int)} always returns
+ * {@code true} — because the exact version that namespaced the chunk status could not be established;
+ * the wiki's own change history for the field carries a notice that it is missing a significant number
+ * of changes. Testing whether {@code Status} already carries a {@code ':'} is correct for every
+ * version in this module's range regardless, and does not depend on a number nobody has actually
+ * read: a status that already has a namespace is left alone, and a bare status is prefixed with
+ * {@code minecraft:}.
+ *
+ * A chunk with no {@code Status} field at all is returned unchanged.
+ *
+ * {@code Sections} is renamed to {@code sections} as it moves, because that is the one field name
+ * that changed rather than only its position — every other child keeps its own name. A {@code yPos}
+ * field is added at the root because 2844 is also the version that introduced it: it names the
+ * lowest section index a chunk stores, which for every chunk below 2844 is {@code 0}, since the
+ * pre-1.18 world height was fixed to sections {@code 0}–{@code 15} and never reached below the
+ * bottom of that range.
+ *
+ * The list of fields this step moves is deliberately not written down here: it moves whatever
+ * {@code Level} actually holds, rather than a fixed set of names invented for this task. A chunk with
+ * no {@code Level} compound at all — already unfolded, or never folded in the first place — is
+ * returned unchanged.
+ *
+ * Every public type here is experimental and may still change in a minor release.
+ *
* {@code Sections} is renamed to {@code sections} as it moves, because that is the one field name
- * that changed rather than only its position — every other child keeps its own name. A {@code yPos}
- * field is added at the root because 2844 is also the version that introduced it: it names the
- * lowest section index a chunk stores, which for every chunk below 2844 is {@code 0}, since the
- * pre-1.18 world height was fixed to sections {@code 0}–{@code 15} and never reached below the
- * bottom of that range.
+ * that changed rather than only its position — every other child keeps its own name. A field that
+ * would silently overwrite a same-named field already present at the root is refused with a
+ * {@link MigrationException} instead: for every pre-1.18 format known to this module that case cannot
+ * happen, since a chunk's own top-level keys and {@code Level}'s children never collide, but a silent
+ * overwrite would contradict this project's fail-loud stance the moment some format this module has
+ * not seen turns out to disagree.
+ *
+ * {@code yPos = 0} is provisional, not a settled answer. A {@code yPos} field is added at the
+ * root because 2844 is also the version that introduced it. {@code 0} is correct for what the
+ * source chunk means: every version below 2844 fixed the world height to sections
+ * {@code 0}–{@code 15}, so a pre-1.18 chunk's own lowest stored section is always {@code 0}. What
+ * {@code yPos} means for the target version is a separate, still-open question — the field's
+ * own wiki documentation ("Lowest Y section position in the chunk (e.g. -4 in 1.18)") is ambiguous
+ * between "the lowest section this chunk itself stores" and "the bottom of the dimension's height
+ * range", and vanilla data can never tell the two apart because vanilla always writes every section
+ * down to the dimension floor. A converted chunk, which does not invent sections below 0 it never
+ * had, is exactly the case where the two readings split. Resolving that split, and therefore whether
+ * {@code 0} is the value this step should keep producing, is Task 5's {@code SettleYRange} step, not
+ * this one — a reader of this class should not conclude the question is answered because a number is
+ * already here.
*
* The list of fields this step moves is deliberately not written down here: it moves whatever
@@ -53,6 +70,16 @@ public boolean appliesTo(int sourceVersion) {
return sourceVersion < APPLIES_BELOW;
}
+ /**
+ * {@inheritDoc}
+ *
+ * @param chunk {@inheritDoc}
+ * @param context {@inheritDoc}
+ * @return {@inheritDoc}
+ * @throws MigrationException if a child of {@code Level} — after the {@code Sections}-to-
+ * {@code sections} rename, where applicable — has the same name as a
+ * field already present at the chunk's root, naming that field
+ */
@Override
public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context) {
if (!(chunk.get(LEVEL_KEY) instanceof CompoundBinaryTag level)) {
@@ -62,6 +89,10 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context
CompoundBinaryTag root = chunk.remove(LEVEL_KEY);
for (Map.Entry
- * {@link #migrate(CompoundBinaryTag, int)} reads {@code DataVersion} off the chunk, declines a chunk
- * older than {@link #MINIMUM_SOURCE_VERSION} with a {@link MigrationException}, runs every
- * {@link MigrationStep} whose {@link MigrationStep#appliesTo(int)} accepts the source version in the
- * chain's declared order, and finally stamps the target version onto the result.
+ * {@link #migrate(CompoundBinaryTag, int)} reads {@code DataVersion} off the chunk and delegates to
+ * {@link #migrate(CompoundBinaryTag, MigrationContext)}, which declines a chunk older than
+ * {@link #MINIMUM_SOURCE_VERSION} with a {@link MigrationException}, runs every {@link MigrationStep}
+ * whose {@link MigrationStep#appliesTo(int)} accepts the source version in the chain's declared order,
+ * and finally stamps the target version onto the result.
*
- * This task wires only the three steps that move or delete data without needing any renaming
- * knowledge: {@link UnfoldLevel}, {@link NamespaceStatus} and {@link DiscardHeightmapsAndLight}, in
- * that order — the order the design's step table gives them (steps 3, 6 and 8). The remaining steps
- * of that table (bit packing, entity counting, biome rebuilding, Y-range widening and block-state
- * renaming) belong to later tasks and are not part of this chain yet.
+ * This chain wires every step of the design's table: {@link NormaliseBitPacking} (step 1),
+ * {@link CountEntities} (step 2), {@link UnfoldLevel} (step 3), {@link RebuildBiomes} (step 4),
+ * {@link SettleYRange} (step 5), {@link NamespaceStatus} (step 6), {@link TranslateBlockStates} and
+ * {@link TranslateBlockEntities} (step 7's block and block-entity halves) and
+ * {@link DiscardHeightmapsAndLight} (step 8), in that order.
*
+ * This is the entry point a caller reaches for when it needs to read {@code context} back
+ * afterward — {@link MigrationContext#entitiesLeftBehind()} in particular, which only accumulates
+ * across chunks migrated through the exact same context instance. {@link #migrate(CompoundBinaryTag, int)}
+ * is the convenience wrapper every other caller uses; it builds a fresh, single-use context from
+ * the chunk itself and delegates here. This overload trusts {@code context.sourceVersion()} as
+ * given rather than re-reading the chunk's own {@code DataVersion} field, so a caller driving
+ * several chunks through one shared context is responsible for the source version actually
+ * matching each of them.
+ *
* A step decides whether it runs at all from {@code sourceVersion} alone, through
* {@link MigrationStep#appliesTo(int)}. {@code targetVersion} is carried for the steps that have to
* know how far a chunk is going, not only where it started — none of the three structural steps built
- * in this task need it, but a step that resolves a rename table for a specific target does.
+ * in Task 4 need it, but a step that resolves a rename table for a specific target does.
+ *
+ * {@code entityCounter} is this context's counting sink: {@code CountEntities} adds to it through
+ * {@link #countEntitiesLeftBehind(int)} once per chunk it finds still carrying entities, and a caller
+ * driving several chunks through the same context — a batch run over a whole world, in particular —
+ * reads {@link #entitiesLeftBehind()} afterwards for the total the design's "entity debt" warning
+ * requires. It is exposed as a plain record component rather than hidden behind this class alone
+ * because {@link MigrationContext} is already the one object every step in the chain receives and can
+ * write into; a second, parallel channel for this one count would only duplicate what the context
+ * already is.
*
+ * From Minecraft 1.17 the server reads entities only from separate {@code entities/} region files; a
+ * chunk below that version still carries them inline, under {@code Level.Entities}. This step only
+ * ever runs before {@link UnfoldLevel} in
+ * {@link net.onelitefeather.falco.migration.ChunkMigration}'s chain, so a chunk it applies to still
+ * has its {@code Level} compound intact when this step sees it — {@code Entities} is read from there,
+ * never from the chunk's root.
+ *
+ * This step does not move that list anywhere, and neither does anything else in this chain.
+ * The design's "entity debt" is stated plainly because it is the same failure mode the loader's
+ * version guard exists to end: a world converted by this module keeps its entity data in the chunk,
+ * where the target version will never look for it. Every mob, item frame, armour stand, painting and
+ * dropped item is effectively gone — the bytes are still there, nothing is deleted, but nothing reads
+ * them either. This step's only job is to make that consequence visible instead of silent: it adds
+ * the size of every chunk's own {@code Entities} list to
+ * {@link MigrationContext#countEntitiesLeftBehind(int)}, so a caller can report the total rather than
+ * discover a converted world's missing mobs the hard way.
+ *
+ * {@link #appliesTo(int)} accepts a source version strictly below {@value #APPLIES_BELOW} —
+ * {@code DataVersion} 2681, snapshot 20w45a, the snapshot that actually extracted entities into
+ * their own {@code entities/} region files. This is deliberately not 2724, the
+ * {@code DataVersion} of the 1.17 release that snapshot led up to: the design's own step table names
+ * 2724, which is the release number substituted for the snapshot number that actually carries the
+ * change — the exact mistake this module's block-state rules already found and corrected once for
+ * this identical snapshot (grass_path's rename also landed in 20w45a). Verified against two
+ * independent fetches of 20w45a's own wiki infobox and changelog text, 2026-08-04; see the task
+ * report. A chunk at or above {@value #APPLIES_BELOW} has already had its entities extracted by the
+ * time it reaches this module, so whatever remains of its own {@code Entities} list, if any, is not
+ * this step's concern.
+ *
+ * A chunk with no {@code Level} compound, or no {@code Entities} list within it, contributes nothing
+ * to the count and is returned unchanged either way.
+ *
+ * A block entity is part of the chunk in every version this module supports and stays there, so
+ * translating it is a rename applied to a tag that is already in the right place — unlike an entity,
+ * which has to move to another file from 1.17 onwards (see {@link CountEntities}). Two things about a
+ * block entity are explicitly out of scope for this step, and both are passed through
+ * untouched rather than fixed: the items inside a block entity — a chest's contents carry item
+ * ids that were renamed too, and this step does not look inside {@code Items} or any other nested
+ * list — and per-block-entity field changes that are not a rename of {@code id}, of which the
+ * sign text rework in Minecraft 1.20 (single {@code Text1}-{@code Text4} lines becoming
+ * {@code front_text}/{@code back_text} components) is the largest. A converted block entity is in the
+ * right place with the right {@code id}; what is inside it, and every field but {@code id}, has not
+ * been looked at.
+ *
+ * No verified block-entity {@code id} rename exists for the whole 1.13-26.1 span, so
+ * {@link #RENAMES} is empty. The 1.13 mapping file this module's block rules are checked against
+ * carries no {@code blockentities} list at all, so the difference that settles the block-rename
+ * question cannot be computed for block entities the same way. The route this task's research took
+ * instead: 26.1's {@code blockentities} registry list has 49 entries, small enough to check
+ * exhaustively. Diffing that list across every version between 1.18 (the earliest version whose
+ * mapping file carries one at all) and 26.1 found not one name ever removed — every change across
+ * that whole span was an addition. Separately, PaperMC/DataConverter — the GPL rebuild of Mojang's
+ * own converter this module already treats as authoritative for structural questions no registry diff
+ * can answer — registers exactly one block-entity rename in its entire fix history from V99 to V4661:
+ * {@code minecraft:suspicious_sand} to {@code minecraft:brushable_block}, which cannot appear in a
+ * 1.13 world because {@code suspicious_sand} did not exist before 1.20 snapshots. Both independent
+ * sources agree: nothing a 1.13 world can contain ever had its block-entity {@code id} renamed on the
+ * way to 26.1. See the task report for the full per-entry accounting.
+ *
+ * Known gap, not fixed here: the block-entity list's own container key is not renamed by this
+ * chain. {@code Level.TileEntities} became the root-level {@code block_entities} in the same
+ * snapshot (21w43a, DataVersion 2844) that {@code UnfoldLevel} already treats as its own threshold for
+ * unfolding {@code Level} and renaming {@code Sections} to {@code sections} — but {@code UnfoldLevel}'s
+ * own Javadoc states that every child other than {@code Sections} keeps its name as it moves, which is
+ * inaccurate for this one field. This step reads whichever of {@code block_entities} or
+ * {@code TileEntities} is actually present at the chunk's root and writes the (possibly id-renamed)
+ * list back under that same key — it does not decide which name the output should carry, because
+ * doing so would mean quietly reaching into a decision {@code UnfoldLevel} (a different task's file)
+ * already made. A chunk converted from a pre-2844 source therefore still carries its block entities
+ * under {@code TileEntities} after this whole chain runs, a name the target version's reader will
+ * never look under. Reported here and in the task report rather than patched, matching how this
+ * module has handled every other gap found next to a task rather than inside it.
+ *
+ * Package-private so a test can exercise the substitution mechanism itself against a rename table
+ * of its own, independent of {@link #RENAMES} — which this task's research found to be empty for
+ * every {@code id} a 1.13 world can actually contain; see this class's Javadoc.
+ *
+ * {@code falco-anvil}'s {@link net.onelitefeather.falco.anvil.BitPacker} cannot read that layout:
+ * its own {@code pack} is documented "without letting an entry span two longs", and its
+ * {@code unpack} computes {@code longIndex = index / entriesPerLong} — an offset that restarts at
+ * a long boundary for every entry rather than walking a single continuous bit stream. Below 1.16 the
+ * format instead lays entries out back to back with no padding at all, so an entry whose bit range
+ * crosses a 64-bit boundary is genuinely split across {@code packed[n]} and {@code packed[n + 1]}.
+ * This class reads exactly that: a continuous bit offset, {@code index * bitsPerEntry}, with no
+ * restart.
+ *
+ * {@link net.onelitefeather.falco.migration.steps.NormaliseBitPacking} is this reader's only caller:
+ * it reads a legacy section once with this class and re-packs the result with {@code BitPacker}, so
+ * every step downstream of it only ever sees the long-aligned layout {@code BitPacker} already
+ * understands.
+ *
+ * This step runs before {@link UnfoldLevel} in {@code ChunkMigration}'s chain — its own threshold,
+ * 2566, is strictly below {@code UnfoldLevel}'s, 2844, so every chunk this step applies to still
+ * carries the pre-1.18 {@code Level} wrapper when this step sees it. It therefore reads
+ * {@code Level.Sections}, not the root {@code sections} list later steps use, and only touches each
+ * section's {@code BlockStates} long array — the key names and every other field are left exactly as
+ * they were; restructuring the container itself into the modern
+ * {@code block_states: \{palette, data\}} shape is
+ * {@link TranslateBlockStates}'s job, which runs after this step has made every section's packing
+ * long-aligned.
+ *
+ * A section whose palette holds a single entry carries no {@code BlockStates} array at all — the
+ * format omits it and lets the one palette entry fill the whole section — and is returned unchanged,
+ * as is a chunk with no {@code Level} compound.
+ *
+ * Runs after {@link UnfoldLevel} in {@code ChunkMigration}'s chain, so it reads the root
+ * {@code sections} list and the root {@code Biomes} field {@code UnfoldLevel} already moved there
+ * for a pre-1.18 chunk; a chunk with no {@code Biomes} field is returned unchanged.
+ *
+ * Two source shapes. Below DataVersion 2203 (snapshot 19w36a — confirmed against that
+ * snapshot's own changelog and infobox on minecraft.wiki, 2026-08-04) {@code Biomes} holds 256
+ * entries, one per column of the chunk's 16-by-16 footprint, with no variance by height. From 2203
+ * up to 2844 it holds 1024 entries: the same footprint split into 4-by-4 columns, crossed with 16
+ * four-block-tall layers, still stored for the whole chunk rather than per section. Both shapes are
+ * converted into the same per-section, 64-entry (4x4x4) form before palettising.
+ *
+ * Widening a 256-entry array is not an average or a guess. It reproduces
+ * PaperMC/DataConverter's own
+ * {@code V2202} (commit {@code 0782df72}, GPL-3.0, DataVersion 2203) bit for bit: for each 4x4
+ * quadrant of the 16x16 grid it samples the single column at the quadrant's centre — not an average
+ * of the four — and then repeats that resulting 4x4 layer across every one of the 64 four-block-tall
+ * layers a 1024-entry array holds, because a pre-1.15 chunk has no biome variance by height to
+ * preserve in the first place.
+ *
+ * Every entry, in either shape, is a legacy numeric biome id, not a name. String biome names
+ * only exist in chunk data from 1.18 onward. Resolving a numeric id therefore needs a per-version
+ * table, and — unlike blocks — this project's own vendored source for such tables, the ViaVersion
+ * {@code mapping-
+ * An id this table does not know throws, rather than substituting {@code minecraft:plains} the
+ * way DataConverter itself does. That is a deliberate divergence from the upstream source of the
+ * table's facts, made to keep faith with this project's own stance on an unmappable value: silently
+ * inventing a biome is the same silent corruption an unmappable block is refused for elsewhere in
+ * this module.
+ *
+ * The question this step answers, and the two sources that answer it. The field's own
+ * documentation on minecraft.wiki's {@code Chunk format} article (fetched 2026-08-04) reads
+ * "[Int] yPos: Lowest Y section position in the chunk (e.g. {@code -4} in 1.18)" — wording
+ * that, read alone, is ambiguous between "the lowest section this chunk itself stores" and "the
+ * bottom of the dimension's height range". The same article settles which reading is load-bearing,
+ * in its description of the {@code sections} list itself: "All sections in the world's height are
+ * present in this list, even those who are empty (filled with air)." Vanilla always writes every
+ * section down to the dimension floor, so for a vanilla file the two readings coincide by
+ * construction — but a chunk converted by this module does not invent sections it never had, which is
+ * exactly the case where they would split if the field meant the dimension floor. The field's own
+ * name and wording — "in the chunk" — settle it as the chunk's own lowest section, so this step
+ * computes {@code yPos} from what the chunk's {@code sections} list actually contains.
+ *
+ * The second source — what the target actually does with the value — confirms this carries no
+ * risk either way. Minestom's own {@code AnvilLoader} (checked in the sources jar of
+ * {@code net.minestom:minestom}, {@code loadSections}) never reads {@code yPos} at all: it derives
+ * the section range purely from {@code chunk.getMinSection()} / {@code chunk.getMaxSection()} — the
+ * running instance's own dimension type — and discards any section tag whose own {@code Y} falls
+ * outside that range, with the comment "Vanilla stores a section below and above the world for
+ * lighting, throw it out." It writes {@code yPos = chunk.getMinSection()} on save, but that is an
+ * echo of its own understanding for other readers, never something it reads back. Falco's own
+ * {@code FalcoAnvilLoader} mirrors this exactly. The two sources therefore do not disagree: the wiki
+ * states the field's declared meaning, and the loader that actually reads the file confirms that
+ * meaning is inert to it operationally, because it never consults the field at all. Writing anything
+ * other than the chunk's own true lowest section would misdescribe the chunk to every reader that
+ * does consult it, for zero benefit to the one reader this project controls.
+ *
+ * Runs after {@link UnfoldLevel}, so it reads the root {@code sections} list. A chunk whose
+ * {@code sections} list is empty, or absent, gets {@code yPos = 0} — the floor every version below
+ * 2844 in this module's range actually used (see {@code UnfoldLevel}'s own javadoc) — rather than an
+ * arbitrary default.
+ *
+ * Runs at every version — {@link #appliesTo(int)} always returns {@code true} — because a rename can
+ * apply anywhere in this module's whole 1.13-to-today range, and because this step is also the one
+ * that restructures a legacy section's block data into the modern container shape.
+ *
+ * A legacy section's block palette lives at the top level: {@code Palette} and
+ * {@code BlockStates}, siblings of {@code Y}. The nested {@code block_states:
+ * \{palette, data\}} container only exists from DataVersion 2844 onward — the same version that
+ * removed {@code Level} and introduced {@code yPos} — and no step earlier in the chain restructures
+ * it: {@link NormaliseBitPacking} only fixes the bit layout of the legacy {@code BlockStates} array,
+ * it does not rename or nest it. This step is therefore the one place that shape changes, for exactly
+ * the versions that still have it; a section that already carries {@code block_states} is read from
+ * there instead and always re-written as {@code block_states}, whether or not any rule fired.
+ *
+ * A palette entry is a compound with {@code Name} and, if the block has any, {@code Properties} —
+ * unchanged in shape across the whole range this module covers, only its container's position moved.
+ * Translating a palette entry can change its name, its properties, or both, but never the number of
+ * entries or which index of the packed data addresses which entry, so the packed indices themselves
+ * are carried through unchanged; only the palette they address is rewritten.
+ *
- * {@code yPos = 0} is provisional, not a settled answer. A {@code yPos} field is added at the
- * root because 2844 is also the version that introduced it. {@code 0} is correct for what the
- * source chunk means: every version below 2844 fixed the world height to sections
- * {@code 0}–{@code 15}, so a pre-1.18 chunk's own lowest stored section is always {@code 0}. What
- * {@code yPos} means for the target version is a separate, still-open question — the field's
- * own wiki documentation ("Lowest Y section position in the chunk (e.g. -4 in 1.18)") is ambiguous
- * between "the lowest section this chunk itself stores" and "the bottom of the dimension's height
- * range", and vanilla data can never tell the two apart because vanilla always writes every section
- * down to the dimension floor. A converted chunk, which does not invent sections below 0 it never
- * had, is exactly the case where the two readings split. Resolving that split, and therefore whether
- * {@code 0} is the value this step should keep producing, is Task 5's {@code SettleYRange} step, not
- * this one — a reader of this class should not conclude the question is answered because a number is
- * already here.
+ * {@code yPos} itself is not set here. DataVersion 2844 is also the version that introduced
+ * the field, but this step hands the decision of what it should hold to {@link SettleYRange}, which
+ * runs later in the chain, after biomes and block states have found their place in the unfolded
+ * {@code sections} list. See that class's javadoc for the sourced answer — the chunk's own lowest
+ * stored section, never an invented dimension floor — and why the two only diverge for a converted
+ * chunk in the first place.
*
* The list of fields this step moves is deliberately not written down here: it moves whatever
@@ -57,7 +50,6 @@ public final class UnfoldLevel implements MigrationStep {
private static final String LEVEL_KEY = "Level";
private static final String SECTIONS_KEY = "Sections";
private static final String LOWERCASE_SECTIONS_KEY = "sections";
- private static final String Y_POS_KEY = "yPos";
/**
* Creates a new instance of this stateless step.
@@ -95,6 +87,6 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context
}
root = root.put(key, child.getValue());
}
- return root.putInt(Y_POS_KEY, 0);
+ return root;
}
}
diff --git a/falco-migration/src/test/java/net/onelitefeather/falco/migration/ChunkMigrationTest.java b/falco-migration/src/test/java/net/onelitefeather/falco/migration/ChunkMigrationTest.java
index 463145c..db75734 100644
--- a/falco-migration/src/test/java/net/onelitefeather/falco/migration/ChunkMigrationTest.java
+++ b/falco-migration/src/test/java/net/onelitefeather/falco/migration/ChunkMigrationTest.java
@@ -35,10 +35,10 @@ void testAPreEighteenChunkGetsItsFieldsOnTheRoot() {
assertEquals(3, migrated.getInt("xPos"));
assertEquals("minecraft:full", migrated.getString("Status"));
assertNotNull(migrated.get("sections"));
- // Pins today's value, not a claim that it is the right one: yPos = 0 is UnfoldLevel's
- // provisional stand-in until Task 5's SettleYRange decides what yPos means for the target
- // version (see UnfoldLevel's javadoc). This assertion exists so that change is visible as a
- // deliberate edit to this test rather than slipping past unnoticed.
+ // yPos = 0 because SettleYRange computes it from the chunk's own (here: empty) sections
+ // list rather than assuming a fixed floor — see that step's javadoc for the sourced reading
+ // of what yPos means ("the chunk's own lowest section", not the dimension floor) and why an
+ // empty sections list falls back to 0 rather than an arbitrary default.
assertEquals(0, migrated.getInt("yPos"));
}
diff --git a/falco-migration/src/test/java/net/onelitefeather/falco/migration/LegacyBitReaderTest.java b/falco-migration/src/test/java/net/onelitefeather/falco/migration/LegacyBitReaderTest.java
new file mode 100644
index 0000000..f892595
--- /dev/null
+++ b/falco-migration/src/test/java/net/onelitefeather/falco/migration/LegacyBitReaderTest.java
@@ -0,0 +1,58 @@
+package net.onelitefeather.falco.migration;
+
+import net.onelitefeather.falco.anvil.BitPacker;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Pins down {@link LegacyBitReader}: that it reads an entry whole even when it spans two longs, and
+ * that {@code falco-anvil}'s {@link BitPacker} — which restarts at a long boundary for every entry —
+ * would read the same bytes differently.
+ */
+class LegacyBitReaderTest {
+
+ @Test
+ void testAnEntryThatSpansTwoLongsIsReadWhole() {
+ // 5 bits per entry, entry index 12: its continuous bit offset is 12 * 5 = 60, so its 5 bits
+ // occupy continuous positions 60, 61, 62, 63, 64 — the last four bits of packed[0] and the
+ // first bit of packed[1] (bit positions are LSB-numbered within each long, matching
+ // BitPacker's own "<< bitOffset" convention).
+ //
+ // packed[0] = 0xF000_0000_0000_0000L: its top hex digit (F = 1111) occupies bits 60-63, so
+ // bit60=1, bit61=1, bit62=1, bit63=1; every other bit of packed[0] is 0.
+ // packed[1] = 0x0000_0000_0000_0001L: only its bit 0 is set, which is continuous bit 64.
+ //
+ // Reading the 5 bits of entry 12 from the low end to the high end: bit60=1 (entry bit 0),
+ // bit61=1 (bit 1), bit62=1 (bit 2), bit63=1 (bit 3), bit64=1 (bit 4). All five bits are 1,
+ // so the entry's value is 0b11111 = 31 — not 1 (which is what only reading packed[1]'s low
+ // bits, ignoring the four bits packed[0] contributes, would give).
+ long[] packed = {0xF000_0000_0000_0000L, 0x0000_0000_0000_0001L};
+
+ int[] values = LegacyBitReader.unpack(packed, 5, 13);
+
+ assertEquals(0b11111, values[12], "the entry crosses the long boundary and must be read whole");
+ }
+
+ @Test
+ void testTheModernReaderWouldGetThatWrong() {
+ // BitPacker restarts at a long boundary for every entry instead of walking a continuous bit
+ // stream: with 5 bits per entry, 64 / 5 = 12 entries fit in one long (using only its low 60
+ // bits; BitPacker leaves the top 4 bits of every long unused rather than letting an entry
+ // spill into the next one). Entry 12 is therefore the FIRST entry of the SECOND long:
+ // longIndex = 12 / 12 = 1, bitOffset = (12 % 12) * 5 = 0 — it reads bits 0-4 of packed[1],
+ // which is 0b00001 = 1, not the 31 LegacyBitReader reads for the same bytes.
+ //
+ // BitPacker.unpack's own parameter order is (packed, entryCount, bitsPerEntry) — the
+ // opposite of LegacyBitReader.unpack's (packed, bitsPerEntry, entryCount) — so the call
+ // below is BitPacker.unpack(packed, 13, 5), not BitPacker.unpack(packed, 5, 13).
+ long[] packed = {0xF000_0000_0000_0000L, 0x0000_0000_0000_0001L};
+
+ int[] modern = BitPacker.unpack(packed, 13, 5);
+ int[] legacy = LegacyBitReader.unpack(packed, 5, 13);
+
+ assertNotEquals(modern[12], legacy[12],
+ "if these agree, the legacy reader is not doing anything and this module does not need it");
+ }
+}
diff --git a/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java b/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java
new file mode 100644
index 0000000..c7ac31e
--- /dev/null
+++ b/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java
@@ -0,0 +1,270 @@
+package net.onelitefeather.falco.migration.steps;
+
+import net.kyori.adventure.nbt.BinaryTagTypes;
+import net.kyori.adventure.nbt.CompoundBinaryTag;
+import net.kyori.adventure.nbt.ListBinaryTag;
+import net.onelitefeather.falco.anvil.BitPacker;
+import net.onelitefeather.falco.migration.ChunkMigration;
+import net.onelitefeather.falco.migration.MigrationContext;
+import net.onelitefeather.falco.migration.MigrationException;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Pins down the three section-facing steps this task adds: {@link NormaliseBitPacking}, which
+ * re-packs a pre-1.16 section's boundary-spanning block data into the long-aligned layout
+ * {@code BitPacker} can read; {@link RebuildBiomes}, which turns a whole-chunk biome array into a
+ * palettised container per section; and {@link TranslateBlockStates}, which walks every section's
+ * palette through {@link net.onelitefeather.falco.migration.BlockStateRules#translate}.
+ */
+class SectionStepsTest {
+
+ private static final MigrationContext ANY_CONTEXT = new MigrationContext(1519, 4790);
+
+ // --- NormaliseBitPacking -------------------------------------------------------------------
+
+ @Test
+ void testASectionsSpanningBlockStatesAreReadableWithBitPackerAfterNormalising() {
+ // A palette of 17 distinct entries forces 5 bits per entry (BitPacker.bitsPerEntry(17, 4)):
+ // 5 does not divide 64, so the legacy (pre-1.16) packing this fixture builds by hand
+ // genuinely lets entries span a long boundary, exactly the case this step exists for.
+ int bitsPerEntry = 5;
+ int[] values = new int[16 * 16 * 16];
+ for (int i = 0; i < values.length; i++) {
+ values[i] = i % 17;
+ }
+ long[] legacyPacked = legacyPack(values, bitsPerEntry);
+
+ ListBinaryTag palette = ListBinaryTag.empty();
+ for (int i = 0; i < 17; i++) {
+ palette = palette.add(CompoundBinaryTag.builder().putString("Name", "minecraft:test_" + i).build());
+ }
+
+ CompoundBinaryTag chunk = CompoundBinaryTag.builder()
+ .put("Level", CompoundBinaryTag.builder()
+ .put("Sections", ListBinaryTag.from(List.of(CompoundBinaryTag.builder()
+ .putByte("Y", (byte) 0)
+ .put("Palette", palette)
+ .putLongArray("BlockStates", legacyPacked)
+ .build())))
+ .build())
+ .build();
+
+ CompoundBinaryTag normalised = new NormaliseBitPacking().apply(chunk, ANY_CONTEXT);
+
+ long[] repacked = normalised.getCompound("Level").getList("Sections").getCompound(0).getLongArray("BlockStates");
+ int[] roundTripped = BitPacker.unpack(repacked, values.length, bitsPerEntry);
+ assertArrayEquals(values, roundTripped,
+ "BitPacker must read back exactly what the legacy, spanning layout held");
+ }
+
+ @Test
+ void testASingleValueSectionWithNoBlockStatesArrayIsLeftAlone() {
+ CompoundBinaryTag section = CompoundBinaryTag.builder()
+ .putByte("Y", (byte) 0)
+ .put("Palette", ListBinaryTag.from(List.of(
+ CompoundBinaryTag.builder().putString("Name", "minecraft:air").build())))
+ .build();
+ CompoundBinaryTag chunk = CompoundBinaryTag.builder()
+ .put("Level", CompoundBinaryTag.builder()
+ .put("Sections", ListBinaryTag.from(List.of(section)))
+ .build())
+ .build();
+
+ CompoundBinaryTag normalised = new NormaliseBitPacking().apply(chunk, ANY_CONTEXT);
+
+ assertEquals(section, normalised.getCompound("Level").getList("Sections").getCompound(0));
+ }
+
+ // --- RebuildBiomes ---------------------------------------------------------------------------
+
+ @Test
+ void testAWidenedTwentyFourEntryBiomeArrayBecomesAUniformPaletteForItsSection() {
+ int[] biomes = new int[1024];
+ java.util.Arrays.fill(biomes, 0, 64, 1); // section Y=0: every cell is id 1 (plains)
+
+ CompoundBinaryTag chunk = CompoundBinaryTag.builder()
+ .putIntArray("Biomes", biomes)
+ .put("sections", ListBinaryTag.from(List.of(
+ CompoundBinaryTag.builder().putInt("Y", 0).build())))
+ .build();
+
+ CompoundBinaryTag rebuilt = new RebuildBiomes().apply(chunk, ANY_CONTEXT);
+
+ assertNull(rebuilt.get("Biomes"), "the whole-chunk array is consumed, not kept alongside the new containers");
+ CompoundBinaryTag section = rebuilt.getList("sections").getCompound(0);
+ CompoundBinaryTag biomesContainer = section.getCompound("biomes");
+ assertEquals(1, biomesContainer.getList("palette").size());
+ assertEquals("minecraft:plains", biomesContainer.getList("palette").getString(0));
+ assertNull(biomesContainer.get("data"), "a single-entry palette carries no packed data");
+ }
+
+ @Test
+ void testAPreFifteenTwoHundredFiftySixEntryArrayIsSampledAtEachQuadrantsCentreAndRepeatedByHeight() {
+ // Mirrors PaperMC/DataConverter's V2202 (DataVersion 2203) exactly: for XZ quadrant (i, j)
+ // the sampled column is old index ((i*4+2) << 4) | (j*4+2). Placing legacy biome id i*4+j at
+ // exactly those 16 centre columns, and an id that must never be sampled everywhere else,
+ // means the resulting section palette must be built purely from the centre samples in
+ // exactly that (i, j) order, proving the quadrant math rather than an average or a different
+ // sampling point. Ids 0-15 are themselves real, sourced entries of this step's own legacy
+ // biome table (ocean through mushroom_field_shore), so the expected names below are not
+ // invented for the test.
+ int[] legacy = new int[256];
+ java.util.Arrays.fill(legacy, 999); // never assigned in this step's table; must not be sampled
+ for (int i = 0; i < 4; i++) {
+ for (int j = 0; j < 4; j++) {
+ int l = (i << 2) + 2;
+ int k = (j << 2) + 2;
+ legacy[(l << 4) | k] = i * 4 + j; // 16 distinct sampled ids, 0..15, in scan order
+ }
+ }
+ List
* A step decides whether it runs at all from {@code sourceVersion} alone, through
* {@link MigrationStep#appliesTo(int)}. {@code targetVersion} is carried for the steps that have to
- * know how far a chunk is going, not only where it started — none of the three structural steps built
- * in Task 4 need it, but a step that resolves a rename table for a specific target does.
+ * know how far a chunk is going, not only where it started — a step that only moves or deletes data
+ * (unfolding {@code Level}, discarding heightmaps and light) never needs it, but a step that resolves
+ * a rename table for a specific target does.
*
* {@code entityCounter} is this context's counting sink: {@code CountEntities} adds to it through
diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NormaliseBitPacking.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NormaliseBitPacking.java
index 1330509..429e6fa 100644
--- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NormaliseBitPacking.java
+++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NormaliseBitPacking.java
@@ -44,8 +44,9 @@ public final class NormaliseBitPacking implements MigrationStep {
* in chunks has been slightly changed... {@code BlockStates} in {@code Sections} elements no
* longer contain values stretching over multiple 64-bit fields", DataVersion 2529. This is the
* same release-instead-of-snapshot mistake this module has already found and corrected several
- * times over for the block-state rename table and, in Task 6, {@code CountEntities} — so it was
- * checked here too rather than trusted. A chunk in the gap this correction closes, DataVersion
+ * times over elsewhere — in the block-state rename table and in {@code CountEntities}'s own
+ * threshold — so it was checked here too rather than trusted. A chunk in the gap this correction
+ * closes, DataVersion
* 2529 up to but not including 2566 (20w17a's own pre-releases through 1.16 Release Candidate 1),
* already writes the non-spanning layout; treating it as spanning would have this step and
* {@link LegacyBitReader} misread already-correct data for any bits-per-entry that does not evenly
@@ -55,18 +56,10 @@ public final class NormaliseBitPacking implements MigrationStep {
private static final String LEVEL_KEY = "Level";
private static final String SECTIONS_KEY = "Sections";
- private static final String PALETTE_KEY = "Palette";
private static final String BLOCK_STATES_KEY = "BlockStates";
private static final int BLOCK_ENTRIES = 16 * 16 * 16;
-
- /**
- * Minestom's {@code net.minestom.server.instance.palette.Palette.BLOCK_PALETTE_MIN_BITS}
- * (checked in the sources jar of {@code net.minestom:minestom}). This module cannot depend on
- * {@code net.minestom} — {@code falco-archunit}'s {@code migrationKnowsNoMinestom} forbids it —
- * so the constant is pinned here instead, with its source named, rather than imported.
- */
- private static final int BLOCK_PALETTE_MIN_BITS = 4;
+ private static final int BITS_PER_LONG = Long.SIZE;
/**
* Creates a new instance of this stateless step.
@@ -105,12 +98,33 @@ private static CompoundBinaryTag normalise(CompoundBinaryTag section) {
return section;
}
- ListBinaryTag palette = section.getList(PALETTE_KEY, BinaryTagTypes.COMPOUND);
- int bitsPerEntry = BitPacker.bitsPerEntry(palette.size(), BLOCK_PALETTE_MIN_BITS);
+ long[] packed = legacy.value();
+ int bitsPerEntry = exactBitsPerEntry(packed.length);
- int[] indices = LegacyBitReader.unpack(legacy.value(), bitsPerEntry, BLOCK_ENTRIES);
+ int[] indices = LegacyBitReader.unpack(packed, bitsPerEntry, BLOCK_ENTRIES);
long[] repacked = BitPacker.pack(indices, bitsPerEntry);
return section.putLongArray(BLOCK_STATES_KEY, repacked);
}
+
+ /**
+ * Derives the bits-per-entry a legacy writer actually used from the packed array's own length,
+ * rather than from the palette size the way an earlier version of this method did.
+ *
+ * The format explicitly allows a writer to use more bits than a palette strictly needs —
+ * {@code falco-anvil}'s own {@code PaletteData.read} carries the same caveat for the modern
+ * layout, and assuming the minimum here silently misreads an over-width-packed section and then
+ * corrupts it on repacking, without throwing. This derivation is exact rather than a best guess:
+ * the legacy layout has no padding at all — every long is completely full except possibly the
+ * last few bits of the final one — and {@link #BLOCK_ENTRIES} (4096) is itself a multiple of 64,
+ * so {@code longCount * 64} is always evenly divisible by {@code BLOCK_ENTRIES} with no rounding,
+ * for any {@code bitsPerEntry} a real writer could have used.
+ *
+ * A section outside the fixed pre-1.18 range, {@code Y} {@value #MIN_SECTION_Y} to
+ * {@value #MAX_SECTION_Y}, is discarded before anything else runs. Vanilla itself writes one
+ * extra section below and one above that range purely to carry lighting data for the sections that
+ * border them — Minestom's own Anvil loader throws exactly these away on load, with the comment
+ * "Vanilla stores a section below and above the world for lighting, throw it out" (checked in the
+ * sources jar of {@code net.minestom:minestom}, {@code AnvilLoader.java:207-209}). Nothing upstream of
+ * this step drops them, so without this check {@code Y = -1} would index this step's widened biome
+ * array at a negative offset and throw {@link ArrayIndexOutOfBoundsException} — an unchecked failure
+ * this module's fail-loud stance does not consider acceptable — and a border section that happened
+ * to unpack cleanly would otherwise survive into the target's own {@code -4}..{@code 19} range as a
+ * spurious empty section nobody asked for.
+ *
+ * A section outside {@value #MIN_SECTION_Y}..{@value #MAX_SECTION_Y} does not count. Vanilla
+ * writes one extra section below and one above the real range purely to carry lighting data — see
+ * {@link RebuildBiomes}'s own javadoc, which discards these before this step ever runs, for the
+ * sourcing. This step re-checks the same range on its own rather than trusting that ordering: were it
+ * ever exercised on a chunk {@code RebuildBiomes} had not already cleaned — directly, in a test, or
+ * because the chain is reordered later — a lighting-only section at {@code Y = -1} would otherwise
+ * compute {@code yPos = -1}, which is not a section this chunk has any real content for.
+ *
+ *
+ *
- * Known gap, not fixed here: the block-entity list's own container key is not renamed by this - * chain. {@code Level.TileEntities} became the root-level {@code block_entities} in the same - * snapshot (21w43a, DataVersion 2844) that {@code UnfoldLevel} already treats as its own threshold for - * unfolding {@code Level} and renaming {@code Sections} to {@code sections} — but {@code UnfoldLevel}'s - * own Javadoc states that every child other than {@code Sections} keeps its name as it moves, which is - * inaccurate for this one field. This step reads whichever of {@code block_entities} or - * {@code TileEntities} is actually present at the chunk's root and writes the (possibly id-renamed) - * list back under that same key — it does not decide which name the output should carry, because - * doing so would mean quietly reaching into a decision {@code UnfoldLevel} (a different task's file) - * already made. A chunk converted from a pre-2844 source therefore still carries its block entities - * under {@code TileEntities} after this whole chain runs, a name the target version's reader will - * never look under. Reported here and in the task report rather than patched, matching how this - * module has handled every other gap found next to a task rather than inside it. + * The block-entity list's own container key is renamed too, but not by this step. + * {@code Level.TileEntities} becomes the root-level {@code block_entities} in {@link UnfoldLevel} — + * the same snapshot (21w43a, DataVersion 2844) that removed {@code Level} and renamed + * {@code Sections} to {@code sections} also renamed this field, and {@code UnfoldLevel} handles both + * renames identically. By the time a pre-2844 chunk reaches this step in the chain, its block entities + * are therefore already under {@code block_entities}. This step still checks {@code TileEntities} as a + * fallback — reading whichever of the two keys is actually present — purely for its own robustness + * against being exercised outside the full chain (directly in a test, or ahead of a future reordering); + * in the chain this module builds, that fallback is never actually reached. *
* * @since 2.1.0 @@ -68,8 +63,7 @@ public final class TranslateBlockEntities implements MigrationStep { /** * The block-entity {@code id} renames this module has verified for the 1.13-26.1 span. Empty — - * see this class's Javadoc for why, and the task report for the per-entry accounting that - * established it. + * see this class's Javadoc for why. */ private static final Map* Package-private so a test can exercise the substitution mechanism itself against a rename table - * of its own, independent of {@link #RENAMES} — which this task's research found to be empty for - * every {@code id} a 1.13 world can actually contain; see this class's Javadoc. + * of its own, independent of {@link #RENAMES} — which was found to be empty for every {@code id} + * a 1.13 world can actually contain; see this class's Javadoc. *
* * @param id the block entity's current {@code id} diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/TranslateBlockStates.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/TranslateBlockStates.java index 268c5ba..a451ec4 100644 --- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/TranslateBlockStates.java +++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/TranslateBlockStates.java @@ -10,6 +10,7 @@ import net.onelitefeather.falco.migration.BlockState; import net.onelitefeather.falco.migration.BlockStateRules; import net.onelitefeather.falco.migration.MigrationContext; +import net.onelitefeather.falco.migration.MigrationException; import net.onelitefeather.falco.migration.MigrationStep; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; @@ -62,8 +63,12 @@ public final class TranslateBlockStates implements MigrationStep { private static final int BLOCK_ENTRIES = 16 * 16 * 16; /** - * Minestom's {@code net.minestom.server.instance.palette.Palette.BLOCK_PALETTE_MIN_BITS}, - * pinned here for the same reason {@link NormaliseBitPacking#BLOCK_PALETTE_MIN_BITS} is. + * Minestom's {@code net.minestom.server.instance.palette.Palette.BLOCK_PALETTE_MIN_BITS} + * (checked in the sources jar of {@code net.minestom:minestom}). This module cannot depend on + * {@code net.minestom} — {@code falco-archunit}'s {@code migrationKnowsNoMinestom} forbids it — + * so the constant is pinned here instead, with its source named, rather than imported. Used only + * when this step chooses its own bits-per-entry while writing a container — never while + * reading one back; see {@link #readFrom} for why a palette-derived guess is not safe there. */ private static final int BLOCK_PALETTE_MIN_BITS = 4; @@ -95,14 +100,26 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context } private static CompoundBinaryTag translate(CompoundBinaryTag section, int sourceVersion) { + boolean alreadyModernShape = section.get(BLOCK_STATES_KEY) instanceof CompoundBinaryTag; PaletteContainer container = readContainer(section); if (container == null) { return section; } List+ * The bits-per-entry a writer actually used is not assumed from the palette size. The + * format explicitly permits a writer to use more bits than a palette strictly needs, which is + * exactly why {@code falco-anvil}'s own {@code PaletteData.read} resolves the width against the + * packed array's own length instead of trusting the palette-derived minimum — and why this method + * does the same, through {@code BitPacker.resolveBitsPerEntry}, rather than repeating the mistake + * a palette-size guess would make: silently reading a 6-bit-packed palette of 17 entries as if it + * were the 5 bits the palette alone would suggest, corrupting every block in the section without + * throwing. + *
+ */ private static PaletteContainer readFrom(ListBinaryTag paletteTag, @Nullable BinaryTag dataTag) { List- * {@code Sections} is renamed to {@code sections} as it moves, because that is the one field name - * that changed rather than only its position — every other child keeps its own name. A field that - * would silently overwrite a same-named field already present at the root is refused with a - * {@link MigrationException} instead: for every pre-1.18 format known to this module that case cannot - * happen, since a chunk's own top-level keys and {@code Level}'s children never collide, but a silent - * overwrite would contradict this project's fail-loud stance the moment some format this module has - * not seen turns out to disagree. + * Two field names change as they move, because the snapshot that removed {@code Level} (21w43a, + * DataVersion 2844) renamed them in the same breath: {@code Sections} becomes {@code sections}, and + * {@code TileEntities} becomes {@code block_entities}. Both renames are confirmed directly by that + * snapshot's own minecraft.wiki changelog: "Removed chunk's {@code Level} and moved everything it + * contained up. {@code Level.Entities} has moved to {@code entities}. {@code Level.TileEntities} has + * moved to {@code block_entities}..." — checked 2026-08-04. Every other child keeps its own name as + * it moves. A field that would silently overwrite a same-named field already present at the root is + * refused with a {@link MigrationException} instead: for every pre-1.18 format known to this module + * that case cannot happen, since a chunk's own top-level keys and {@code Level}'s children never + * collide, but a silent overwrite would contradict this project's fail-loud stance the moment some + * format this module has not seen turns out to disagree. *
** {@code yPos} itself is not set here. DataVersion 2844 is also the version that introduced @@ -31,7 +35,7 @@ *
** The list of fields this step moves is deliberately not written down here: it moves whatever - * {@code Level} actually holds, rather than a fixed set of names invented for this task. A chunk with + * {@code Level} actually holds, rather than an invented fixed set of names. A chunk with * no {@code Level} compound at all — already unfolded, or never folded in the first place — is * returned unchanged. *
@@ -48,8 +52,15 @@ public final class UnfoldLevel implements MigrationStep { private static final int APPLIES_BELOW = 2844; private static final String LEVEL_KEY = "Level"; - private static final String SECTIONS_KEY = "Sections"; - private static final String LOWERCASE_SECTIONS_KEY = "sections"; + + /** + * The two {@code Level} children whose own name changed in the same snapshot that removed + * {@code Level} itself — see this class's own javadoc for the source. Every other child keeps + * its name as it moves to the root. + */ + private static final Map+ * This is not the obvious rule. Nearly every waterloggable block defaults {@code waterlogged} to + * {@code false} — fences, stairs, walls — which makes {@code false} look like a safe universal + * substitute. It is not: every coral variant and the conduit default to {@code waterlogged=true}, + * because they cannot exist outside water in the first place. A converter that filled the gap with + * {@code false} for every block would quietly dry out every reef and beach every conduit in a + * migrated world, and the chunk would still load without complaint. + *
+ *+ * The exact call this test exercises, {@code Block.fromKey(name).withProperties(partialProperties)}, + * is the same one {@link BlockPaletteResolver#toId} already uses in production — so this pins + * Minestom's real resolution path, not a hypothetical one. + *
+ *+ * This test lives in {@code falco-anvil} rather than {@code falco-migration} because resolving a + * default requires {@link Block}, and {@code falco-migration}'s own ArchUnit rule + * ({@code MigrationBoundaryTest.migrationKnowsNoMinestom}) forbids that module from depending on + * Minestom at all — the boundary is worth more than the convenience of putting this test next to the + * engine it documents. + *
+ *+ * The defaults asserted below are cross-checked against {@code net.minestom:data}'s own + * {@code block.json} (the {@code defaultStateId} recorded for {@code minecraft:tube_coral}, + * {@code minecraft:conduit}, and {@code minecraft:oak_fence} each resolve to the state asserted + * here) rather than assumed from general Minecraft knowledge. + *
+ */ +@ExtendWith(MicrotusExtension.class) +class MigrationEnginePropertyDefaultTest { + + @Test + void testAConvertedCoralWithoutWaterloggedEndsUpWaterloggedTrue() { + Block coral = Block.fromKey("minecraft:tube_coral").withProperties(Map.of()); + + assertEquals("true", coral.properties().get("waterlogged"), + "a 1.13 coral entry that carries no waterlogged property must resolve to Minestom's " + + "own default for the target block, which is true for coral - not a " + + "hardcoded false"); + } + + @Test + void testAConvertedConduitWithoutWaterloggedEndsUpWaterloggedTrue() { + Block conduit = Block.fromKey("minecraft:conduit").withProperties(Map.of()); + + assertEquals("true", conduit.properties().get("waterlogged"), + "a conduit is not a coral subtype - this is a second, independently sourced fact, " + + "not a corollary of the coral case above"); + } + + @Test + void testAnOrdinaryWaterloggableBlockWithoutWaterloggedEndsUpWaterloggedFalse() { + Block fence = Block.fromKey("minecraft:oak_fence").withProperties(Map.of()); + + assertEquals("false", fence.properties().get("waterlogged"), + "the contrast case: for most blocks the target default really is false, so the rule " + + "a migrated chunk must follow is \"the target version's default\", never a " + + "rule that would look like \"always true\" if only coral were checked"); + } +} From 627cdd0dfb966e7bd22ad13eb27c43f1ca2dba96 Mon Sep 17 00:00:00 2001 From: TheMeinerLP+ * Every other test either module has asserts key by key against an in-memory {@link CompoundBinaryTag}. + * That is exactly the gap the final review found: {@code NamespaceStatus} namespaced a chunk status + * without translating its value, and nothing upstream of the loader noticed, because nothing exercised + * the loader at all. This test instead runs the whole pipeline a converted world actually goes + * through — a genuine 1.13 chunk, through the nine-step {@link ChunkMigration} chain, written into a + * real region file exactly {@link FalcoAnvilLoaderIntegrationTest}'s own {@code writeRawChunk} helper + * does, then loaded back through the production {@link FalcoAnvilLoader} — and checks that a block + * placed by the original 1.13 chunk actually arrives. Both the chunk's {@code Status} value + * ({@code postprocessed}, not {@code full} — see {@code NamespaceStatusTest} in {@code falco-migration} + * for why) and its packed block data (built by hand in the pre-1.16 boundary-spanning layout, the + * shape {@code NormaliseBitPacking} exists to re-pack) are the real, sourced 1.13 shapes, not stand-ins. + */ +@ExtendWith(MicrotusExtension.class) +class MigrationRoundTripTest { + + private static final Key OVERWORLD = Key.key("minecraft:overworld"); + private static final int SECTION_BLOCK_ENTRIES = 16 * 16 * 16; + + @TempDir + private Path worldRoot; + + @Test + void testANineteenThirteenChunkMigratedAndReloadedThroughTheProductionLoaderKeepsItsBlocks(Env env) throws Exception { + // A 2-entry palette (air, stone) packed at 4 bits per entry (BitPacker.bitsPerEntry(2, 4)), + // in the pre-1.16 boundary-spanning layout every DataVersion below 2529 actually wrote. Every + // block in the section is air (palette index 0) except one, index 0 of the packed array + // itself (local x=0, y=0, z=0), which is stone (palette index 1). + int[] values = new int[SECTION_BLOCK_ENTRIES]; + values[0] = 1; + long[] legacyPacked = legacyPack(values, 4); + + CompoundBinaryTag section = CompoundBinaryTag.builder() + .putByte("Y", (byte) 2) + .put("Palette", ListBinaryTag.from(List.of( + CompoundBinaryTag.builder().putString("Name", "minecraft:air").build(), + CompoundBinaryTag.builder().putString("Name", "minecraft:stone").build()))) + .putLongArray("BlockStates", legacyPacked) + .build(); + + CompoundBinaryTag legacyChunk = CompoundBinaryTag.builder() + .putInt("DataVersion", 1519) // Minecraft 1.13 release, ChunkMigration's own floor + .put("Level", CompoundBinaryTag.builder() + .putInt("xPos", 5) + .putInt("zPos", 5) + .putString("Status", "postprocessed") // the real 1.13 terminal status + .put("Sections", ListBinaryTag.from(List.of(section))) + .build()) + .build(); + + CompoundBinaryTag migrated = ChunkMigration.migrate(legacyChunk, MinecraftServer.DATA_VERSION); + writeRawChunk(5, 5, migrated); + + try (FalcoAnvilLoader loader = new FalcoAnvilLoader(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + Chunk loaded = loader.loadChunk(instance, 5, 5); + + assertNotNull(loaded, "the loader's own status guard must accept the migrated chunk's " + + "translated status (minecraft:full), not just its namespace " + + "(minecraft:postprocessed would still be refused)"); + assertEquals(Block.STONE, blockAt(loaded, 0, 32, 0), + "the one block the 1.13 chunk actually placed must survive the whole round trip"); + assertEquals(Block.AIR, blockAt(loaded, 1, 32, 0), + "every other block in the section must stay air, proving the packed indices " + + "themselves, not just the palette, survived"); + } + } + + /** + * Writes chunk data straight into the region file of the temporary world, exactly + * {@link FalcoAnvilLoaderIntegrationTest}'s own private helper of the same name does — duplicated + * here rather than shared, since that helper is {@code private} to its own test class. + * + * @param chunkX the absolute chunk x coordinate + * @param chunkZ the absolute chunk z coordinate + * @param data the chunk data to store + * @throws Exception if the chunk cannot be written + */ + private void writeRawChunk(int chunkX, int chunkZ, CompoundBinaryTag data) throws Exception { + Path directory = this.worldRoot.resolve("dimensions/minecraft/overworld/region"); + Files.createDirectories(directory); + ByteArrayOutputStream target = new ByteArrayOutputStream(); + BinaryTagIO.writer().writeNamed( + Map.entry("", data), target, BinaryTagIO.Compression.NONE + ); + + try (RegionFile file = RegionFile.open(directory.resolve("r." + (chunkX >> 5) + "." + (chunkZ >> 5) + ".mca"))) { + file.writeRaw(chunkX, chunkZ, ChunkCompression.ZLIB, ChunkCompression.ZLIB.compress(target.toByteArray())); + } + } + + /** + * Reads a block of the given chunk while holding its read lock, exactly + * {@link FalcoAnvilLoaderIntegrationTest}'s own private helper of the same name does. + * + * @param chunk the chunk to read + * @param x the x coordinate inside the chunk + * @param y the y coordinate of the block + * @param z the z coordinate inside the chunk + * @return the block at the given position + */ + private static Block blockAt(Chunk chunk, int x, int y, int z) { + chunk.lockReadLock(); + try { + return chunk.getBlock(x, y, z); + } finally { + chunk.unlockReadLock(); + } + } + + /** + * Packs {@code values} using the pre-1.16 layout, in which an entry is allowed to span a long + * boundary — the same fixture helper {@code SectionStepsTest} uses in {@code falco-migration}, + * duplicated here rather than shared across modules for the same reason + * {@code FalcoAnvilLoaderIntegrationTest.writeRawChunk} is duplicated rather than exposed. + */ + private static long[] legacyPack(int[] values, int bitsPerEntry) { + long totalBits = (long) values.length * bitsPerEntry; + long[] packed = new long[(int) ((totalBits + 63) / 64)]; + long mask = (1L << bitsPerEntry) - 1L; + + for (int index = 0; index < values.length; index++) { + long bitOffset = (long) index * bitsPerEntry; + int longIndex = (int) (bitOffset / 64); + int bitInLong = (int) (bitOffset % 64); + long value = values[index] & mask; + + packed[longIndex] |= value << bitInLong; + int bitsWrittenInFirstLong = 64 - bitInLong; + if (bitsWrittenInFirstLong < bitsPerEntry) { + packed[longIndex + 1] |= value >>> bitsWrittenInFirstLong; + } + } + return packed; + } +} diff --git a/falco-migration/build.gradle.kts b/falco-migration/build.gradle.kts index 99efbfb..541e63c 100644 --- a/falco-migration/build.gradle.kts +++ b/falco-migration/build.gradle.kts @@ -16,3 +16,11 @@ dependencies { testImplementation(libs.junit.platform.launcher) testRuntimeOnly(libs.junit.jupiter.engine) } + +// This module gets -Werror on javadoc from the root build, but nothing otherwise calls javadoc for +// it: unlike falco-anvil/-light/-instance it has no withJavadocJar(), on purpose (no publishing and +// no japicmp baseline for a module that has never been released — see the design doc). Without this, +// a broken javadoc comment would compile clean and pass check regardless. +tasks.named("check") { + dependsOn(tasks.named("javadoc")) +} diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockStateRule.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockStateRule.java index 80a3289..1db90d8 100644 --- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockStateRule.java +++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/BlockStateRule.java @@ -28,8 +28,9 @@ public interface BlockStateRule { *
* This is the version a fix landed in, not the version the rule targets: a rule applies to a * chunk's source version exactly when the source is older than this number, i.e. when - * {@code since() > sourceVersion}. A rule with {@code since() == 1802} therefore applies to a - * 1.13 world ({@code 1519 < 1802}) and leaves a 1.16 world ({@code 2566 > 1802}) alone, because + * {@code since() > sourceVersion}. A rule with {@code since() == 1901} — {@code stone_slab}'s own + * rule in {@link BlockStateRules}, DataVersion 1901, snapshot 18w43a — therefore applies to a + * 1.13 world ({@code 1519 < 1901}) and leaves a 1.16 world ({@code 2566 > 1901}) alone, because * by 1.16 the change already happened and the state already carries its later meaning. *
* diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/ChunkMigration.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/ChunkMigration.java index 10e13ce..8dbfdc5 100644 --- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/ChunkMigration.java +++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/ChunkMigration.java @@ -67,10 +67,21 @@ private ChunkMigration() { * @param chunk the chunk's root compound, as read from a region file * @param targetVersion the {@code DataVersion} the result should carry * @return the converted chunk, stamped with {@code targetVersion} - * @throws MigrationException if the chunk's own {@code DataVersion} is older than + * @throws MigrationException if {@code chunk} carries no {@code DataVersion} field at all, or if + * the value it does carry is older than * {@link #MINIMUM_SOURCE_VERSION} */ public static CompoundBinaryTag migrate(CompoundBinaryTag chunk, int targetVersion) { + if (chunk.get(DATA_VERSION_KEY) == null) { + // Distinguished from "present but too old" on purpose: a missing field is a chunk this + // module was never told the age of, while a present-but-low one is a chunk this module + // knows for certain predates its floor. CompoundBinaryTag#getInt below would otherwise + // collapse both into the same misleading "DataVersion 0", a number no real chunk of any + // age actually carries. + throw new MigrationException( + "The chunk carries no DataVersion field at all, so its source version cannot be " + + "determined and this module cannot decide which steps would even apply"); + } int sourceVersion = chunk.getInt(DATA_VERSION_KEY); return migrate(chunk, new MigrationContext(sourceVersion, targetVersion)); } diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NamespaceStatus.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NamespaceStatus.java index 5485572..902b3fc 100644 --- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NamespaceStatus.java +++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/NamespaceStatus.java @@ -6,17 +6,49 @@ import net.onelitefeather.falco.migration.MigrationStep; import org.jetbrains.annotations.ApiStatus; +import java.util.Map; + /** - * Rewrites a chunk status without a namespace, such as {@code full}, into its namespaced form, - * {@code minecraft:full}. + * Rewrites a chunk status without a namespace into its namespaced form, translating the handful of + * pre-1.14 values this module can prove a meaning for along the way — most importantly, a genuinely + * complete 1.13 chunk's own value, which is not {@code full}. ** This step tests no {@code DataVersion} at all — {@link #appliesTo(int)} always returns * {@code true} — because the exact version that namespaced the chunk status could not be established; * the wiki's own change history for the field carries a notice that it is missing a significant number * of changes. Testing whether {@code Status} already carries a {@code ':'} is correct for every * version in this module's range regardless, and does not depend on a number nobody has actually - * read: a status that already has a namespace is left alone, and a bare status is prefixed with - * {@code minecraft:}. + * read: a status that already has a namespace is left alone, and a bare status is looked up in + * {@link #RENAMED_ON_NAMESPACE} before being prefixed with {@code minecraft:} — the value that lookup + * does not recognize is prefixed exactly as it was read, unchanged, because this module encodes only + * what it can source rather than guess a rename it cannot prove. + *
+ *+ * A 1.13 chunk's own terminal status is not {@code full}. Namespaced status ids, and + * {@code full} as a name at all, did not exist until Minecraft 1.14's own development snapshots; + * DataVersion 1519 (this module's floor) writes one of the ten values 1.13.2's own + * {@code ChunkStatus} registers, in pipeline order: {@code empty}, {@code base}, {@code carved}, + * {@code liquid_carved}, {@code decorated}, {@code lighted}, {@code mobs_spawned}, {@code finalized}, + * {@code fullchunk}, {@code postprocessed} — confirmed directly against the decompiled 1.13.2 source, + * {@code Akarin-project/Minecraft}'s {@code 1.13.2/spigot/net/minecraft/server/ChunkStatus.java}, + * 2026-08-05. Both {@code fullchunk} (a proto-chunk that has become a full, loaded chunk but has + * never been postprocessed — generated terrain a player has simply never walked near) and + * {@code postprocessed} (a full chunk that has) describe a completely generated chunk; the game + * itself later folds the distinction away, confirmed by + * {@code PaperMC/DataConverter}'s own two-stage fix at commit {@code dcde1f1f89dd6882b56246fe60233ed6a1cb5abb}: + * {@code V1905.java} (DataVersion 1905, 18w43c+2) renames {@code postprocessed} to {@code fullchunk} + * outright, and {@code V1911.java} (DataVersion 1911, 18w46a+1) then maps {@code fullchunk} — by then + * the only surviving name for "complete" — to {@code full}, in the same table that renames every + * other pipeline stage to its 1.14-development-era name ({@code base} to {@code surface}, + * {@code carved} to {@code carvers}, and so on). Both fetches checked 2026-08-05. This step therefore + * maps {@code fullchunk} and {@code postprocessed} to {@code full} directly, the one edge this + * module's own loader ({@code FalcoAnvilLoader.isFullyGenerated}) actually gates a chunk's load on; + * see {@code NamespaceStatusTest.testAGenuineNineteenThirteenTerminalStatusBecomesMinecraftFull} for + * the sourced fixture. The eight remaining, non-terminal 1.13 values are left namespaced but + * otherwise unrenamed rather than guessed at: they describe an incomplete proto-chunk either way, and + * {@code FalcoAnvilLoader} skips anything that is not exactly {@code minecraft:full} regardless of + * which non-terminal name it carries, so a wrong (but still non-{@code full}) guess for one of them + * would not silently corrupt a load the way the {@code full} case would have. *
*
* A chunk with no {@code Status} field at all is returned unchanged.
@@ -30,6 +62,17 @@ public final class NamespaceStatus implements MigrationStep {
private static final String STATUS_KEY = "Status";
private static final String MINECRAFT_NAMESPACE = "minecraft:";
+ /**
+ * The only two bare, pre-namespace {@code Status} values this module can source a modern meaning
+ * for — both the 1.13.2 terminal ("chunk is completely generated") status, under its two names
+ * across that version's own chunk-loading pipeline. See this class's own javadoc for the sourced
+ * chain ({@code fullchunk}/{@code postprocessed} to {@code full}) that justifies this table; every
+ * bare value not in it is namespaced without being renamed, rather than guessed at.
+ */
+ private static final Map
* This step runs before {@link UnfoldLevel} in {@code ChunkMigration}'s chain — its own threshold,
- * 2566, is strictly below {@code UnfoldLevel}'s, 2844, so every chunk this step applies to still
+ * 2529, is strictly below {@code UnfoldLevel}'s, 2844, so every chunk this step applies to still
* carries the pre-1.18 {@code Level} wrapper when this step sees it. It therefore reads
* {@code Level.Sections}, not the root {@code sections} list later steps use, and only touches each
* section's {@code BlockStates} long array — the key names and every other field are left exactly as
@@ -31,6 +33,24 @@
* format omits it and lets the one palette entry fill the whole section — and is returned unchanged,
* as is a chunk with no {@code Level} compound.
*
+ * The width a section is re-packed at is not the width it was read at. An earlier version of
+ * this step preserved whatever width {@link #exactBitsPerEntry(int)} recovered from the legacy array,
+ * on the reasoning that the exact original width is always recoverable from the spanning format's own
+ * length with no ambiguity (unlike the long-aligned format, which pads). That is true, but it does not
+ * survive the round trip: {@link TranslateBlockStates}, reading the long-aligned array back, has only
+ * the array's length and the palette size to work from, and {@code BitPacker.expectedLongCount} maps
+ * more than one width to the same long count for {@link #BLOCK_ENTRIES} entries — 11 and 12 bits both
+ * produce 820 longs, for instance — so a width a writer over-provisioned (the format allows more bits
+ * than a palette strictly needs) is not always recoverable downstream, and a wrong guess corrupts the
+ * section silently rather than throwing. Repacking at the palette's own canonical minimum,
+ * {@code BitPacker.bitsPerEntry(paletteSize, 4)}, removes the ambiguity constructively instead of
+ * hoping the disambiguation heuristic guesses right: the indices this step unpacked are carried
+ * through exactly, only the width they are written back at changes, and every section this step
+ * touches is guaranteed to carry the one width {@link TranslateBlockStates} would derive from its
+ * palette alone. See {@code SectionStepsTest.testAnOverWidthPackedSectionIsCanonicalisedRatherThanPreservingTheAmbiguousWidth}
+ * for the measured 11/12-bit collision this closes.
+ *
* The format explicitly allows a writer to use more bits than a palette strictly needs —
* {@code falco-anvil}'s own {@code PaletteData.read} carries the same caveat for the modern
- * layout, and assuming the minimum here silently misreads an over-width-packed section and then
- * corrupts it on repacking, without throwing. This derivation is exact rather than a best guess:
- * the legacy layout has no padding at all — every long is completely full except possibly the
- * last few bits of the final one — and {@link #BLOCK_ENTRIES} (4096) is itself a multiple of 64,
- * so {@code longCount * 64} is always evenly divisible by {@code BLOCK_ENTRIES} with no rounding,
- * for any {@code bitsPerEntry} a real writer could have used.
+ * layout — which is why this derivation reads the width the writer actually used rather than
+ * assuming the palette's own minimum. It is exact rather than a best guess: the legacy layout has
+ * no padding at all — every long is completely full except possibly the last few bits of the
+ * final one — and {@link #BLOCK_ENTRIES} (4096) is itself a multiple of 64, so
+ * {@code longCount * 64} is always evenly divisible by {@code BLOCK_ENTRIES} with no rounding,
+ * for any {@code bitsPerEntry} a real writer could have used. (What this method's result is then
+ * re-packed at is a separate decision — see {@link #normalise(CompoundBinaryTag)}, which does not
+ * reuse it.)
*
- * A section outside the fixed pre-1.18 range, {@code Y} {@value #MIN_SECTION_Y} to - * {@value #MAX_SECTION_Y}, is discarded before anything else runs. Vanilla itself writes one - * extra section below and one above that range purely to carry lighting data for the sections that - * border them — Minestom's own Anvil loader throws exactly these away on load, with the comment - * "Vanilla stores a section below and above the world for lighting, throw it out" (checked in the - * sources jar of {@code net.minestom:minestom}, {@code AnvilLoader.java:207-209}). Nothing upstream of - * this step drops them, so without this check {@code Y = -1} would index this step's widened biome - * array at a negative offset and throw {@link ArrayIndexOutOfBoundsException} — an unchecked failure - * this module's fail-loud stance does not consider acceptable — and a border section that happened - * to unpack cleanly would otherwise survive into the target's own {@code -4}..{@code 19} range as a - * spurious empty section nobody asked for. + * A section that carries no block data is discarded before anything else runs, regardless of its + * {@code Y}. Vanilla itself writes one extra section below and one above a chunk's real content + * purely to carry lighting data for the sections that border them — Minestom's own Anvil loader + * throws exactly these away on load, with the comment "Vanilla stores a section below and above the + * world for lighting, throw it out" (checked in the sources jar of {@code net.minestom:minestom}, + * {@code AnvilLoader.java:207-209}). An earlier version of this step told such a section apart from a + * real one by a fixed {@code Y} range, {@code 0} to {@code 15} — which only holds for a world whose + * height is the pre-1.18 default, {@code 0}..{@code 255}. Configurable world height is not a 1.18 + * feature: it landed three snapshots into 1.17's own cycle, 20w49a, DataVersion 2685 (confirmed + * against that snapshot's own minecraft.wiki changelog, fetched 2026-08-05: "Added {@code height} and + * {@code min_y} variables to dimension types, allowing for the height limit to be increased in custom + * world settings") — below every source version this step ever runs for. A world converted while + * using such a custom height genuinely stores content sections outside {@code 0}..{@code 15}; the + * fixed-range check silently discarded those instead of the lighting-only sections it meant to catch, + * for both a custom-height 1.17 world and a stock 1.18-generation Overworld caught mid-conversion + * (DataVersion 2836-2843) whose sections already span the new {@code -4}..{@code 19} range under the + * old {@code Level}-based layout. This step instead discards a section carrying none of + * {@code Palette}, {@code BlockStates}, or {@code block_states} — the only names a legacy or an + * already-normalised section's block data is ever stored under — which is exactly the lighting-only + * sections vanilla writes, independent of where they sit, and never a section that actually holds + * blocks. Nothing upstream of this step drops them, so without this check a lighting-only section at + * a negative {@code Y} would otherwise index this step's widened biome array at a negative offset and + * throw {@link ArrayIndexOutOfBoundsException} — an unchecked failure this module's fail-loud stance + * does not consider acceptable — and a border section that happened to unpack cleanly would otherwise + * survive into the target's own section range as a spurious empty section nobody asked for. + *
+ *+ * Where the surviving sections actually start still has to be known to place biomes correctly, and + * this step refuses to guess it. {@link #biomeContainer} indexes the widened, whole-chunk biome + * array with {@code sectionY * SECTION_BIOME_ENTRIES}, which only lands on the right layer if the + * chunk's lowest surviving section is {@code Y = 0} — true for every fixed-height pre-1.18 world, but + * not for a world converted from a custom height range whose sections start below zero, which + * {@code Y} alone no longer distinguishes from the fixed-height case now that content is what decides + * a section's survival. Rather than writing a biome layer 64 blocks off from the section it actually + * belongs to, this step throws {@link MigrationException} the moment a whole-chunk {@code Biomes} + * array is present alongside any surviving section at {@code Y < 0}. *
* * @since 2.1.0 @@ -99,18 +124,19 @@ public final class RebuildBiomes implements MigrationStep { private static final String DATA_KEY = "data"; private static final String SECTION_Y_KEY = "Y"; + /** + * The three names a section's own block data is ever stored under in this module's source range — + * see this class's own javadoc for why their absence, rather than a fixed {@code Y} range, is what + * marks a lighting-only section. + */ + private static final String LEGACY_PALETTE_KEY = "Palette"; + private static final String LEGACY_BLOCK_STATES_KEY = "BlockStates"; + private static final String MODERN_BLOCK_STATES_KEY = "block_states"; + private static final int PRE_WIDENING_ENTRIES = 256; private static final int WIDENED_ENTRIES = 1024; private static final int SECTION_BIOME_ENTRIES = 4 * 4 * 4; - /** - * The fixed section range every pre-1.18 chunk in this module's range actually stores content - * for — see this class's own javadoc for why a section outside it is discarded rather than - * processed. - */ - private static final int MIN_SECTION_Y = 0; - private static final int MAX_SECTION_Y = 15; - /** * Minestom's {@code net.minestom.server.instance.palette.Palette.BIOME_PALETTE_MIN_BITS} * (checked in the sources jar of {@code net.minestom:minestom}), pinned here for the same reason @@ -138,12 +164,13 @@ public boolean appliesTo(int sourceVersion) { * @param chunk {@inheritDoc} * @param context {@inheritDoc} * @return {@inheritDoc} - * @throws MigrationException if {@code Biomes} holds neither 256 nor 1024 entries, or holds a - * legacy numeric id this step's table does not know + * @throws MigrationException if {@code Biomes} holds neither 256 nor 1024 entries, holds a legacy + * numeric id this step's table does not know, or is present alongside + * a surviving section at {@code Y < 0} */ @Override public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context) { - chunk = discardSectionsOutsideTheFixedRange(chunk); + chunk = discardSectionsWithoutBlockData(chunk); if (!(chunk.get(BIOMES_KEY) instanceof IntArrayBinaryTag biomesTag)) { return chunk; @@ -158,6 +185,17 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context } ListBinaryTag sections = chunk.getList(SECTIONS_KEY, BinaryTagTypes.COMPOUND); + for (BinaryTag sectionTag : sections) { + if (sectionTag instanceof CompoundBinaryTag section && section.getInt(SECTION_Y_KEY) < 0) { + throw new MigrationException("The chunk carries a whole-chunk 'Biomes' array and a " + + "surviving section at Y = " + section.getInt(SECTION_Y_KEY) + "; this step's " + + "biome offset (Y * " + SECTION_BIOME_ENTRIES + ") assumes the chunk's lowest " + + "section is Y = 0, which does not hold for a world with a shifted " + + "(below-zero) height range, so the chunk is refused instead of silently " + + "writing every biome layer at the wrong height"); + } + } + ListBinaryTag rebuilt = ListBinaryTag.empty(); for (BinaryTag sectionTag : sections) { if (!(sectionTag instanceof CompoundBinaryTag section)) { @@ -172,13 +210,13 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context } /** - * Drops every section whose {@code Y} falls outside {@value #MIN_SECTION_Y}.. - * {@value #MAX_SECTION_Y} — see this class's own javadoc for why vanilla writes them and why they - * must not reach the biome-rebuilding logic below. Runs first, and unconditionally, so it also - * protects a chunk that has no {@code Biomes} field at all (an early return further down would - * otherwise skip this check entirely for such a chunk). + * Drops every section carrying none of {@link #LEGACY_PALETTE_KEY}, {@link #LEGACY_BLOCK_STATES_KEY} + * or {@link #MODERN_BLOCK_STATES_KEY} — see this class's own javadoc for why the absence of block + * data, rather than a fixed {@code Y} range, is what marks a lighting-only section. Runs first, and + * unconditionally, so it also protects a chunk that has no {@code Biomes} field at all (an early + * return further down would otherwise skip this check entirely for such a chunk). */ - private static CompoundBinaryTag discardSectionsOutsideTheFixedRange(CompoundBinaryTag chunk) { + private static CompoundBinaryTag discardSectionsWithoutBlockData(CompoundBinaryTag chunk) { ListBinaryTag sections = chunk.getList(SECTIONS_KEY, BinaryTagTypes.COMPOUND); if (sections.size() == 0) { return chunk; @@ -187,18 +225,21 @@ private static CompoundBinaryTag discardSectionsOutsideTheFixedRange(CompoundBin ListBinaryTag kept = ListBinaryTag.empty(); boolean droppedAny = false; for (BinaryTag sectionTag : sections) { - if (sectionTag instanceof CompoundBinaryTag section) { - int sectionY = section.getInt(SECTION_Y_KEY); - if (sectionY < MIN_SECTION_Y || sectionY > MAX_SECTION_Y) { - droppedAny = true; - continue; - } + if (sectionTag instanceof CompoundBinaryTag section && !hasBlockData(section)) { + droppedAny = true; + continue; } kept = kept.add(sectionTag); } return droppedAny ? chunk.put(SECTIONS_KEY, kept) : chunk; } + private static boolean hasBlockData(CompoundBinaryTag section) { + return section.get(LEGACY_PALETTE_KEY) != null + || section.get(LEGACY_BLOCK_STATES_KEY) != null + || section.get(MODERN_BLOCK_STATES_KEY) != null; + } + private static int[] widen(int[] legacy) { int[] widened = new int[WIDENED_ENTRIES]; for (int i = 0; i < 4; i++) { diff --git a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/SettleYRange.java b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/SettleYRange.java index 7de66d8..ab03d64 100644 --- a/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/SettleYRange.java +++ b/falco-migration/src/main/java/net/onelitefeather/falco/migration/steps/SettleYRange.java @@ -42,18 +42,22 @@ * ** Runs after {@link UnfoldLevel}, so it reads the root {@code sections} list. A chunk whose - * {@code sections} list is empty, or absent, gets {@code yPos = 0} — the floor every version below - * 2844 in this module's range actually used (see {@code UnfoldLevel}'s own javadoc) — rather than an - * arbitrary default. + * {@code sections} list is empty, or absent, is returned unchanged rather than stamped with an + * invented {@code yPos} — there is no section in the chunk for any such value to describe. *
*- * A section outside {@value #MIN_SECTION_Y}..{@value #MAX_SECTION_Y} does not count. Vanilla - * writes one extra section below and one above the real range purely to carry lighting data — see - * {@link RebuildBiomes}'s own javadoc, which discards these before this step ever runs, for the - * sourcing. This step re-checks the same range on its own rather than trusting that ordering: were it - * ever exercised on a chunk {@code RebuildBiomes} had not already cleaned — directly, in a test, or - * because the chain is reordered later — a lighting-only section at {@code Y = -1} would otherwise - * compute {@code yPos = -1}, which is not a section this chunk has any real content for. + * Every remaining section counts, without a fixed range filter. An earlier version of this + * step ignored a section outside a fixed {@code Y} range of {@code 0}..{@code 15} on the theory that + * only vanilla's own lighting-only border sections — one below and one above a chunk's real content, + * discarded by {@link RebuildBiomes} earlier in the chain — could ever fall outside it. That theory + * held only for a world at the pre-1.18 fixed height, {@code 0}..{@code 255}; see + * {@link RebuildBiomes}'s own javadoc for why a world converted from a custom height (possible from + * DataVersion 2685 onward, well below every source version this step runs for) can genuinely store + * real content sections outside that range, and for why {@code Y} alone can no longer be trusted to + * tell such a section apart from a lighting-only one. This step now trusts {@link RebuildBiomes} to + * have already removed every section that carries no block data, and simply takes the lowest + * {@code Y} among whatever sections remain — never filtering by, or falling back to, a fixed number + * that might not correspond to any section the chunk actually has. *
* * @since 2.1.0 @@ -71,13 +75,6 @@ public final class SettleYRange implements MigrationStep { private static final String SECTION_Y_KEY = "Y"; private static final String Y_POS_KEY = "yPos"; - /** - * The fixed section range every pre-1.18 chunk in this module's range actually stores content - * for — see this class's own javadoc for why a section outside it must not count. - */ - private static final int MIN_SECTION_Y = 0; - private static final int MAX_SECTION_Y = 15; - /** * Creates a new instance of this stateless step. */ @@ -98,9 +95,6 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context for (BinaryTag sectionTag : sections) { if (sectionTag instanceof CompoundBinaryTag section) { int sectionY = section.getInt(SECTION_Y_KEY); - if (sectionY < MIN_SECTION_Y || sectionY > MAX_SECTION_Y) { - continue; - } if (!any || sectionY < lowest) { lowest = sectionY; any = true; @@ -108,6 +102,6 @@ public CompoundBinaryTag apply(CompoundBinaryTag chunk, MigrationContext context } } - return chunk.putInt(Y_POS_KEY, lowest); + return any ? chunk.putInt(Y_POS_KEY, lowest) : chunk; } } diff --git a/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java b/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java index 8189ad3..e13338a 100644 --- a/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java +++ b/falco-migration/src/test/java/net/onelitefeather/falco/migration/steps/SectionStepsTest.java @@ -265,7 +265,7 @@ void testAOneThousandTwentyFourEntryBiomeArrayBecomesAUniformPaletteForItsSectio CompoundBinaryTag chunk = CompoundBinaryTag.builder() .putIntArray("Biomes", biomes) .put("sections", ListBinaryTag.from(List.of( - CompoundBinaryTag.builder().putInt("Y", 0).build()))) + CompoundBinaryTag.builder().putInt("Y", 0).put("Palette", airPalette()).build()))) .build(); CompoundBinaryTag rebuilt = new RebuildBiomes().apply(chunk, ANY_CONTEXT); @@ -307,8 +307,8 @@ void testAPreFifteenTwoHundredFiftySixEntryArrayIsSampledAtEachQuadrantsCentreAn CompoundBinaryTag chunk = CompoundBinaryTag.builder() .putIntArray("Biomes", legacy) .put("sections", ListBinaryTag.from(List.of( - CompoundBinaryTag.builder().putInt("Y", 0).build(), - CompoundBinaryTag.builder().putInt("Y", 5).build()))) + CompoundBinaryTag.builder().putInt("Y", 0).put("Palette", airPalette()).build(), + CompoundBinaryTag.builder().putInt("Y", 5).put("Palette", airPalette()).build()))) .build(); CompoundBinaryTag rebuilt = new RebuildBiomes().apply(chunk, ANY_CONTEXT); @@ -343,7 +343,7 @@ void testAnUnknownLegacyBiomeIdFailsRatherThanInventingAName() { CompoundBinaryTag chunk = CompoundBinaryTag.builder() .putIntArray("Biomes", biomes) .put("sections", ListBinaryTag.from(List.of( - CompoundBinaryTag.builder().putInt("Y", 0).build()))) + CompoundBinaryTag.builder().putInt("Y", 0).put("Palette", airPalette()).build()))) .build(); MigrationException exception = assertThrows(MigrationException.class, @@ -351,36 +351,75 @@ void testAnUnknownLegacyBiomeIdFailsRatherThanInventingAName() { assertTrue(exception.getMessage().contains("253")); } - // --- Sections outside the fixed 0..15 range --------------------------------------------------- + // --- Sections without block data, discarded by content rather than by Y range ----------------- @Test - void testASectionBelowZeroDoesNotCrashRebuildBiomesAndIsDroppedRatherThanKept() { - // Vanilla writes one extra section below the real 0..15 range (Y = -1) purely to carry - // lighting data for the section it borders. Without discarding it first, its offset into - // the widened biome array (-1 * 64 = -64) is negative and indexing it throws - // ArrayIndexOutOfBoundsException rather than a MigrationException. - int[] biomes = new int[1024]; - java.util.Arrays.fill(biomes, 1); - + void testASectionWithoutBlockDataAtNegativeYIsDroppedRatherThanKept() { + // Vanilla writes one extra section below a chunk's real content (Y = -1) purely to carry + // lighting data for the section it borders; such a section never carries Palette, + // BlockStates or block_states. No Biomes array is present, so only the content-based + // discarding itself is exercised here, independent of the Y < 0 guard around the biome + // offset (that guard is proven separately below). + CompoundBinaryTag lightingOnly = CompoundBinaryTag.builder().putInt("Y", -1).build(); + CompoundBinaryTag realContent = CompoundBinaryTag.builder().putInt("Y", 0).put("Palette", airPalette()).build(); CompoundBinaryTag chunk = CompoundBinaryTag.builder() - .putIntArray("Biomes", biomes) - .put("sections", ListBinaryTag.from(List.of( - CompoundBinaryTag.builder().putInt("Y", -1).build(), - CompoundBinaryTag.builder().putInt("Y", 0).build()))) + .put("sections", ListBinaryTag.from(List.of(lightingOnly, realContent))) .build(); CompoundBinaryTag rebuilt = new RebuildBiomes().apply(chunk, ANY_CONTEXT); ListBinaryTag sections = rebuilt.getList("sections"); - assertEquals(1, sections.size(), "the Y=-1 lighting-only section must not survive into the output"); + assertEquals(1, sections.size(), "the section without block data must not survive into the output"); assertEquals(0, sections.getCompound(0).getInt("Y")); } @Test - void testAnOutOfRangeSectionDoesNotCountTowardsSettleYRangesMinimum() { - // Exercised directly against SettleYRange, independent of whether RebuildBiomes already - // ran and already cleaned the list - the step re-checks the range itself rather than - // trusting the chain's ordering (see its own Javadoc). + void testASectionWithBlockDataAtNegativeYSurvivesWhenNoBiomesArrayIsPresent() { + // A configurable-height world (possible from DataVersion 2685 onward) can genuinely store + // real content below Y = 0; a section carrying block data there must survive discarding, + // regardless of its Y. No Biomes array is present, so the offset guard below never fires. + CompoundBinaryTag realContentBelowZero = CompoundBinaryTag.builder() + .putInt("Y", -1) + .put("Palette", airPalette()) + .build(); + CompoundBinaryTag chunk = CompoundBinaryTag.builder() + .put("sections", ListBinaryTag.from(List.of(realContentBelowZero))) + .build(); + + CompoundBinaryTag rebuilt = new RebuildBiomes().apply(chunk, ANY_CONTEXT); + + ListBinaryTag sections = rebuilt.getList("sections"); + assertEquals(1, sections.size(), "a section with real block data must survive regardless of its Y"); + assertEquals(-1, sections.getCompound(0).getInt("Y")); + } + + @Test + void testABiomesArrayWithASurvivingSectionBelowZeroThrowsRatherThanMisplacingTheOffset() { + // The section at Y = -1 carries block data, so it survives discarding and reaches the + // offset guard; this step's Y * SECTION_BIOME_ENTRIES offset assumes the lowest section is + // Y = 0, which does not hold here, so the chunk must be refused rather than writing every + // biome layer 64 blocks off from where it belongs. + int[] biomes = new int[1024]; + java.util.Arrays.fill(biomes, 1); + CompoundBinaryTag sectionBelowZero = CompoundBinaryTag.builder() + .putInt("Y", -1) + .put("Palette", airPalette()) + .build(); + CompoundBinaryTag chunk = CompoundBinaryTag.builder() + .putIntArray("Biomes", biomes) + .put("sections", ListBinaryTag.from(List.of(sectionBelowZero))) + .build(); + + MigrationException exception = assertThrows(MigrationException.class, + () -> new RebuildBiomes().apply(chunk, ANY_CONTEXT)); + assertTrue(exception.getMessage().contains("-1")); + } + + @Test + void testSettleYRangeUsesTheActualLowestSectionEvenWhenItIsNegative() { + // The range filter this step used to apply is gone: every remaining section counts, because + // RebuildBiomes (earlier in the chain) is now trusted to have already removed anything + // without block data, whatever its Y. CompoundBinaryTag chunk = CompoundBinaryTag.builder() .put("sections", ListBinaryTag.from(List.of( CompoundBinaryTag.builder().putInt("Y", -1).build()))) @@ -388,15 +427,25 @@ void testAnOutOfRangeSectionDoesNotCountTowardsSettleYRangesMinimum() { CompoundBinaryTag settled = new SettleYRange().apply(chunk, ANY_CONTEXT); - assertEquals(0, settled.getInt("yPos"), - "no in-range section is present, so this falls back to the same default an empty list gets"); + assertEquals(-1, settled.getInt("yPos"), "the chunk's only section is at Y = -1, so yPos must be -1"); + } + + @Test + void testSettleYRangeLeavesAChunkWithNoSectionsUnchangedRatherThanInventingYPosZero() { + CompoundBinaryTag chunk = CompoundBinaryTag.builder() + .put("sections", ListBinaryTag.empty()) + .build(); + + CompoundBinaryTag settled = new SettleYRange().apply(chunk, ANY_CONTEXT); + + assertNull(settled.get("yPos"), "there is no section for yPos to describe, so none must be invented"); } @Test - void testSectionsAtYMinusOneAndYSixteenSurviveTheWholeChainDiscardedRatherThanCorruptingItOrYPos() { - // The end-to-end version of the two direct tests above: a chunk with a lighting-only - // section on both sides of the real range must migrate cleanly, end up with only its one - // real section, and settle yPos on that real section's own Y rather than the discarded one. + void testASectionWithoutBlockDataAtBothBordersIsDiscardedByTheWholeChainRatherThanCorruptingItOrYPos() { + // The end-to-end version of the direct tests above: a chunk with a lighting-only section on + // both sides of its real content must migrate cleanly, end up with only its one real + // section, and settle yPos on that real section's own Y rather than a discarded one. int[] biomes = new int[1024]; java.util.Arrays.fill(biomes, 1); @@ -409,7 +458,8 @@ void testSectionsAtYMinusOneAndYSixteenSurviveTheWholeChainDiscardedRatherThanCo .putIntArray("Biomes", biomes) .put("Sections", ListBinaryTag.from(List.of( CompoundBinaryTag.builder().putByte("Y", (byte) -1).build(), - CompoundBinaryTag.builder().putByte("Y", (byte) 0).build(), + CompoundBinaryTag.builder().putByte("Y", (byte) 0) + .put("Palette", airPalette()).build(), CompoundBinaryTag.builder().putByte("Y", (byte) 16).build()))) .build()) .build(); @@ -417,9 +467,20 @@ void testSectionsAtYMinusOneAndYSixteenSurviveTheWholeChainDiscardedRatherThanCo CompoundBinaryTag migrated = ChunkMigration.migrate(chunk, 4790); ListBinaryTag sections = migrated.getList("sections"); - assertEquals(1, sections.size(), "only Y=0 is real content; Y=-1 and Y=16 are lighting-only"); + assertEquals(1, sections.size(), "only Y=0 carries block data; Y=-1 and Y=16 are lighting-only"); assertEquals(0, sections.getCompound(0).getInt("Y")); - assertEquals(0, migrated.getInt("yPos"), "the lowest REAL section is 0, not the discarded Y=-1"); + assertEquals(0, migrated.getInt("yPos"), "the lowest REAL section is 0, not a discarded lighting-only one"); + } + + /** + * A section built by hand for these tests, carrying just enough block data ({@code Palette} with + * one air entry, no {@code BlockStates} — a single-entry palette needs no packed data, per + * {@link NormaliseBitPacking}'s own javadoc) to survive {@link RebuildBiomes}'s content-based + * discarding without affecting a test's own assertions. + */ + private static ListBinaryTag airPalette() { + return ListBinaryTag.from(List.of( + CompoundBinaryTag.builder().putString("Name", "minecraft:air").build())); } // --- TranslateBlockStates, including the full-chain Gegenprobe target ----------------------- From 7aba9202684c7297f02d4df55b8ba1dbf599c971 Mon Sep 17 00:00:00 2001 From: TheMeinerLP- * {@code redstone_wire} (V2531, 20w17a, Minecraft 1.16, 144 states) is not in this list. - * The research document establishes that a direction's new value depends on the other three - * directions of the same state (DataConverter's {@code connectedX}/{@code connectedZ} in V2531), - * that a per-property implementation is guaranteed wrong, and that 9 of the 81 direction - * combinations change (times 16 {@code power} values = 144 states). It does not say which 9 - * combinations change or what they become — only the count. That is not enough to reproduce - * V2531's logic exactly, and a guessed whole-state table for redstone wiring would be exactly the - * silent corruption this module exists to avoid: the chunk still loads, the wiring looks - * subtly different, and nobody notices. A state this module does not recognize — including every - * {@code redstone_wire} state — passes through {@link #translate(BlockState, int)} unchanged; - * see {@code BlockStateRulesTest.testAStateNoRuleKnowsAboutPassesThroughUnchanged}. + * {@code redstone_wire} (DataVersion 2532, snapshot 20w18a, Minecraft 1.16, 144 of 1296 + * states) took a second pass to land in this list. The research document establishes only the + * shape of the change — a direction's new value depends on the other three directions of the + * same state, a per-property implementation is guaranteed wrong, and 9 of the 81 direction + * combinations change (times 16 {@code power} values = 144 states) — not which nine or what they + * become. That was not enough on its own: a guessed whole-state table for redstone wiring would + * have been exactly the silent corruption this module exists to avoid, the chunk still loads, the + * wiring looks subtly different, and nobody notices. The rule below closes that gap directly from + * the connection semantics: a direction becomes {@code side} exactly when it is itself + * unconnected AND the axis perpendicular to it has no connection either — {@code north}/ + * {@code south} are decided by whether {@code east}/{@code west} connects, {@code east}/ + * {@code west} by whether {@code north}/{@code south} connects. The axes are crossed on purpose, + * not mirrored, and getting that backwards produces a rule that looks plausible and is wrong; see + * {@code BlockStateRulesTest} for the nine affected combinations this guards against. It was + * cross-checked against DataConverter's V2531, Yarn's {@code RedstoneConnectionsFix}, and + * Chunker's {@code JavaLegacyRedstonePreTransformHandler} — three independently maintained + * reimplementations of the same 1.16 change — all of which agree with the derivation above. + * {@code power} is a plain multiplier and is neither read nor written by this rule. + *
+ *+ * A state this module does not otherwise recognize passes through + * {@link #translate(BlockState, int)} unchanged; see + * {@code BlockStateRulesTest.testAStateNoRuleKnowsAboutPassesThroughUnchanged}. *
*/ private static final List- * A state no rule recognizes — including every {@code redstone_wire} state, for which this - * module deliberately carries no rule — passes through unchanged. + * A state no rule recognizes passes through unchanged. *
* * @param state the state as read from the source chunk, or as already transformed by an @@ -212,6 +253,74 @@ private static BlockState rewriteWallSides(BlockState state) { return new BlockState(state.name(), properties); } + private static BlockStateRule redstoneWireRule(int since) { + return new Rule(since, + state -> state.name().equals("minecraft:redstone_wire"), + BlockStateRules::rewriteRedstoneWireConnections); + } + + /** + * Recomputes {@code redstone_wire}'s four direction properties from each other, crossing the + * axes on purpose: whether {@code north}/{@code south} collapse to {@code side} is decided by + * {@code east}/{@code west}'s connection state, and whether {@code east}/{@code west} collapse is + * decided by {@code north}/{@code south}'s. All four outputs are computed from the original, + * unmodified values first and only written afterward, so an earlier write in this same call can + * never leak into a later read — sequential writing gets the all-{@code none} case wrong. + * + * @param state a {@code redstone_wire} state as read from the source chunk + * @return {@code state} with its four direction properties resolved to their later meaning + */ + private static BlockState rewriteRedstoneWireConnections(BlockState state) { + Map