diff --git a/README.md b/README.md index ed3c65d..b5af078 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ nothing to do with speed — and it claims none. | Module | What it is | | --- | --- | -| [`falco-anvil`](https://github.com/OneLiteFeatherNET/Falco/wiki/Anvil-Chunk-Loader) | A `ChunkLoader` for the Anvil region format. Genuinely parallel: reading, decompression and NBT parsing do not share one lock. A read failure throws instead of reporting the chunk as absent, so the server cannot overwrite real data with a freshly generated chunk. | +| [`falco-anvil`](https://github.com/OneLiteFeatherNET/Falco/wiki/Anvil-Chunk-Loader) | A `ChunkLoader` for the Anvil region format. Genuinely parallel: reading, decompression and NBT parsing do not share one lock. A read failure throws instead of reporting the chunk as absent, so the server cannot overwrite real data with a freshly generated chunk. It refuses worlds older than snapshot 21w43a instead of reading them as air; a world at or above that floor is read with the current schema regardless of how old its DataVersion actually is, with no DataFixer involved. | | [`falco-light`](https://github.com/OneLiteFeatherNET/Falco/wiki/Light-Engine) | A block and sky light engine. Thread-safe per call and tied to no chunk implementation, so it works with chunk types Minestom's own engine ignores. Call it yourself, or let a chunk keep its own light up to date. | | [`falco-instance`](https://github.com/OneLiteFeatherNET/Falco/wiki/Rationale-Instances-And-Chunks) | An `Instance` and its `Chunk`. **No speed gain is claimed and none is measured** — ticking lives in the server's global `ThreadDispatcher`, not in the instance. What it buys is an unload path of its own, where `InstanceManager.unregisterInstance` leaks every chunk a foreign instance ever loaded. It cannot back a `SharedInstance`; shared worlds are served by `FalcoSharedInstance` on a plain container instead. | diff --git a/docs/superpowers/plans/2026-08-03-anvil-version-guard.md b/docs/superpowers/plans/2026-08-03-anvil-version-guard.md new file mode 100644 index 0000000..20c5aa3 --- /dev/null +++ b/docs/superpowers/plans/2026-08-03-anvil-version-guard.md @@ -0,0 +1,774 @@ +# Anvil version guard — implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `FalcoAnvilLoader` rejects a chunk it cannot read instead of returning it as air. + +**Architecture:** One guard call at the single seam in `loadChunk` where the full root compound +exists and nothing has been interpreted yet. It checks the *layout* first — a root without +`sections` but with a `Level` compound is the pre-1.18 shape — and the stored `DataVersion` second, +against a configurable floor. Both rejections throw `ChunkDataException` with one new `Reason` and +are counted per version value in `AnvilDiagnostics`. + +**Tech Stack:** Java 25, Gradle, Adventure NBT (`net.kyori.adventure.nbt`), JUnit 5, Minestom +(`compileOnly`), MicrotusExtension for the environment-backed tests. + +**Spec:** `docs/superpowers/specs/2026-08-03-anvil-version-guard-design.md` + +## Global Constraints + +- Base branch `feat/anvil-version-guard`, worktree `/mnt/projects/oss/onelitefeather/Falco-worktrees/anvil-version-guard`, off `origin/main` (`2d3955d8`, the 1.0.0 baseline). +- **The sealed hierarchy is not touched.** `AnvilFormatException permits ChunkDataException, RegionFormatException` stays exactly as it is. +- **`isFullyGenerated(null) == true` stays.** It is not the defect. +- **The save path is not touched.** +- Every new public type and member carries `@ApiStatus.Experimental`, Javadoc with `@param`/`@return`, and `@since 1.1.0`. Every modified type's `@version` is raised by one minor. +- Javadoc runs under `-Werror`; a missing tag fails the build. +- `checkApiCompatibility` runs on this module. **Every signature change must be additive** — no member is removed, renamed, or has its parameters changed. This binds the API surface, not the behaviour: Task 3 deliberately changes what the loader *does* with a pre-1.18 world, which is why its commit carries the `!` marker, and that is not a contradiction of this line. Binary compatibility and behavioural compatibility are separate promises here, and only the first one is enforced by the build. +- Builders in this project are immutable: every setter returns a **new** `Builder` with all fields passed through. Adding a field means touching the constructor, `build()`, and **every** existing setter. +- Test method names in this module read as sentences: `testLoadingAnAbsentChunkReturnsNull`. +- Commit messages are Conventional Commits, lower case, and say what changed and why. +- No timing figure may be produced or quoted anywhere in this work. + +## File Structure + +| File | Responsibility | Change | +| --- | --- | --- | +| `falco-anvil/src/main/java/…/ChunkDataException.java` | The fault type and its reasons | Modify: one new `Reason` constant, last in the declaration | +| `falco-anvil/src/main/java/…/AnvilDiagnostics.java` | Counters and per-value breakdowns | Modify: one constant, one field pair, `reportUnsupportedChunkVersion`, `unsupportedChunkVersions`, `chunksSkippedAsUnsupported` | +| `falco-anvil/src/main/java/…/FalcoAnvilLoader.java` | The loader, its builder, the guard | Modify: two key constants, one builder field with its setter and pass-throughs, the guard method, one call at the seam | +| `falco-anvil/src/test/java/…/AnvilDiagnosticsTest.java` | Diagnostics unit tests | Modify: three cases | +| `falco-anvil/src/test/java/…/FalcoAnvilLoaderBuilderTest.java` | Builder unit tests | Modify: two cases | +| `falco-anvil/src/test/java/…/FalcoAnvilLoaderIntegrationTest.java` | Loader against a running environment | Modify: three cases, reusing the existing `writeRawChunk` helper at `:573` | + +No new file. The guard is twenty lines in the class that owns the seam; a class of its own would +split one decision across two files. + +--- + +### Task 1: The reason and the diagnostics pair + +**Files:** +- Modify: `falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java:37-68` +- Modify: `falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java` +- Test: `falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java` + +**Interfaces:** +- Consumes: nothing. +- Produces: `ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION`; + `AnvilDiagnostics.UNKNOWN_DATA_VERSION` (`String`, value `""`); + `boolean AnvilDiagnostics.reportUnsupportedChunkVersion(String version)`; + `@Unmodifiable Map AnvilDiagnostics.unsupportedChunkVersions()`; + `long AnvilDiagnostics.chunksSkippedAsUnsupported()`. + +- [ ] **Step 1: Write the failing tests** + +In `AnvilDiagnosticsTest.java`: + +```java +@Test +void testAnUnsupportedVersionIsCountedUnderItsOwnValue() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + assertTrue(diagnostics.reportUnsupportedChunkVersion("1976")); + assertFalse(diagnostics.reportUnsupportedChunkVersion("1976")); + assertTrue(diagnostics.reportUnsupportedChunkVersion("2724")); + + assertEquals(3, diagnostics.chunksSkippedAsUnsupported()); + assertEquals(Map.of("1976", 2L, "2724", 1L), diagnostics.unsupportedChunkVersions()); +} + +@Test +void testAChunkWithoutAStoredVersionIsCountedApart() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + diagnostics.reportUnsupportedChunkVersion(AnvilDiagnostics.UNKNOWN_DATA_VERSION); + + assertEquals(Map.of(AnvilDiagnostics.UNKNOWN_DATA_VERSION, 1L), + diagnostics.unsupportedChunkVersions()); +} + +@Test +void testTheVersionBreakdownIsSortedByValue() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + diagnostics.reportUnsupportedChunkVersion("2724"); + diagnostics.reportUnsupportedChunkVersion("1976"); + + assertEquals(List.of("1976", "2724"), + List.copyOf(diagnostics.unsupportedChunkVersions().keySet())); +} +``` + +The third case pins the ordering promise `partialChunkStatuses()` already makes in its Javadoc: a +summary that lists values in a different order on every shutdown cannot be compared between runs. + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*AnvilDiagnosticsTest*"` +Expected: compilation failure — `reportUnsupportedChunkVersion` does not exist. + +- [ ] **Step 3: Add the reason** + +At the **end** of `ChunkDataException.Reason` (after `MISSING_OR_MISTYPED_KEY`, `:67`), preserving +the existing Javadoc style of the constants above it: + +```java + /** + * The chunk comes from a Minecraft version this loader cannot read. Either it carries the + * pre-1.18 layout, which keeps everything under {@code Level}, or its stored + * {@code DataVersion} is below the configured floor. + */ + UNSUPPORTED_CHUNK_VERSION +``` + +Add the comma after `MISSING_OR_MISTYPED_KEY`. Going last matters: a foreign exhaustive `switch` over +`Reason` keeps compiling against every constant that was already there. + +- [ ] **Step 4: Add the diagnostics pair** + +Follow `reportPartialChunk(String)` (`:160`) exactly — the cap, the race comment, the throttle +semantics. Next to `UNKNOWN_STATUS` (`:59`): + +```java + /** + * The value an unsupported chunk is counted under when it stored no {@code DataVersion} at all. + */ + public static final String UNKNOWN_DATA_VERSION = ""; +``` + +Two fields next to `partialChunkStatuses` and `partialChunks`, initialised in the constructor the +same way: + +```java + private final Map unsupportedChunkVersions; + private final LongAdder unsupportedChunks; +``` + +The reporter, mirroring `reportPartialChunk` including the cap behaviour: + +```java + /** + * Reports a chunk which comes from a version this loader cannot read. + *

+ * The throttling is per version value rather than per loader, so a world holding several + * versions names each of them exactly once. A version beyond the cap is still counted in + * {@link #chunksSkippedAsUnsupported()} and only loses its own entry in + * {@link #unsupportedChunkVersions()}. + *

+ * + * @param version the stored data version, or {@link #UNKNOWN_DATA_VERSION} if none was stored + * @return true if the caller should log the problem, otherwise false + */ + public boolean reportUnsupportedChunkVersion(String version) { + this.unsupportedChunks.increment(); + LongAdder counter = this.unsupportedChunkVersions.get(version); + + if (counter == null) { + if (this.unsupportedChunkVersions.size() >= MAX_TRACKED_NAMES) { + return false; + } + LongAdder created = new LongAdder(); + LongAdder previous = this.unsupportedChunkVersions.putIfAbsent(version, created); + + if (previous == null) { + created.increment(); + return true; + } + previous.increment(); + return false; + } + counter.increment(); + return false; + } +``` + +The two getters, mirroring `partialChunkStatuses()` (`:316`) including the `LinkedHashMap` and the +reason for it: + +```java + /** + * Returns how many chunks were refused because their version could not be read. + * + * @return the amount of refused chunks + */ + @Contract(pure = true) + public long chunksSkippedAsUnsupported() { + return this.unsupportedChunks.sum(); + } + + /** + * Returns the amount of refused chunks per stored data version, sorted by the version value. + * + * @return the amount of refused chunks per version + */ + @Contract(pure = true) + public @Unmodifiable Map unsupportedChunkVersions() { + Map snapshot = new LinkedHashMap<>(); + + this.unsupportedChunkVersions.entrySet().stream() + .sorted(Map.Entry.comparingByKey()) + .forEach(entry -> snapshot.put(entry.getKey(), entry.getValue().sum())); + + return Collections.unmodifiableMap(snapshot); + } +``` + +Raise `@version` on both classes by one minor. + +- [ ] **Step 5: Run the tests and watch them pass** + +Run: `./gradlew :falco-anvil:test --tests "*AnvilDiagnosticsTest*"` +Expected: PASS, and the existing cases in that class still pass. + +- [ ] **Step 6: Gegenprobe** + +Drop the `size() >= MAX_TRACKED_NAMES` check. `testAnUnsupportedVersionIsCountedUnderItsOwnValue` +must stay green — it does not reach the cap — so add nothing on that basis; instead delete the +`sorted(...)` line and confirm `testTheVersionBreakdownIsSortedByValue` goes red while the other two +stay green. Revert. If it does not go red, the ordering is accidental and the test is worthless. + +- [ ] **Step 7: Commit** + +```bash +git add falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java \ + falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java \ + falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java +git commit -m "feat(anvil): count the chunks whose version the loader cannot read" +``` + +--- + +### Task 2: The builder slot + +**Files:** +- Modify: `falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java` — builder field `:290-315`, every setter through `:460`, `build()`, the loader field and constructor `:119`/`:207` +- Test: `falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java` + +**Interfaces:** +- Consumes: nothing from Task 1. +- Produces: `FalcoAnvilLoader.Builder minimumDataVersion(int minimumDataVersion)`; + the loader field `private final int minimumDataVersion;`; + the constant `DEFAULT_MINIMUM_DATA_VERSION = 2860`. + +- [ ] **Step 1: Write the failing tests** + +In `FalcoAnvilLoaderBuilderTest.java`, matching the style of the cases already there: + +```java +@Test +void testTheMinimumDataVersionDefaultsToTheFirstRootLayout() { + assertEquals(2860, FalcoAnvilLoader.DEFAULT_MINIMUM_DATA_VERSION); +} + +@Test +void testANegativeMinimumDataVersionIsRefused() { + assertThrows(IllegalArgumentException.class, + () -> FalcoAnvilLoader.builder().minimumDataVersion(-1)); +} +``` + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*FalcoAnvilLoaderBuilderTest*"` +Expected: compilation failure — neither member exists. + +- [ ] **Step 3: Add the constant, the field and the setter** + +Next to the other constants at the top of `FalcoAnvilLoader`: + +```java + /** + * The lowest data version the loader reads by default: 1.18, the first version whose chunks + * carry {@code sections} on the root compound instead of under {@code Level}. + */ + public static final int DEFAULT_MINIMUM_DATA_VERSION = 2860; +``` + +Add `private final int minimumDataVersion;` to **both** `FalcoAnvilLoader` (next to `dataVersion`, +`:119`) and `Builder` (`:297`), assign it in both constructors, and thread it through **every** +existing `Builder` setter and `build()`. There are eight setters; each constructs a new `Builder` +with the full field list, and every one of them needs the new argument. Missing one silently drops +the caller's value. + +The setter itself, placed next to `dataVersion(int)` (`:411`): + +```java + /** + * Sets the lowest data version the loader accepts when reading a chunk. + *

+ * This is the read side and has nothing to do with {@link #dataVersion(int)}, which is the + * version written into every saved chunk. A chunk below this floor is refused rather than + * read, because the layout it carries would otherwise decode to air. + *

+ * + * @param minimumDataVersion the lowest data version the loader accepts + * @return a new builder with this value + * @throws IllegalArgumentException if the version is negative + */ + @Contract(value = "_ -> new", pure = true) + public Builder minimumDataVersion(int minimumDataVersion) { + if (minimumDataVersion < 0) { + throw new IllegalArgumentException( + "The minimum data version must not be negative but was " + minimumDataVersion); + } + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler); + } +``` + +Raise `@version` on `FalcoAnvilLoader`. + +- [ ] **Step 4: Run the tests and watch them pass** + +Run: `./gradlew :falco-anvil:test --tests "*FalcoAnvilLoaderBuilderTest*"` +Expected: PASS, and every existing builder case still passes. + +- [ ] **Step 5: Gegenprobe on the pass-through** + +`Builder` exposes no readers, so the value is asserted through the built loader. Add a +**package-private** reader on `FalcoAnvilLoader`, next to the field — package-private keeps it out of +the published API and therefore out of `checkApiCompatibility`: + +```java + @Contract(pure = true) + int minimumDataVersion() { + return this.minimumDataVersion; + } +``` + +`builder()` is at `:256` and `build(Path worldRoot, Key dimension)` at `:541`, both verified against +this baseline. Add the case: + +```java +@Test +void testTheMinimumDataVersionSurvivesEveryOtherSetter(@TempDir Path worldRoot) { + FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .minimumDataVersion(1519) + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, Key.key("minecraft:overworld")); + + assertEquals(1519, loader.minimumDataVersion()); +} +``` + +Then inject the defect: in `openRegionLimit(int)` at `:337`, replace `this.minimumDataVersion` with +`DEFAULT_MINIMUM_DATA_VERSION`. The case must go **red**, because the value set first is dropped by a +setter called after it — which is exactly the mistake the immutable-builder pattern invites when a +field is added. Revert the defect; the test stays. + +- [ ] **Step 6: Commit** + +```bash +git add falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java \ + falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java +git commit -m "feat(anvil): let a caller say which data version is the floor" +``` + +--- + +### Task 3: The guard at the seam + +**Files:** +- Modify: `falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java:654-655` and the constants block at `:92-98` +- Test: `falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java` + +**Interfaces:** +- Consumes: `Reason.UNSUPPORTED_CHUNK_VERSION` and `reportUnsupportedChunkVersion(String)` from Task 1; `minimumDataVersion` from Task 2. +- Produces: nothing further tasks build on. + +- [ ] **Step 1: Write the failing tests** + +In `FalcoAnvilLoaderIntegrationTest.java`, using the existing `writeRawChunk` helper at `:573`: + +```java +@Test +void testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir(Env env) throws Exception { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("Sections", ListBinaryTag.empty()) + .build()) + .build(); + writeRawChunk(3, 3, legacy); + + try (FalcoAnvilLoader loader = loader()) { + Instance instance = env.createEmptyInstance(loader); + + ChunkDataException failure = assertThrows(ChunkDataException.class, + () -> loader.loadChunk(instance, 3, 3)); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, failure.reason()); + } +} + +@Test +void testAChunkBelowTheFloorIsRefused(Env env) throws Exception { + CompoundBinaryTag old = CompoundBinaryTag.builder() + .putInt("DataVersion", 2724) + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(4, 4, old); + + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .diagnostics(diagnostics) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + assertThrows(ChunkDataException.class, () -> loader.loadChunk(instance, 4, 4)); + assertEquals(Map.of("2724", 1L), diagnostics.unsupportedChunkVersions()); + } +} + +@Test +void testAChunkWithoutAStoredVersionStillLoads(Env env) throws Exception { + CompoundBinaryTag toolWritten = CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(5, 5, toolWritten); + + try (FalcoAnvilLoader loader = loader()) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 5, 5)); + } +} +``` + +The third is the regression guard. A world written by a tool that stores no `DataVersion` must keep +loading — that is what `isFullyGenerated(null) == true` exists for, and the guard must not take it +away. + +Check the exact `build(...)` signature against `FalcoAnvilLoaderBuilderTest` before writing the +second case; adjust the call if it differs. + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*FalcoAnvilLoaderIntegrationTest*"` +Expected: cases 1 and 2 FAIL — case 1 because `loadChunk` returns a chunk rather than throwing, +which **is the defect this whole plan exists for**; case 2 likewise. Case 3 already passes. + +- [ ] **Step 3: Add the two key constants** + +Next to `SECTIONS_KEY` (`:92`): + +```java + private static final String LEGACY_LEVEL_KEY = "Level"; + private static final String DATA_VERSION_KEY = "DataVersion"; +``` + +- [ ] **Step 4: Write the guard** + +Place it next to `chunkStatus` (`:1353`): + +```java + /** + * Refuses a chunk which comes from a version this loader cannot read. + *

+ * The layout is checked before the version, because a version number is a claim about the data + * while the layout is the data: a chunk may carry no version at all, and one that carries a + * version may not hold what that version promises. A root compound without {@code sections} but + * with a {@code Level} compound is the pre-1.18 shape, which would otherwise decode to an empty + * section list and reach the caller as a chunk of air. + *

+ * + * @param data the root compound of the chunk + * @throws ChunkDataException if the chunk cannot be read + */ + private void requireReadableVersion(CompoundBinaryTag data) throws ChunkDataException { + int version = NbtReads.optionalInteger(data, DATA_VERSION_KEY, -1); + String reported = version < 0 ? AnvilDiagnostics.UNKNOWN_DATA_VERSION : Integer.toString(version); + boolean legacyLayout = data.get(SECTIONS_KEY) == null + && NbtReads.optionalCompound(data, LEGACY_LEVEL_KEY) != null; + + if (!legacyLayout && (version < 0 || version >= this.minimumDataVersion)) { + return; + } + + if (this.diagnostics.reportUnsupportedChunkVersion(reported)) { + LOGGER.warn( + "Refusing a chunk from data version {} in {}: {}", + reported, this.regionDirectory, + legacyLayout + ? "the chunk data sits under Level, which this loader does not read" + : "the loader accepts " + this.minimumDataVersion + " and above" + ); + } + + throw new ChunkDataException( + ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, + legacyLayout + ? "The chunk stores its data under Level, which means a version before 1.18" + : "The chunk stores data version " + version + + " but the loader accepts " + this.minimumDataVersion + " and above" + ); + } +``` + +Note the two conditions are deliberately not symmetric: a missing version (`-1`) alone is **not** a +rejection, a legacy layout alone **is**. + +- [ ] **Step 5: Call it at the seam** + +Between `:654` and `:655`, leaving both lines untouched: + +```java + CompoundBinaryTag data = TAG_READER.read(new ByteArrayInputStream(raw.decompress()), BinaryTagIO.Compression.NONE); + requireReadableVersion(data); + String status = chunkStatus(data); +``` + +The `ChunkDataException` propagates into the existing `catch` at `:692` and through `failedLoad`, +which already calls `countError()` (`:712`). The refused chunk is therefore counted twice on purpose: +once as an error, once in the version breakdown. Raise `@version` on the class. + +- [ ] **Step 6: Run the tests and watch them pass** + +Run: `./gradlew :falco-anvil:test --tests "*FalcoAnvilLoaderIntegrationTest*"` +Expected: PASS, all three, and every existing case in the class unchanged. + +- [ ] **Step 7: Gegenprobe** + +Two defects, injected one at a time, each reverted afterwards: + +1. Drop `&& NbtReads.optionalCompound(data, LEGACY_LEVEL_KEY) != null` from `legacyLayout`. + `testAChunkWithoutAStoredVersionStillLoads` must go **red** — the guard now rejects any chunk + without a root `sections`, including tool-written ones. This proves the second half of the + condition earns its place. +2. Change `version >= this.minimumDataVersion` to `version >= 0`. `testAChunkBelowTheFloorIsRefused` + must go red while the other two stay green. + +Verify `git status` is clean after each revert. + +- [ ] **Step 8: Commit** + +```bash +git add falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java \ + falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java +git commit -m "fix(anvil)!: refuse a pre-1.18 world instead of reading it as air" +``` + +The `!` is deliberate: a caller feeding the loader such a world now gets an exception where it got an +air chunk. That is the point of the change and belongs in the changelog as a break. + +--- + +### Task 4: Acceptance + +**Files:** none modified unless a defect is found. + +- [ ] **Step 1: Run every module** + +```bash +./gradlew :falco-anvil:test :falco-light:test :falco-instance:test \ + :falco-demo:test :falco-benchmarks:test :falco-archunit:test --rerun-tasks +``` + +Take the counts **from the JUnit XML** under each module's `build/test-results/test/`, not from the +console summary. No count may fall. The baseline at `2d3955d8` is: anvil 217, light 205, instance +186, demo 166, benchmarks 42 (one skipped — `EmptySectionCensusTest` wants an Anvil world on disk), +archunit 46. Anvil gains eight cases from tasks 1 to 3. + +- [ ] **Step 2: Check the load first, and record it** + +Run `uptime` before and after. Both figures go into the report. **No timing figure is produced here +and none may be quoted** — this task measures counts, not speed. + +- [ ] **Step 3: Build with javadoc and the API check** + +```bash +./gradlew build -x test --rerun-tasks +``` + +Expected: green, javadoc genuinely executed for all four published modules with zero warnings, and +`checkApiCompatibility` executed. If japicmp flags the new enum constant, do not weaken the check — +record what it says and stop; that is a decision for the project owner. + +- [ ] **Step 4: Attack the gate** + +Re-inject the defect from Task 3 Gegenprobe #1 and confirm it is caught by the full suite and not +only by the single test class. Revert, verify the tree is clean, confirm the suites are green again. + +- [ ] **Step 5: Write the report and commit it** + +Append a `## Result` section to this plan: which cases were added, which defect each one caught, +the counts per module from the XML, both load figures, and — explicitly — what this work does **not** +do: it converts nothing, it does not touch the save path, and it does not make a pre-1.18 world +usable. + +```bash +git add docs/superpowers/plans/2026-08-03-anvil-version-guard.md +git commit -m "docs(plan): record what the acceptance measured, and what it did not" +``` + +--- + +### Task 5: Documentation + +**Files:** +- Modify: `README.md` +- Modify: the `Anvil-Chunk-Loader` page in the wiki repository at `/mnt/projects/oss/onelitefeather/Falco.wiki` + +- [ ] **Step 1: State the floor in the README** + +The README table row for `falco-anvil` says a read failure throws "so the server cannot overwrite +real data with a freshly generated chunk". Add one sentence naming the floor: the loader reads worlds +from 1.18 onwards and refuses older ones instead of reading them as air. Do not add a section. + +- [ ] **Step 2: Extend the existing wiki page** + +The long-form documentation lives in the wiki repository, not here. Extend the existing +`Anvil-Chunk-Loader` page with the floor, the builder slot, the new reason and the diagnostics +getter. **No new page**, so the sidebar needs no entry. + +- [ ] **Step 3: Commit both, separately** + +The wiki is its own repository with its own history. + +```bash +git add README.md +git commit -m "docs(anvil): say which worlds the loader reads" +``` + +--- + +## Self-Review + +**Spec coverage.** Every section of the spec maps to a task: the seam and both checks to Task 3; +the reason to Task 1; the builder slot and the 2860 default to Task 2; the counter to Task 1; the +API-compatibility note to Task 4 Step 3; all four test cases of the spec's table to Tasks 1 and 3 +(case 4 of the spec is folded into Task 3's second case, which asserts the breakdown directly); +"what does not change" to the Global Constraints. The spec's out-of-scope section needs no task. + +**Placeholders.** None. Every code step carries the code. Task 2 Step 5 first read "if `Builder` +exposes no reader, do X instead" and named a method that exists nowhere; `Builder` was checked and +exposes none, so the step now states the one route and adds the package-private reader it needs. +`builder()` (`:256`) and `build(Path, Key)` (`:541`) were verified against this baseline rather than +assumed. + +**Type consistency.** `reportUnsupportedChunkVersion(String)` and `unsupportedChunkVersions()` are +named identically in Task 1's Interfaces block, its code, and Task 3's test. `UNKNOWN_DATA_VERSION` +is `""` throughout and distinct from the existing `UNKNOWN_STATUS` (`""`). +`UNSUPPORTED_CHUNK_VERSION` — not `UNSUPPORTED_DATA_VERSION` — in all five places it appears. +`DEFAULT_MINIMUM_DATA_VERSION` is 2860 in Task 2's test, its constant, and the spec. + +**One risk this plan cannot remove.** 2860 comes from the research, not from a source read +first-hand. If it is wrong, Task 2's first test locks in the wrong number — but the layout check of +Task 3 rejects pre-1.18 worlds regardless, so the guard still works and only the message misleads. +Whoever implements Task 2 should confirm 2860 against minecraft.wiki's chunk-format history and +correct the constant, its Javadoc and the test together if it differs. + +--- + +## Result + +Acceptance run against `a74cd042` (branch tip), worktree +`/mnt/projects/oss/onelitefeather/Falco-worktrees/anvil-version-guard`, base `2d3955d8`. + +### Cases added + +Task 2's implementer corrected `DEFAULT_MINIMUM_DATA_VERSION` to **2844**, not the plan's researched +2860 — the "one risk this plan cannot remove" above was exercised and resolved during implementation. + +13 test cases were added to `falco-anvil` (not the 8 estimated in the task-4 brief; Task 3's own +review follow-up added 4 more edge-case witnesses beyond its original commit, and Task 1 added 3): + +- `AnvilDiagnosticsTest`: `testAnUnsupportedVersionIsCountedUnderItsOwnValue`, + `testAChunkWithoutAStoredVersionIsCountedApart`, `testTheVersionBreakdownIsSortedByValue` +- `FalcoAnvilLoaderBuilderTest`: `testTheMinimumDataVersionDefaultsToTheFirstRootLayout`, + `testANegativeMinimumDataVersionIsRefused`, `testTheMinimumDataVersionSurvivesEveryOtherSetter` +- `FalcoAnvilLoaderIntegrationTest`: `testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir`, + `testAChunkBelowTheFloorIsRefused`, `testAChunkWithoutAStoredVersionStillLoads`, + `testAChunkWithNeitherSectionsNorLevelIsNotRefused`, `testAChunkWithBothSectionsAndLevelIsNotRefused`, + `testAChunkWithADataVersionStoredAsTheWrongTypeIsRefused`, `testAChunkWithANegativeDataVersionIsRefused` + +### Gate attack + +Re-injected Task 3 Gegenprobe #1 — dropped the `Level` half of `legacyChunkLayout`: + +```java +boolean legacyChunkLayout = data.get(SECTIONS_KEY) == null; +``` + +Ran the **full** `:falco-anvil:test` module (230 cases, not a single filtered class). Result: `230 +tests completed, 2 failed` — + +- `FalcoAnvilLoaderIntegrationTest > testAChunkWithNeitherSectionsNorLevelIsNotRefused` +- `FalcoAnvilLoaderIntegrationTest > testAPartiallyGeneratedChunkIsCountedUnderItsStatus` + +The second is a **pre-existing** test that predates this whole plan — it never anticipated the guard, +yet the mutated condition broke it too, which is stronger evidence than the dedicated regression test +alone. The task-4-brief's predicted witness, `testAChunkWithoutAStoredVersionStillLoads`, stayed green +under this mutation, exactly as Task 3's own report already found: that fixture carries a present +(if empty) `sections` list, so the dropped condition half never gets a chance to fire for it. + +Reverted the mutation; `git status --short` was empty afterward; `:falco-anvil:test --rerun-tasks` was +green again (230/230). + +### Module counts (from JUnit XML under `build/test-results/test/`, counted via `` elements +— not the console summary) + +| Module | Count at `2d3955d8` (measured) | Count now | Delta | +| --- | --- | --- | --- | +| falco-anvil | 217 | 230 | +13 | +| falco-light | 223 | 223 | 0 | +| falco-instance | 259 | 259 | 0 | +| falco-demo | 167 | 167 | 0 | +| falco-benchmarks | 42 (1 skipped) | 42 (1 skipped) | 0 | +| falco-archunit | 47 | 47 | 0 | + +No count fell. **Discrepancy against the task-4 brief's stated baseline**: the brief quoted light 205, +instance 186, demo 166, archunit 46. `git diff 2d3955d8..HEAD --stat` shows this branch touches only +`falco-anvil` files — so light/instance/demo/archunit at `2d3955d8` are, by construction, identical to +what this run measured (223/259/167/47). The brief's numbers predate the `feat(instance): a shared +instance that repairs what it inherits (#40)` merge that landed on `main` before `2d3955d8`, which is +almost certainly where the instance-module gap (186 → 259) comes from. This is a stale reference in +the brief, not a regression — flagged here rather than silently reconciled. + +### Machine load + +`uptime` before the module run (09:33:57): `load average: 0.72, 2.99, 4.34` +`uptime` after the module run (09:36:08): `load average: 4.63, 4.42, 4.73` + +No timing figure was produced or is quoted anywhere in this section. + +### `./gradlew build -x test --rerun-tasks` + +`BUILD SUCCESSFUL`. `javadoc` genuinely executed (no `UP-TO-DATE`/`FROM-CACHE`, zero warnings in the +full output) for all four modules that carry a javadoc task: falco-anvil, falco-light, falco-instance, +falco-demo. `checkApiCompatibility` genuinely executed for the three modules configured for it — +falco-anvil, falco-light, falco-instance (`build.gradle.kts:122`, `publishedModules - falco-bom`; +falco-demo is not in `publishedModules` and carries no japicmp task; falco-bom is a platform artifact +with no jar to compare). japicmp raised no complaint about the new `UNSUPPORTED_CHUNK_VERSION` enum +constant — adding an enum constant is additive under `onlyBinaryIncompatibleModified`, so no exception +handling in `gradle/api-breaks.properties` was needed and none was added. + +### What this work does not do + +- It converts nothing. A pre-1.18 world is refused, not rewritten into the post-1.18 layout. +- It does not touch the save path. Only the read seam in `loadChunk` gained the guard. +- It does not make a pre-1.18 world usable. The loader now fails loudly instead of silently returning + air; the world itself remains unreadable by this loader. + +### Known, checked, still true + +- `decodeSections` still reads `sections` via `NbtReads.optionalList`. A chunk stamped + `minecraft:full`, with neither `sections` nor `Level`, passes the guard (no legacy layout — `Level` + is absent) and reaches the caller as an air chunk. Confirmed still open and intentionally outside + this plan's scope. +- A `DataVersion` tag present but of the wrong NBT type is read as `-1` by + `NbtReads.optionalInteger` (falls through its `instanceof NumberBinaryTag` check), and reported as + `"-1"` — indistinguishable in the diagnostics breakdown from a genuine `DataVersion` of `-1`. Both + are correctly rejected by the guard; only the breakdown conflates them. Confirmed still true. +- `FalcoAnvilLoader.Builder`'s `@version` javadoc tag is still `1.0.0` (`FalcoAnvilLoader.java:300`), + unraised despite the outer class moving to `1.2.0`. Confirmed still true. diff --git a/docs/superpowers/specs/2026-08-03-anvil-version-guard-design.md b/docs/superpowers/specs/2026-08-03-anvil-version-guard-design.md new file mode 100644 index 0000000..e01f7e6 --- /dev/null +++ b/docs/superpowers/specs/2026-08-03-anvil-version-guard-design.md @@ -0,0 +1,174 @@ +# A version guard for the Anvil chunk loader + +Design of 2026-08-03. `FalcoAnvilLoader` reads a world it does not understand as air and reports +success. This adds the check that says so, and nothing else. + +All line numbers are against `2d3955d8`, the 1.0.0 baseline. + +## Why + +The loader expects the chunk layout Minecraft writes since 1.18: `sections` on the root compound. A +world written by 1.17 or earlier keeps everything one level down, under `Level`. Three independent +decisions, each defensible on its own, combine into silent data loss: + +1. **The status is looked for on the root.** `chunkStatus` reads `Status` and falls back to `status` + (`FalcoAnvilLoader.java:1354-1357`), both against `data` itself. A 1.17 chunk carries + `Level.Status`, so the answer is `null`. +2. **A chunk without a status counts as generated.** `isFullyGenerated` returns true for `null` + (`:1371-1372`). Its Javadoc gives the reason, and the reason is good: a world written by a tool + that stores no status would otherwise be unreadable in its entirety. +3. **A missing section list is not an error.** `decodeSections` calls + `NbtReads.optionalList(data, SECTIONS_KEY, …)` (`:1386`), and `NbtReads.optionalList` returns + `ListBinaryTag.empty()` when the key is absent or mistyped (`NbtReads.java:162-167`). The 1.17 key + is `Level.Sections`, so the list is empty and nothing throws. + +The chunk is then counted as loaded and consists entirely of air. Whether a later save writes that +air back over the stored region file has **not been measured** and is not claimed here; the read +side alone is the defect. + +`DataVersion` would answer the question outright, and the loader writes it — `putInt("DataVersion", +this.dataVersion)` at `:1586` is the **only** occurrence of the name in the whole module. It is never +read. There is no `Reason` for an unreadable version and no counter in `AnvilDiagnostics`. + +This contradicts the property the module is sold on. The README says a read failure "throws instead +of reporting the chunk as absent, so the server cannot overwrite real data with a freshly generated +chunk". For a pre-1.18 world it does the opposite, and more quietly. + +## Decisions + +| Question | Decision | +| --- | --- | +| What is checked | The layout first, the version second | +| Where | Between `:654` and `:655`, the one point where the full root compound exists and nothing is interpreted yet | +| How it fails | `ChunkDataException` with a new `Reason`, thrown | +| New exception type | None — the sealed hierarchy is not touched | +| Upper version bound | None | +| `isFullyGenerated(null) == true` | Unchanged | +| Save path | Unchanged | + +**Why the layout is checked before the version.** A version number is a claim about the data; the +layout is the data. A world written by a third-party tool may carry no `DataVersion` at all, and a +world may carry one that does not match what it contains. The check that decides whether the loader +can proceed therefore asks what is actually in the compound. The version is read to *explain* the +failure to whoever has to fix it, not to detect it. This also means the guard does not rest on a +version table being correct. + +**Why no upper bound.** The damage that is proven runs downwards: older layouts read as air. A future +format break would be a different failure and cannot be anticipated by a constant, while a hard +ceiling would turn every Minecraft release into a Falco release. The lower bound is enough. + +**Why a `Reason` and not a new exception type.** `AnvilFormatException` is +`sealed … permits ChunkDataException, RegionFormatException` (`AnvilFormatException.java:38-39`). +Extending that list is a change to a published hierarchy on a 1.0.0 artefact that runs +`checkApiCompatibility`. A new enum constant is additive and carries the same information. + +## The change + +### 1. Read the version at the seam + +```java +CompoundBinaryTag data = TAG_READER.read(…); // :654, unchanged +// new: guard here +String status = chunkStatus(data); // :655, unchanged +``` + +Everything downstream — `chunkStatus`, `decodeSections` (`:673`), `applyBlockEntities` (`:680`) — is +covered by a single call at this point, because it is the only place that holds the root compound +before anything is interpreted. + +### 2. The layout check + +The compound is rejected when it carries **no `sections` list on the root while holding a `Level` +compound**. That is the pre-1.18 shape, and it is the shape that produces the air chunk today. Both +halves are required: a root without `sections` and without `Level` is a genuinely empty or corrupt +chunk, which the existing paths already handle. + +### 3. The version bound + +`DataVersion` is read as an optional int — absent is not an error, since tools write worlds without +it. When present and below the configured minimum, the chunk is rejected with the same `Reason`. + +The default minimum is **2860**, the first version whose chunks carry `sections` on the root +(1.18; per the Minecraft chunk-format history). It is configurable through a new builder slot, which +sits next to the existing `dataVersion(int)` at `:411` — that one is the version *written* on save, +this one is the lowest version *accepted* on load. The two must not be conflated in the Javadoc. + +If 2860 is off by a release, the layout check of step 2 still rejects the world correctly. The +constant changes only what the message says. + +### 4. The reason + +A new constant `UNSUPPORTED_CHUNK_VERSION` at the end of `ChunkDataException.Reason` +(`ChunkDataException.java:37-68`, six constants today). + +**Not `UNSUPPORTED_DATA_VERSION`.** The layout check of step 2 fires on chunks that may carry no +`DataVersion` at all, and naming the reason after a field that is absent in the case that triggers it +most often would send the reader looking for the wrong thing. One constant covers both checks, +because both say the same thing — this chunk comes from a version the loader cannot read — and both +have the same remedy. Which check fired is carried by the message, not by a second constant. + +The message names the version found (or that none was stored), the minimum accepted, and which of +the two checks fired. That is the whole content of the report; without it the reader is left where +the missing status left them before. + +### 5. The counter + +`AnvilDiagnostics` gains `reportUnsupportedChunkVersion(String version)` returning `boolean` on first +sight of a value, plus an `@Unmodifiable Map unsupportedChunkVersions()` getter. This +follows `reportPartialChunk(String)` / `partialChunkStatuses()` (`:160`, `:316`) — the existing pair +that keeps a per-value breakdown rather than a bare count, and the one whose shape fits here, because +"which versions did this world contain" is the question an operator actually asks. Chunks carrying no +`DataVersion` are counted under a constant, in the way `UNKNOWN_STATUS` (`:59`) already serves that +role for the status breakdown. + +## What does not change + +- **`isFullyGenerated(null)` stays true.** It is not the defect. It only became one in combination + with an unchecked layout, and step 2 removes that combination. +- **The sealed hierarchy**, as decided above. +- **The save path.** `snapshot(chunk)` (`:1559`) builds from the runtime chunk, not from foreign NBT; + there is nothing there to migrate, only a target version to stamp. Note for later work: the save + path swallows every `AnvilFormatException` (`:766-773`, log and report, no rethrow). + +## API compatibility + +The module runs `checkApiCompatibility` since 1.0.0. Three additions, all outward: + +- a new enum constant — binary compatible; source-incompatible only for an exhaustive `switch` over + `Reason` in foreign code, which is why it goes last in the declaration; +- a new builder method; +- a new diagnostics method and getter. + +**One behavioural change, and it is deliberate:** a caller feeding the loader a pre-1.18 world gets an +exception where it got an air chunk before. That is the point of the change, and it belongs in the +changelog as such rather than as a fix note. + +## Tests + +Fixtures are built as NBT by hand — a root compound holding `Level.Sections` is a few lines and needs +neither a real old world nor Minecraft. + +| # | Input | Expectation | +| --- | --- | --- | +| 1 | `Level` compound holding `Sections`, no root `sections`, no `DataVersion` | throws, `Reason.UNSUPPORTED_CHUNK_VERSION` | +| 2 | Root `sections`, `DataVersion` below the minimum | throws, same reason | +| 3 | Root `sections`, no `DataVersion` | loads normally | +| 4 | Case 1 and case 2 | `unsupportedChunkVersions()` holds one entry each — the stored version for case 2, the unknown-version constant for case 1 | + +Case 1 is the one that pins the defect: today it returns a chunk and reports success. +Case 3 is the regression guard for tool-written worlds — it is what keeps the check from being too +sharp. + +**Gegenprobe.** With the guard removed, cases 1 and 2 must go red and case 3 must stay green. A case +3 that also goes red means the check rejects worlds it should read, and the test caught the wrong +thing. + +## Out of scope + +This converts nothing. It reads a version, checks a layout, and fails honestly. Migrating a world is +the subject of `falco-migration`, specified separately: an NBT-to-NBT engine over the existing +`RegionFile` API, data-driven from the vendored ViaVersion mappings, upgrade direction first, lower +bound 1.16 initially. The flattening break and the world parts outside `region/` — `entities/`, +`poi/`, `playerdata/`, `level.dat` — are stages after that. + +Reading the source version is the precondition for all of it: without it no mapping can be selected. diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java index 3328853..1a0ddef 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/AnvilDiagnostics.java @@ -41,7 +41,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -58,9 +58,15 @@ public final class AnvilDiagnostics { */ public static final String UNKNOWN_STATUS = ""; + /** + * The value an unsupported chunk is counted under when it stored no {@code DataVersion} at all. + */ + public static final String UNKNOWN_DATA_VERSION = ""; + private final Set unknownBlocks; private final Set unknownBiomes; private final Map partialChunkStatuses; + private final Map unsupportedChunkVersions; private final AtomicBoolean missingRegionFileReported; private final AtomicBoolean missingChunkEntryReported; private final AtomicBoolean sectionRangeReported; @@ -70,6 +76,7 @@ public final class AnvilDiagnostics { private final LongAdder chunksWithoutRegionFile; private final LongAdder chunksWithoutEntry; private final LongAdder partialChunks; + private final LongAdder unsupportedChunks; /** * Creates a new diagnostics instance with empty counters. @@ -78,6 +85,7 @@ public AnvilDiagnostics() { this.unknownBlocks = ConcurrentHashMap.newKeySet(); this.unknownBiomes = ConcurrentHashMap.newKeySet(); this.partialChunkStatuses = new ConcurrentHashMap<>(); + this.unsupportedChunkVersions = new ConcurrentHashMap<>(); this.missingRegionFileReported = new AtomicBoolean(); this.missingChunkEntryReported = new AtomicBoolean(); this.sectionRangeReported = new AtomicBoolean(); @@ -87,6 +95,7 @@ public AnvilDiagnostics() { this.chunksWithoutRegionFile = new LongAdder(); this.chunksWithoutEntry = new LongAdder(); this.partialChunks = new LongAdder(); + this.unsupportedChunks = new LongAdder(); } /** @@ -196,6 +205,45 @@ public boolean reportPartialChunk() { return reportPartialChunk(UNKNOWN_STATUS); } + /** + * Reports a chunk which comes from a version this loader cannot read. + *

+ * The throttling is per version value rather than per loader, so a world holding several + * versions names each of them exactly once. A version beyond the cap is still counted in + * {@link #chunksSkippedAsUnsupported()} and only loses its own entry in + * {@link #unsupportedChunkVersions()}. + *

+ * + * @param version the stored data version, or {@link #UNKNOWN_DATA_VERSION} if none was stored + * @return true if the caller should log the problem, otherwise false + * @since 1.1.0 + */ + public boolean reportUnsupportedChunkVersion(String version) { + this.unsupportedChunks.increment(); + LongAdder counter = this.unsupportedChunkVersions.get(version); + + if (counter == null) { + // The size check and the insertion cannot be one atomic step, so racing threads can + // push the version map past the cap by at most one entry each. The map is not trimmed + // back afterwards: a version which was already counted cannot be removed without losing + // the count it carries, and tracking the counts per version is the point of this map. + if (this.unsupportedChunkVersions.size() >= MAX_TRACKED_NAMES) { + return false; + } + LongAdder created = new LongAdder(); + LongAdder previous = this.unsupportedChunkVersions.putIfAbsent(version, created); + + if (previous == null) { + created.increment(); + return true; + } + previous.increment(); + return false; + } + counter.increment(); + return false; + } + /** * Checks whether a section outside of the dimension height should be reported. * @@ -326,6 +374,34 @@ public long chunksSkipped() { return Collections.unmodifiableMap(snapshot); } + /** + * Returns how many chunks were refused because their version could not be read. + * + * @return the amount of refused chunks + * @since 1.1.0 + */ + @Contract(pure = true) + public long chunksSkippedAsUnsupported() { + return this.unsupportedChunks.sum(); + } + + /** + * Returns the amount of refused chunks per stored data version, sorted by the version value. + * + * @return the amount of refused chunks per version + * @since 1.1.0 + */ + @Contract(pure = true) + public @Unmodifiable Map unsupportedChunkVersions() { + Map snapshot = new LinkedHashMap<>(); + + this.unsupportedChunkVersions.entrySet().stream() + .sorted(Map.Entry.comparingByKey()) + .forEach(entry -> snapshot.put(entry.getKey(), entry.getValue().sum())); + + return Collections.unmodifiableMap(snapshot); + } + /** * Returns the amount of distinct unknown block names which were reported. * diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java index 1117daf..2b167a6 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkDataException.java @@ -15,7 +15,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 1.0.0 */ @ApiStatus.Experimental @@ -30,7 +30,7 @@ public final class ChunkDataException extends AnvilFormatException { *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 1.0.0 */ @ApiStatus.Experimental @@ -64,7 +64,14 @@ public enum Reason { /** * A key the format requires is absent, or holds a tag of another type. */ - MISSING_OR_MISTYPED_KEY + MISSING_OR_MISTYPED_KEY, + + /** + * The chunk comes from a Minecraft version this loader cannot read. Either it carries the + * pre-1.18 layout, which keeps everything under {@code Level}, or its stored + * {@code DataVersion} is below the configured floor. + */ + UNSUPPORTED_CHUNK_VERSION } /** diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java index 16d5c89..541482a 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/FalcoAnvilLoader.java @@ -7,6 +7,7 @@ import net.kyori.adventure.nbt.ByteArrayBinaryTag; import net.kyori.adventure.nbt.CompoundBinaryTag; import net.kyori.adventure.nbt.ListBinaryTag; +import net.kyori.adventure.nbt.NumberBinaryTag; import net.kyori.adventure.nbt.StringBinaryTag; import net.minestom.server.MinecraftServer; import net.minestom.server.coordinate.CoordConversion; @@ -78,7 +79,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -90,6 +91,8 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { private static final BinaryTagIO.Writer TAG_WRITER = BinaryTagIO.writer(); private static final String SECTIONS_KEY = "sections"; + private static final String LEGACY_LEVEL_KEY = "Level"; + private static final String DATA_VERSION_KEY = "DataVersion"; private static final String BLOCK_STATES_KEY = "block_states"; private static final String BIOMES_KEY = "biomes"; private static final String BLOCK_ENTITIES_KEY = "block_entities"; @@ -105,6 +108,14 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { */ public static final int DEFAULT_OPEN_REGION_LIMIT = 64; + /** + * The lowest data version the loader reads by default: {@code 21w43a}, the first version whose + * chunks carry {@code sections} on the root compound instead of under {@code Level}. + * + * @since 1.1.0 + */ + public static final int DEFAULT_MINIMUM_DATA_VERSION = 2844; + private final int openRegionLimit; private final int compressionLevel; private final Path regionDirectory; @@ -117,6 +128,7 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { private final Map> trackedChunks; private final Semaphore saveLimit; private final int dataVersion; + private final int minimumDataVersion; /** * Where failures are reported, or null for the exception manager of the running server. @@ -205,6 +217,7 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { this.trackedChunks = new ConcurrentHashMap<>(); this.saveLimit = new Semaphore(settings.saveParallelism); this.dataVersion = settings.dataVersion; + this.minimumDataVersion = settings.minimumDataVersion; this.exceptionHandler = settings.exceptionHandler; this.closeLock = new ReentrantLock(); @@ -256,7 +269,7 @@ private void reportException(Throwable exception) { public static Builder builder() { return new Builder(DEFAULT_OPEN_REGION_LIMIT, ChunkCompression.DEFAULT_LEVEL, Math.max(Runtime.getRuntime().availableProcessors(), 2), MinecraftServer.DATA_VERSION, - null, null, null, null); + DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null); } /** @@ -285,7 +298,7 @@ public static Builder builder() { *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.4.0 */ @ApiStatus.Experimental @@ -295,19 +308,22 @@ public static final class Builder { private final int compressionLevel; private final int saveParallelism; private final int dataVersion; + private final int minimumDataVersion; private final @Nullable AnvilDiagnostics diagnostics; private final @Nullable PaletteEntryResolver blockResolver; private final @Nullable PaletteEntryResolver biomeResolver; private final @Nullable Consumer exceptionHandler; private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, int dataVersion, - @Nullable AnvilDiagnostics diagnostics, @Nullable PaletteEntryResolver blockResolver, + int minimumDataVersion, @Nullable AnvilDiagnostics diagnostics, + @Nullable PaletteEntryResolver blockResolver, @Nullable PaletteEntryResolver biomeResolver, @Nullable Consumer exceptionHandler) { this.openRegionLimit = openRegionLimit; this.compressionLevel = compressionLevel; this.saveParallelism = saveParallelism; this.dataVersion = dataVersion; + this.minimumDataVersion = minimumDataVersion; this.diagnostics = diagnostics; this.blockResolver = blockResolver; this.biomeResolver = biomeResolver; @@ -334,6 +350,7 @@ public Builder openRegionLimit(int openRegionLimit) { this.compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, this.blockResolver, this.biomeResolver, @@ -364,6 +381,7 @@ public Builder compressionLevel(int compressionLevel) { compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, this.blockResolver, this.biomeResolver, @@ -390,6 +408,7 @@ public Builder saveParallelism(int saveParallelism) { this.compressionLevel, saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, this.blockResolver, this.biomeResolver, @@ -413,6 +432,37 @@ public Builder dataVersion(int dataVersion) { this.compressionLevel, this.saveParallelism, dataVersion, + this.minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler); + } + + /** + * Sets the lowest data version the loader accepts when reading a chunk. + *

+ * This is the read side and has nothing to do with {@link #dataVersion(int)}, which is the + * version written into every saved chunk. A chunk below this floor is refused rather than + * read, because the layout it carries would otherwise decode to air. + *

+ * + * @param minimumDataVersion the lowest data version the loader accepts + * @return a new builder with this value + * @throws IllegalArgumentException if the version is negative + * @since 1.1.0 + */ + @Contract(value = "_ -> new", pure = true) + public Builder minimumDataVersion(int minimumDataVersion) { + if (minimumDataVersion < 0) { + throw new IllegalArgumentException( + "The minimum data version must not be negative but was " + minimumDataVersion); + } + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + minimumDataVersion, this.diagnostics, this.blockResolver, this.biomeResolver, @@ -441,6 +491,7 @@ public Builder diagnostics(AnvilDiagnostics diagnostics) { this.compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, diagnostics, this.blockResolver, this.biomeResolver, @@ -466,6 +517,7 @@ public Builder blockResolver(PaletteEntryResolver blockResolver) { this.compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, blockResolver, this.biomeResolver, @@ -488,6 +540,7 @@ public Builder biomeResolver(PaletteEntryResolver biomeResolver) { this.compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, this.blockResolver, biomeResolver, @@ -519,6 +572,7 @@ public Builder exceptionHandler(Consumer exceptionHandler) { this.compressionLevel, this.saveParallelism, this.dataVersion, + this.minimumDataVersion, this.diagnostics, this.blockResolver, this.biomeResolver, @@ -652,6 +706,7 @@ private record ResolvedRegionDirectory(Path directory, boolean legacyLayout) { } CompoundBinaryTag data = TAG_READER.read(new ByteArrayInputStream(raw.decompress()), BinaryTagIO.Compression.NONE); + requireReadableVersion(data); String status = chunkStatus(data); if (!isFullyGenerated(status)) { @@ -999,6 +1054,20 @@ public boolean legacyLayout() { return this.legacyLayout; } + /** + * Returns the lowest data version this loader accepts when reading a chunk. + *

+ * Package-private on purpose: this reader exists for the Gegenprobe of the builder slot and for + * the guard a later change adds to the load path, not for a caller outside this package. + *

+ * + * @return the lowest data version the loader accepts + */ + @Contract(pure = true) + int minimumDataVersion() { + return this.minimumDataVersion; + } + /** * Closes every region file the loader opened and reports a summary of its work. *

@@ -1341,6 +1410,64 @@ public int openRegionCount() { return this.regions.size(); } + /** + * Refuses a chunk which comes from a version this loader cannot read. + *

+ * The layout is checked before the version, because a version number is a claim about the data + * while the layout is the data: a chunk may carry no version at all, and one that carries a + * version may not hold what that version promises. A root compound without {@code sections} but + * with a {@code Level} compound is the pre-1.18 shape, which would otherwise decode to an empty + * section list and reach the caller as a chunk of air. + *

+ *

+ * A missing {@code DataVersion} is the one case that is not a rejection: a tool which writes + * {@code sections} on the root but never learned to stamp a version has to keep loading, or a + * whole category of externally-written world becomes unreadable. A key that is present but is not + * the number it claims to be, and a key that holds a negative number, are both a different + * situation from absent: something wrote a value there and it does not describe a version this + * loader can trust, so both are refused rather than waved through the same path as "nothing was + * ever written". + *

+ * + * @param data the root compound of the chunk + * @throws ChunkDataException if the chunk cannot be read + */ + private void requireReadableVersion(CompoundBinaryTag data) throws ChunkDataException { + boolean versionMissing = data.get(DATA_VERSION_KEY) == null; + // A stored value that is not a number falls back to the same -1 as an absent key, but the + // two are not the same failure: this flag is what lets the exception below say "not a + // number" instead of misreporting a value ("-1") that was never actually stored. + boolean versionMistyped = !versionMissing && !(data.get(DATA_VERSION_KEY) instanceof NumberBinaryTag); + int version = NbtReads.optionalInteger(data, DATA_VERSION_KEY, -1); + String reported = versionMissing ? AnvilDiagnostics.UNKNOWN_DATA_VERSION : Integer.toString(version); + boolean legacyChunkLayout = !(data.get(SECTIONS_KEY) instanceof ListBinaryTag) + && NbtReads.optionalCompound(data, LEGACY_LEVEL_KEY) != null; + + if (!legacyChunkLayout && (versionMissing || version >= this.minimumDataVersion)) { + return; + } + + if (this.diagnostics.reportUnsupportedChunkVersion(reported)) { + LOGGER.warn( + "Refusing a chunk from data version {} in {}: {}", + reported, this.regionDirectory, + legacyChunkLayout + ? "the chunk data sits under Level, which this loader does not read" + : "the loader accepts " + this.minimumDataVersion + " and above" + ); + } + + throw new ChunkDataException( + ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, + legacyChunkLayout + ? "The chunk stores its data under Level, which means a version before 1.18" + : versionMistyped + ? "The chunk does not store its DataVersion as a number" + : "The chunk stores data version " + version + + " but the loader accepts " + this.minimumDataVersion + " and above" + ); + } + /** * Reads the generation status of the given chunk data. * The key is read in both spellings because Minestom writes it in lower case while the game diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java index ef311c5..ba55d9c 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/AnvilDiagnosticsTest.java @@ -241,4 +241,37 @@ void testConcurrentCountingLosesNoIncrement() throws InterruptedException, Execu } assertEquals((long) threadCount * perThread, diagnostics.chunksLoaded()); } + + @Test + void testAnUnsupportedVersionIsCountedUnderItsOwnValue() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + assertTrue(diagnostics.reportUnsupportedChunkVersion("1976")); + assertFalse(diagnostics.reportUnsupportedChunkVersion("1976")); + assertTrue(diagnostics.reportUnsupportedChunkVersion("2724")); + + assertEquals(3, diagnostics.chunksSkippedAsUnsupported()); + assertEquals(Map.of("1976", 2L, "2724", 1L), diagnostics.unsupportedChunkVersions()); + } + + @Test + void testAChunkWithoutAStoredVersionIsCountedApart() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + diagnostics.reportUnsupportedChunkVersion(AnvilDiagnostics.UNKNOWN_DATA_VERSION); + + assertEquals(Map.of(AnvilDiagnostics.UNKNOWN_DATA_VERSION, 1L), + diagnostics.unsupportedChunkVersions()); + } + + @Test + void testTheVersionBreakdownIsSortedByValue() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + diagnostics.reportUnsupportedChunkVersion("2724"); + diagnostics.reportUnsupportedChunkVersion("1976"); + + assertEquals(List.of("1976", "2724"), + List.copyOf(diagnostics.unsupportedChunkVersions().keySet())); + } } diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java index ff05681..e18eca6 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderBuilderTest.java @@ -295,6 +295,30 @@ public CompoundBinaryTag toEntry(int id) { } } + @Test + void testTheMinimumDataVersionDefaultsToTheFirstRootLayout() { + assertEquals(2844, FalcoAnvilLoader.DEFAULT_MINIMUM_DATA_VERSION); + } + + @Test + void testANegativeMinimumDataVersionIsRefused() { + assertThrows(IllegalArgumentException.class, + () -> FalcoAnvilLoader.builder().minimumDataVersion(-1)); + } + + @Test + void testTheMinimumDataVersionSurvivesEveryOtherSetter(@TempDir Path worldRoot) throws Exception { + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .minimumDataVersion(1519) + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, OVERWORLD)) { + + assertEquals(1519, loader.minimumDataVersion()); + } + } + @Test void testTheBuilderCanBeReusedAfterASlotChanged() throws Exception { FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder().openRegionLimit(8); diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java index bd6889a..6a3d097 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/FalcoAnvilLoaderIntegrationTest.java @@ -32,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -583,6 +584,206 @@ private void writeRawChunk(int chunkX, int chunkZ, CompoundBinaryTag data) throw } } + @Test + void testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir(Env env) throws Exception { + // A custom (swallowing) exception handler is required here, not just style: the default + // handler reaches MinecraftServer's exception manager, which the test environment turns + // into its own "Server threw exception" assertion failure before the loader's own exception + // ever reaches this lambda. See testACorruptedChunkFailsInsteadOfLookingAbsent for the same + // trap hit head-on. + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("Sections", ListBinaryTag.empty()) + .build()) + .build(); + writeRawChunk(3, 3, legacy); + + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .diagnostics(diagnostics) + .exceptionHandler(ignored -> { + }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> loader.loadChunk(instance, 3, 3)); + ChunkDataException cause = assertInstanceOf(ChunkDataException.class, failure.getCause()); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, cause.reason()); + // This chunk carries no DataVersion at all, so the version breakdown has to file it under + // the "" bucket rather than under a literal "-1" — the two mean different things, + // and only the diagnostics assertion here can tell them apart. + assertEquals(Map.of(AnvilDiagnostics.UNKNOWN_DATA_VERSION, 1L), diagnostics.unsupportedChunkVersions()); + } + } + + @Test + void testAChunkBelowTheFloorIsRefused(Env env) throws Exception { + CompoundBinaryTag old = CompoundBinaryTag.builder() + .putInt("DataVersion", 2724) + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(4, 4, old); + + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .diagnostics(diagnostics) + .exceptionHandler(ignored -> { + }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> loader.loadChunk(instance, 4, 4)); + ChunkDataException cause = assertInstanceOf(ChunkDataException.class, failure.getCause()); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, cause.reason()); + assertEquals(Map.of("2724", 1L), diagnostics.unsupportedChunkVersions()); + // A refused chunk is counted once through the version breakdown above and once more as + // a load error, deliberately -- failedLoad's countError() runs in addition to the guard's + // own reporting, not instead of it. Without this line a change that moved the guard behind + // failedLoad, or dropped its reporting altogether, would pass unnoticed. + assertEquals(1, diagnostics.errors()); + } + } + + @Test + void testAChunkWithoutAStoredVersionStillLoads(Env env) throws Exception { + CompoundBinaryTag toolWritten = CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(5, 5, toolWritten); + + try (FalcoAnvilLoader loader = loader()) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 5, 5)); + } + } + + @Test + void testAChunkWithNeitherSectionsNorLevelIsNotRefused(Env env) throws Exception { + // Only the presence of a Level compound makes the pre-1.18 shape, not merely the absence of + // sections on its own. A chunk that simply has not finished generating yet -- no sections, no + // Level, just a partial Status -- is not a legacy chunk and must not be caught by the guard. + // It still comes back as null, but for an unrelated reason further down in loadChunk: it is + // not fully generated. + writeRawChunk(6, 6, CompoundBinaryTag.builder().putString("Status", "minecraft:features").build()); + try (FalcoAnvilLoader loader = loader()) { + assertNull(loader.loadChunk(env.createEmptyInstance(loader), 6, 6)); + } + } + + @Test + void testAChunkWithBothSectionsAndLevelIsNotRefused(Env env) throws Exception { + // A chunk that carries both a root sections list and a Level compound -- plausible leftover + // from some conversion tool -- is not the legacy shape: the loader reads sections from the + // root either way. The first half of the guard's condition only fires when sections is + // genuinely absent, so this fixture has to load normally rather than being refused. + CompoundBinaryTag both = CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .put("Level", CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .build()) + .build(); + writeRawChunk(7, 7, both); + + try (FalcoAnvilLoader loader = loader()) { + assertNotNull(loader.loadChunk(env.createEmptyInstance(loader), 7, 7)); + } + } + + @Test + void testAChunkWithADataVersionStoredAsTheWrongTypeIsRefused(Env env) throws Exception { + // A DataVersion key that holds something other than a number is not "missing": something did + // write a value there. NbtReads.optionalInteger cannot tell "wrong type" apart from "absent" + // by itself, both fall back to the same default, so the guard has to make that distinction + // itself instead of waving a corrupted DataVersion through the path meant for tools that never + // wrote one at all. + CompoundBinaryTag malformed = CompoundBinaryTag.builder() + .putString("DataVersion", "not-a-number") + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(8, 8, malformed); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .exceptionHandler(ignored -> { + }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> loader.loadChunk(instance, 8, 8)); + ChunkDataException cause = assertInstanceOf(ChunkDataException.class, failure.getCause()); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, cause.reason()); + } + } + + @Test + void testAChunkWithANegativeDataVersionIsRefused(Env env) throws Exception { + // A negative DataVersion is not "missing" either: it is a value someone actually wrote, and it + // cannot be a real Minecraft data version. Treating "present but negative" the same as "absent" + // would let a corrupted chunk load as though a tool had simply never stamped a version on it. + CompoundBinaryTag malformed = CompoundBinaryTag.builder() + .putInt("DataVersion", -5) + .putString("Status", "minecraft:full") + .put("sections", ListBinaryTag.empty()) + .build(); + writeRawChunk(9, 9, malformed); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .exceptionHandler(ignored -> { + }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> loader.loadChunk(instance, 9, 9)); + ChunkDataException cause = assertInstanceOf(ChunkDataException.class, failure.getCause()); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, cause.reason()); + } + } + + @Test + void testASectionsKeyStoredAsTheWrongTypeWithLevelIsRefused(Env env) throws Exception { + // The layout half of the guard used to ask only whether "sections" was present at all + // (data.get(SECTIONS_KEY) == null). A root that carries a "sections" key of the wrong type -- + // here a string, sitting next to a full Level compound -- passed that presence check and + // reached decodeSections, which reads nothing usable from a string and hands back air. The + // check has to ask what type sections holds, not merely whether the key exists, so a chunk + // shaped like this is refused instead of silently decoding to air. + CompoundBinaryTag malformed = CompoundBinaryTag.builder() + .putString("sections", "not-a-list") + .put("Level", CompoundBinaryTag.builder() + .putString("Status", "minecraft:full") + .put("Sections", ListBinaryTag.empty()) + .build()) + .build(); + writeRawChunk(10, 10, malformed); + + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .diagnostics(diagnostics) + .exceptionHandler(ignored -> { + }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> loader.loadChunk(instance, 10, 10)); + ChunkDataException cause = assertInstanceOf(ChunkDataException.class, failure.getCause()); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, cause.reason()); + // No DataVersion was stamped either, so this is the layout branch of the guard, not the + // version branch -- the same distinction testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir + // makes for the plain "sections absent" shape. + assertEquals(Map.of(AnvilDiagnostics.UNKNOWN_DATA_VERSION, 1L), diagnostics.unsupportedChunkVersions()); + } + } + @Test void testUnloadingAForeignChunkIsIgnored(Env env) throws Exception { Instance instance = env.createEmptyInstance(loader());