From f62a08d1afffb443214551c9396880b8a8ecc126 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 12:57:36 +0200 Subject: [PATCH 01/18] docs: carry the extension-point spec and plan onto their own branch Stacked on feat/anvil-version-guard rather than main, because the plan moves the body of requireReadableVersion and that only exists on #45. Rebase onto main once #45 lands. Co-Authored-By: Claude Opus 5 (1M context) --- .../2026-08-04-anvil-extension-points.md | 631 ++++++++++++++++++ ...026-08-04-anvil-extension-points-design.md | 130 ++++ 2 files changed, 761 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-04-anvil-extension-points.md create mode 100644 docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md diff --git a/docs/superpowers/plans/2026-08-04-anvil-extension-points.md b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md new file mode 100644 index 0000000..ce69fac --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md @@ -0,0 +1,631 @@ +# Anvil extension points — 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:** Two hard-wired policies in `falco-anvil` — which worlds are readable, and what an unknown +palette entry becomes — become services a caller can replace or discover. + +**Architecture:** One shared resolution helper implements the discovery rules once. Two service +interfaces use it: `ChunkVersionPolicy` (the guard from #45, moved rather than rewritten) and +`UnknownEntryPolicy` (consulted where the two palette resolvers substitute today). Each has a +built-in implementation, an explicit builder slot, and a `discover…()` slot. + +**Tech Stack:** Java 25, Gradle, `java.util.ServiceLoader` (plain classpath, no JPMS), Adventure NBT, +JUnit 5. + +**Spec:** `docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md` + +**Base:** `main` **after #45 is merged**. This plan moves the body of `requireReadableVersion`, which +only exists on that branch. Do not start before the merge. + +## Global Constraints + +- All three interfaces live in `net.onelitefeather.falco.anvil`. No new module. +- **No `module-info.java`** is added. Plain classpath services. +- `checkApiCompatibility` runs: every signature change is additive. **`PaletteEntryResolver.toId` and `toEntry` keep their exact signatures** — see the decision below. +- Javadoc under `-Werror`, `@param`/`@return`/`@throws` complete, `@since 1.2.0` on new members, `@version` of every changed type raised one minor. +- Builders are immutable: a new field means the constructor, `build()`, and **every** existing setter. +- Test names read as sentences. Tests are package-private, plain JUnit assertions. +- Conventional Commits, lower case. +- No timing figure anywhere. +- Check `uptime` before any test run and record it. Counts come from the JUnit XML, not the console. + +## Two decisions this plan makes that the spec left open + +Both are recorded here because the spec's "Open for the plan" section names them, and because an +implementer would otherwise have to invent them. + +**1. `ChunkVersionPolicy` does not count and does not log.** The spec's sketch was +`check(CompoundBinaryTag, int)`, but the body being moved reads three instance fields of the loader: +`minimumDataVersion`, `diagnostics` and `regionDirectory`. Passing all three into a service would put +the loader's infrastructure into a public contract. Instead the policy **only decides and throws**; +the loader catches, counts and logs, deriving the reported version from the compound it already has. +This keeps the interface free of `AnvilDiagnostics` and keeps every diagnostic in one place. + +**2. `UnknownEntryPolicy` throws an *unchecked* fault.** `PaletteEntryResolver.toId` is +`int toId(String, CompoundBinaryTag)` with no `throws` clause, and it is published API on a 1.0.0 +artefact. Adding a checked exception to it would break every implementor. `AnvilChunkException` +already exists as `non-sealed class … extends RuntimeException implements AnvilFault`, so a policy +that refuses throws that, and no published signature changes. + +## File Structure + +| File | Responsibility | Change | +| --- | --- | --- | +| `…/anvil/ServiceResolution.java` | The discovery rules, once | Create (package-private) | +| `…/anvil/ChunkVersionPolicy.java` | The readable-world contract | Create | +| `…/anvil/DefaultChunkVersionPolicy.java` | #45's body, moved | Create | +| `…/anvil/UnknownEntryPolicy.java` | The unknown-entry contract | Create | +| `…/anvil/DefaultUnknownEntryPolicy.java` | Air and plains, as today | Create | +| `…/anvil/FalcoAnvilLoader.java` | Loader and builder | Modify: two field pairs, four builder slots, `requireReadableVersion` becomes a call | +| `…/anvil/BlockPaletteResolver.java` | Block palette | Modify: substitution goes through the policy | +| `…/anvil/BiomePaletteResolver.java` | Biome palette | Modify: same | +| `falco-anvil/src/main/resources/META-INF/services/…ChunkVersionPolicy` | `falco-anvil`'s own registration | Create | +| Five test classes | | Create/modify per task | + +--- + +### Task 1: The resolution rules, once + +**Files:** +- Create: `falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java` +- Test: `falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java` + +**Interfaces:** +- Consumes: nothing. +- Produces: `static @Nullable T ServiceResolution.discover(Class service)` — returns the single + provider, `null` if none, throws `IllegalStateException` naming all providers if more than one; + `static @Nullable T ServiceResolution.choose(Class service, @Nullable T explicit, boolean discover)` + — throws `IllegalStateException` if both an explicit instance and `discover` are given, otherwise + returns the explicit one, the discovered one, or `null`. + +- [ ] **Step 1: Write the failing tests** + +The tests need services to find. Register two dummies through the **test** resources so the real +module is unaffected: + +`falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy` +containing two lines, the two nested implementation class names. + +```java +class ServiceResolutionTest { + + interface Dummy { String name(); } + + public static final class FirstDummy implements Dummy { + public FirstDummy() { } + @Override public String name() { return "first"; } + } + + public static final class SecondDummy implements Dummy { + public SecondDummy() { } + @Override public String name() { return "second"; } + } + + interface Absent { } + + @Test + void testAServiceWithNoProviderResolvesToNothing() { + assertNull(ServiceResolution.discover(Absent.class)); + } + + @Test + void testTwoProvidersAreRefusedAndBothAreNamed() { + IllegalStateException failure = + assertThrows(IllegalStateException.class, () -> ServiceResolution.discover(Dummy.class)); + + assertTrue(failure.getMessage().contains("FirstDummy"), failure.getMessage()); + assertTrue(failure.getMessage().contains("SecondDummy"), failure.getMessage()); + } + + @Test + void testAnExplicitInstanceAndDiscoveryTogetherAreRefused() { + Dummy explicit = () -> "explicit"; + + assertThrows(IllegalStateException.class, + () -> ServiceResolution.choose(Dummy.class, explicit, true)); + } + + @Test + void testAnExplicitInstanceIsUsedWithoutTouchingTheClasspath() { + Dummy explicit = () -> "explicit"; + + assertEquals("explicit", ServiceResolution.choose(Dummy.class, explicit, false).name()); + } + + @Test + void testNeitherExplicitNorDiscoveredResolvesToNothing() { + assertNull(ServiceResolution.choose(Dummy.class, null, false)); + } +} +``` + +The fourth case matters more than it looks: it proves an explicit instance short-circuits before +`ServiceLoader` runs. Without that, `Dummy`'s two providers would make it throw. + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*ServiceResolutionTest*"` +Expected: compilation failure — `ServiceResolution` does not exist. + +- [ ] **Step 3: Implement it** + +```java +final class ServiceResolution { + + private ServiceResolution() { + } + + /** + * Finds the single provider of the given service on the classpath. + * + * @param service the service interface + * @param the service type + * @return the provider, or null if the classpath carries none + * @throws IllegalStateException if more than one provider is registered + */ + static @Nullable T discover(Class service) { + List providers = new ArrayList<>(); + ServiceLoader.load(service).forEach(providers::add); + + if (providers.isEmpty()) { + return null; + } + if (providers.size() > 1) { + // Naming them is the whole value of this branch: "several providers" sends the reader + // to the classpath, the two class names send them to the jar that should not be there. + throw new IllegalStateException( + "Several providers of " + service.getName() + " are registered and none can be chosen for you: " + + providers.stream().map(provider -> provider.getClass().getName()).sorted().toList() + + ". Set one explicitly on the builder instead." + ); + } + return providers.getFirst(); + } + + /** + * Chooses between an explicitly configured instance and classpath discovery. + * + * @param service the service interface + * @param explicit the instance the caller configured, or null + * @param discover whether the caller asked for discovery + * @param the service type + * @return the chosen provider, or null if the caller asked for neither + * @throws IllegalStateException if the caller asked for both, or if discovery is ambiguous + */ + static @Nullable T choose(Class service, @Nullable T explicit, boolean discover) { + if (explicit != null && discover) { + throw new IllegalStateException( + "An explicit " + service.getSimpleName() + " and discovery were both configured. " + + "Choose one: the explicit instance, or the classpath." + ); + } + if (explicit != null) { + return explicit; + } + return discover ? discover(service) : null; + } +} +``` + +- [ ] **Step 4: Run them and watch them pass** + +Run: `./gradlew :falco-anvil:test --tests "*ServiceResolutionTest*"` +Expected: PASS, five cases. + +- [ ] **Step 5: Gegenprobe** + +Change `providers.size() > 1` to `providers.size() > 2`. +`testTwoProvidersAreRefusedAndBothAreNamed` must go red and the other four stay green. Revert, verify +`git status` is clean. + +- [ ] **Step 6: Commit** + +```bash +git add falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java \ + falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java \ + falco-anvil/src/test/resources/META-INF/services/ +git commit -m "feat(anvil): resolve a service once, and refuse to guess between two" +``` + +--- + +### Task 2: The version policy + +**Files:** +- Create: `…/anvil/ChunkVersionPolicy.java`, `…/anvil/DefaultChunkVersionPolicy.java` +- Create: `falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy` +- Modify: `…/anvil/FalcoAnvilLoader.java` — the guard call, one field, two builder slots +- Test: `…/anvil/ChunkVersionPolicyTest.java`, and the existing `FalcoAnvilLoaderIntegrationTest` + +**Interfaces:** +- Consumes: `ServiceResolution.choose` from Task 1. +- Produces: `ChunkVersionPolicy` with + `void check(CompoundBinaryTag data, int minimumDataVersion) throws ChunkDataException`; + `DefaultChunkVersionPolicy` implementing it; builder slots + `versionPolicy(ChunkVersionPolicy)` and `discoverVersionPolicy()`. + +- [ ] **Step 1: Write the failing tests** + +In a new `ChunkVersionPolicyTest`, against the default policy directly — no loader, no environment: + +```java +@Test +void testTheDefaultPolicyRefusesALevelLayout() { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .build(); + + ChunkDataException failure = assertThrows(ChunkDataException.class, + () -> new DefaultChunkVersionPolicy().check(legacy, 2844)); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, failure.reason()); +} + +@Test +void testTheDefaultPolicyAcceptsAChunkWithoutAStoredVersion() throws Exception { + CompoundBinaryTag toolWritten = CompoundBinaryTag.builder() + .put("sections", ListBinaryTag.empty()) + .build(); + + new DefaultChunkVersionPolicy().check(toolWritten, 2844); +} + +@Test +void testTheDefaultPolicyRefusesAMistypedVersion() { + CompoundBinaryTag broken = CompoundBinaryTag.builder() + .putString("DataVersion", "not-a-number") + .put("sections", ListBinaryTag.empty()) + .build(); + + assertThrows(ChunkDataException.class, () -> new DefaultChunkVersionPolicy().check(broken, 2844)); +} +``` + +In `FalcoAnvilLoaderIntegrationTest`, that the loader honours a replacement — the fixture helper +`writeRawChunk` is already there: + +```java +@Test +void testAPolicyThatAllowsEverythingLetsALegacyChunkThrough(Env env) throws Exception { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .putString("Status", "minecraft:full") + .build(); + writeRawChunk(11, 11, legacy); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy((data, minimum) -> { }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 11, 11)); + } +} + +@Test +void testWithoutAnyPolicyALegacyChunkIsNotChecked(Env env) throws Exception { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .putString("Status", "minecraft:full") + .build(); + writeRawChunk(12, 12, legacy); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy(null) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 12, 12)); + } +} +``` + +**The second case is the one the spec asks for in executable form.** It documents the cost of the +chosen default: with no policy, a pre-21w43a chunk loads, and it loads as air. Its Javadoc says so. + +Check what `versionPolicy(null)` should mean before writing it — if the builder rejects null, express +"no policy" the way the implementation actually offers it, and say so in the report. + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*ChunkVersionPolicyTest*" --tests "*FalcoAnvilLoaderIntegrationTest*"` +Expected: compilation failure — the type and the slots do not exist. + +- [ ] **Step 3: Create the interface** + +```java +/** + * Decides whether a chunk is one this loader can read. + *

+ * A policy only decides. It does not count and it does not log: the loader catches the failure, + * records it in its {@link AnvilDiagnostics} and writes the log line, so every diagnostic of a load + * stays in one place and this contract stays free of the loader's infrastructure. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public interface ChunkVersionPolicy { + + /** + * Checks the given chunk data and throws if the loader cannot read it. + * + * @param data the root compound of the chunk + * @param minimumDataVersion the lowest data version the loader was configured to accept + * @throws ChunkDataException if the chunk cannot be read + */ + void check(CompoundBinaryTag data, int minimumDataVersion) throws ChunkDataException; +} +``` + +- [ ] **Step 4: Move #45's body into the default** + +`DefaultChunkVersionPolicy` takes the body of `requireReadableVersion` **unchanged in its decision +logic**: `versionMissing` from `data.get(DATA_VERSION_KEY) == null`, `versionMistyped`, +`legacyChunkLayout` from `!(… instanceof ListBinaryTag) && optionalCompound(Level) != null`, the same +early return, the same three message branches. What it drops is the diagnostics call and the log +line, which move to the loader per the decision above. + +- [ ] **Step 5: Register it and wire the loader** + +`falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy` +holds one line: `net.onelitefeather.falco.anvil.DefaultChunkVersionPolicy`. + +In `FalcoAnvilLoader`: one field `@Nullable ChunkVersionPolicy versionPolicy`, resolved in the +constructor through `ServiceResolution.choose(...)`. Two builder slots, threaded through **every** +setter. At the seam, `requireReadableVersion(data)` becomes: + +```java + if (this.versionPolicy != null) { + checkVersion(data); + } +``` + +where `checkVersion` calls the policy, catches `ChunkDataException`, does the counting and logging +#45 did, and rethrows. + +Extend the existing startup log line with the resolved policy's class name or `none`. + +- [ ] **Step 6: Run them and watch them pass** + +Run: `./gradlew :falco-anvil:test` +Expected: PASS. **Every case #45 added must still pass** — they now exercise the default policy +through the loader instead of a private method, which is the point. + +- [ ] **Step 7: Gegenprobe** + +Two defects, one at a time, each reverted: + +1. Make `checkVersion` swallow the exception instead of rethrowing. Every #45 refusal case must go + red. +2. Delete the `META-INF/services` line. `testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir` must go + red, because nothing is registered and nothing checks — **and that is the documented cost of the + chosen default, reproduced on purpose.** Note in the report which cases went red; that list is + what the acceptance quotes. + +- [ ] **Step 8: Commit** + +```bash +git add falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ \ + falco-anvil/src/main/resources/META-INF/services/ \ + falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ +git commit -m "feat(anvil): make the version guard a service the caller can replace" +``` + +--- + +### Task 3: The unknown-entry policy + +**Files:** +- Create: `…/anvil/UnknownEntryPolicy.java`, `…/anvil/DefaultUnknownEntryPolicy.java` +- Modify: `…/anvil/BlockPaletteResolver.java`, `…/anvil/BiomePaletteResolver.java`, `…/anvil/FalcoAnvilLoader.java` +- Test: `…/anvil/UnknownEntryPolicyTest.java` + +**Interfaces:** +- Consumes: `ServiceResolution.choose` from Task 1. +- Produces: `UnknownEntryPolicy` with + `int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties)` and + `int onUnknownBiome(String name)`, both unchecked; + `DefaultUnknownEntryPolicy`; builder slots `unknownEntryPolicy(...)` and `discoverUnknownEntryPolicy()`. + +- [ ] **Step 1: Write the failing tests** + +```java +@Test +void testTheDefaultPolicyReplacesAnUnknownBlockWithAir() { + assertEquals(Block.AIR.stateId(), new DefaultUnknownEntryPolicy().onUnknownBlock("falco:nope", null)); +} + +@Test +void testARefusingPolicyFailsTheChunkInsteadOfSubstituting() { + UnknownEntryPolicy refusing = new UnknownEntryPolicy() { + @Override public int onUnknownBlock(String name, CompoundBinaryTag properties) { + throw new AnvilChunkException("The block " + name + " has no mapping"); + } + @Override public int onUnknownBiome(String name) { + throw new AnvilChunkException("The biome " + name + " has no mapping"); + } + }; + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> new BlockPaletteResolver(new AnvilDiagnostics(), refusing).toId("falco:nope", null)); + assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); +} + +@Test +void testTheResolverStillCountsWhenThePolicySubstitutes() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + new BlockPaletteResolver(diagnostics, new DefaultUnknownEntryPolicy()).toId("falco:nope", null); + + assertEquals(1, diagnostics.unknownBlockCount()); +} +``` + +Check `AnvilChunkException`'s public constructors before writing case two and use one that exists. +The third case pins that counting stays in the resolver and does not move into the policy — losing it +would make a substituting run silent. + +- [ ] **Step 2: Run them and watch them fail** + +Run: `./gradlew :falco-anvil:test --tests "*UnknownEntryPolicyTest*"` +Expected: compilation failure — the type and the two-argument resolver constructor do not exist. + +- [ ] **Step 3: Create the interface and the default** + +```java +/** + * Decides what becomes of a palette entry the running server does not know. + *

+ * Returning an id substitutes it; throwing {@link AnvilChunkException} fails the chunk. Substituting + * is right for a server that wants a world to stay loadable and wrong for a tool that converts one, + * which is why this is a policy and not a constant. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public interface UnknownEntryPolicy { + + /** + * Decides what an unknown block becomes. + * + * @param name the block name stored in the palette + * @param properties the stored properties, or null if the entry carries none + * @return the state id to use instead + * @throws AnvilChunkException if the chunk should fail rather than carry a substitute + */ + int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties); + + /** + * Decides what an unknown biome becomes. + * + * @param name the biome name stored in the palette + * @return the id to use instead + * @throws AnvilChunkException if the chunk should fail rather than carry a substitute + */ + int onUnknownBiome(String name); +} +``` + +`DefaultUnknownEntryPolicy` returns `Block.AIR.stateId()` and the plains id, exactly the values the +two resolvers hard-code today. + +- [ ] **Step 4: Route the resolvers through it** + +`BlockPaletteResolver` gains a second constructor argument. **Keep the one-argument constructor**, +delegating to the new one with the default policy — it is published API and removing it would break +`checkApiCompatibility`. In `toId`, the branch that today does + +```java + if (this.diagnostics.reportUnknownBlock(name)) { + LOGGER.warn("The block '{}' is unknown and is replaced with air, …", name); + } + return Block.AIR.stateId(); +``` + +keeps the reporting and the log **and** ends with `return this.policy.onUnknownBlock(name, properties);`. +The log wording must stop claiming "is replaced with air" unconditionally, because with a refusing +policy it is not. `BiomePaletteResolver` gets the same treatment. + +Add the two builder slots to `FalcoAnvilLoader`, threaded through every setter, and pass the resolved +policy into the resolvers the loader constructs itself. + +- [ ] **Step 5: Run them and watch them pass** + +Run: `./gradlew :falco-anvil:test` +Expected: PASS, including every existing resolver test unchanged. + +- [ ] **Step 6: Gegenprobe** + +Remove the `reportUnknownBlock` call while keeping the policy call. +`testTheResolverStillCountsWhenThePolicySubstitutes` must go red alone. Revert. + +- [ ] **Step 7: Commit** + +```bash +git add falco-anvil/src/ +git commit -m "feat(anvil): let a caller decide what an unknown palette entry becomes" +``` + +--- + +### Task 4: Acceptance + +- [ ] **Step 1: Check the load, then run every module** + +`uptime` before and after, both into the report. Then: + +```bash +./gradlew :falco-anvil:test :falco-light:test :falco-instance:test \ + :falco-demo:test :falco-benchmarks:test :falco-archunit:test --rerun-tasks +``` + +Counts from the JUnit XML. No count may fall against the baseline recorded at the merge of #45. + +- [ ] **Step 2: Build with javadoc and the API check** + +```bash +./gradlew build -x test --rerun-tasks +``` + +Javadoc genuinely executed for all published modules, zero warnings, `checkApiCompatibility` +executed. **If japicmp flags anything, stop and record it verbatim** — the plan's claim is that every +change is additive, and a flag means that claim is wrong. + +- [ ] **Step 3: Attack the gate** + +Re-inject Task 2's Gegenprobe #2 — delete the `META-INF/services` registration — and confirm the full +suite catches it, not just one class. This is also the executable record of what the optional guard +costs. Revert; `git status --short` empty. + +- [ ] **Step 4: Write the result into the plan and commit** + +Append `## Result`: cases added per task, which injected defect each caught, module counts from the +XML, both load figures, and explicitly what this does **not** change — no module, no JPMS, no +published signature, and no behaviour at all when nothing is registered except that the guard is now +losable. + +--- + +### Task 5: Documentation + +- [ ] **Step 1: README** + +One paragraph under the `falco-anvil` material: the loader has replaceable policies, the guard is one +of them, and it can be removed. Do not add a section. + +- [ ] **Step 2: The wiki page** + +Extend `Anvil-Chunk-Loader` in `/mnt/projects/oss/onelitefeather/Falco.wiki` — the page already has a +`### The version floor` subsection from #45, which is where this belongs. Cover all three services, +the three resolution rules, and **the consequence of removing the guard, in the same words the spec +uses.** No new page, so the sidebar is untouched. + +- [ ] **Step 3: Commit both, separately** + +The wiki is its own repository. Do not push either. + +--- + +## Self-Review + +**Spec coverage.** `ChunkVersionPolicy` → Task 2; `UnknownEntryPolicy` → Task 3; the three resolution +rules → Task 1, exercised again in 2 and 3; `falco-anvil`'s own registration → Task 2 Step 5; the +startup line → Task 2 Step 5; the Gegenprobe the spec explicitly asks for → Task 2 Step 7 case 2 and +Task 4 Step 3; documentation → Task 5. `ChunkMigrator` is out of scope and belongs to the migration +plan. + +**Placeholders.** None. Two steps carry a verify-before-you-write instruction rather than an +assumption — `versionPolicy(null)` in Task 2 Step 1 and `AnvilChunkException`'s constructors in Task 3 +Step 1 — because both depend on code this plan does not quote in full, and guessing them would put a +wrong call into a test. + +**Type consistency.** `ServiceResolution.discover`/`choose` are named identically in Task 1's +Interfaces block, its code, and the consumers in Tasks 2 and 3. `ChunkVersionPolicy.check` takes +`(CompoundBinaryTag, int)` everywhere. `UnknownEntryPolicy` has exactly the two methods named in the +spec, both unchecked. `@since 1.2.0` throughout, because #45 already took the module to 1.1.0. diff --git a/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md new file mode 100644 index 0000000..c123659 --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md @@ -0,0 +1,130 @@ +# Extension points for the Anvil loader + +Design of 2026-08-04. `falco-anvil` turns three hard-wired decisions into services a caller can +replace: the version guard, the fallback for an unknown palette entry, and — already specified +separately — chunk migration. All three use the platform `ServiceLoader`. + +Sits between [#45](https://github.com/OneLiteFeatherNET/Falco/pull/45), which built the guard, and +`falco-migration`, which needs the third of them. `2026-08-04-falco-migration-design.md` specifies +`ChunkMigrator` and is not repeated here. + +## Why + +Two behaviours in the loader are decisions the project made for its own use and then fixed in code: + +1. **Which worlds are readable.** #45 refuses anything below `DEFAULT_MINIMUM_DATA_VERSION` or + carrying a `Level` compound. That is right for a server that only wants worlds it can read, and + wrong for a tool that wants to inspect one it cannot. +2. **What an unknown block or biome becomes.** `BlockPaletteResolver` substitutes air, and + `BiomePaletteResolver` substitutes plains, both counting the substitution in `AnvilDiagnostics`. + For a server that keeps a world loadable this is the reasonable last resort. For anything that + converts or audits a world it is the wrong reaction, because a substitution that is counted is + still a substitution written back on the next save. + +Neither is wrong. Both are policy, and policy that only one consumer can choose is not policy. + +`PaletteEntryResolver` already proves the shape: an interface in `falco-anvil`, handed in through the +builder. What it does not offer is discovery — a consumer must know the type exists and wire it. The +services below add that, and keep the builder slot as the explicit route. + +## Decisions + +| Question | Decision | +| --- | --- | +| Mechanism | Plain classpath `ServiceLoader`. No `module-info.java` exists in this project and none is added | +| Guard default | **None. No provider means no version check** | +| Guard shipped by `falco-anvil` | Yes, as a registered provider of its own | +| Fallback default | The current behaviour: air and plains, counted | +| Explicit route | Kept. A builder slot overrides discovery for every service | +| API compatibility | Additive only. No existing signature changes | + +**The guard is fully optional, and that is the project owner's decision, taken with its consequence +stated.** The consequence: a loader with no guard provider reads a pre-21w43a world as air again, +with no error and no log line — the defect #45 exists to close. This spec does not soften that. What +it does is make the state visible rather than silent: + +- `falco-anvil` registers its own guard in its `META-INF/services`, so a normal dependency on the + module has the guard. Losing it takes an exclusion somebody writes, not a classpath that happens to + be empty. +- The loader's existing startup line — which already reports the chosen region layout, and exists + because "without this line the choice between the two layouts happens invisibly" — gains the guard + it resolved, or the word `none`. + +## The services + +### `ChunkVersionPolicy` + +```java +public interface ChunkVersionPolicy { + + void check(CompoundBinaryTag data, int minimumDataVersion) throws ChunkDataException; +} +``` + +Called at the seam where `requireReadableVersion` is called today. The built-in implementation is the +body #45 wrote, moved rather than rewritten: layout first, version second, one `Reason`, the +diagnostics counter. + +A policy that wants to allow everything implements an empty body. A policy that wants a different +floor reads its own configuration. The interface deliberately takes the whole compound, not a version +number, because the layout check does not rest on a version at all. + +### `UnknownEntryPolicy` + +```java +public interface UnknownEntryPolicy { + + int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties) throws ChunkDataException; + + int onUnknownBiome(String name) throws ChunkDataException; +} +``` + +Consulted by `BlockPaletteResolver` and `BiomePaletteResolver` where they substitute today. Returning +an id substitutes it; throwing fails the chunk. The built-in implementation returns air and plains and +keeps the existing counting, so behaviour without a provider is byte-for-byte what it is now. + +This is the seam `falco-migration` needs: a converter installs a policy that throws, because on the +upgrade path an unmappable block means the mapping data is incomplete and must be seen. + +### Resolution rules, shared by all three services + +The same three rules the migration spec sets out, applied uniformly: + +- **More than one provider throws**, naming them. Silent selection is how a world gets read under a + policy nobody chose. +- **The builder slot and discovery are exclusive.** Setting both is a configuration error. +- **A builder slot always wins over the classpath** when only it is used — that is what "explicit" + means. + +Discovery happens once, when the loader is built, not per chunk. + +## What this does not do + +- **No behaviour change with no provider present**, for the fallback. Air and plains, counted, exactly + as today. +- **A behaviour change for the guard**, and it is the point of the change: the guard becomes losable. + See the consequence above. +- **No new module.** All three interfaces live in `falco-anvil`. +- **No JPMS.** Plain classpath services. +- **`ChunkMigrator` is not specified here** — see the migration design. + +## Evidence + +- One case per service that the built-in provider is used when nothing is registered, and that + behaviour matches the current tree. For the fallback this is a regression guard on all of #45's + and the existing resolver tests; for the guard it is the assertion that `falco-anvil`'s own + registration is found. +- One case per service that a registered provider replaces the built-in one. +- One case that two providers throw, and one that builder slot plus discovery throws. +- **The Gegenprobe that matters:** remove `falco-anvil`'s own guard registration and assert that a + pre-21w43a world loads as air again. That test documents the cost of the chosen default in + executable form, so nobody has to take this document's word for it. + +## Open for the plan + +- Whether `ChunkVersionPolicy` and `UnknownEntryPolicy` are one service or two. They are written as + two here because they answer unrelated questions and a consumer will usually want one, not both. +- Where the built-in providers live: a package of their own, or beside the interfaces. +- Whether `AnvilDiagnostics` gains a counter for "policy replaced", so a run says it did not use the + defaults. From a57f48d4ce4ab0fbd9cab8286f8fbe49c48ac387 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 13:01:56 +0200 Subject: [PATCH 02/18] feat(anvil): resolve a service once, and refuse to guess between two --- .../falco/anvil/ServiceResolution.java | 78 +++++++++++++++++ .../falco/anvil/ServiceResolutionTest.java | 86 +++++++++++++++++++ ...er.falco.anvil.ServiceResolutionTest$Dummy | 2 + 3 files changed, 166 insertions(+) create mode 100644 falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java create mode 100644 falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java create mode 100644 falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java new file mode 100644 index 0000000..6592233 --- /dev/null +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java @@ -0,0 +1,78 @@ +package net.onelitefeather.falco.anvil; + +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; +import java.util.ServiceLoader; + +/** + * Resolves the two decisions that currently sit hardcoded in the loader as classpath services: + * which worlds are readable, and what an unknown block becomes. This type builds the resolution + * rules once, so the extension points built on top of it do not each reinvent them. + *

+ * A caller either configures an instance explicitly, or asks for classpath discovery, but not both + * at once: {@link #choose(Class, Object, boolean)} refuses the combination outright rather than + * silently preferring one side. Discovery itself, in {@link #discover(Class)}, refuses to guess + * between more than one registered provider. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.0.0 + */ +final class ServiceResolution { + + private ServiceResolution() { + } + + /** + * Finds the single provider of the given service on the classpath. + * + * @param service the service interface + * @param the service type + * @return the provider, or null if the classpath carries none + * @throws IllegalStateException if more than one provider is registered + */ + static @Nullable T discover(Class service) { + List providers = new ArrayList<>(); + ServiceLoader.load(service).forEach(providers::add); + + if (providers.isEmpty()) { + return null; + } + if (providers.size() > 1) { + // Naming them is the whole value of this branch: "several providers" sends the reader + // to the classpath, the two class names send them to the jar that should not be there. + throw new IllegalStateException( + "Several providers of " + service.getName() + " are registered and none can be chosen for you: " + + providers.stream().map(provider -> provider.getClass().getName()).sorted().toList() + + ". Set one explicitly on the builder instead." + ); + } + return providers.getFirst(); + } + + /** + * Chooses between an explicitly configured instance and classpath discovery. + * + * @param service the service interface + * @param explicit the instance the caller configured, or null + * @param discover whether the caller asked for discovery + * @param the service type + * @return the chosen provider, or null if the caller asked for neither + * @throws IllegalStateException if the caller asked for both, or if discovery is ambiguous + */ + static @Nullable T choose(Class service, @Nullable T explicit, boolean discover) { + if (explicit != null && discover) { + throw new IllegalStateException( + "An explicit " + service.getSimpleName() + " and discovery were both configured. " + + "Choose one: the explicit instance, or the classpath." + ); + } + if (explicit != null) { + return explicit; + } + return discover ? discover(service) : null; + } +} diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java new file mode 100644 index 0000000..984e571 --- /dev/null +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java @@ -0,0 +1,86 @@ +package net.onelitefeather.falco.anvil; + +import org.junit.jupiter.api.Test; + +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 resolution rules {@link ServiceResolution} offers to the two extension points built + * on top of it: how a service is discovered on the classpath, and how an explicitly configured + * instance relates to that discovery. + *

+ * The dummy providers this test resolves are registered through the test resources, under + * {@code META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy}, so the main + * module registers nothing extra. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.0.0 + */ +class ServiceResolutionTest { + + interface Dummy { + String name(); + } + + public static final class FirstDummy implements Dummy { + public FirstDummy() { + } + + @Override + public String name() { + return "first"; + } + } + + public static final class SecondDummy implements Dummy { + public SecondDummy() { + } + + @Override + public String name() { + return "second"; + } + } + + interface Absent { + } + + @Test + void testAServiceWithNoProviderResolvesToNothing() { + assertNull(ServiceResolution.discover(Absent.class)); + } + + @Test + void testTwoProvidersAreRefusedAndBothAreNamed() { + IllegalStateException failure = + assertThrows(IllegalStateException.class, () -> ServiceResolution.discover(Dummy.class)); + + assertTrue(failure.getMessage().contains("FirstDummy"), failure.getMessage()); + assertTrue(failure.getMessage().contains("SecondDummy"), failure.getMessage()); + } + + @Test + void testAnExplicitInstanceAndDiscoveryTogetherAreRefused() { + Dummy explicit = () -> "explicit"; + + assertThrows(IllegalStateException.class, + () -> ServiceResolution.choose(Dummy.class, explicit, true)); + } + + @Test + void testAnExplicitInstanceIsUsedWithoutTouchingTheClasspath() { + Dummy explicit = () -> "explicit"; + + assertEquals("explicit", ServiceResolution.choose(Dummy.class, explicit, false).name()); + } + + @Test + void testNeitherExplicitNorDiscoveredResolvesToNothing() { + assertNull(ServiceResolution.choose(Dummy.class, null, false)); + } +} diff --git a/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy new file mode 100644 index 0000000..a9c2788 --- /dev/null +++ b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy @@ -0,0 +1,2 @@ +net.onelitefeather.falco.anvil.ServiceResolutionTest$FirstDummy +net.onelitefeather.falco.anvil.ServiceResolutionTest$SecondDummy From ab88dd403ae033bcc5d887db1e239dc9eb511b7c Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 13:23:06 +0200 Subject: [PATCH 03/18] feat(anvil): make the version guard a service the caller can replace Moves the decision logic of requireReadableVersion into ChunkVersionPolicy / DefaultChunkVersionPolicy, unchanged, and resolves it through ServiceResolution.choose the way Task 1 built it. The loader keeps counting and logging a refusal itself; the policy only decides and throws. A builder that never touches versionPolicy()/discoverVersionPolicy() keeps discovering the default, so a plain constructor call refuses the same chunks it always did. versionPolicy(null) is the only way to skip the check entirely. --- .../falco/anvil/ChunkVersionPolicy.java | 29 +++ .../anvil/DefaultChunkVersionPolicy.java | 88 +++++++ .../falco/anvil/FalcoAnvilLoader.java | 240 +++++++++++++----- ...litefeather.falco.anvil.ChunkVersionPolicy | 1 + .../falco/anvil/ChunkVersionPolicyTest.java | 50 ++++ .../FalcoAnvilLoaderIntegrationTest.java | 42 +++ 6 files changed, 382 insertions(+), 68 deletions(-) create mode 100644 falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java create mode 100644 falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java create mode 100644 falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy create mode 100644 falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java new file mode 100644 index 0000000..ccc09d6 --- /dev/null +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java @@ -0,0 +1,29 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import org.jetbrains.annotations.ApiStatus; + +/** + * Decides whether a chunk is one this loader can read. + *

+ * A policy only decides. It does not count and it does not log: the loader catches the failure, + * records it in its {@link AnvilDiagnostics} and writes the log line, so every diagnostic of a load + * stays in one place and this contract stays free of the loader's infrastructure. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public interface ChunkVersionPolicy { + + /** + * Checks the given chunk data and throws if the loader cannot read it. + * + * @param data the root compound of the chunk + * @param minimumDataVersion the lowest data version the loader was configured to accept + * @throws ChunkDataException if the chunk cannot be read + */ + void check(CompoundBinaryTag data, int minimumDataVersion) throws ChunkDataException; +} diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java new file mode 100644 index 0000000..ea41922 --- /dev/null +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java @@ -0,0 +1,88 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import net.kyori.adventure.nbt.ListBinaryTag; +import net.kyori.adventure.nbt.NumberBinaryTag; +import org.jetbrains.annotations.ApiStatus; + +/** + * The {@link ChunkVersionPolicy} a loader resolves when nothing else was configured: refuses the + * pre-1.18 {@code Level} layout and a stored {@code DataVersion} below the configured floor, and + * accepts a chunk that carries no {@code DataVersion} at all. + *

+ * This is the guard against a specific piece of data loss: before snapshot {@code 21w43a}, a + * chunk's block data lived under a {@code Level} compound instead of {@code sections} on the root. + * Reading such a chunk with a loader that only looks for {@code sections} on the root does not + * fail, it silently decodes to an empty section list — a chunk of air. This policy is what turns + * that silent data loss into a thrown exception. + *

+ *

+ * This type is experimental, like everything else in this package. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public final class DefaultChunkVersionPolicy implements ChunkVersionPolicy { + + private static final String SECTIONS_KEY = "sections"; + private static final String LEGACY_LEVEL_KEY = "Level"; + private static final String DATA_VERSION_KEY = "DataVersion"; + + /** + * Creates the policy. It holds no state of its own, so every instance behaves the same way. + */ + public DefaultChunkVersionPolicy() { + } + + /** + * Checks the given chunk data and throws if the loader cannot read it. + *

+ * 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 + * @param minimumDataVersion the lowest data version the loader was configured to accept + * @throws ChunkDataException if the chunk cannot be read + */ + @Override + public void check(CompoundBinaryTag data, int minimumDataVersion) 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); + boolean legacyChunkLayout = !(data.get(SECTIONS_KEY) instanceof ListBinaryTag) + && NbtReads.optionalCompound(data, LEGACY_LEVEL_KEY) != null; + + if (!legacyChunkLayout && (versionMissing || version >= minimumDataVersion)) { + return; + } + + 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 " + minimumDataVersion + " and above" + ); + } +} 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 541482a..ee0b97b 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,7 +7,6 @@ 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; @@ -79,7 +78,7 @@ *

* * @author TheMeinerLP - * @version 1.1.0 + * @version 1.2.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -91,7 +90,6 @@ 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"; @@ -130,6 +128,22 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { private final int dataVersion; private final int minimumDataVersion; + /** + * The policy consulted before a chunk is decoded, or null to skip that check entirely. + *

+ * Resolved once, in the constructor, through {@link ServiceResolution#choose(Class, Object, + * boolean)}. A builder which never touches {@link Builder#versionPolicy(ChunkVersionPolicy)} or + * {@link Builder#discoverVersionPolicy()} discovers {@link DefaultChunkVersionPolicy} through the + * classpath by default, which is what keeps every loader built the way earlier versions built + * one refusing the same chunks it always refused. Calling {@code versionPolicy(null)} is the + * only way to leave this field null, and {@link #checkVersion(CompoundBinaryTag)} treats that as + * "check nothing" rather than substituting the default itself. + *

+ * + * @since 1.2.0 + */ + private final @Nullable ChunkVersionPolicy versionPolicy; + /** * Where failures are reported, or null for the exception manager of the running server. *

@@ -219,6 +233,8 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { this.dataVersion = settings.dataVersion; this.minimumDataVersion = settings.minimumDataVersion; this.exceptionHandler = settings.exceptionHandler; + this.versionPolicy = ServiceResolution.choose( + ChunkVersionPolicy.class, settings.versionPolicy, settings.discoverVersionPolicy); this.closeLock = new ReentrantLock(); // Which directory was chosen, and how many region files are in it, is the first thing @@ -226,12 +242,13 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { // two layouts happens invisibly, and a world whose files sit in the other one looks exactly // like a world which is empty. LOGGER.info( - "Opening the anvil loader for region={} layout={} exists={} regionFiles={} dim={}", + "Opening the anvil loader for region={} layout={} exists={} regionFiles={} dim={} versionPolicy={}", this.regionDirectory, this.legacyLayout ? "legacy /region" : "dimension /dimensions///region", Files.isDirectory(this.regionDirectory), describeRegionFileCount(this.regionDirectory), - this.dimensionLabel + this.dimensionLabel, + this.versionPolicy == null ? "none" : this.versionPolicy.getClass().getName() ); } @@ -258,8 +275,11 @@ private void reportException(Throwable exception) { * Returns a builder for a loader whose defaults are those of the constructors. *

* The builder reaches the values the constructors set for themselves — the compression level, - * the diagnostics, both palette resolvers, the save parallelism and the data version. The world - * directory and the dimension are not among them: they are required, so they sit in + * the diagnostics, both palette resolvers, the save parallelism and the data version. It also + * defaults to discovering a {@link ChunkVersionPolicy} from the classpath, which is what keeps + * every loader built through a constructor rather than an explicit + * {@link Builder#versionPolicy(ChunkVersionPolicy)} call refusing the chunks it always refused. + * The world directory and the dimension are not among them: they are required, so they sit in * {@link Builder#build(Path, Key)} rather than in a slot. *

* @@ -269,7 +289,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, - DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null); + DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null, null, true); } /** @@ -298,7 +318,7 @@ public static Builder builder() { *

* * @author TheMeinerLP - * @version 1.1.0 + * @version 1.2.0 * @since 0.4.0 */ @ApiStatus.Experimental @@ -313,12 +333,16 @@ public static final class Builder { private final @Nullable PaletteEntryResolver blockResolver; private final @Nullable PaletteEntryResolver biomeResolver; private final @Nullable Consumer exceptionHandler; + private final @Nullable ChunkVersionPolicy versionPolicy; + private final boolean discoverVersionPolicy; private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, int dataVersion, int minimumDataVersion, @Nullable AnvilDiagnostics diagnostics, @Nullable PaletteEntryResolver blockResolver, @Nullable PaletteEntryResolver biomeResolver, - @Nullable Consumer exceptionHandler) { + @Nullable Consumer exceptionHandler, + @Nullable ChunkVersionPolicy versionPolicy, + boolean discoverVersionPolicy) { this.openRegionLimit = openRegionLimit; this.compressionLevel = compressionLevel; this.saveParallelism = saveParallelism; @@ -328,6 +352,8 @@ private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, this.blockResolver = blockResolver; this.biomeResolver = biomeResolver; this.exceptionHandler = exceptionHandler; + this.versionPolicy = versionPolicy; + this.discoverVersionPolicy = discoverVersionPolicy; } /** @@ -354,7 +380,9 @@ public Builder openRegionLimit(int openRegionLimit) { this.diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -385,7 +413,9 @@ public Builder compressionLevel(int compressionLevel) { this.diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -412,7 +442,9 @@ public Builder saveParallelism(int saveParallelism) { this.diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -436,7 +468,9 @@ public Builder dataVersion(int dataVersion) { this.diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -466,7 +500,9 @@ public Builder minimumDataVersion(int minimumDataVersion) { this.diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -495,7 +531,9 @@ public Builder diagnostics(AnvilDiagnostics diagnostics) { diagnostics, this.blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -521,7 +559,9 @@ public Builder blockResolver(PaletteEntryResolver blockResolver) { this.diagnostics, blockResolver, this.biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -544,7 +584,9 @@ public Builder biomeResolver(PaletteEntryResolver biomeResolver) { this.diagnostics, this.blockResolver, biomeResolver, - this.exceptionHandler); + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); } /** @@ -576,7 +618,74 @@ public Builder exceptionHandler(Consumer exceptionHandler) { this.diagnostics, this.blockResolver, this.biomeResolver, - exceptionHandler); + exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy); + } + + /** + * Sets the policy consulted before a chunk is decoded, or clears it so nothing is consulted. + *

+ * Calling this slot always turns discovery off, whatever value is passed: an explicit + * decision about the policy — including the explicit decision "none" — has to win over the + * classpath without {@link ServiceResolution#choose(Class, Object, boolean)} seeing both set + * at once and refusing to guess between them. A builder that never calls this slot keeps + * discovering the default instead, which is what {@link #builder()} starts with. + *

+ *

+ * Passing {@code null} is not "use the default": it is "check nothing". A pre-{@code + * 21w43a} chunk that would otherwise be refused loads as a chunk of air instead, exactly as + * this loader read one before {@link ChunkVersionPolicy} existed. That cost is deliberate — + * see {@code testWithoutAnyPolicyALegacyChunkIsNotChecked} in the loader's integration + * tests for the case that documents it. + *

+ * + * @param versionPolicy the policy to consult before a chunk is decoded, or null to consult + * none + * @return a new builder with this value + * @since 1.2.0 + */ + @Contract(value = "_ -> new", pure = true) + public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + this.minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler, + versionPolicy, + false); + } + + /** + * Asks the loader to discover its {@link ChunkVersionPolicy} from the classpath instead of + * using an explicit instance. + *

+ * This is what {@link #builder()} already defaults to, so calling it only matters after an + * earlier {@link #versionPolicy(ChunkVersionPolicy)} call on the same chain, to undo it. A + * classpath that registers more than one provider makes {@link Builder#build(Path, Key)} + * throw {@link IllegalStateException} rather than guess between them. + *

+ * + * @return a new builder with this value + * @since 1.2.0 + */ + @Contract(value = "-> new", pure = true) + public Builder discoverVersionPolicy() { + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + this.minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler, + null, + true); } /** @@ -706,7 +815,9 @@ private record ResolvedRegionDirectory(Path directory, boolean legacyLayout) { } CompoundBinaryTag data = TAG_READER.read(new ByteArrayInputStream(raw.decompress()), BinaryTagIO.Compression.NONE); - requireReadableVersion(data); + if (this.versionPolicy != null) { + checkVersion(data); + } String status = chunkStatus(data); if (!isFullyGenerated(status)) { @@ -1411,61 +1522,54 @@ public int openRegionCount() { } /** - * Refuses a chunk which comes from a version this loader cannot read. + * Consults {@link #versionPolicy} about a chunk, and reports and counts a refusal before it + * reaches the caller. *

- * 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". + * The policy only decides and throws; it does not know about {@link AnvilDiagnostics} or the + * logger, on purpose — see {@link ChunkVersionPolicy}. Counting and logging the refusal is + * therefore the loader's job, done here rather than duplicated at every call site, and done + * only when {@link #versionPolicy} is not null: the caller at {@link #loadChunk(Instance, int, + * int)} already guards the call for that reason. *

* * @param data the root compound of the chunk - * @throws ChunkDataException if the chunk cannot be read + * @throws ChunkDataException if the policy refuses the chunk */ - 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" - ); + private void checkVersion(CompoundBinaryTag data) throws ChunkDataException { + try { + this.versionPolicy.check(data, this.minimumDataVersion); + } catch (ChunkDataException failure) { + String reported = reportedDataVersion(data); + + if (this.diagnostics.reportUnsupportedChunkVersion(reported)) { + LOGGER.warn( + "Refusing a chunk from data version {} in {}: {}", + reported, this.regionDirectory, failure.getMessage() + ); + } + throw failure; } + } - 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" - ); + /** + * Renders the stored {@code DataVersion} of a chunk the way the version breakdown of + * {@link AnvilDiagnostics} groups it: by the number it holds, or by + * {@link AnvilDiagnostics#UNKNOWN_DATA_VERSION} for a chunk which stores none. + *

+ * This mirrors only the presentation the guard used before it became a policy, not its + * decision: a key that is present but not a number renders as {@code "-1"} here exactly as it + * always did, because {@link NbtReads#optionalInteger(CompoundBinaryTag, String, int)} falls + * back to that default for a mistyped value the same way it does for a missing one. + *

+ * + * @param data the root compound of the chunk + * @return the data version to report, or {@link AnvilDiagnostics#UNKNOWN_DATA_VERSION} + */ + @Contract(pure = true) + private static String reportedDataVersion(CompoundBinaryTag data) { + return data.get(DATA_VERSION_KEY) == null + ? AnvilDiagnostics.UNKNOWN_DATA_VERSION + : Integer.toString(NbtReads.optionalInteger(data, DATA_VERSION_KEY, -1)); } /** diff --git a/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy b/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy new file mode 100644 index 0000000..67b2e89 --- /dev/null +++ b/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy @@ -0,0 +1 @@ +net.onelitefeather.falco.anvil.DefaultChunkVersionPolicy diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java new file mode 100644 index 0000000..ebeada7 --- /dev/null +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java @@ -0,0 +1,50 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import net.kyori.adventure.nbt.ListBinaryTag; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Tests {@link DefaultChunkVersionPolicy} directly, against chunk data built by hand rather than + * through the loader. What used to be the loader's private {@code requireReadableVersion} guard now + * lives here, unchanged in its decision logic and reachable without a running Minestom environment. + * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +class ChunkVersionPolicyTest { + + @Test + void testTheDefaultPolicyRefusesALevelLayout() { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .build(); + + ChunkDataException failure = assertThrows(ChunkDataException.class, + () -> new DefaultChunkVersionPolicy().check(legacy, 2844)); + assertEquals(ChunkDataException.Reason.UNSUPPORTED_CHUNK_VERSION, failure.reason()); + } + + @Test + void testTheDefaultPolicyAcceptsAChunkWithoutAStoredVersion() throws Exception { + CompoundBinaryTag toolWritten = CompoundBinaryTag.builder() + .put("sections", ListBinaryTag.empty()) + .build(); + + new DefaultChunkVersionPolicy().check(toolWritten, 2844); + } + + @Test + void testTheDefaultPolicyRefusesAMistypedVersion() { + CompoundBinaryTag broken = CompoundBinaryTag.builder() + .putString("DataVersion", "not-a-number") + .put("sections", ListBinaryTag.empty()) + .build(); + + assertThrows(ChunkDataException.class, () -> new DefaultChunkVersionPolicy().check(broken, 2844)); + } +} 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 6a3d097..10bb7ab 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 @@ -784,6 +784,48 @@ void testASectionsKeyStoredAsTheWrongTypeWithLevelIsRefused(Env env) throws Exce } } + @Test + void testAPolicyThatAllowsEverythingLetsALegacyChunkThrough(Env env) throws Exception { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .putString("Status", "minecraft:full") + .build(); + writeRawChunk(11, 11, legacy); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy((data, minimum) -> { }) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 11, 11)); + } + } + + /** + * Documents the cost of the loader's default: without any {@link ChunkVersionPolicy}, nothing + * checks a chunk's data version at all, so a pre-{@code 21w43a} chunk which stores its data + * under {@code Level} is not refused. It loads exactly as it did before this loader ever gained + * a version guard — as a chunk of air, because {@code sections} is absent from the root. This is + * a deliberate choice of the project, not a regression: a caller has to opt out of the check + * explicitly, with {@code versionPolicy(null)}, to reach this behaviour. + */ + @Test + void testWithoutAnyPolicyALegacyChunkIsNotChecked(Env env) throws Exception { + CompoundBinaryTag legacy = CompoundBinaryTag.builder() + .put("Level", CompoundBinaryTag.builder().put("Sections", ListBinaryTag.empty()).build()) + .putString("Status", "minecraft:full") + .build(); + writeRawChunk(12, 12, legacy); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy(null) + .build(this.worldRoot, OVERWORLD)) { + Instance instance = env.createEmptyInstance(loader); + + assertNotNull(loader.loadChunk(instance, 12, 12)); + } + } + @Test void testUnloadingAForeignChunkIsIgnored(Env env) throws Exception { Instance instance = env.createEmptyInstance(loader()); From b32c4ba8c6a2add5b996602b1ad457453e104484 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 13:43:27 +0200 Subject: [PATCH 04/18] fix(anvil): let a foreign version policy win over the shipped default Review of the version-guard service extraction found the discovery path was unusable for any third party: falco-anvil registers its own DefaultChunkVersionPolicy, so a caller who registered a policy the documented way always hit "two providers" and got refused. Teaches ServiceResolution.discover/choose an optional shippedDefault class that steps aside for any single foreign provider; two foreign providers still refuse each other, so "no silent choice between undocumented opinions" still holds. Also: ServiceLoader.load now pins the service's own classloader instead of trusting the thread-context one, which is not reliable across a CloudNet/extension/plugin boundary; documents the new IllegalStateException on the three public entry points that can now throw it; adds the pass-through tests the two new builder setters were missing; and corrects a wrong test count in the task report. --- .../falco/anvil/FalcoAnvilLoader.java | 54 +++++++++--- .../falco/anvil/ServiceResolution.java | 74 +++++++++++++++- .../falco/anvil/ChunkVersionPolicyTest.java | 8 +- .../anvil/FalcoAnvilLoaderBuilderTest.java | 32 +++++++ .../falco/anvil/ServiceResolutionTest.java | 88 ++++++++++++++++++- ...o.anvil.ServiceResolutionTest$DefaultAware | 2 + ...eResolutionTest$DefaultAwareWithTwoForeign | 3 + 7 files changed, 241 insertions(+), 20 deletions(-) create mode 100644 falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAware create mode 100644 falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAwareWithTwoForeign 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 ee0b97b..b41daa1 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 @@ -132,12 +132,15 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { * The policy consulted before a chunk is decoded, or null to skip that check entirely. *

* Resolved once, in the constructor, through {@link ServiceResolution#choose(Class, Object, - * boolean)}. A builder which never touches {@link Builder#versionPolicy(ChunkVersionPolicy)} or - * {@link Builder#discoverVersionPolicy()} discovers {@link DefaultChunkVersionPolicy} through the - * classpath by default, which is what keeps every loader built the way earlier versions built - * one refusing the same chunks it always refused. Calling {@code versionPolicy(null)} is the - * only way to leave this field null, and {@link #checkVersion(CompoundBinaryTag)} treats that as - * "check nothing" rather than substituting the default itself. + * boolean, Class)}, naming {@link DefaultChunkVersionPolicy} as the shipped default — so a + * foreign {@link ChunkVersionPolicy} registered through {@code META-INF/services} is chosen over + * it, and only a second foreign provider is refused as ambiguous. A builder which never touches + * {@link Builder#versionPolicy(ChunkVersionPolicy)} or {@link Builder#discoverVersionPolicy()} + * discovers a policy through the classpath by default, which is what keeps every loader built + * the way earlier versions built one refusing the same chunks it always refused. Calling + * {@code versionPolicy(null)} is the only way to leave this field null, and + * {@link #checkVersion(CompoundBinaryTag)} treats that as "check nothing" rather than + * substituting the default itself. *

* * @since 1.2.0 @@ -172,6 +175,8 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { * * @param worldRoot the root directory of the world * @param dimension the key of the dimension the loader reads and writes + * @throws IllegalStateException if the classpath registers more than one foreign + * {@link ChunkVersionPolicy} */ public FalcoAnvilLoader(Path worldRoot, Key dimension) { this(worldRoot, dimension, DEFAULT_OPEN_REGION_LIMIT); @@ -190,6 +195,8 @@ public FalcoAnvilLoader(Path worldRoot, Key dimension) { * @param dimension the key of the dimension the loader reads and writes * @param openRegionLimit the amount of region files the loader keeps open * @throws IllegalArgumentException if the limit is not positive + * @throws IllegalStateException if the classpath registers more than one foreign + * {@link ChunkVersionPolicy} */ public FalcoAnvilLoader(Path worldRoot, Key dimension, int openRegionLimit) { this(worldRoot, dimension, builder().openRegionLimit(openRegionLimit)); @@ -234,7 +241,8 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { this.minimumDataVersion = settings.minimumDataVersion; this.exceptionHandler = settings.exceptionHandler; this.versionPolicy = ServiceResolution.choose( - ChunkVersionPolicy.class, settings.versionPolicy, settings.discoverVersionPolicy); + ChunkVersionPolicy.class, settings.versionPolicy, settings.discoverVersionPolicy, + DefaultChunkVersionPolicy.class); this.closeLock = new ReentrantLock(); // Which directory was chosen, and how many region files are in it, is the first thing @@ -628,9 +636,9 @@ public Builder exceptionHandler(Consumer exceptionHandler) { *

* Calling this slot always turns discovery off, whatever value is passed: an explicit * decision about the policy — including the explicit decision "none" — has to win over the - * classpath without {@link ServiceResolution#choose(Class, Object, boolean)} seeing both set - * at once and refusing to guess between them. A builder that never calls this slot keeps - * discovering the default instead, which is what {@link #builder()} starts with. + * classpath without {@link ServiceResolution#choose(Class, Object, boolean, Class)} seeing + * both set at once and refusing to guess between them. A builder that never calls this slot + * keeps discovering the default instead, which is what {@link #builder()} starts with. *

*

* Passing {@code null} is not "use the default": it is "check nothing". A pre-{@code @@ -665,9 +673,11 @@ public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { * using an explicit instance. *

* This is what {@link #builder()} already defaults to, so calling it only matters after an - * earlier {@link #versionPolicy(ChunkVersionPolicy)} call on the same chain, to undo it. A - * classpath that registers more than one provider makes {@link Builder#build(Path, Key)} - * throw {@link IllegalStateException} rather than guess between them. + * earlier {@link #versionPolicy(ChunkVersionPolicy)} call on the same chain, to undo it. The + * shipped {@link DefaultChunkVersionPolicy} steps aside for a single registered foreign + * provider; a classpath that registers more than one foreign provider makes + * {@link Builder#build(Path, Key)} throw {@link IllegalStateException} rather than guess + * between them. *

* * @return a new builder with this value @@ -699,6 +709,9 @@ public Builder discoverVersionPolicy() { * @param worldRoot the root directory of the world * @param dimension the key of the dimension the loader reads and writes * @return a new loader, independent of every other loader from this builder + * @throws IllegalStateException if this builder was left on classpath discovery and the + * classpath registers more than one foreign + * {@link ChunkVersionPolicy} */ @Contract(value = "_, _ -> new", pure = true) public FalcoAnvilLoader build(Path worldRoot, Key dimension) { @@ -1179,6 +1192,21 @@ int minimumDataVersion() { return this.minimumDataVersion; } + /** + * Returns the policy this loader resolved, or null if it checks nothing. + *

+ * Package-private for the same reason as {@link #minimumDataVersion()}: this exists for the + * builder's own pass-through tests, not for a caller outside this package. + *

+ * + * @return the resolved policy, or null if the loader checks no chunk version at all + * @since 1.2.0 + */ + @Contract(pure = true) + @Nullable ChunkVersionPolicy versionPolicy() { + return this.versionPolicy; + } + /** * Closes every region file the loader opened and reports a summary of its work. *

diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java index 6592233..b6c28ca 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java @@ -14,11 +14,13 @@ * A caller either configures an instance explicitly, or asks for classpath discovery, but not both * at once: {@link #choose(Class, Object, boolean)} refuses the combination outright rather than * silently preferring one side. Discovery itself, in {@link #discover(Class)}, refuses to guess - * between more than one registered provider. + * between more than one registered provider — except that a module's own shipped default, named + * through {@link #discover(Class, Class)}, always steps aside for a foreign one rather than + * counting as a second vote. Two foreign providers still refuse each other. *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 1.0.0 */ final class ServiceResolution { @@ -35,12 +37,56 @@ private ServiceResolution() { * @throws IllegalStateException if more than one provider is registered */ static @Nullable T discover(Class service) { + return discover(service, null); + } + + /** + * Finds the single provider of the given service on the classpath, letting a module's own + * shipped default step aside for a foreign one instead of counting as a competing vote. + *

+ * This is not the "no silent choice between two providers" rule weakening: a module's own + * default is a known quantity, registered by the module itself, not a second undocumented + * opinion on the classpath. A caller who registers one foreign provider has stated an + * unambiguous intent, and that intent should not be refused just because the module also ships + * its own fallback. Two foreign providers are a different situation — neither of them + * is the known default, so the classpath genuinely holds two competing, undocumented opinions, + * and this still refuses to guess between them exactly as {@link #discover(Class)} does. + *

+ * + * @param service the service interface + * @param shippedDefault the class of the module's own default implementation, which yields to + * any other registered provider, or null if the service has no such + * default + * @param the service type + * @return the provider, or null if the classpath carries none + * @throws IllegalStateException if more than one provider other than {@code shippedDefault} is + * registered + * @since 1.2.0 + */ + static @Nullable T discover(Class service, @Nullable Class shippedDefault) { List providers = new ArrayList<>(); - ServiceLoader.load(service).forEach(providers::add); + // The thread's context classloader is not a safe bet here: in a CloudNet, extension or + // plugin classloader environment it may not see this jar at all, which would make discovery + // silently resolve to nothing instead of finding the provider that is right there on this + // module's own classloader. The service interface and its providers live in the same + // module, so that module's classloader is what has to be asked, not whatever classloader + // happens to be current on the calling thread. + ServiceLoader.load(service, service.getClassLoader()).forEach(providers::add); if (providers.isEmpty()) { return null; } + + if (shippedDefault != null) { + List foreign = providers.stream() + .filter(provider -> !shippedDefault.equals(provider.getClass())) + .toList(); + + if (!foreign.isEmpty()) { + providers = foreign; + } + } + if (providers.size() > 1) { // Naming them is the whole value of this branch: "several providers" sends the reader // to the classpath, the two class names send them to the jar that should not be there. @@ -64,6 +110,26 @@ private ServiceResolution() { * @throws IllegalStateException if the caller asked for both, or if discovery is ambiguous */ static @Nullable T choose(Class service, @Nullable T explicit, boolean discover) { + return choose(service, explicit, discover, null); + } + + /** + * Chooses between an explicitly configured instance and classpath discovery, letting discovery + * treat a module's own shipped default as stepping aside for a foreign provider. + * + * @param service the service interface + * @param explicit the instance the caller configured, or null + * @param discover whether the caller asked for discovery + * @param shippedDefault the class of the module's own default implementation, passed on to + * {@link #discover(Class, Class)}, or null if the service has no such + * default + * @param the service type + * @return the chosen provider, or null if the caller asked for neither + * @throws IllegalStateException if the caller asked for both, or if discovery is ambiguous + * @since 1.2.0 + */ + static @Nullable T choose(Class service, @Nullable T explicit, boolean discover, + @Nullable Class shippedDefault) { if (explicit != null && discover) { throw new IllegalStateException( "An explicit " + service.getSimpleName() + " and discovery were both configured. " @@ -73,6 +139,6 @@ private ServiceResolution() { if (explicit != null) { return explicit; } - return discover ? discover(service) : null; + return discover ? discover(service, shippedDefault) : null; } } diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java index ebeada7..c42012e 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Tests {@link DefaultChunkVersionPolicy} directly, against chunk data built by hand rather than @@ -45,6 +46,11 @@ void testTheDefaultPolicyRefusesAMistypedVersion() { .put("sections", ListBinaryTag.empty()) .build(); - assertThrows(ChunkDataException.class, () -> new DefaultChunkVersionPolicy().check(broken, 2844)); + ChunkDataException failure = assertThrows(ChunkDataException.class, + () -> new DefaultChunkVersionPolicy().check(broken, 2844)); + // Pins the distinction the message is for, not its exact wording: a mistyped DataVersion is + // not the same failure as a DataVersion that is simply too old, and the loader's log line now + // relays this message instead of recomputing that distinction itself. + assertTrue(failure.getMessage().contains("number"), failure.getMessage()); } } 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 e18eca6..f339bb2 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 @@ -20,6 +20,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; @@ -319,6 +320,37 @@ void testTheMinimumDataVersionSurvivesEveryOtherSetter(@TempDir Path worldRoot) } } + @Test + void testAnExplicitVersionPolicySurvivesEveryOtherSetter(@TempDir Path worldRoot) throws Exception { + ChunkVersionPolicy policy = (data, minimum) -> { + }; + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy(policy) + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, OVERWORLD)) { + + assertSame(policy, loader.versionPolicy()); + } + } + + @Test + void testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne(@TempDir Path worldRoot) throws Exception { + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .versionPolicy((data, minimum) -> { + }) + .discoverVersionPolicy() + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, OVERWORLD)) { + + assertInstanceOf(DefaultChunkVersionPolicy.class, loader.versionPolicy()); + } + } + @Test void testTheBuilderCanBeReusedAfterASlotChanged() throws Exception { FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder().openRegionLimit(8); diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java index 984e571..abb2737 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java @@ -13,12 +13,13 @@ * instance relates to that discovery. *

* The dummy providers this test resolves are registered through the test resources, under - * {@code META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy}, so the main + * {@code META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$Dummy} and two more + * files named after {@link DefaultAware} and {@link DefaultAwareWithTwoForeign} below, so the main * module registers nothing extra. *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 1.0.0 */ class ServiceResolutionTest { @@ -50,6 +51,73 @@ public String name() { interface Absent { } + /** + * A service with a "shipped default" registered next to a single foreign provider, for the two + * {@link ServiceResolution#discover(Class, Class)} cases below. + */ + interface DefaultAware { + String name(); + } + + public static final class ShippedDefault implements DefaultAware { + public ShippedDefault() { + } + + @Override + public String name() { + return "shipped"; + } + } + + public static final class ForeignProvider implements DefaultAware { + public ForeignProvider() { + } + + @Override + public String name() { + return "foreign"; + } + } + + /** + * A separate service, registered with a shipped default and two foreign providers, so + * that "the default steps aside" and "two foreign providers still refuse each other" can be + * pinned down independently of one another. + */ + interface DefaultAwareWithTwoForeign { + String name(); + } + + public static final class AnotherShippedDefault implements DefaultAwareWithTwoForeign { + public AnotherShippedDefault() { + } + + @Override + public String name() { + return "shipped"; + } + } + + public static final class FirstForeignProvider implements DefaultAwareWithTwoForeign { + public FirstForeignProvider() { + } + + @Override + public String name() { + return "first-foreign"; + } + } + + public static final class SecondForeignProvider implements DefaultAwareWithTwoForeign { + public SecondForeignProvider() { + } + + @Override + public String name() { + return "second-foreign"; + } + } + @Test void testAServiceWithNoProviderResolvesToNothing() { assertNull(ServiceResolution.discover(Absent.class)); @@ -83,4 +151,20 @@ void testAnExplicitInstanceIsUsedWithoutTouchingTheClasspath() { void testNeitherExplicitNorDiscoveredResolvesToNothing() { assertNull(ServiceResolution.choose(Dummy.class, null, false)); } + + @Test + void testAForeignProviderWinsOverTheShippedDefault() { + DefaultAware resolved = ServiceResolution.discover(DefaultAware.class, ShippedDefault.class); + + assertEquals("foreign", resolved.name()); + } + + @Test + void testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered() { + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> ServiceResolution.discover(DefaultAwareWithTwoForeign.class, AnotherShippedDefault.class)); + + assertTrue(failure.getMessage().contains("FirstForeignProvider"), failure.getMessage()); + assertTrue(failure.getMessage().contains("SecondForeignProvider"), failure.getMessage()); + } } diff --git a/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAware b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAware new file mode 100644 index 0000000..2f4677d --- /dev/null +++ b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAware @@ -0,0 +1,2 @@ +net.onelitefeather.falco.anvil.ServiceResolutionTest$ShippedDefault +net.onelitefeather.falco.anvil.ServiceResolutionTest$ForeignProvider diff --git a/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAwareWithTwoForeign b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAwareWithTwoForeign new file mode 100644 index 0000000..e6193dc --- /dev/null +++ b/falco-anvil/src/test/resources/META-INF/services/net.onelitefeather.falco.anvil.ServiceResolutionTest$DefaultAwareWithTwoForeign @@ -0,0 +1,3 @@ +net.onelitefeather.falco.anvil.ServiceResolutionTest$AnotherShippedDefault +net.onelitefeather.falco.anvil.ServiceResolutionTest$FirstForeignProvider +net.onelitefeather.falco.anvil.ServiceResolutionTest$SecondForeignProvider From a7b9c041eaa83becca7c053a4baff3d4b96b4205 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 13:47:56 +0200 Subject: [PATCH 05/18] docs(spec): the shipped default has to step aside, or the seam is decoration The resolution rules as written made discovery useless: falco-anvil registers its own provider, so a third party taking the documented route would always produce two and always get the refusal. Discovery could never return anything but the default. Found by the review of the first implementation, fixed there, and now corrected here so the document does not keep teaching the version that does not work. Two rules gained along the way. Only foreign providers are counted for the ambiguity check, so the default never appears in the error message. And discovery loads with the service's own class loader rather than the thread context one - under CloudNet or extension class loaders the context loader may not see the jar, and discovery would then silently find nothing and put the air chunk back. Co-Authored-By: Claude Opus 5 (1M context) --- ...026-08-04-anvil-extension-points-design.md | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md index c123659..08d58df 100644 --- a/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md +++ b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md @@ -89,13 +89,25 @@ upgrade path an unmappable block means the mapping data is incomplete and must b ### Resolution rules, shared by all three services -The same three rules the migration spec sets out, applied uniformly: - -- **More than one provider throws**, naming them. Silent selection is how a world gets read under a - policy nobody chose. +Applied uniformly to every service here: + +- **The shipped default steps aside for a foreign provider.** `falco-anvil` registers its own + implementations, so without this rule a third party taking the documented route would *always* + produce two providers and always hit the refusal below. Discovery could never return anything but + the default, and the extension point would be decoration. A named default is therefore removed from + the candidate set before the candidates are counted. **Added after the first implementation review + found the point unusable as this document originally specified it.** +- **More than one *foreign* provider throws**, naming them — the default is not among the names, + because by then it is not a candidate. Silent selection between two foreign providers is how a + world gets read under a policy nobody chose. - **The builder slot and discovery are exclusive.** Setting both is a configuration error. - **A builder slot always wins over the classpath** when only it is used — that is what "explicit" - means. + means, and it short-circuits before the class path is consulted at all. +- **Discovery loads with the service's own class loader**, not the thread context loader. Under + CloudNet, extension or plugin class loaders the context loader may not see the `falco-anvil` jar; + discovery would then find nothing, resolve to no policy, and put the pre-21w43a air chunk back — + silently, which is the failure mode this whole line of work exists to end. The contract and its + shipped provider live in the same module, so that module's loader is the one to ask. Discovery happens once, when the loader is built, not per chunk. From 4767af302ed5bfce0e2b0b71b0f73604ea33da53 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 13:59:41 +0200 Subject: [PATCH 06/18] feat(anvil): let a caller decide what an unknown palette entry becomes --- .../falco/anvil/BiomePaletteResolver.java | 82 +++++--- .../falco/anvil/BlockPaletteResolver.java | 35 +++- .../anvil/DefaultUnknownEntryPolicy.java | 97 ++++++++++ .../falco/anvil/FalcoAnvilLoader.java | 183 ++++++++++++++++-- .../falco/anvil/UnknownEntryPolicy.java | 45 +++++ ...litefeather.falco.anvil.UnknownEntryPolicy | 1 + .../anvil/FalcoAnvilLoaderBuilderTest.java | 29 +++ .../falco/anvil/UnknownEntryPolicyTest.java | 59 ++++++ 8 files changed, 471 insertions(+), 60 deletions(-) create mode 100644 falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java create mode 100644 falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java create mode 100644 falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.UnknownEntryPolicy create mode 100644 falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java index 78c95f1..4198c63 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java @@ -16,9 +16,10 @@ * The {@link BiomePaletteResolver} class translates between the biome entries of the Anvil format * and the biome ids of the server registry. *

- * A biome the registry does not know is replaced with the plains biome instead of failing, which - * follows the behaviour of the built-in loader. Every replaced name is reported once through the - * diagnostics. + * A biome the registry does not know is handed to an {@link UnknownEntryPolicy} instead of failing + * outright: the default substitutes plains, which follows the behaviour of the built-in loader, but + * a caller that converts or checks a world can configure a policy which refuses it instead. Every + * unknown name is reported once through the diagnostics, regardless of what the policy does with it. *

*

* The registry is resolved on the first use instead of in a static initializer or in the @@ -32,7 +33,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -43,17 +44,31 @@ public final class BiomePaletteResolver implements PaletteEntryResolver { private static final String NAME_KEY = "Name"; private final AnvilDiagnostics diagnostics; + private final UnknownEntryPolicy policy; private final Supplier> registrySupplier; - private volatile @Nullable Registries resolved; + private volatile @Nullable DynamicRegistry resolvedRegistry; /** - * Creates a new resolver which uses the biome registry of the running server. + * Creates a new resolver which uses the biome registry of the running server and replaces an + * unknown biome following {@link DefaultUnknownEntryPolicy}. * * @param diagnostics the diagnostics which throttle the reports */ public BiomePaletteResolver(AnvilDiagnostics diagnostics) { - this(diagnostics, MinecraftServer::getBiomeRegistry); + this(diagnostics, new DefaultUnknownEntryPolicy(), MinecraftServer::getBiomeRegistry); + } + + /** + * Creates a new resolver which uses the biome registry of the running server and decides what + * an unknown biome becomes through the given policy. + * + * @param diagnostics the diagnostics which throttle the reports + * @param policy the policy consulted for a biome the registry does not know + * @since 1.2.0 + */ + public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy) { + this(diagnostics, policy, MinecraftServer::getBiomeRegistry); } /** @@ -68,62 +83,65 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics) { * @param registrySupplier the supplier which provides the registry of the known biomes */ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier> registrySupplier) { + this(diagnostics, new DefaultUnknownEntryPolicy(registrySupplier), registrySupplier); + } + + /** + * Creates a new resolver which uses the registry the given supplier provides and decides what an + * unknown biome becomes through the given policy. + * + * @param diagnostics the diagnostics which throttle the reports + * @param policy the policy consulted for a biome the registry does not know + * @param registrySupplier the supplier which provides the registry of the known biomes + * @since 1.2.0 + */ + public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy, + Supplier> registrySupplier) { this.diagnostics = diagnostics; + this.policy = policy; this.registrySupplier = registrySupplier; } /** * Returns the registry of this resolver and resolves it on the first call. * - * @return the registry and the id of the fallback biome + * @return the registry of the known biomes */ - private Registries registries() { - Registries current = this.resolved; + private DynamicRegistry registry() { + DynamicRegistry current = this.resolvedRegistry; if (current != null) { return current; } synchronized (this) { - Registries created = this.resolved; + DynamicRegistry created = this.resolvedRegistry; if (created == null) { - DynamicRegistry registry = this.registrySupplier.get(); - created = new Registries(registry, registry.getId(Biome.PLAINS)); - this.resolved = created; + created = this.registrySupplier.get(); + this.resolvedRegistry = created; } return created; } } - /** - * The {@link Registries} record holds the resolved registry together with the id of the biome - * which replaces an unknown one. - * - * @param registry the registry which holds the known biomes - * @param fallbackId the id of the biome which replaces an unknown one - * @author TheMeinerLP - * @version 1.0.0 - * @since 0.1.0 - */ - private record Registries(DynamicRegistry registry, int fallbackId) { - } - /** * {@inheritDoc} + * + * @throws AnvilChunkException if the configured policy refuses an unknown biome */ @Override public int toId(String name, @Nullable CompoundBinaryTag properties) { - Registries registries = registries(); - int id = registries.registry().getId(RegistryKey.unsafeOf(name)); + DynamicRegistry registry = registry(); + int id = registry.getId(RegistryKey.unsafeOf(name)); if (id != -1) { return id; } if (this.diagnostics.reportUnknownBiome(name)) { - LOGGER.warn("The biome '{}' is unknown and is replaced with plains, further chunks with it are not reported", name); + LOGGER.warn("The biome '{}' is unknown, further chunks with it are not reported", name); } - return registries.fallbackId(); + return this.policy.onUnknownBiome(name); } /** @@ -131,7 +149,7 @@ public int toId(String name, @Nullable CompoundBinaryTag properties) { */ @Override public CompoundBinaryTag toEntry(int id) { - RegistryKey key = registries().registry().getKey(id); + RegistryKey key = registry().getKey(id); String name = key == null ? Biome.PLAINS.key().asString() : key.key().asString(); return CompoundBinaryTag.builder().putString(NAME_KEY, name).build(); } diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java index ccf5705..fd80c54 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java @@ -16,10 +16,12 @@ * The {@link BlockPaletteResolver} class translates between the block entries of the Anvil format * and the block state ids of Minestom. *

- * An entry the server does not know is replaced with air instead of failing. A world can hold - * blocks of a mod or of a newer game version and rejecting the whole chunk over a single unknown - * block would lose far more data than it protects. Every replaced name is reported once through - * the diagnostics so the problem stays visible without flooding the log. + * An entry the server does not know is handed to an {@link UnknownEntryPolicy} instead of failing + * outright: the default substitutes air, which keeps a world holding blocks of a mod or of a newer + * game version loadable instead of losing a whole chunk over a single unknown block, but a caller + * that converts or checks a world can configure a policy which refuses it instead. Every unknown + * name is reported once through the diagnostics so the problem stays visible without flooding the + * log, regardless of what the policy does with it. *

* *

@@ -28,7 +30,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -40,18 +42,35 @@ public final class BlockPaletteResolver implements PaletteEntryResolver { private static final String PROPERTIES_KEY = "Properties"; private final AnvilDiagnostics diagnostics; + private final UnknownEntryPolicy policy; /** - * Creates a new resolver which reports unknown blocks to the given diagnostics. + * Creates a new resolver which reports unknown blocks to the given diagnostics and replaces + * them following {@link DefaultUnknownEntryPolicy}. * * @param diagnostics the diagnostics which throttle the reports */ public BlockPaletteResolver(AnvilDiagnostics diagnostics) { + this(diagnostics, new DefaultUnknownEntryPolicy()); + } + + /** + * Creates a new resolver which reports unknown blocks to the given diagnostics and decides what + * they become through the given policy. + * + * @param diagnostics the diagnostics which throttle the reports + * @param policy the policy consulted for a block the server does not know + * @since 1.2.0 + */ + public BlockPaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy) { this.diagnostics = diagnostics; + this.policy = policy; } /** * {@inheritDoc} + * + * @throws AnvilChunkException if the configured policy refuses an unknown block */ @Override public int toId(String name, @Nullable CompoundBinaryTag properties) { @@ -59,9 +78,9 @@ public int toId(String name, @Nullable CompoundBinaryTag properties) { if (block == null) { if (this.diagnostics.reportUnknownBlock(name)) { - LOGGER.warn("The block '{}' is unknown and is replaced with air, further chunks with it are not reported", name); + LOGGER.warn("The block '{}' is unknown, further chunks with it are not reported", name); } - return Block.AIR.stateId(); + return this.policy.onUnknownBlock(name, properties); } if (properties == null || properties.size() == 0) { return block.stateId(); diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java new file mode 100644 index 0000000..1eac789 --- /dev/null +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java @@ -0,0 +1,97 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import net.minestom.server.MinecraftServer; +import net.minestom.server.instance.block.Block; +import net.minestom.server.registry.DynamicRegistry; +import net.minestom.server.world.biome.Biome; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +import java.util.function.Supplier; + +/** + * The {@link UnknownEntryPolicy} a resolver falls back to when nothing else was configured: + * substitutes air for an unknown block and plains for an unknown biome, exactly the values + * {@link BlockPaletteResolver} and {@link BiomePaletteResolver} hard-coded before this policy + * existed. + *

+ * The plains id is resolved from the biome registry lazily rather than in the constructor, for the + * same reason {@link BiomePaletteResolver} resolves its registry lazily: a policy is often built + * while the server is still starting, and reading the registry too early would fail before the + * policy ever meets a chunk. + *

+ *

+ * This type is experimental, like everything else in this package. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public final class DefaultUnknownEntryPolicy implements UnknownEntryPolicy { + + private final Supplier> registrySupplier; + + private volatile @Nullable Integer resolvedFallbackBiomeId; + + /** + * Creates a new policy which resolves the plains id from the biome registry of the running + * server. + */ + public DefaultUnknownEntryPolicy() { + this(MinecraftServer::getBiomeRegistry); + } + + /** + * Creates a new policy which resolves the plains id from the registry the given supplier + * provides. + *

+ * The registry is resolved on the first use instead of in the constructor. A policy is often + * built while the server is still starting and reading the registry too early would fail before + * the policy ever meets a chunk. + *

+ * + * @param registrySupplier the supplier which provides the registry of the known biomes + */ + public DefaultUnknownEntryPolicy(Supplier> registrySupplier) { + this.registrySupplier = registrySupplier; + } + + /** + * {@inheritDoc} + *

+ * Always returns {@link Block#AIR}'s state id. + *

+ */ + @Override + public int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties) { + return Block.AIR.stateId(); + } + + /** + * {@inheritDoc} + *

+ * Always returns the id of {@link Biome#PLAINS} in the resolved registry. + *

+ */ + @Override + public int onUnknownBiome(String name) { + Integer current = this.resolvedFallbackBiomeId; + + if (current != null) { + return current; + } + + synchronized (this) { + Integer created = this.resolvedFallbackBiomeId; + + if (created == null) { + created = this.registrySupplier.get().getId(Biome.PLAINS); + this.resolvedFallbackBiomeId = created; + } + return created; + } + } +} 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 b41daa1..72b89ee 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 @@ -78,7 +78,7 @@ *

* * @author TheMeinerLP - * @version 1.2.0 + * @version 1.3.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -147,6 +147,25 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { */ private final @Nullable ChunkVersionPolicy versionPolicy; + /** + * The policy consulted for a palette entry the running server does not know, resolved once in + * the constructor the same way {@link #versionPolicy} is: through {@link + * ServiceResolution#choose(Class, Object, boolean, Class)}, naming {@link + * DefaultUnknownEntryPolicy} as the shipped default. + *

+ * Unlike {@link #versionPolicy}, this field is never null. A builder which never touches + * {@link Builder#unknownEntryPolicy(UnknownEntryPolicy)} or + * {@link Builder#discoverUnknownEntryPolicy()} discovers a policy from the classpath by default, + * falling back to {@link DefaultUnknownEntryPolicy} when the classpath registers none — there is + * no "consult nothing" state for this decision the way {@code versionPolicy(null)} lets a caller + * skip the version check entirely, because an id is always required for the loader to keep + * decoding. + *

+ * + * @since 1.2.0 + */ + private final UnknownEntryPolicy unknownEntryPolicy; + /** * Where failures are reported, or null for the exception manager of the running server. *

@@ -228,11 +247,17 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { this.legacyLayout = resolved.legacyLayout(); this.dimensionLabel = dimension.asString(); this.diagnostics = effective; + UnknownEntryPolicy resolvedUnknownEntryPolicy = ServiceResolution.choose( + UnknownEntryPolicy.class, settings.unknownEntryPolicy, settings.discoverUnknownEntryPolicy, + DefaultUnknownEntryPolicy.class); + this.unknownEntryPolicy = resolvedUnknownEntryPolicy == null + ? new DefaultUnknownEntryPolicy() + : resolvedUnknownEntryPolicy; this.blockResolver = settings.blockResolver == null - ? new BlockPaletteResolver(effective) + ? new BlockPaletteResolver(effective, this.unknownEntryPolicy) : settings.blockResolver; this.biomeResolver = settings.biomeResolver == null - ? new BiomePaletteResolver(effective) + ? new BiomePaletteResolver(effective, this.unknownEntryPolicy) : settings.biomeResolver; this.regions = new ConcurrentHashMap<>(); this.trackedChunks = new ConcurrentHashMap<>(); @@ -284,9 +309,10 @@ private void reportException(Throwable exception) { *

* The builder reaches the values the constructors set for themselves — the compression level, * the diagnostics, both palette resolvers, the save parallelism and the data version. It also - * defaults to discovering a {@link ChunkVersionPolicy} from the classpath, which is what keeps - * every loader built through a constructor rather than an explicit - * {@link Builder#versionPolicy(ChunkVersionPolicy)} call refusing the chunks it always refused. + * defaults to discovering a {@link ChunkVersionPolicy} and an {@link UnknownEntryPolicy} from + * the classpath, which is what keeps every loader built through a constructor rather than an + * explicit {@link Builder#versionPolicy(ChunkVersionPolicy)} or + * {@link Builder#unknownEntryPolicy(UnknownEntryPolicy)} call behaving the way it always did. * The world directory and the dimension are not among them: they are required, so they sit in * {@link Builder#build(Path, Key)} rather than in a slot. *

@@ -297,7 +323,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, - DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null, null, true); + DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null, null, true, null, true); } /** @@ -326,7 +352,7 @@ public static Builder builder() { *

* * @author TheMeinerLP - * @version 1.2.0 + * @version 1.3.0 * @since 0.4.0 */ @ApiStatus.Experimental @@ -343,6 +369,8 @@ public static final class Builder { private final @Nullable Consumer exceptionHandler; private final @Nullable ChunkVersionPolicy versionPolicy; private final boolean discoverVersionPolicy; + private final @Nullable UnknownEntryPolicy unknownEntryPolicy; + private final boolean discoverUnknownEntryPolicy; private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, int dataVersion, int minimumDataVersion, @Nullable AnvilDiagnostics diagnostics, @@ -350,7 +378,9 @@ private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, @Nullable PaletteEntryResolver biomeResolver, @Nullable Consumer exceptionHandler, @Nullable ChunkVersionPolicy versionPolicy, - boolean discoverVersionPolicy) { + boolean discoverVersionPolicy, + @Nullable UnknownEntryPolicy unknownEntryPolicy, + boolean discoverUnknownEntryPolicy) { this.openRegionLimit = openRegionLimit; this.compressionLevel = compressionLevel; this.saveParallelism = saveParallelism; @@ -362,6 +392,8 @@ private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, this.exceptionHandler = exceptionHandler; this.versionPolicy = versionPolicy; this.discoverVersionPolicy = discoverVersionPolicy; + this.unknownEntryPolicy = unknownEntryPolicy; + this.discoverUnknownEntryPolicy = discoverUnknownEntryPolicy; } /** @@ -390,7 +422,9 @@ public Builder openRegionLimit(int openRegionLimit) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -423,7 +457,9 @@ public Builder compressionLevel(int compressionLevel) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -452,7 +488,9 @@ public Builder saveParallelism(int saveParallelism) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -478,7 +516,9 @@ public Builder dataVersion(int dataVersion) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -510,7 +550,9 @@ public Builder minimumDataVersion(int minimumDataVersion) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -541,7 +583,9 @@ public Builder diagnostics(AnvilDiagnostics diagnostics) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -569,7 +613,9 @@ public Builder blockResolver(PaletteEntryResolver blockResolver) { this.biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -594,7 +640,9 @@ public Builder biomeResolver(PaletteEntryResolver biomeResolver) { biomeResolver, this.exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -628,7 +676,9 @@ public Builder exceptionHandler(Consumer exceptionHandler) { this.biomeResolver, exceptionHandler, this.versionPolicy, - this.discoverVersionPolicy); + this.discoverVersionPolicy, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -665,7 +715,9 @@ public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { this.biomeResolver, this.exceptionHandler, versionPolicy, - false); + false, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); } /** @@ -695,6 +747,81 @@ public Builder discoverVersionPolicy() { this.biomeResolver, this.exceptionHandler, null, + true, + this.unknownEntryPolicy, + this.discoverUnknownEntryPolicy); + } + + /** + * Sets the policy consulted for a palette entry the running server does not know, or clears + * it so the classpath default is used instead. + *

+ * Calling this slot always turns discovery off, whatever value is passed, for the same + * reason {@link #versionPolicy(ChunkVersionPolicy)} does: an explicit decision has to win + * over the classpath without {@link ServiceResolution#choose(Class, Object, boolean, Class)} + * seeing both set at once and refusing to guess between them. A builder that never calls + * this slot keeps discovering the default instead, which is what {@link #builder()} starts + * with. + *

+ *

+ * Unlike {@link #versionPolicy(ChunkVersionPolicy)}, passing {@code null} here is not "check + * nothing": {@link #build(Path, Key)} always resolves a usable policy, falling back to + * {@link DefaultUnknownEntryPolicy} when neither an explicit instance nor a foreign + * classpath provider is found, because a resolver always needs an id for the entry it could + * not otherwise decode. + *

+ * + * @param unknownEntryPolicy the policy to consult for an unknown palette entry, or null to + * fall back to the classpath default + * @return a new builder with this value + * @since 1.2.0 + */ + @Contract(value = "_ -> new", pure = true) + public Builder unknownEntryPolicy(@Nullable UnknownEntryPolicy unknownEntryPolicy) { + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + this.minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy, + unknownEntryPolicy, + false); + } + + /** + * Asks the loader to discover its {@link UnknownEntryPolicy} from the classpath instead of + * using an explicit instance. + *

+ * This is what {@link #builder()} already defaults to, so calling it only matters after an + * earlier {@link #unknownEntryPolicy(UnknownEntryPolicy)} call on the same chain, to undo it. + * The shipped {@link DefaultUnknownEntryPolicy} steps aside for a single registered foreign + * provider; a classpath that registers more than one foreign provider makes + * {@link Builder#build(Path, Key)} throw {@link IllegalStateException} rather than guess + * between them. + *

+ * + * @return a new builder with this value + * @since 1.2.0 + */ + @Contract(value = "-> new", pure = true) + public Builder discoverUnknownEntryPolicy() { + return new Builder(this.openRegionLimit, + this.compressionLevel, + this.saveParallelism, + this.dataVersion, + this.minimumDataVersion, + this.diagnostics, + this.blockResolver, + this.biomeResolver, + this.exceptionHandler, + this.versionPolicy, + this.discoverVersionPolicy, + null, true); } @@ -711,7 +838,8 @@ public Builder discoverVersionPolicy() { * @return a new loader, independent of every other loader from this builder * @throws IllegalStateException if this builder was left on classpath discovery and the * classpath registers more than one foreign - * {@link ChunkVersionPolicy} + * {@link ChunkVersionPolicy} or more than one foreign + * {@link UnknownEntryPolicy} */ @Contract(value = "_, _ -> new", pure = true) public FalcoAnvilLoader build(Path worldRoot, Key dimension) { @@ -1207,6 +1335,21 @@ int minimumDataVersion() { return this.versionPolicy; } + /** + * Returns the policy this loader resolved for an unknown palette entry. + *

+ * Package-private for the same reason as {@link #versionPolicy()}: this exists for the + * builder's own pass-through tests, not for a caller outside this package. + *

+ * + * @return the resolved policy, never null + * @since 1.2.0 + */ + @Contract(pure = true) + UnknownEntryPolicy unknownEntryPolicy() { + return this.unknownEntryPolicy; + } + /** * Closes every region file the loader opened and reports a summary of its work. *

diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java new file mode 100644 index 0000000..c33beb7 --- /dev/null +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java @@ -0,0 +1,45 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +/** + * Decides what becomes of a palette entry the running server does not know. + *

+ * Returning an id substitutes it; throwing {@link AnvilChunkException} fails the chunk. Substituting + * is right for a server that wants a world to stay loadable and wrong for a tool that converts one, + * which is why this is a policy and not a constant. + *

+ *

+ * A policy only decides. It does not count and it does not log: the resolver which consults it keeps + * reporting the name to its {@link AnvilDiagnostics} and writing the log line regardless of what the + * policy does with the entry, so a substituting run stays as visible as a refusing one. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +@ApiStatus.Experimental +public interface UnknownEntryPolicy { + + /** + * Decides what an unknown block becomes. + * + * @param name the block name stored in the palette + * @param properties the stored properties, or null if the entry carries none + * @return the state id to use instead + * @throws AnvilChunkException if the chunk should fail rather than carry a substitute + */ + int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties); + + /** + * Decides what an unknown biome becomes. + * + * @param name the biome name stored in the palette + * @return the id to use instead + * @throws AnvilChunkException if the chunk should fail rather than carry a substitute + */ + int onUnknownBiome(String name); +} diff --git a/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.UnknownEntryPolicy b/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.UnknownEntryPolicy new file mode 100644 index 0000000..1d14ce4 --- /dev/null +++ b/falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.UnknownEntryPolicy @@ -0,0 +1 @@ +net.onelitefeather.falco.anvil.DefaultUnknownEntryPolicy 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 f339bb2..ffe1ebf 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 @@ -351,6 +351,35 @@ void testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne } } + @Test + void testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter(@TempDir Path worldRoot) throws Exception { + UnknownEntryPolicy policy = new DefaultUnknownEntryPolicy(); + + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .unknownEntryPolicy(policy) + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, OVERWORLD)) { + + assertSame(policy, loader.unknownEntryPolicy()); + } + } + + @Test + void testDiscoverUnknownEntryPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne(@TempDir Path worldRoot) throws Exception { + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .unknownEntryPolicy(new DefaultUnknownEntryPolicy()) + .discoverUnknownEntryPolicy() + .openRegionLimit(4) + .compressionLevel(3) + .saveParallelism(2) + .build(worldRoot, OVERWORLD)) { + + assertInstanceOf(DefaultUnknownEntryPolicy.class, loader.unknownEntryPolicy()); + } + } + @Test void testTheBuilderCanBeReusedAfterASlotChanged() throws Exception { FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder().openRegionLimit(8); diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java new file mode 100644 index 0000000..14c70d2 --- /dev/null +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java @@ -0,0 +1,59 @@ +package net.onelitefeather.falco.anvil; + +import net.kyori.adventure.nbt.CompoundBinaryTag; +import net.minestom.server.instance.block.Block; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests {@link DefaultUnknownEntryPolicy} directly and pins how {@link BlockPaletteResolver} uses + * whichever {@link UnknownEntryPolicy} it was built with. + *

+ * The third case is the one that matters most: counting an unknown entry is the resolver's job, not + * the policy's, and it has to keep happening even when the policy substitutes instead of throwing. + * Losing it would make a substituting run silent again, which is exactly what this whole extension + * point exists to prevent. + *

+ * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.2.0 + */ +class UnknownEntryPolicyTest { + + @Test + void testTheDefaultPolicyReplacesAnUnknownBlockWithAir() { + assertEquals(Block.AIR.stateId(), new DefaultUnknownEntryPolicy().onUnknownBlock("falco:nope", null)); + } + + @Test + void testARefusingPolicyFailsTheChunkInsteadOfSubstituting() { + UnknownEntryPolicy refusing = new UnknownEntryPolicy() { + @Override + public int onUnknownBlock(String name, CompoundBinaryTag properties) { + throw new AnvilChunkException("The block " + name + " has no mapping"); + } + + @Override + public int onUnknownBiome(String name) { + throw new AnvilChunkException("The biome " + name + " has no mapping"); + } + }; + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> new BlockPaletteResolver(new AnvilDiagnostics(), refusing).toId("falco:nope", null)); + assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); + } + + @Test + void testTheResolverStillCountsWhenThePolicySubstitutes() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + new BlockPaletteResolver(diagnostics, new DefaultUnknownEntryPolicy()).toId("falco:nope", null); + + assertEquals(1, diagnostics.unknownBlockCount()); + } +} From c8d73cb9396b20355c9ae7823c31b8b5b4d3f969 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 14:19:39 +0200 Subject: [PATCH 07/18] fix(anvil): address review findings on the unknown-entry policy - PaletteEntryResolver.toId's Javadoc now describes the contract this task's own change created: an implementation is allowed to fail, unchecked, instead of unconditionally promising a substitute. - Add the biome mirror of every UnknownEntryPolicyTest case (BiomePaletteResolver had none before this). - Narrow BiomePaletteResolver's three-argument constructor to package-private; it has no production caller and testability alone does not justify public API. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../falco/anvil/BiomePaletteResolver.java | 20 ++++++- .../anvil/DefaultUnknownEntryPolicy.java | 5 ++ .../falco/anvil/PaletteEntryResolver.java | 17 ++++-- .../falco/anvil/UnknownEntryPolicyTest.java | 60 ++++++++++++++++--- 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java index 4198c63..10798b9 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java @@ -89,14 +89,23 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier + * Package-private on purpose: nothing in this module needs both a custom policy and a custom + * registry at once, and there is no reason to promise that combination as public API before a + * caller actually needs it. It exists for this package's own tests, which use it to exercise + * {@link #toId(String, CompoundBinaryTag)} against a fake registry without a running server, + * exactly the way {@link DefaultUnknownEntryPolicy}'s own two-argument constructor lets its + * biome fallback be tested — see the note on {@link #registry()} for why the two lazy + * resolutions are not shared. + *

* * @param diagnostics the diagnostics which throttle the reports * @param policy the policy consulted for a biome the registry does not know * @param registrySupplier the supplier which provides the registry of the known biomes * @since 1.2.0 */ - public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy, - Supplier> registrySupplier) { + BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy, + Supplier> registrySupplier) { this.diagnostics = diagnostics; this.policy = policy; this.registrySupplier = registrySupplier; @@ -104,6 +113,13 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy pol /** * Returns the registry of this resolver and resolves it on the first call. + *

+ * This is the same lazy, double-checked-locking derivation {@link DefaultUnknownEntryPolicy} uses + * to resolve its own plains fallback from the same kind of supplier. The two are not shared: a + * shared holder would need a third class injected into both, for roughly ten lines saved, and + * this resolver's copy also has to hand the result to {@link #toEntry(int)} for the id-to-name + * direction, which the policy has no reason to do. + *

* * @return the registry of the known biomes */ diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java index 1eac789..e471dc4 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java @@ -84,6 +84,11 @@ public int onUnknownBiome(String name) { return current; } + // The same lazy, double-checked-locking derivation BiomePaletteResolver#registry() uses for + // its own resolution from the same kind of supplier, kept as its own copy rather than a + // shared holder: a shared holder would need a third class injected into both for roughly ten + // lines saved, and this method only ever needs an id, never the registry object itself the + // way the resolver does for its id-to-name direction. synchronized (this) { Integer created = this.resolvedFallbackBiomeId; diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/PaletteEntryResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/PaletteEntryResolver.java index b78a4c3..11384fe 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/PaletteEntryResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/PaletteEntryResolver.java @@ -20,7 +20,7 @@ *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -29,15 +29,22 @@ public interface PaletteEntryResolver { /** * Resolves the id which belongs to the given palette entry. *

- * An implementation must not fail for an unknown name. A world can hold entries of a mod or of - * a newer game version and losing a whole chunk over a single unknown entry would destroy more - * data than it protects. An implementation is expected to return a replacement id instead and - * to report the name to the caller. + * An implementation is allowed to fail for an unknown name — unchecked, since this method + * declares no {@code throws} clause. {@link BlockPaletteResolver} and + * {@link BiomePaletteResolver} both delegate that decision to a caller-supplied + * {@link UnknownEntryPolicy} rather than deciding it themselves: the shipped default + * substitutes a replacement id, which keeps a world holding entries of a mod or of a newer game + * version loadable instead of losing a whole chunk over a single unknown entry, but a policy + * configured to refuse instead throws {@link AnvilChunkException} from here. A caller of + * {@code toId} therefore has to be ready for either outcome, depending on how the resolver it + * holds was configured. *

* * @param name the name of the palette entry * @param properties the properties of the palette entry or null if it carries none * @return the id which belongs to the entry + * @throws AnvilChunkException if the implementation was configured to refuse an unknown name + * instead of substituting one */ int toId(String name, @Nullable CompoundBinaryTag properties); diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java index 14c70d2..b952ffd 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java @@ -2,6 +2,8 @@ import net.kyori.adventure.nbt.CompoundBinaryTag; import net.minestom.server.instance.block.Block; +import net.minestom.server.registry.DynamicRegistry; +import net.minestom.server.world.biome.Biome; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -9,17 +11,23 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Tests {@link DefaultUnknownEntryPolicy} directly and pins how {@link BlockPaletteResolver} uses - * whichever {@link UnknownEntryPolicy} it was built with. + * Tests {@link DefaultUnknownEntryPolicy} directly and pins how {@link BlockPaletteResolver} and + * {@link BiomePaletteResolver} each use whichever {@link UnknownEntryPolicy} they were built with. *

- * The third case is the one that matters most: counting an unknown entry is the resolver's job, not - * the policy's, and it has to keep happening even when the policy substitutes instead of throwing. - * Losing it would make a substituting run silent again, which is exactly what this whole extension - * point exists to prevent. + * The counting cases are the ones that matter most: counting an unknown entry is the resolver's job, + * not the policy's, and it has to keep happening even when the policy substitutes instead of + * throwing. Losing it would make a substituting run silent again, which is exactly what this whole + * extension point exists to prevent. + *

+ *

+ * The biome cases use {@link Biome#createDefaultRegistry()} rather than the biome registry of a + * running server, so this class needs no Minestom test environment: {@link BiomePaletteResolver}'s + * package-private three-argument constructor exists for exactly this, to inject both a policy and a + * registry supplier without starting a server. *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 1.2.0 */ class UnknownEntryPolicyTest { @@ -29,6 +37,14 @@ void testTheDefaultPolicyReplacesAnUnknownBlockWithAir() { assertEquals(Block.AIR.stateId(), new DefaultUnknownEntryPolicy().onUnknownBlock("falco:nope", null)); } + @Test + void testTheDefaultPolicyReplacesAnUnknownBiomeWithPlains() { + DynamicRegistry registry = Biome.createDefaultRegistry(); + + assertEquals(registry.getId(Biome.PLAINS), + new DefaultUnknownEntryPolicy(() -> registry).onUnknownBiome("falco:nope")); + } + @Test void testARefusingPolicyFailsTheChunkInsteadOfSubstituting() { UnknownEntryPolicy refusing = new UnknownEntryPolicy() { @@ -48,6 +64,26 @@ public int onUnknownBiome(String name) { assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); } + @Test + void testARefusingPolicyFailsTheChunkInsteadOfSubstitutingForABiome() { + UnknownEntryPolicy refusing = new UnknownEntryPolicy() { + @Override + public int onUnknownBlock(String name, CompoundBinaryTag properties) { + throw new AnvilChunkException("The block " + name + " has no mapping"); + } + + @Override + public int onUnknownBiome(String name) { + throw new AnvilChunkException("The biome " + name + " has no mapping"); + } + }; + + AnvilChunkException failure = assertThrows(AnvilChunkException.class, + () -> new BiomePaletteResolver(new AnvilDiagnostics(), refusing, Biome::createDefaultRegistry) + .toId("falco:nope", null)); + assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); + } + @Test void testTheResolverStillCountsWhenThePolicySubstitutes() { AnvilDiagnostics diagnostics = new AnvilDiagnostics(); @@ -56,4 +92,14 @@ void testTheResolverStillCountsWhenThePolicySubstitutes() { assertEquals(1, diagnostics.unknownBlockCount()); } + + @Test + void testTheResolverStillCountsWhenThePolicySubstitutesForABiome() { + AnvilDiagnostics diagnostics = new AnvilDiagnostics(); + + new BiomePaletteResolver(diagnostics, new DefaultUnknownEntryPolicy(Biome::createDefaultRegistry), + Biome::createDefaultRegistry).toId("falco:nope", null); + + assertEquals(1, diagnostics.unknownBiomeCount()); + } } From 1664dcddff0fa12754d57938a3a07a2ce91f50cd Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 14:37:32 +0200 Subject: [PATCH 08/18] docs(anvil): date the new members to the release they will actually land in #45 merged with a breaking-change marker, so release-please raised the next version to 2.0.0 (PR #46). Everything on this branch is additive on top of that, which makes it 2.1.0 - not the 1.2.0 the tags carried from when this branch was written against a 1.x line. Nineteen tags across ten files. @version tags are untouched: those count a class's own revisions, not the artefact's. Co-Authored-By: Claude Opus 5 (1M context) --- .../falco/anvil/BiomePaletteResolver.java | 4 ++-- .../falco/anvil/BlockPaletteResolver.java | 2 +- .../falco/anvil/ChunkVersionPolicy.java | 2 +- .../falco/anvil/DefaultChunkVersionPolicy.java | 2 +- .../falco/anvil/DefaultUnknownEntryPolicy.java | 2 +- .../falco/anvil/FalcoAnvilLoader.java | 16 ++++++++-------- .../falco/anvil/ServiceResolution.java | 4 ++-- .../falco/anvil/UnknownEntryPolicy.java | 2 +- .../falco/anvil/ChunkVersionPolicyTest.java | 2 +- .../falco/anvil/UnknownEntryPolicyTest.java | 2 +- 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java index 10798b9..b4ff272 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java @@ -65,7 +65,7 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics) { * * @param diagnostics the diagnostics which throttle the reports * @param policy the policy consulted for a biome the registry does not know - * @since 1.2.0 + * @since 2.1.0 */ public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy) { this(diagnostics, policy, MinecraftServer::getBiomeRegistry); @@ -102,7 +102,7 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier> registrySupplier) { diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java index fd80c54..6f11770 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java @@ -60,7 +60,7 @@ public BlockPaletteResolver(AnvilDiagnostics diagnostics) { * * @param diagnostics the diagnostics which throttle the reports * @param policy the policy consulted for a block the server does not know - * @since 1.2.0 + * @since 2.1.0 */ public BlockPaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy policy) { this.diagnostics = diagnostics; diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java index ccc09d6..6e3c569 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java @@ -13,7 +13,7 @@ * * @author TheMeinerLP * @version 1.0.0 - * @since 1.2.0 + * @since 2.1.0 */ @ApiStatus.Experimental public interface ChunkVersionPolicy { diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java index ea41922..e6cbae8 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultChunkVersionPolicy.java @@ -22,7 +22,7 @@ * * @author TheMeinerLP * @version 1.0.0 - * @since 1.2.0 + * @since 2.1.0 */ @ApiStatus.Experimental public final class DefaultChunkVersionPolicy implements ChunkVersionPolicy { diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java index e471dc4..62c4ded 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java @@ -27,7 +27,7 @@ * * @author TheMeinerLP * @version 1.0.0 - * @since 1.2.0 + * @since 2.1.0 */ @ApiStatus.Experimental public final class DefaultUnknownEntryPolicy implements UnknownEntryPolicy { 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 72b89ee..6c21333 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 @@ -143,7 +143,7 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { * substituting the default itself. *

* - * @since 1.2.0 + * @since 2.1.0 */ private final @Nullable ChunkVersionPolicy versionPolicy; @@ -162,7 +162,7 @@ public final class FalcoAnvilLoader implements ChunkLoader, AutoCloseable { * decoding. *

* - * @since 1.2.0 + * @since 2.1.0 */ private final UnknownEntryPolicy unknownEntryPolicy; @@ -701,7 +701,7 @@ public Builder exceptionHandler(Consumer exceptionHandler) { * @param versionPolicy the policy to consult before a chunk is decoded, or null to consult * none * @return a new builder with this value - * @since 1.2.0 + * @since 2.1.0 */ @Contract(value = "_ -> new", pure = true) public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { @@ -733,7 +733,7 @@ public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { *

* * @return a new builder with this value - * @since 1.2.0 + * @since 2.1.0 */ @Contract(value = "-> new", pure = true) public Builder discoverVersionPolicy() { @@ -774,7 +774,7 @@ public Builder discoverVersionPolicy() { * @param unknownEntryPolicy the policy to consult for an unknown palette entry, or null to * fall back to the classpath default * @return a new builder with this value - * @since 1.2.0 + * @since 2.1.0 */ @Contract(value = "_ -> new", pure = true) public Builder unknownEntryPolicy(@Nullable UnknownEntryPolicy unknownEntryPolicy) { @@ -806,7 +806,7 @@ public Builder unknownEntryPolicy(@Nullable UnknownEntryPolicy unknownEntryPolic *

* * @return a new builder with this value - * @since 1.2.0 + * @since 2.1.0 */ @Contract(value = "-> new", pure = true) public Builder discoverUnknownEntryPolicy() { @@ -1328,7 +1328,7 @@ int minimumDataVersion() { *

* * @return the resolved policy, or null if the loader checks no chunk version at all - * @since 1.2.0 + * @since 2.1.0 */ @Contract(pure = true) @Nullable ChunkVersionPolicy versionPolicy() { @@ -1343,7 +1343,7 @@ int minimumDataVersion() { *

* * @return the resolved policy, never null - * @since 1.2.0 + * @since 2.1.0 */ @Contract(pure = true) UnknownEntryPolicy unknownEntryPolicy() { diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java index b6c28ca..0d66d88 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ServiceResolution.java @@ -61,7 +61,7 @@ private ServiceResolution() { * @return the provider, or null if the classpath carries none * @throws IllegalStateException if more than one provider other than {@code shippedDefault} is * registered - * @since 1.2.0 + * @since 2.1.0 */ static @Nullable T discover(Class service, @Nullable Class shippedDefault) { List providers = new ArrayList<>(); @@ -126,7 +126,7 @@ private ServiceResolution() { * @param the service type * @return the chosen provider, or null if the caller asked for neither * @throws IllegalStateException if the caller asked for both, or if discovery is ambiguous - * @since 1.2.0 + * @since 2.1.0 */ static @Nullable T choose(Class service, @Nullable T explicit, boolean discover, @Nullable Class shippedDefault) { diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java index c33beb7..1b89e19 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java @@ -19,7 +19,7 @@ * * @author TheMeinerLP * @version 1.0.0 - * @since 1.2.0 + * @since 2.1.0 */ @ApiStatus.Experimental public interface UnknownEntryPolicy { diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java index c42012e..588710a 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ChunkVersionPolicyTest.java @@ -15,7 +15,7 @@ * * @author TheMeinerLP * @version 1.0.0 - * @since 1.2.0 + * @since 2.1.0 */ class ChunkVersionPolicyTest { diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java index b952ffd..325eecc 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java @@ -28,7 +28,7 @@ * * @author TheMeinerLP * @version 1.1.0 - * @since 1.2.0 + * @since 2.1.0 */ class UnknownEntryPolicyTest { From de206f741f8ec5493b541393c1ecad9ca112fea6 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 14:51:14 +0200 Subject: [PATCH 09/18] docs(anvil): record task 4 acceptance, including the archunit regression it found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six modules re-run with --rerun-tasks, both gate attacks re-executed and reverted, javadoc/japicmp re-verified. falco-anvil grew from 230 to 253 cases and every other module held; falco-archunit's ForeignCouplingTest now fails 4 cases because its allow-list regex was never extended for the new policy classes — reported here rather than fixed, per the acceptance's own instruction not to silently repair a defect found during acceptance. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../2026-08-04-anvil-extension-points.md | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/docs/superpowers/plans/2026-08-04-anvil-extension-points.md b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md index ce69fac..d294502 100644 --- a/docs/superpowers/plans/2026-08-04-anvil-extension-points.md +++ b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md @@ -629,3 +629,232 @@ wrong call into a test. Interfaces block, its code, and the consumers in Tasks 2 and 3. `ChunkVersionPolicy.check` takes `(CompoundBinaryTag, int)` everywhere. `UnknownEntryPolicy` has exactly the two methods named in the spec, both unchecked. `@since 1.2.0` throughout, because #45 already took the module to 1.1.0. + +## Result + +Acceptance run against `1664dcdd` (branch tip before this section), worktree +`/mnt/projects/oss/onelitefeather/Falco-worktrees/anvil-extension-points`, base `1bdd0cca` (#45, +merged into `origin/main`). The branch had already been rebased onto `origin/main` and the 19 `@since` +tags redated to `2.1.0` before this acceptance began (`1664dcdd`); both were re-verified below rather +than taken on faith. + +### `@since`/`@version` check (redone, not assumed) + +`grep -rn "@since 1.2.0" falco-anvil/src/main` returns nothing — no stale tag survived the rebase. +`grep -c "@since 2.1.0"` finds 17 in `falco-anvil/src/main` plus 2 more in the two new test classes +(`ChunkVersionPolicyTest`, `UnknownEntryPolicyTest`), 19 total, matching commit `1664dcdd`'s own +"Nineteen tags across ten files" exactly. `@version` tags on the touched types are untouched by that +commit and still read as the individual tasks left them (`1.0.0`–`1.3.0` depending on the type) — they +count class revisions, not the artefact version, per the commit's own message. + +### Cases added per task (measured, not estimated) + +22 new `@Test` methods, confirmed by `git diff 1bdd0cca..HEAD -- falco-anvil/src/test` (22 `+` lines +carrying `@Test`, 0 removed) and cross-checked per file against the `` count in this run's +JUnit XML: + +- Task 1 (`ServiceResolution`) — `ServiceResolutionTest.java` (new file): **7** — + `testAServiceWithNoProviderResolvesToNothing`, `testTwoProvidersAreRefusedAndBothAreNamed`, + `testAnExplicitInstanceAndDiscoveryTogetherAreRefused`, + `testAnExplicitInstanceIsUsedWithoutTouchingTheClasspath`, + `testNeitherExplicitNorDiscoveredResolvesToNothing` (the original five), plus + `testAForeignProviderWinsOverTheShippedDefault` and + `testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered` (added during Task 2's + review fix round, since the shipped-default-yields rule lives in `ServiceResolution`). +- Task 2 (`ChunkVersionPolicy`) — **7**: `ChunkVersionPolicyTest.java` (new file, 3 cases — + `testTheDefaultPolicyRefusesALevelLayout`, `testTheDefaultPolicyAcceptsAChunkWithoutAStoredVersion`, + `testTheDefaultPolicyRefusesAMistypedVersion`); `FalcoAnvilLoaderIntegrationTest` +2 + (`testAPolicyThatAllowsEverythingLetsALegacyChunkThrough`, + `testWithoutAnyPolicyALegacyChunkIsNotChecked`); `FalcoAnvilLoaderBuilderTest` +2 + (`testAnExplicitVersionPolicySurvivesEveryOtherSetter`, + `testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne`). +- Task 3 (`UnknownEntryPolicy`) — **8**: `UnknownEntryPolicyTest.java` (new file, 6 cases — the 3 + original block cases plus 3 biome mirrors added in the review fix round — + `testTheDefaultPolicyReplacesAnUnknownBlockWithAir`, + `testARefusingPolicyFailsTheChunkInsteadOfSubstituting`, + `testTheResolverStillCountsWhenThePolicySubstitutes`, + `testTheDefaultPolicyReplacesAnUnknownBiomeWithPlains`, + `testARefusingPolicyFailsTheChunkInsteadOfSubstitutingForABiome`, + `testTheResolverStillCountsWhenThePolicySubstitutesForABiome`); `FalcoAnvilLoaderBuilderTest` +2 + (`testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter`, + `testDiscoverUnknownEntryPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne`). + +Total: 22 identified by name. The measured `falco-anvil` module delta from the JUnit XML is **+23** +(230 → 253, see table below), one higher than the 22 named methods above. Every individual touched +file's `` count reconciles exactly against the per-task breakdown its own task report +recorded (`ServiceResolutionTest` 7, `ChunkVersionPolicyTest` 3, `UnknownEntryPolicyTest` 6, +`FalcoAnvilLoaderIntegrationTest` 35 total against a recorded baseline of 33, +`FalcoAnvilLoaderBuilderTest` 20 total against a recorded baseline of 16) — so the one-test gap sits in +the *baseline* figure (230) carried over from the prior acceptance report, not in anything this branch +added. Flagged for transparency rather than silently rounded; it does not change the "nothing fell" +conclusion, since it only moves in the direction of "more tests than accounted for," not fewer. + +### Gate attack 1 — delete the `ChunkVersionPolicy` service registration + +Removed `falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy` +and ran the **full** `:falco-anvil:test` module (253 cases, not a filtered class): + +``` +253 tests completed, 6 failed +``` + +- `FalcoAnvilLoaderIntegrationTest.testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir` — the specific + regression this whole plan (and #45 before it) exists to prevent: a pre-21w43a chunk goes back to + loading as air, silently. +- `FalcoAnvilLoaderIntegrationTest.testAChunkBelowTheFloorIsRefused` +- `FalcoAnvilLoaderIntegrationTest.testASectionsKeyStoredAsTheWrongTypeWithLevelIsRefused` +- `FalcoAnvilLoaderIntegrationTest.testAChunkWithADataVersionStoredAsTheWrongTypeIsRefused` +- `FalcoAnvilLoaderIntegrationTest.testAChunkWithANegativeDataVersionIsRefused` +- `FalcoAnvilLoaderBuilderTest.testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne` + — new in this run relative to Task 2's own Gegenprobe: with nothing registered, `discoverVersionPolicy()` + resolves to `null` instead of a `DefaultChunkVersionPolicy` instance, so the pass-through assertion + fails too. This is the executable proof that the failure mode reaches beyond the five rejection cases + into the builder's own contract test — two classes, not one. + +This is also the executable record of what the optional guard costs: with no `ChunkVersionPolicy` +provider on the classpath, every chunk that would have been refused loads instead, unchecked, and a +caller who explicitly asked to fall back to discovery silently gets nothing. + +Reverted (`git checkout -- falco-anvil/src/main/resources/META-INF/services/...ChunkVersionPolicy`). +`git status --short` empty afterward. + +### Gate attack 2 — drop the two new policy fields from one builder setter + +In `FalcoAnvilLoader.Builder.openRegionLimit(int)`, replaced the trailing constructor arguments +`this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, this.discoverUnknownEntryPolicy` +with `null, true, null, true` — simulating "forgot to thread the new fields through this setter." +Ran `FalcoAnvilLoaderBuilderTest`: + +``` +20 tests completed, 2 failed +``` + +- `testAnExplicitVersionPolicySurvivesEveryOtherSetter` — chaining `.versionPolicy(policy).openRegionLimit(4)…` + now loses the explicit policy at the mutated setter. +- `testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter` — same loss for the other policy. + +Both pass-through tests went red, exactly the ones the mutation should bite; the other 18 cases, +including the two `discover…SurvivesEveryOtherSetter` tests (whose expected post-mutation state +happens to coincide with the mutation's fallback), stayed green. Reverted +(`git checkout -- falco-anvil/.../FalcoAnvilLoader.java`); `git diff --stat` before the revert showed +exactly the one intended file, `git status --short` empty after. Re-ran the full `falco-anvil` module +afterward: 253/253 green, no residue. + +### Module counts (from JUnit XML under `build/test-results/test/`, `` elements counted +directly — not the console summary, not the `testsuite` summary attribute, though the two agreed on +every file in this run) + +| Module | Baseline at `1bdd0cca` (#45, from the prior acceptance report¹) | Count now | Delta | +| --- | --- | --- | --- | +| falco-anvil | 230 | 253 | +23 | +| 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 total, **4 failing** | **regression** | + +No count *fell* in the sense of fewer test cases existing. **falco-archunit is the exception that +matters**: total case count held at 47, but 4 that passed at the baseline now fail — see "Defect +found" below. + +¹ `docs/superpowers/plans/2026-08-03-anvil-version-guard.md`, its own `## Result` section, measured at +branch tip `a74cd042` (which became `1bdd0cca` on `origin/main` via #45) — reused per this task's +instruction to take baseline numbers from the last acceptance report of an earlier undertaking rather +than standing up a second worktree. That report itself notes light/instance/demo/archunit at its +baseline commit `2d3955d8` were, by construction, identical to `1bdd0cca` (the branch touched only +`falco-anvil` files), so these five numbers are read as valid for `1bdd0cca` directly. No new worktree +was created for this acceptance. + +### Defect found: `falco-archunit`'s `ForeignCouplingTest` was never updated for the new policy classes + +**Not fixed — reported, per this task's explicit instruction not to silently repair a real defect found +during acceptance.** + +`./gradlew :falco-anvil:test :falco-light:test :falco-instance:test :falco-demo:test +:falco-benchmarks:test :falco-archunit:test --rerun-tasks` fails on `:falco-archunit:test`. Four cases +in `net.onelitefeather.falco.architecture.ForeignCouplingTest` go red: + +- `anvilCoreKnowsNoMinestom` (9 violations) +- `blockRegistryOnlyInAdapters` (1 violation) +- `dynamicRegistryOnlyInBiomeResolver` (3 violations) +- `byteLayerKnowsNoNbt` (9 violations) + +All four rules use an allow-list regex naming the specific classes in `net.onelitefeather.falco.anvil` +that are permitted to depend on Minestom/Kyori-NBT types, e.g. (`ForeignCouplingTest.java:52`): + +```java ++ "(FalcoAnvilLoader|BlockPaletteResolver|BiomePaletteResolver)(\\$.*)?"; +``` + +Tasks 1–3 added `ChunkVersionPolicy`, `DefaultChunkVersionPolicy`, `UnknownEntryPolicy` and +`DefaultUnknownEntryPolicy` to `net.onelitefeather.falco.anvil`. Three of these four legitimately touch +the types the rule forbids — `DefaultChunkVersionPolicy` reads `CompoundBinaryTag` (needed to decide +chunk readability), `DefaultUnknownEntryPolicy` calls `Block.AIR`/`Block.AIR.stateId()` and +`MinecraftServer.getBiomeRegistry()`/`DynamicRegistry.getId(...)` (needed to reproduce the exact +air/plains fallback the two resolvers used to hard-code), and `UnknownEntryPolicy`'s own interface +method signature carries a `CompoundBinaryTag` parameter. None of these four classes is named in the +allow-list regex, so every one of these — architecturally intentional — dependencies now reads as a +violation. + +**Why none of the three implementation tasks caught this themselves:** each task's own verification ran +`:falco-anvil:test` (and `javadoc`/`checkApiCompatibility`), never `:falco-archunit:test` — that module +lives outside `falco-anvil` and was outside each task's own file list. This acceptance is the first point +in the plan that runs all six modules together, which is exactly why it exists. + +This is a real defect against the plan's own claim of "no module, no JPMS, no published signature" — +the *behaviour* is unchanged, but the *architecture rule* meant to keep `falco-anvil`'s core free of a +running-server dependency no longer reflects where the boundary actually is now that policies live +inside that same package. Left unfixed here; the two defensible directions for whoever picks this up +are (a) widen the allow-list regex to include the four new classes, or (b) move +`DefaultChunkVersionPolicy`/`DefaultUnknownEntryPolicy` to a place the existing rules already permit — +a design call outside this acceptance's scope. + +### `./gradlew build -x test --rerun-tasks` + +`BUILD SUCCESSFUL`. `javadoc` genuinely executed (no `UP-TO-DATE`, full `--rerun-tasks` output grepped +for "warning" case-insensitively: zero matches) for the four modules that carry a javadoc task: +falco-anvil, falco-light, falco-instance, falco-demo. `checkApiCompatibility` genuinely executed for +the three configured modules — falco-anvil, falco-light, falco-instance. All three reports read +literally: + +``` +Comparing binary compatibility of falco-anvil-1.0.0.jar against falco-anvil-1.0.0.jar +No changes. +``` + +(same for `falco-light` and `falco-instance`, each against its own `-1.0.0.jar`). `onlyBinaryIncompatibleModified` +is `true` for this task, so purely-additive surface (every new type and setter this plan added) does +not appear here by design — consistent with every prior task's own japicmp run. No exception entry was +needed in `gradle/api-breaks.properties` and none was added. + +### Machine load + +`uptime` before the module run (14:39:57): `load average: 4.17, 5.18, 3.56` +`uptime` after the gate attacks and the full re-verification run (14:45:49): `load average: 8.68, 8.57, +5.79` + +No timing figure was produced or is quoted anywhere in this section. + +### What this work does not do + +- No new module. Everything lives in `net.onelitefeather.falco.anvil`, in the six existing published + modules. +- No `module-info.java`, no JPMS. Plain classpath `ServiceLoader`. +- No published signature was removed or changed — `checkApiCompatibility` confirms this literally + ("No changes.") for all three configured modules. +- No behaviour changes for a caller who registers nothing and calls the builder as before: + `discoverVersionPolicy`/`discoverUnknownEntryPolicy` default to `true`, so `builder()` and the two + public constructors resolve `DefaultChunkVersionPolicy`/`DefaultUnknownEntryPolicy` exactly as #45's + guard and the two resolvers' hard-coded fallbacks always did — **except that the guard is now + losable**: deleting one `META-INF/services` line (Gate attack 1, above) turns the same "default" + configuration into "no check at all," which was not possible before this plan. That loss is a + deliberate project decision (documented in Task 2's own report and re-confirmed by this run), not an + oversight. + +### Status + +**DONE_WITH_CONCERNS.** All four acceptance steps ran to completion; every module's test count held or +grew; javadoc and `checkApiCompatibility` are clean; both gate attacks landed exactly where expected and +both were reverted cleanly. The concern is the `falco-archunit` regression above — a real defect, +reported rather than silently repaired, that needs a follow-up decision before this branch should be +considered fully clean. From fe569f5b8bba094eec362e577486d9ff18d5b2c4 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 15:27:22 +0200 Subject: [PATCH 10/18] fix(anvil): let the unknown-entry policy name a substitute instead of resolving one DefaultUnknownEntryPolicy resolved Block.AIR and the biome registry itself, which archunit's ForeignCouplingTest rightly refuses (registry access belongs to exactly one adapter per kind). UnknownEntryPolicy now returns a palette name instead of an id; the resolver, which already holds the registry from the original lookup, resolves the substitute itself and fails the chunk - without asking the policy twice - if that name is unknown too. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../falco/anvil/BiomePaletteResolver.java | 34 ++++---- .../falco/anvil/BlockPaletteResolver.java | 20 ++++- .../anvil/DefaultUnknownEntryPolicy.java | 75 +++++------------ .../falco/anvil/UnknownEntryPolicy.java | 28 +++++-- .../falco/anvil/UnknownEntryPolicyTest.java | 83 +++++++++++++++---- 5 files changed, 144 insertions(+), 96 deletions(-) diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java index b4ff272..8a35996 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BiomePaletteResolver.java @@ -33,7 +33,7 @@ *

* * @author TheMeinerLP - * @version 1.1.0 + * @version 1.2.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -83,7 +83,7 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy pol * @param registrySupplier the supplier which provides the registry of the known biomes */ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier> registrySupplier) { - this(diagnostics, new DefaultUnknownEntryPolicy(registrySupplier), registrySupplier); + this(diagnostics, new DefaultUnknownEntryPolicy(), registrySupplier); } /** @@ -93,10 +93,7 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier * * @param diagnostics the diagnostics which throttle the reports @@ -113,13 +110,6 @@ public BiomePaletteResolver(AnvilDiagnostics diagnostics, Supplier - * This is the same lazy, double-checked-locking derivation {@link DefaultUnknownEntryPolicy} uses - * to resolve its own plains fallback from the same kind of supplier. The two are not shared: a - * shared holder would need a third class injected into both, for roughly ten lines saved, and - * this resolver's copy also has to hand the result to {@link #toEntry(int)} for the id-to-name - * direction, which the policy has no reason to do. - *

* * @return the registry of the known biomes */ @@ -144,7 +134,11 @@ private DynamicRegistry registry() { /** * {@inheritDoc} * - * @throws AnvilChunkException if the configured policy refuses an unknown biome + * @throws AnvilChunkException if the configured policy refuses an unknown biome + * @throws IllegalStateException if the name the policy substitutes is itself unknown; not an + * {@link AnvilChunkException}, because {@code FalcoAnvilLoader} is + * the only place that constructs one, and it wraps this into one + * when a chunk is read through the loader */ @Override public int toId(String name, @Nullable CompoundBinaryTag properties) { @@ -157,7 +151,17 @@ public int toId(String name, @Nullable CompoundBinaryTag properties) { if (this.diagnostics.reportUnknownBiome(name)) { LOGGER.warn("The biome '{}' is unknown, further chunks with it are not reported", name); } - return this.policy.onUnknownBiome(name); + String substituteName = this.policy.onUnknownBiome(name); + int substituteId = registry.getId(RegistryKey.unsafeOf(substituteName)); + + // The policy is not consulted a second time for its own substitute, for the same reason + // BlockPaletteResolver does not: a second call could loop, and a substitute the registry + // does not know either is a failure that has to reach the caller, not carry on silently. + if (substituteId == -1) { + throw new IllegalStateException("The biome '" + name + "' is unknown and its substitute '" + + substituteName + "' is unknown too"); + } + return substituteId; } /** diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java index 6f11770..d7aa498 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/BlockPaletteResolver.java @@ -30,7 +30,7 @@ *

* * @author TheMeinerLP - * @version 1.1.0 + * @version 1.2.0 * @since 0.1.0 */ @ApiStatus.Experimental @@ -70,7 +70,11 @@ public BlockPaletteResolver(AnvilDiagnostics diagnostics, UnknownEntryPolicy pol /** * {@inheritDoc} * - * @throws AnvilChunkException if the configured policy refuses an unknown block + * @throws AnvilChunkException if the configured policy refuses an unknown block + * @throws IllegalStateException if the name the policy substitutes is itself unknown; not an + * {@link AnvilChunkException}, because {@code FalcoAnvilLoader} is + * the only place that constructs one, and it wraps this into one + * when a chunk is read through the loader */ @Override public int toId(String name, @Nullable CompoundBinaryTag properties) { @@ -80,7 +84,17 @@ public int toId(String name, @Nullable CompoundBinaryTag properties) { if (this.diagnostics.reportUnknownBlock(name)) { LOGGER.warn("The block '{}' is unknown, further chunks with it are not reported", name); } - return this.policy.onUnknownBlock(name, properties); + String substituteName = this.policy.onUnknownBlock(name, properties); + Block substitute = Block.fromKey(substituteName); + + // The policy is not consulted a second time for its own substitute: doing so could loop + // if a policy ever named itself as the replacement, and a substitute the running server + // does not know either is exactly the kind of failure a chunk should not carry silently. + if (substitute == null) { + throw new IllegalStateException("The block '" + name + "' is unknown and its substitute '" + + substituteName + "' is unknown too"); + } + return substitute.stateId(); } if (properties == null || properties.size() == 0) { return block.stateId(); diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java index 62c4ded..a7cc7ce 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/DefaultUnknownEntryPolicy.java @@ -1,102 +1,67 @@ package net.onelitefeather.falco.anvil; import net.kyori.adventure.nbt.CompoundBinaryTag; -import net.minestom.server.MinecraftServer; -import net.minestom.server.instance.block.Block; -import net.minestom.server.registry.DynamicRegistry; -import net.minestom.server.world.biome.Biome; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; -import java.util.function.Supplier; - /** * The {@link UnknownEntryPolicy} a resolver falls back to when nothing else was configured: * substitutes air for an unknown block and plains for an unknown biome, exactly the values * {@link BlockPaletteResolver} and {@link BiomePaletteResolver} hard-coded before this policy * existed. *

- * The plains id is resolved from the biome registry lazily rather than in the constructor, for the - * same reason {@link BiomePaletteResolver} resolves its registry lazily: a policy is often built - * while the server is still starting, and reading the registry too early would fail before the - * policy ever meets a chunk. + * Both names are plain string literals. This policy resolves nothing itself — {@link + * UnknownEntryPolicy} hands back a name, not an id, precisely so that the shipped default needs + * neither Minestom nor a registry to answer. The resolver that consults this policy already holds + * the registry it needs to turn {@code "minecraft:air"} or {@code "minecraft:plains"} into an id, + * because it just used that same registry to look the original, unknown name up. *

*

* This type is experimental, like everything else in this package. *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 2.1.0 */ @ApiStatus.Experimental public final class DefaultUnknownEntryPolicy implements UnknownEntryPolicy { - private final Supplier> registrySupplier; - - private volatile @Nullable Integer resolvedFallbackBiomeId; + /** + * The name substituted for an unknown block. + */ + static final String AIR = "minecraft:air"; /** - * Creates a new policy which resolves the plains id from the biome registry of the running - * server. + * The name substituted for an unknown biome. */ - public DefaultUnknownEntryPolicy() { - this(MinecraftServer::getBiomeRegistry); - } + static final String PLAINS = "minecraft:plains"; /** - * Creates a new policy which resolves the plains id from the registry the given supplier - * provides. - *

- * The registry is resolved on the first use instead of in the constructor. A policy is often - * built while the server is still starting and reading the registry too early would fail before - * the policy ever meets a chunk. - *

- * - * @param registrySupplier the supplier which provides the registry of the known biomes + * Creates the policy. It holds no state of its own, so every instance behaves the same way. */ - public DefaultUnknownEntryPolicy(Supplier> registrySupplier) { - this.registrySupplier = registrySupplier; + public DefaultUnknownEntryPolicy() { } /** * {@inheritDoc} *

- * Always returns {@link Block#AIR}'s state id. + * Always returns {@code "minecraft:air"}. *

*/ @Override - public int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties) { - return Block.AIR.stateId(); + public String onUnknownBlock(String name, @Nullable CompoundBinaryTag properties) { + return AIR; } /** * {@inheritDoc} *

- * Always returns the id of {@link Biome#PLAINS} in the resolved registry. + * Always returns {@code "minecraft:plains"}. *

*/ @Override - public int onUnknownBiome(String name) { - Integer current = this.resolvedFallbackBiomeId; - - if (current != null) { - return current; - } - - // The same lazy, double-checked-locking derivation BiomePaletteResolver#registry() uses for - // its own resolution from the same kind of supplier, kept as its own copy rather than a - // shared holder: a shared holder would need a third class injected into both for roughly ten - // lines saved, and this method only ever needs an id, never the registry object itself the - // way the resolver does for its id-to-name direction. - synchronized (this) { - Integer created = this.resolvedFallbackBiomeId; - - if (created == null) { - created = this.registrySupplier.get().getId(Biome.PLAINS); - this.resolvedFallbackBiomeId = created; - } - return created; - } + public String onUnknownBiome(String name) { + return PLAINS; } } diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java index 1b89e19..2c37801 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java @@ -7,18 +7,28 @@ /** * Decides what becomes of a palette entry the running server does not know. *

- * Returning an id substitutes it; throwing {@link AnvilChunkException} fails the chunk. Substituting - * is right for a server that wants a world to stay loadable and wrong for a tool that converts one, - * which is why this is a policy and not a constant. + * Returning a name substitutes it; throwing {@link AnvilChunkException} fails the chunk. + * Substituting is right for a server that wants a world to stay loadable and wrong for a tool that + * converts one, which is why this is a policy and not a constant. + *

+ *

+ * A policy names a replacement; it does not resolve one. It returns a palette name such as + * {@code "minecraft:stone"}, not an id — the resolver that consults it owns the registry lookup + * that turns a name into an id, the same registry it already needed to look the original, unknown + * name up in the first place. That split keeps this interface, and any implementation of it, free of + * a dependency on Minestom or any registry: naming {@code "minecraft:air"} takes no more than a + * string literal. *

*

* A policy only decides. It does not count and it does not log: the resolver which consults it keeps * reporting the name to its {@link AnvilDiagnostics} and writing the log line regardless of what the - * policy does with the entry, so a substituting run stays as visible as a refusing one. + * policy does with the entry, so a substituting run stays as visible as a refusing one. Nor does a + * policy get asked twice: if the name it returns is itself unknown, the resolver fails the chunk + * instead of consulting the policy again, which would risk a loop. *

* * @author TheMeinerLP - * @version 1.0.0 + * @version 1.1.0 * @since 2.1.0 */ @ApiStatus.Experimental @@ -29,17 +39,17 @@ public interface UnknownEntryPolicy { * * @param name the block name stored in the palette * @param properties the stored properties, or null if the entry carries none - * @return the state id to use instead + * @return the name of the block to use instead * @throws AnvilChunkException if the chunk should fail rather than carry a substitute */ - int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties); + String onUnknownBlock(String name, @Nullable CompoundBinaryTag properties); /** * Decides what an unknown biome becomes. * * @param name the biome name stored in the palette - * @return the id to use instead + * @return the name of the biome to use instead * @throws AnvilChunkException if the chunk should fail rather than carry a substitute */ - int onUnknownBiome(String name); + String onUnknownBiome(String name); } diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java index 325eecc..36538a0 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/UnknownEntryPolicyTest.java @@ -1,11 +1,11 @@ package net.onelitefeather.falco.anvil; import net.kyori.adventure.nbt.CompoundBinaryTag; -import net.minestom.server.instance.block.Block; -import net.minestom.server.registry.DynamicRegistry; import net.minestom.server.world.biome.Biome; import org.junit.jupiter.api.Test; +import java.util.concurrent.atomic.AtomicInteger; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -20,6 +20,15 @@ * extension point exists to prevent. *

*

+ * {@link UnknownEntryPolicy} hands back a name, not an id — the resolver owns the registry lookup + * that turns a name into one, using the same registry it already needed for the original, unknown + * name. That is what the "unusable substitute" cases pin: the resolver has to fail if the name a + * policy substitutes is itself unresolvable, and it must not ask the policy a second time to find + * out, which could loop. The failure surfaces as {@link IllegalStateException} rather than {@link + * AnvilChunkException} directly, because {@code FalcoAnvilLoader} is the only class that constructs + * the latter — it wraps this into one when a chunk is read through the loader. + *

+ *

* The biome cases use {@link Biome#createDefaultRegistry()} rather than the biome registry of a * running server, so this class needs no Minestom test environment: {@link BiomePaletteResolver}'s * package-private three-argument constructor exists for exactly this, to inject both a policy and a @@ -27,34 +36,31 @@ *

* * @author TheMeinerLP - * @version 1.1.0 + * @version 2.0.0 * @since 2.1.0 */ class UnknownEntryPolicyTest { @Test void testTheDefaultPolicyReplacesAnUnknownBlockWithAir() { - assertEquals(Block.AIR.stateId(), new DefaultUnknownEntryPolicy().onUnknownBlock("falco:nope", null)); + assertEquals("minecraft:air", new DefaultUnknownEntryPolicy().onUnknownBlock("falco:nope", null)); } @Test void testTheDefaultPolicyReplacesAnUnknownBiomeWithPlains() { - DynamicRegistry registry = Biome.createDefaultRegistry(); - - assertEquals(registry.getId(Biome.PLAINS), - new DefaultUnknownEntryPolicy(() -> registry).onUnknownBiome("falco:nope")); + assertEquals("minecraft:plains", new DefaultUnknownEntryPolicy().onUnknownBiome("falco:nope")); } @Test void testARefusingPolicyFailsTheChunkInsteadOfSubstituting() { UnknownEntryPolicy refusing = new UnknownEntryPolicy() { @Override - public int onUnknownBlock(String name, CompoundBinaryTag properties) { + public String onUnknownBlock(String name, CompoundBinaryTag properties) { throw new AnvilChunkException("The block " + name + " has no mapping"); } @Override - public int onUnknownBiome(String name) { + public String onUnknownBiome(String name) { throw new AnvilChunkException("The biome " + name + " has no mapping"); } }; @@ -68,12 +74,12 @@ public int onUnknownBiome(String name) { void testARefusingPolicyFailsTheChunkInsteadOfSubstitutingForABiome() { UnknownEntryPolicy refusing = new UnknownEntryPolicy() { @Override - public int onUnknownBlock(String name, CompoundBinaryTag properties) { + public String onUnknownBlock(String name, CompoundBinaryTag properties) { throw new AnvilChunkException("The block " + name + " has no mapping"); } @Override - public int onUnknownBiome(String name) { + public String onUnknownBiome(String name) { throw new AnvilChunkException("The biome " + name + " has no mapping"); } }; @@ -97,9 +103,58 @@ void testTheResolverStillCountsWhenThePolicySubstitutes() { void testTheResolverStillCountsWhenThePolicySubstitutesForABiome() { AnvilDiagnostics diagnostics = new AnvilDiagnostics(); - new BiomePaletteResolver(diagnostics, new DefaultUnknownEntryPolicy(Biome::createDefaultRegistry), - Biome::createDefaultRegistry).toId("falco:nope", null); + new BiomePaletteResolver(diagnostics, new DefaultUnknownEntryPolicy(), Biome::createDefaultRegistry) + .toId("falco:nope", null); assertEquals(1, diagnostics.unknownBiomeCount()); } + + @Test + void testAnUnusableSubstituteBlockFailsTheChunkAndNamesBothNames() { + AtomicInteger calls = new AtomicInteger(); + UnknownEntryPolicy nonsense = new UnknownEntryPolicy() { + @Override + public String onUnknownBlock(String name, CompoundBinaryTag properties) { + calls.incrementAndGet(); + return "falco:still-nope"; + } + + @Override + public String onUnknownBiome(String name) { + throw new AssertionError("not exercised by this test"); + } + }; + + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> new BlockPaletteResolver(new AnvilDiagnostics(), nonsense).toId("falco:nope", null)); + + assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); + assertTrue(failure.getMessage().contains("falco:still-nope"), failure.getMessage()); + assertEquals(1, calls.get(), "the policy is not asked a second time for its own substitute"); + } + + @Test + void testAnUnusableSubstituteBiomeFailsTheChunkAndNamesBothNames() { + AtomicInteger calls = new AtomicInteger(); + UnknownEntryPolicy nonsense = new UnknownEntryPolicy() { + @Override + public String onUnknownBlock(String name, CompoundBinaryTag properties) { + throw new AssertionError("not exercised by this test"); + } + + @Override + public String onUnknownBiome(String name) { + calls.incrementAndGet(); + return "falco:still-nope"; + } + }; + + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> new BiomePaletteResolver(new AnvilDiagnostics(), nonsense, Biome::createDefaultRegistry) + .toId("falco:nope", null)); + + assertTrue(failure.getMessage().contains("falco:nope"), failure.getMessage()); + assertTrue(failure.getMessage().contains("falco:still-nope"), failure.getMessage()); + assertEquals(1, calls.get(), "the policy is not asked a second time for its own substitute"); + } } From 4985431372cc9d34cfa04694b4acd2a5284ccb4a Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 15:33:33 +0200 Subject: [PATCH 11/18] test(archunit): widen byteLayerKnowsNoNbt for the anvil policy classes ChunkVersionPolicy, DefaultChunkVersionPolicy, UnknownEntryPolicy and DefaultUnknownEntryPolicy carry NBT in their contract by design, one layer above RegionFile's pure-byte guarantee that this rule actually protects. Add all four to ANVIL_NBT_LAYER and correct the rule's javadoc, which wrongly claimed the exemption was a self-maintaining complement rather than the hand-maintained name list it always was - the reason nobody extended it when these classes were added. --- .../falco/architecture/ForeignCouplingTest.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/falco-archunit/src/test/java/net/onelitefeather/falco/architecture/ForeignCouplingTest.java b/falco-archunit/src/test/java/net/onelitefeather/falco/architecture/ForeignCouplingTest.java index e7ca831..9430626 100644 --- a/falco-archunit/src/test/java/net/onelitefeather/falco/architecture/ForeignCouplingTest.java +++ b/falco-archunit/src/test/java/net/onelitefeather/falco/architecture/ForeignCouplingTest.java @@ -54,7 +54,9 @@ class ForeignCouplingTest { private static final String ANVIL_NBT_LAYER = "net\\.onelitefeather\\.falco\\.anvil\\." + "(FalcoAnvilLoader|SectionCodec|NbtReads|PaletteEntryResolver" - + "|BlockPaletteResolver|BiomePaletteResolver)(\\$.*)?"; + + "|BlockPaletteResolver|BiomePaletteResolver" + + "|ChunkVersionPolicy|DefaultChunkVersionPolicy" + + "|UnknownEntryPolicy|DefaultUnknownEntryPolicy)(\\$.*)?"; private static final String ANVIL_FILE_BOUNDARY = "net\\.onelitefeather\\.falco\\.anvil\\.(RegionFile|FalcoAnvilLoader)(\\$.*)?"; @@ -251,9 +253,14 @@ class ForeignCouplingTest { * import down here is not untidy, it is the door through which the loss of predictability under * concurrency comes back. * - *

Phrased as a complement rather than a hand-maintained allow list, so it covers - * {@code PaletteData}, {@code AnvilChunkException} and nested types such as - * {@code RegionFile$RawChunk} without maintenance when a class is added. + *

{@code ANVIL_NBT_LAYER} is a hand-maintained list, not a complement: it names the ten + * classes of the NBT layer as of this writing — {@code FalcoAnvilLoader}, + * {@code SectionCodec}, {@code NbtReads}, {@code PaletteEntryResolver}, + * {@code BlockPaletteResolver}, {@code BiomePaletteResolver}, {@code ChunkVersionPolicy}, + * {@code DefaultChunkVersionPolicy}, {@code UnknownEntryPolicy} and + * {@code DefaultUnknownEntryPolicy} — and a class added to that layer later has to be + * added here too, the way the four policy classes were not when they were first written. + * Nested types need no separate entry; they come along through the {@code (\$.*)?} suffix. */ @ArchTest static final ArchRule byteLayerKnowsNoNbt = noClasses() From a7f7b574b5bc6a9f6afc0295b956e2c30c0428d4 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 15:39:27 +0200 Subject: [PATCH 12/18] docs(anvil): note that the version guard and unknown-entry fallback are replaceable --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index b5af078..0374206 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,15 @@ diagnostics.reportUnknownBlock("mod:strange_block"); // true the first time, f pre-26.1 layout, and `openRegionCount()` how many files are open right now. `close()` flushes every one of them and is what `ownsLoader(true)` calls for you. +The version guard and the unknown-entry fallback above are both policies, not fixed behaviour: +`ChunkVersionPolicy` decides whether a chunk is readable at all, `UnknownEntryPolicy` decides what an +unknown block or biome becomes, and `falco-anvil` ships a default for each — `DefaultChunkVersionPolicy` +is the 21w43a guard, `DefaultUnknownEntryPolicy` is the air/plains substitution — discovered from the +classpath via `ServiceLoader` unless the builder's `versionPolicy()`/`unknownEntryPolicy()` slots are +used instead. The guard can be removed: `versionPolicy(null)` turns the check off, and nothing stands +in for it — a loader with no guard reads a pre-21w43a world as air again, with no error and no log +line. + ### falco-light — block and sky light Three entry points, in order of how much they do: From 38d3b9b1e713e3b0b59d55811f425a3db7b6e793 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 15:49:19 +0200 Subject: [PATCH 13/18] docs(anvil): re-measure task 4 acceptance after the archunit fix closed the gap Three commits landed since the last measurement (fe569f5b, 49854313, a7f7b574): UnknownEntryPolicy now names a substitute instead of resolving one, ANVIL_NBT_LAYER was widened and its Javadoc corrected, and README gained the replaceable-policy paragraph. Re-ran all six modules and the build/javadoc/japicmp check against the new tip: falco-archunit is back to 47/47, falco-anvil grew to 255 cases, no other module moved. Replaces the previous Result section rather than appending a second one, and keeps the archunit defect's discovery and actual fix in the history instead of quietly re-measuring it away. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../2026-08-04-anvil-extension-points.md | 355 ++++++++---------- 1 file changed, 162 insertions(+), 193 deletions(-) diff --git a/docs/superpowers/plans/2026-08-04-anvil-extension-points.md b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md index d294502..7c87a8d 100644 --- a/docs/superpowers/plans/2026-08-04-anvil-extension-points.md +++ b/docs/superpowers/plans/2026-08-04-anvil-extension-points.md @@ -632,229 +632,198 @@ spec, both unchecked. `@since 1.2.0` throughout, because #45 already took the mo ## Result -Acceptance run against `1664dcdd` (branch tip before this section), worktree +Acceptance run against `a7f7b574` (branch tip), worktree `/mnt/projects/oss/onelitefeather/Falco-worktrees/anvil-extension-points`, base `1bdd0cca` (#45, -merged into `origin/main`). The branch had already been rebased onto `origin/main` and the 19 `@since` -tags redated to `2.1.0` before this acceptance began (`1664dcdd`); both were re-verified below rather -than taken on faith. - -### `@since`/`@version` check (redone, not assumed) - -`grep -rn "@since 1.2.0" falco-anvil/src/main` returns nothing — no stale tag survived the rebase. -`grep -c "@since 2.1.0"` finds 17 in `falco-anvil/src/main` plus 2 more in the two new test classes -(`ChunkVersionPolicyTest`, `UnknownEntryPolicyTest`), 19 total, matching commit `1664dcdd`'s own -"Nineteen tags across ten files" exactly. `@version` tags on the touched types are untouched by that -commit and still read as the individual tasks left them (`1.0.0`–`1.3.0` depending on the type) — they -count class revisions, not the artefact version, per the commit's own message. - -### Cases added per task (measured, not estimated) - -22 new `@Test` methods, confirmed by `git diff 1bdd0cca..HEAD -- falco-anvil/src/test` (22 `+` lines -carrying `@Test`, 0 removed) and cross-checked per file against the `` count in this run's -JUnit XML: - -- Task 1 (`ServiceResolution`) — `ServiceResolutionTest.java` (new file): **7** — - `testAServiceWithNoProviderResolvesToNothing`, `testTwoProvidersAreRefusedAndBothAreNamed`, - `testAnExplicitInstanceAndDiscoveryTogetherAreRefused`, - `testAnExplicitInstanceIsUsedWithoutTouchingTheClasspath`, - `testNeitherExplicitNorDiscoveredResolvesToNothing` (the original five), plus - `testAForeignProviderWinsOverTheShippedDefault` and - `testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered` (added during Task 2's - review fix round, since the shipped-default-yields rule lives in `ServiceResolution`). -- Task 2 (`ChunkVersionPolicy`) — **7**: `ChunkVersionPolicyTest.java` (new file, 3 cases — - `testTheDefaultPolicyRefusesALevelLayout`, `testTheDefaultPolicyAcceptsAChunkWithoutAStoredVersion`, - `testTheDefaultPolicyRefusesAMistypedVersion`); `FalcoAnvilLoaderIntegrationTest` +2 - (`testAPolicyThatAllowsEverythingLetsALegacyChunkThrough`, - `testWithoutAnyPolicyALegacyChunkIsNotChecked`); `FalcoAnvilLoaderBuilderTest` +2 - (`testAnExplicitVersionPolicySurvivesEveryOtherSetter`, +merged into `origin/main`). The branch was already rebased onto `origin/main` and the 19 `@since` tags +redated to `2.1.0` before this measurement. + +This section supersedes an earlier version of itself. That earlier pass measured `1664dcdd` and found +`falco-archunit` red — 4 of `ForeignCouplingTest`'s cases failed, because the three implementation +tasks added `ChunkVersionPolicy`/`DefaultChunkVersionPolicy`/`UnknownEntryPolicy`/ +`DefaultUnknownEntryPolicy` to `net.onelitefeather.falco.anvil` without ever running +`:falco-archunit:test` against them. That finding was reported, not silently fixed, and is preserved +below as "The archunit defect, and how it was actually closed" — it is the most instructive part of +this task and stays in the history rather than being measured away. Three commits landed since that +measurement (`fe569f5b`, `49854313`, `a7f7b574`) and are folded into the numbers below, which are a full +re-measurement, not a patch on the old ones. + +### `@since`/`@version` check (re-verified against the current tip) + +`grep -rn "@since 1.2.0" falco-anvil/src` returns nothing. 17 `@since 2.1.0` tags in +`falco-anvil/src/main`, 2 more in the two new test classes (`ChunkVersionPolicyTest`, +`UnknownEntryPolicyTest`) — 19 total, unaffected by the three commits since the last measurement (none +of them touched an `@since` tag). `@version` tags remain untouched, as they were before. + +### Cases added per task (falco-anvil, measured from the JUnit XML) + +`falco-anvil` now carries 255 cases (was 230 at `1bdd0cca`, +25). 24 new `@Test` methods are +identifiable by name via `git diff 1bdd0cca..HEAD -- falco-anvil/src/test`: + +- **Task 1** (`ServiceResolution`) — `ServiceResolutionTest.java` (new file): 7 — the original five + resolution-rule cases plus `testAForeignProviderWinsOverTheShippedDefault` and + `testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered`, added during Task 2's + review fix round. +- **Task 2** (`ChunkVersionPolicy`) — 7: `ChunkVersionPolicyTest` (new file, 3 cases) + + `FalcoAnvilLoaderIntegrationTest` (+2: `testAPolicyThatAllowsEverythingLetsALegacyChunkThrough`, + `testWithoutAnyPolicyALegacyChunkIsNotChecked`) + `FalcoAnvilLoaderBuilderTest` (+2: + `testAnExplicitVersionPolicySurvivesEveryOtherSetter`, `testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne`). -- Task 3 (`UnknownEntryPolicy`) — **8**: `UnknownEntryPolicyTest.java` (new file, 6 cases — the 3 - original block cases plus 3 biome mirrors added in the review fix round — - `testTheDefaultPolicyReplacesAnUnknownBlockWithAir`, - `testARefusingPolicyFailsTheChunkInsteadOfSubstituting`, - `testTheResolverStillCountsWhenThePolicySubstitutes`, - `testTheDefaultPolicyReplacesAnUnknownBiomeWithPlains`, - `testARefusingPolicyFailsTheChunkInsteadOfSubstitutingForABiome`, - `testTheResolverStillCountsWhenThePolicySubstitutesForABiome`); `FalcoAnvilLoaderBuilderTest` +2 - (`testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter`, +- **Task 3** (`UnknownEntryPolicy`) — 10: `UnknownEntryPolicyTest` (8 cases — the 3 original block + cases, 3 biome mirrors added in Task 3's own review fix round, and 2 more — + `testAnUnusableSubstituteBlockFailsTheChunkAndNamesBothNames`, + `testAnUnusableSubstituteBiomeFailsTheChunkAndNamesBothNames` — added by the interface rework below) + + `FalcoAnvilLoaderBuilderTest` (+2: `testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter`, `testDiscoverUnknownEntryPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne`). -Total: 22 identified by name. The measured `falco-anvil` module delta from the JUnit XML is **+23** -(230 → 253, see table below), one higher than the 22 named methods above. Every individual touched -file's `` count reconciles exactly against the per-task breakdown its own task report -recorded (`ServiceResolutionTest` 7, `ChunkVersionPolicyTest` 3, `UnknownEntryPolicyTest` 6, -`FalcoAnvilLoaderIntegrationTest` 35 total against a recorded baseline of 33, -`FalcoAnvilLoaderBuilderTest` 20 total against a recorded baseline of 16) — so the one-test gap sits in -the *baseline* figure (230) carried over from the prior acceptance report, not in anything this branch -added. Flagged for transparency rather than silently rounded; it does not change the "nothing fell" -conclusion, since it only moves in the direction of "more tests than accounted for," not fewer. - -### Gate attack 1 — delete the `ChunkVersionPolicy` service registration - -Removed `falco-anvil/src/main/resources/META-INF/services/net.onelitefeather.falco.anvil.ChunkVersionPolicy` -and ran the **full** `:falco-anvil:test` module (253 cases, not a filtered class): - -``` -253 tests completed, 6 failed -``` - -- `FalcoAnvilLoaderIntegrationTest.testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir` — the specific - regression this whole plan (and #45 before it) exists to prevent: a pre-21w43a chunk goes back to - loading as air, silently. -- `FalcoAnvilLoaderIntegrationTest.testAChunkBelowTheFloorIsRefused` -- `FalcoAnvilLoaderIntegrationTest.testASectionsKeyStoredAsTheWrongTypeWithLevelIsRefused` -- `FalcoAnvilLoaderIntegrationTest.testAChunkWithADataVersionStoredAsTheWrongTypeIsRefused` -- `FalcoAnvilLoaderIntegrationTest.testAChunkWithANegativeDataVersionIsRefused` -- `FalcoAnvilLoaderBuilderTest.testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne` - — new in this run relative to Task 2's own Gegenprobe: with nothing registered, `discoverVersionPolicy()` - resolves to `null` instead of a `DefaultChunkVersionPolicy` instance, so the pass-through assertion - fails too. This is the executable proof that the failure mode reaches beyond the five rejection cases - into the builder's own contract test — two classes, not one. - -This is also the executable record of what the optional guard costs: with no `ChunkVersionPolicy` -provider on the classpath, every chunk that would have been refused loads instead, unchecked, and a -caller who explicitly asked to fall back to discovery silently gets nothing. - -Reverted (`git checkout -- falco-anvil/src/main/resources/META-INF/services/...ChunkVersionPolicy`). -`git status --short` empty afterward. - -### Gate attack 2 — drop the two new policy fields from one builder setter - -In `FalcoAnvilLoader.Builder.openRegionLimit(int)`, replaced the trailing constructor arguments -`this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, this.discoverUnknownEntryPolicy` -with `null, true, null, true` — simulating "forgot to thread the new fields through this setter." -Ran `FalcoAnvilLoaderBuilderTest`: - -``` -20 tests completed, 2 failed -``` - -- `testAnExplicitVersionPolicySurvivesEveryOtherSetter` — chaining `.versionPolicy(policy).openRegionLimit(4)…` - now loses the explicit policy at the mutated setter. -- `testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter` — same loss for the other policy. - -Both pass-through tests went red, exactly the ones the mutation should bite; the other 18 cases, -including the two `discover…SurvivesEveryOtherSetter` tests (whose expected post-mutation state -happens to coincide with the mutation's fallback), stayed green. Reverted -(`git checkout -- falco-anvil/.../FalcoAnvilLoader.java`); `git diff --stat` before the revert showed -exactly the one intended file, `git status --short` empty after. Re-ran the full `falco-anvil` module -afterward: 253/253 green, no residue. - -### Module counts (from JUnit XML under `build/test-results/test/`, `` elements counted -directly — not the console summary, not the `testsuite` summary attribute, though the two agreed on -every file in this run) - -| Module | Baseline at `1bdd0cca` (#45, from the prior acceptance report¹) | Count now | Delta | +Named total: 24. The measured module delta is +25 (230 → 255) — the same one-test gap this section +flagged at the previous measurement persists (traced there to the *baseline* figure carried over from +the prior acceptance report, not to anything this branch added or removed since), plus the interface +rework's own net +2 (`testTheDefaultPolicyReplacesAnUnknownBlockWithAir` and +`testTheDefaultPolicyReplacesAnUnknownBiomeWithPlains` were rewritten in place, not added — only the +two "unusable substitute" cases are new). Still a "more than accounted for," not "fewer," gap. + +### The archunit defect, and how it was actually closed + +**Found by this acceptance, reported rather than silently repaired, then fixed by the coordinator in +two separate, targeted commits — not by loosening the rules to fit the code.** + +At the previous measurement (`1664dcdd`), `net.onelitefeather.falco.architecture.ForeignCouplingTest` +failed 4 cases: `anvilCoreKnowsNoMinestom`, `blockRegistryOnlyInAdapters`, +`dynamicRegistryOnlyInBiomeResolver`, `byteLayerKnowsNoNbt`. The four new policy classes touched +Minestom and Kyori-NBT types that pre-existing allow-list regexes in that test did not name, because +none of the three implementation tasks ran `:falco-archunit:test` — it lives outside `falco-anvil` and +outside each task's own file list. + +Two different fixes landed, for two different reasons: + +1. **`fe569f5b` — `fix(anvil): let the unknown-entry policy name a substitute instead of resolving + one`.** This is a real interface change, not a workaround: `UnknownEntryPolicy.onUnknownBlock`/ + `onUnknownBiome` now return a palette **name** (`String`, e.g. `"minecraft:air"`) instead of an + **id** (`int`). `DefaultUnknownEntryPolicy` dropped its `Supplier>` field and + the double-checked-locking lazy resolution entirely — it now returns the literals + `"minecraft:air"`/`"minecraft:plains"` and touches neither Minestom nor a registry. The resolver + that already holds the registry (it just used it to look the original, unknown name up) resolves the + substitute name itself, and fails the chunk — without asking the policy a second time — if that name + is itself unknown, using `IllegalStateException` rather than `AnvilChunkException`: only + `FalcoAnvilLoader` is allowed to construct `AnvilChunkException` (an `exactlyOneTranslationPoint` + rule this same archunit suite enforces elsewhere), and a resolver is not that class. This closed + three of the four rules — `anvilCoreKnowsNoMinestom`, `blockRegistryOnlyInAdapters`, + `dynamicRegistryOnlyInBiomeResolver` — by removing the dependency the rules objected to, not by + widening any allow-list. `ANVIL_MINESTOM_BOUNDARY` (`FalcoAnvilLoader|BlockPaletteResolver| + BiomePaletteResolver`) is unchanged from before this whole plan. + Two new tests, `testAnUnusableSubstituteBlockFailsTheChunkAndNamesBothNames` and + `testAnUnusableSubstituteBiomeFailsTheChunkAndNamesBothNames`, assert the resolver throws + `IllegalStateException` naming both the original and the unusable substitute name, and that the + policy is asked for a substitute exactly once, not twice. +2. **`49854313` — `test(archunit): widen byteLayerKnowsNoNbt for the anvil policy classes`.** The + fourth rule stayed red on its own merits: `ChunkVersionPolicy`/`DefaultChunkVersionPolicy` and + `UnknownEntryPolicy`/`DefaultUnknownEntryPolicy` carry `CompoundBinaryTag` in their contracts by + design (deciding chunk readability, and identifying an unknown block/biome, both need the NBT data), + one layer above `RegionFile`'s pure-byte guarantee that this rule actually protects — there was no + dependency to remove here, unlike the Minestom case. `ANVIL_NBT_LAYER`, the hand-maintained name + list this rule checks against, had its own Javadoc corrected: it previously claimed to be a + self-maintaining complement, which is why nobody had extended it when the four policy classes were + first added. All four are now in the list, and the Javadoc says plainly that it is hand-maintained + and that a class added to this layer later has to be added here too — naming the four policy classes + as the example of what happens when that step is skipped. A Gegenprobe was run for this fix: + removing one class from the list turned the rule red again, and the failure message named exactly + that class. + +### `./gradlew :falco-anvil:test :falco-light:test :falco-instance:test :falco-demo:test :falco-benchmarks:test :falco-archunit:test --rerun-tasks` + +`BUILD SUCCESSFUL`. Module counts (from JUnit XML under `build/test-results/test/`, `` +elements counted directly and cross-checked against each file's `testsuite` summary attributes — the +two agreed on every file): + +| Module | Baseline at `1bdd0cca` (#45) | Count now | Delta | | --- | --- | --- | --- | -| falco-anvil | 230 | 253 | +23 | +| falco-anvil | 230 | 255 | +25 | | 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 total, **4 failing** | **regression** | - -No count *fell* in the sense of fewer test cases existing. **falco-archunit is the exception that -matters**: total case count held at 47, but 4 that passed at the baseline now fail — see "Defect -found" below. - -¹ `docs/superpowers/plans/2026-08-03-anvil-version-guard.md`, its own `## Result` section, measured at -branch tip `a74cd042` (which became `1bdd0cca` on `origin/main` via #45) — reused per this task's -instruction to take baseline numbers from the last acceptance report of an earlier undertaking rather -than standing up a second worktree. That report itself notes light/instance/demo/archunit at its -baseline commit `2d3955d8` were, by construction, identical to `1bdd0cca` (the branch touched only -`falco-anvil` files), so these five numbers are read as valid for `1bdd0cca` directly. No new worktree -was created for this acceptance. - -### Defect found: `falco-archunit`'s `ForeignCouplingTest` was never updated for the new policy classes - -**Not fixed — reported, per this task's explicit instruction not to silently repair a real defect found -during acceptance.** - -`./gradlew :falco-anvil:test :falco-light:test :falco-instance:test :falco-demo:test -:falco-benchmarks:test :falco-archunit:test --rerun-tasks` fails on `:falco-archunit:test`. Four cases -in `net.onelitefeather.falco.architecture.ForeignCouplingTest` go red: - -- `anvilCoreKnowsNoMinestom` (9 violations) -- `blockRegistryOnlyInAdapters` (1 violation) -- `dynamicRegistryOnlyInBiomeResolver` (3 violations) -- `byteLayerKnowsNoNbt` (9 violations) - -All four rules use an allow-list regex naming the specific classes in `net.onelitefeather.falco.anvil` -that are permitted to depend on Minestom/Kyori-NBT types, e.g. (`ForeignCouplingTest.java:52`): - -```java -+ "(FalcoAnvilLoader|BlockPaletteResolver|BiomePaletteResolver)(\\$.*)?"; -``` +| falco-archunit | 47 | **47** | **0 — the regression from the previous measurement is closed** | -Tasks 1–3 added `ChunkVersionPolicy`, `DefaultChunkVersionPolicy`, `UnknownEntryPolicy` and -`DefaultUnknownEntryPolicy` to `net.onelitefeather.falco.anvil`. Three of these four legitimately touch -the types the rule forbids — `DefaultChunkVersionPolicy` reads `CompoundBinaryTag` (needed to decide -chunk readability), `DefaultUnknownEntryPolicy` calls `Block.AIR`/`Block.AIR.stateId()` and -`MinecraftServer.getBiomeRegistry()`/`DynamicRegistry.getId(...)` (needed to reproduce the exact -air/plains fallback the two resolvers used to hard-code), and `UnknownEntryPolicy`'s own interface -method signature carries a `CompoundBinaryTag` parameter. None of these four classes is named in the -allow-list regex, so every one of these — architecturally intentional — dependencies now reads as a -violation. - -**Why none of the three implementation tasks caught this themselves:** each task's own verification ran -`:falco-anvil:test` (and `javadoc`/`checkApiCompatibility`), never `:falco-archunit:test` — that module -lives outside `falco-anvil` and was outside each task's own file list. This acceptance is the first point -in the plan that runs all six modules together, which is exactly why it exists. - -This is a real defect against the plan's own claim of "no module, no JPMS, no published signature" — -the *behaviour* is unchanged, but the *architecture rule* meant to keep `falco-anvil`'s core free of a -running-server dependency no longer reflects where the boundary actually is now that policies live -inside that same package. Left unfixed here; the two defensible directions for whoever picks this up -are (a) widen the allow-list regex to include the four new classes, or (b) move -`DefaultChunkVersionPolicy`/`DefaultUnknownEntryPolicy` to a place the existing rules already permit — -a design call outside this acceptance's scope. +No count fell. `falco-archunit` is back to 47/47 green, for the reasons above, not because the rules +were softened. ### `./gradlew build -x test --rerun-tasks` -`BUILD SUCCESSFUL`. `javadoc` genuinely executed (no `UP-TO-DATE`, full `--rerun-tasks` output grepped -for "warning" case-insensitively: zero matches) for the four modules that carry a javadoc task: +`BUILD SUCCESSFUL`. `javadoc` genuinely executed (no `UP-TO-DATE`; full `--rerun-tasks` output grepped +case-insensitively for "warning": zero matches) for the four modules that carry a javadoc task — falco-anvil, falco-light, falco-instance, falco-demo. `checkApiCompatibility` genuinely executed for -the three configured modules — falco-anvil, falco-light, falco-instance. All three reports read -literally: +the three configured modules — falco-anvil, falco-light, falco-instance. All three reports, verbatim: ``` Comparing binary compatibility of falco-anvil-1.0.0.jar against falco-anvil-1.0.0.jar No changes. ``` -(same for `falco-light` and `falco-instance`, each against its own `-1.0.0.jar`). `onlyBinaryIncompatibleModified` -is `true` for this task, so purely-additive surface (every new type and setter this plan added) does -not appear here by design — consistent with every prior task's own japicmp run. No exception entry was -needed in `gradle/api-breaks.properties` and none was added. +(same text for `falco-light-1.0.0.jar` and `falco-instance-1.0.0.jar`.) japicmp raised nothing — +`onlyBinaryIncompatibleModified` is `true` for this task, so purely-additive surface, including +`UnknownEntryPolicy`'s changed return type (`int` → `String`, a real signature change, but to a type +that was never in a published jar — it was introduced and reworked entirely within this unreleased +branch), does not appear here by design. No exception entry was needed in +`gradle/api-breaks.properties` and none was added. + +### Gate attacks (re-verified against the current tip, not re-run) + +Both gate attacks from the prior measurement still apply; per instruction they were not re-executed, +only checked that the test names they cite still exist at the current tip. All do, unchanged: + +**Attack 1 — delete the `ChunkVersionPolicy` service registration.** Confirmed present: +`FalcoAnvilLoaderIntegrationTest.testAPreRootLayoutChunkIsRefusedInsteadOfReadAsAir`, +`testAChunkBelowTheFloorIsRefused`, `testASectionsKeyStoredAsTheWrongTypeWithLevelIsRefused`, +`testAChunkWithADataVersionStoredAsTheWrongTypeIsRefused`, +`testAChunkWithANegativeDataVersionIsRefused`, and +`FalcoAnvilLoaderBuilderTest.testDiscoverVersionPolicySurvivesEveryOtherSetterAfterClearingAnExplicitOne` +— all six exist at the current tip with the same names. Removing the registration made these six fail +(253 tests completed, 6 failed, at the time of that run) — five rejection cases plus the one +pass-through test whose discovery fallback resolves to `null` instead of a `DefaultChunkVersionPolicy` +instance with nothing registered. This remains the executable record of what the optional guard costs: +with no provider on the classpath, every chunk that would have been refused loads instead, unchecked. +Reverted; `git status --short` was empty afterward. + +**Attack 2 — drop the two new policy fields from one builder setter.** Confirmed present: +`FalcoAnvilLoaderBuilderTest.testAnExplicitVersionPolicySurvivesEveryOtherSetter` and +`testAnExplicitUnknownEntryPolicySurvivesEveryOtherSetter` — both exist unchanged. Replacing the +trailing four constructor arguments in `Builder.openRegionLimit(int)` with `null, true, null, true` +made exactly these two go red (20 tests completed, 2 failed), the other 18 unaffected. Reverted; +`git status --short` empty afterward, full `falco-anvil` module re-run 253/253 green at that time (now +255/255 at the current tip, per the module run above). ### Machine load -`uptime` before the module run (14:39:57): `load average: 4.17, 5.18, 3.56` -`uptime` after the gate attacks and the full re-verification run (14:45:49): `load average: 8.68, 8.57, -5.79` +`uptime` before the module run (15:42:09): `load average: 6.84, 7.27, 7.09` +`uptime` after the module run (15:44:29): `load average: 17.86, 13.84, 9.74` -No timing figure was produced or is quoted anywhere in this section. +The machine was measurably busier by the end (consistent with earlier task reports on this same branch +noting a shared, non-idle machine), but every Gradle run in this measurement completed normally with +consistent, repeatable pass/fail outcomes. No timing figure is produced or quoted anywhere in this +section. -### What this work does not do +### What this work does not change -- No new module. Everything lives in `net.onelitefeather.falco.anvil`, in the six existing published - modules. -- No `module-info.java`, no JPMS. Plain classpath `ServiceLoader`. -- No published signature was removed or changed — `checkApiCompatibility` confirms this literally - ("No changes.") for all three configured modules. -- No behaviour changes for a caller who registers nothing and calls the builder as before: +- No new module. Everything lives in `net.onelitefeather.falco.anvil`, across the six existing + published modules. +- No `module-info.java`, no JPMS — plain classpath `ServiceLoader`, as the plan specified. +- No published signature removed or changed — `checkApiCompatibility` confirms this literally ("No + changes.") for all three configured modules; `UnknownEntryPolicy`'s `int` → `String` return-type + change is a real signature change but to a type with no published jar to break. +- No behaviour change for a caller who registers nothing and calls the builder as before: `discoverVersionPolicy`/`discoverUnknownEntryPolicy` default to `true`, so `builder()` and the two public constructors resolve `DefaultChunkVersionPolicy`/`DefaultUnknownEntryPolicy` exactly as #45's guard and the two resolvers' hard-coded fallbacks always did — **except that the guard is now - losable**: deleting one `META-INF/services` line (Gate attack 1, above) turns the same "default" + losable**: deleting one `META-INF/services` line (Attack 1, above) turns the same "default" configuration into "no check at all," which was not possible before this plan. That loss is a - deliberate project decision (documented in Task 2's own report and re-confirmed by this run), not an - oversight. + deliberate project decision, documented in Task 2's own report and re-confirmed by both measurements + of this section, not an oversight. ### Status -**DONE_WITH_CONCERNS.** All four acceptance steps ran to completion; every module's test count held or -grew; javadoc and `checkApiCompatibility` are clean; both gate attacks landed exactly where expected and -both were reverted cleanly. The concern is the `falco-archunit` regression above — a real defect, -reported rather than silently repaired, that needs a follow-up decision before this branch should be -considered fully clean. +**DONE.** All four acceptance steps ran to completion on the current tip; every module's test count +held or grew, including `falco-archunit`'s return to 47/47; javadoc and `checkApiCompatibility` are +clean; both gate attacks' test names were confirmed still valid and their previously-recorded results +remain accurate. The one open item from the prior measurement — the `falco-archunit` regression — is +closed, by a real interface fix in one case and a corrected, extended allow-list in the other, both +recorded above rather than left as an unexplained diff. From 6a238e7cc2080889f8b8b04a9bc9b5b88f88cf7d Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 16:25:27 +0200 Subject: [PATCH 14/18] fix(anvil): refuse a custom resolver configured together with an UnknownEntryPolicy FalcoAnvilLoader.Builder.build(...) silently dropped the configured UnknownEntryPolicy whenever a caller also supplied their own blockResolver or biomeResolver: the resolver is used exactly as given, so the policy was never reached even though loader.unknownEntryPolicy() kept reporting it as active. A caller who set both, e.g. unknownEntryPolicy(strict).blockResolver(mine), got a loader that silently substituted air for an unknown block instead of enforcing the configured policy. The constructor now refuses that combination with an IllegalStateException naming the slot to use instead, applying the same standard ServiceResolution.choose already holds for explicit configuration versus discovery. A resolver configured without touching either unknownEntryPolicy slot is unaffected: a new unknownEntryPolicyConfigured flag on the builder tracks whether unknownEntryPolicy(...) or discoverUnknownEntryPolicy() was actually called, separate from the default value both already carry. Verified with a temporary mutation: disabling the new guard turned the two new tests red, confirming they actually exercise it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../falco/anvil/FalcoAnvilLoader.java | 99 ++++++++++++++++--- .../anvil/FalcoAnvilLoaderBuilderTest.java | 55 +++++++++++ 2 files changed, 138 insertions(+), 16 deletions(-) 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 6c21333..a84b96b 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 @@ -247,6 +247,27 @@ private FalcoAnvilLoader(Path worldRoot, Key dimension, Builder settings) { this.legacyLayout = resolved.legacyLayout(); this.dimensionLabel = dimension.asString(); this.diagnostics = effective; + // A caller who names both a policy and their own resolver has built a loader in which the + // policy can never be reached: the resolver a builder is handed is used exactly as given, + // never rebuilt around the configured policy, so unknownEntryPolicy() below would keep + // reporting a policy that the actual decoding path never consults. Refusing the combination + // holds this to the same standard ServiceResolution.choose already applies to explicit + // configuration versus discovery: two conflicting explicit decisions are refused outright, + // not silently reconciled by picking one of them. A resolver configured without touching + // either unknownEntryPolicy slot is unaffected, because unknownEntryPolicyConfigured stays + // false for a builder that never called unknownEntryPolicy(...) or + // discoverUnknownEntryPolicy() itself. + if (settings.unknownEntryPolicyConfigured && (settings.blockResolver != null || settings.biomeResolver != null)) { + throw new IllegalStateException( + "An UnknownEntryPolicy was configured together with a custom " + + (settings.blockResolver != null ? "blockResolver" : "biomeResolver") + + ". A resolver supplied through the builder is used exactly as given and never " + + "sees the configured policy, so it would silently keep its own fallback instead. " + + "Pass the policy into the resolver you build instead, e.g. " + + "new BlockPaletteResolver(diagnostics, policy), and stop configuring it on this " + + "builder." + ); + } UnknownEntryPolicy resolvedUnknownEntryPolicy = ServiceResolution.choose( UnknownEntryPolicy.class, settings.unknownEntryPolicy, settings.discoverUnknownEntryPolicy, DefaultUnknownEntryPolicy.class); @@ -323,7 +344,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, - DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null, null, true, null, true); + DEFAULT_MINIMUM_DATA_VERSION, null, null, null, null, null, true, null, true, false); } /** @@ -371,6 +392,7 @@ public static final class Builder { private final boolean discoverVersionPolicy; private final @Nullable UnknownEntryPolicy unknownEntryPolicy; private final boolean discoverUnknownEntryPolicy; + private final boolean unknownEntryPolicyConfigured; private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, int dataVersion, int minimumDataVersion, @Nullable AnvilDiagnostics diagnostics, @@ -380,7 +402,8 @@ private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, @Nullable ChunkVersionPolicy versionPolicy, boolean discoverVersionPolicy, @Nullable UnknownEntryPolicy unknownEntryPolicy, - boolean discoverUnknownEntryPolicy) { + boolean discoverUnknownEntryPolicy, + boolean unknownEntryPolicyConfigured) { this.openRegionLimit = openRegionLimit; this.compressionLevel = compressionLevel; this.saveParallelism = saveParallelism; @@ -394,6 +417,7 @@ private Builder(int openRegionLimit, int compressionLevel, int saveParallelism, this.discoverVersionPolicy = discoverVersionPolicy; this.unknownEntryPolicy = unknownEntryPolicy; this.discoverUnknownEntryPolicy = discoverUnknownEntryPolicy; + this.unknownEntryPolicyConfigured = unknownEntryPolicyConfigured; } /** @@ -424,7 +448,8 @@ public Builder openRegionLimit(int openRegionLimit) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -459,7 +484,8 @@ public Builder compressionLevel(int compressionLevel) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -490,7 +516,8 @@ public Builder saveParallelism(int saveParallelism) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -518,7 +545,8 @@ public Builder dataVersion(int dataVersion) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -552,7 +580,8 @@ public Builder minimumDataVersion(int minimumDataVersion) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -585,7 +614,8 @@ public Builder diagnostics(AnvilDiagnostics diagnostics) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -597,6 +627,14 @@ public Builder diagnostics(AnvilDiagnostics diagnostics) { * blocks although the resolver saw them. {@link PaletteEntryResolver} exposes no * diagnostics, so this cannot be checked in {@code build}. *

+ *

+ * Do not combine this with {@link #unknownEntryPolicy(UnknownEntryPolicy)} or + * {@link #discoverUnknownEntryPolicy()}. Whatever policy those slots resolve to is never + * handed to a resolver supplied here — {@link #build(Path, Key)} refuses the combination + * rather than build a loader whose configured policy is silently unreachable. Pass the + * policy into your own resolver instead, the same way the shipped resolvers accept one in + * their constructor. + *

* * @param blockResolver the resolver for block palette entries * @return a new builder with this value @@ -615,14 +653,17 @@ public Builder blockResolver(PaletteEntryResolver blockResolver) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** * Sets the resolver which turns biome palette entries into ids. *

* The same caveat as {@link #blockResolver(PaletteEntryResolver)}: a resolver of your own - * counts past the diagnostics of the loader. + * counts past the diagnostics of the loader, and the same restriction against combining it + * with {@link #unknownEntryPolicy(UnknownEntryPolicy)} or + * {@link #discoverUnknownEntryPolicy()} applies, for the same reason. *

* * @param biomeResolver the resolver for biome palette entries @@ -642,7 +683,8 @@ public Builder biomeResolver(PaletteEntryResolver biomeResolver) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -678,7 +720,8 @@ public Builder exceptionHandler(Consumer exceptionHandler) { this.versionPolicy, this.discoverVersionPolicy, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -717,7 +760,8 @@ public Builder versionPolicy(@Nullable ChunkVersionPolicy versionPolicy) { versionPolicy, false, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -749,7 +793,8 @@ public Builder discoverVersionPolicy() { null, true, this.unknownEntryPolicy, - this.discoverUnknownEntryPolicy); + this.discoverUnknownEntryPolicy, + this.unknownEntryPolicyConfigured); } /** @@ -771,6 +816,14 @@ public Builder discoverVersionPolicy() { * not otherwise decode. *

* + *

+ * Do not combine this with {@link #blockResolver(PaletteEntryResolver)} or + * {@link #biomeResolver(PaletteEntryResolver)}. A resolver supplied through either of + * those slots is used exactly as given and is never rebuilt around this policy, so + * {@link #build(Path, Key)} refuses the combination instead of building a loader whose + * configured policy would never actually run. + *

+ * * @param unknownEntryPolicy the policy to consult for an unknown palette entry, or null to * fall back to the classpath default * @return a new builder with this value @@ -790,7 +843,8 @@ public Builder unknownEntryPolicy(@Nullable UnknownEntryPolicy unknownEntryPolic this.versionPolicy, this.discoverVersionPolicy, unknownEntryPolicy, - false); + false, + true); } /** @@ -804,6 +858,12 @@ public Builder unknownEntryPolicy(@Nullable UnknownEntryPolicy unknownEntryPolic * {@link Builder#build(Path, Key)} throw {@link IllegalStateException} rather than guess * between them. *

+ *

+ * Calling this together with {@link #blockResolver(PaletteEntryResolver)} or + * {@link #biomeResolver(PaletteEntryResolver)} is refused by {@link #build(Path, Key)} for + * the same reason {@link #unknownEntryPolicy(UnknownEntryPolicy)} is: a resolver supplied + * through either of those slots never sees whatever this discovers. + *

* * @return a new builder with this value * @since 2.1.0 @@ -822,6 +882,7 @@ public Builder discoverUnknownEntryPolicy() { this.versionPolicy, this.discoverVersionPolicy, null, + true, true); } @@ -839,7 +900,13 @@ public Builder discoverUnknownEntryPolicy() { * @throws IllegalStateException if this builder was left on classpath discovery and the * classpath registers more than one foreign * {@link ChunkVersionPolicy} or more than one foreign - * {@link UnknownEntryPolicy} + * {@link UnknownEntryPolicy}, or if + * {@link #unknownEntryPolicy(UnknownEntryPolicy)} or + * {@link #discoverUnknownEntryPolicy()} was called on this + * builder together with {@link #blockResolver(PaletteEntryResolver)} + * or {@link #biomeResolver(PaletteEntryResolver)} — a custom + * resolver never sees the configured policy, so the combination + * would silently drop it instead of applying it */ @Contract(value = "_, _ -> new", pure = true) public FalcoAnvilLoader build(Path worldRoot, Key dimension) { 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 ffe1ebf..fd56a70 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 @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Pins down the builder of the loader. @@ -380,6 +381,60 @@ void testDiscoverUnknownEntryPolicySurvivesEveryOtherSetterAfterClearingAnExplic } } + /** + * The combination this class exists to refuse: an explicit {@link UnknownEntryPolicy} and a + * custom resolver together. A resolver supplied through {@link Builder#blockResolver} is used + * exactly as given, so the configured policy would never actually be consulted while + * {@link FalcoAnvilLoader#unknownEntryPolicy()} kept reporting it as active — the exact trap a + * caller who names both slots would otherwise fall into silently. + */ + @Test + void testCombiningAnExplicitUnknownEntryPolicyWithACustomBlockResolverIsRefused() { + FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder() + .unknownEntryPolicy(new DefaultUnknownEntryPolicy()) + .blockResolver(new RefusingResolver()); + + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> builder.build(this.worldRoot, OVERWORLD)); + assertTrue(failure.getMessage().contains("UnknownEntryPolicy"), failure.getMessage()); + assertTrue(failure.getMessage().contains("blockResolver"), failure.getMessage()); + } + + /** + * The mirror of the block resolver case, and the other slot the guard has to consider: an + * explicit request for classpath discovery combined with a custom biome resolver is refused for + * the same reason a resolved instance is — a resolver supplied through + * {@link Builder#biomeResolver} never sees whatever discovery would have found. + */ + @Test + void testCombiningDiscoverUnknownEntryPolicyWithACustomBiomeResolverIsRefused() { + FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder() + .discoverUnknownEntryPolicy() + .biomeResolver(new RefusingResolver()); + + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> builder.build(this.worldRoot, OVERWORLD)); + assertTrue(failure.getMessage().contains("UnknownEntryPolicy"), failure.getMessage()); + assertTrue(failure.getMessage().contains("biomeResolver"), failure.getMessage()); + } + + /** + * The existing case the guard must not break: a custom resolver on its own, without either + * {@link Builder#unknownEntryPolicy} or {@link Builder#discoverUnknownEntryPolicy} ever being + * called, still builds. {@link #testAGivenBlockResolverIsTheOneTheLoaderDecodesWith(Env)} already + * exercises this shape end to end; this test pins the construction itself so the guard's + * "configured" flag, not merely a null check, is what the refusal above keys on. + */ + @Test + void testACustomBlockResolverWithoutAnyPolicyConfigurationStillBuilds() throws Exception { + try (FalcoAnvilLoader loader = FalcoAnvilLoader.builder() + .blockResolver(new RefusingResolver()) + .build(this.worldRoot, OVERWORLD)) { + + assertNotNull(loader); + } + } + @Test void testTheBuilderCanBeReusedAfterASlotChanged() throws Exception { FalcoAnvilLoader.Builder builder = FalcoAnvilLoader.builder().openRegionLimit(8); From 54ab652361af27939759e4a246672e5d4d829c98 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 16:26:18 +0200 Subject: [PATCH 15/18] docs(anvil): document that both policies are called concurrently and can be shared ChunkVersionPolicy and UnknownEntryPolicy are resolved once when a loader is built and then consulted by every parallel load afterward, since FalcoAnvilLoader.supportsParallelLoading() is true. Neither interface said so, leaving a foreign implementer to discover the requirement by reading the loader rather than the contract. Both javadocs now state the requirement explicitly and note that the shipped defaults are stateless. The two builder slots that accept an explicit instance (versionPolicy(...), unknownEntryPolicy(...)) now carry the same note the diagnostics slot already implies: an instance passed there is shared by every loader built from that builder afterward, so it has to tolerate exactly the concurrent, shared use the interface javadoc now documents. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../falco/anvil/ChunkVersionPolicy.java | 7 +++++++ .../onelitefeather/falco/anvil/FalcoAnvilLoader.java | 12 ++++++++++++ .../falco/anvil/UnknownEntryPolicy.java | 8 ++++++++ 3 files changed, 27 insertions(+) diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java index 6e3c569..2c36309 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/ChunkVersionPolicy.java @@ -10,6 +10,13 @@ * records it in its {@link AnvilDiagnostics} and writes the log line, so every diagnostic of a load * stays in one place and this contract stays free of the loader's infrastructure. *

+ *

+ * Called from several threads at once. The policy is resolved once, when the loader is built, + * and every load after that consults the same instance — including every parallel load, since + * {@link FalcoAnvilLoader#supportsParallelLoading()} reports {@code true}. An implementation + * therefore has to be thread-safe on its own; the loader takes no lock around the call. + * {@link DefaultChunkVersionPolicy}, the shipped default, holds no state and needs none. + *

* * @author TheMeinerLP * @version 1.0.0 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 a84b96b..0339d19 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 @@ -740,6 +740,12 @@ public Builder exceptionHandler(Consumer exceptionHandler) { * see {@code testWithoutAnyPolicyALegacyChunkIsNotChecked} in the loader's integration * tests for the case that documents it. *

+ *

+ * An explicit instance passed here is shared by every loader this builder builds afterward, + * the same way an explicit {@link #diagnostics(AnvilDiagnostics)} instance is — and + * {@link ChunkVersionPolicy} is called from every one of those loaders' parallel loads at + * once, so it has to tolerate that sharing the way {@link AnvilDiagnostics} already does. + *

* * @param versionPolicy the policy to consult before a chunk is decoded, or null to consult * none @@ -823,6 +829,12 @@ public Builder discoverVersionPolicy() { * {@link #build(Path, Key)} refuses the combination instead of building a loader whose * configured policy would never actually run. *

+ *

+ * An explicit instance passed here is shared by every loader this builder builds afterward, + * the same way an explicit {@link #diagnostics(AnvilDiagnostics)} instance is — and + * {@link UnknownEntryPolicy} is called from every one of those loaders' parallel loads at + * once, so it has to tolerate that sharing the way {@link AnvilDiagnostics} already does. + *

* * @param unknownEntryPolicy the policy to consult for an unknown palette entry, or null to * fall back to the classpath default diff --git a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java index 2c37801..b36ec4e 100644 --- a/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java +++ b/falco-anvil/src/main/java/net/onelitefeather/falco/anvil/UnknownEntryPolicy.java @@ -26,6 +26,14 @@ * policy get asked twice: if the name it returns is itself unknown, the resolver fails the chunk * instead of consulting the policy again, which would risk a loop. *

+ *

+ * Called from several threads at once. The policy is resolved once, when the loader is built, + * and both {@link BlockPaletteResolver} and {@link BiomePaletteResolver} keep consulting that same + * instance for as long as the loader lives — including every parallel load, since + * {@link FalcoAnvilLoader#supportsParallelLoading()} reports {@code true}. An implementation + * therefore has to be thread-safe on its own; neither resolver takes a lock around the call. + * {@link DefaultUnknownEntryPolicy}, the shipped default, holds no state and needs none. + *

* * @author TheMeinerLP * @version 1.1.0 From a0706db7bfb9ec20abaf26212ec7c5483455004c Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 16:26:39 +0200 Subject: [PATCH 16/18] test(anvil): assert the legacy no-policy chunk actually decodes to air testWithoutAnyPolicyALegacyChunkIsNotChecked only asserted assertNotNull on the loaded chunk, even though its own javadoc claims "it loads ... as a chunk of air" -- nothing verified that. The test now reads a block back with the existing blockAt(...) helper and asserts it is Block.AIR, so the assertion matches what the test documents. Verified with a temporary mutation: changing the expected block to Block.STONE turned the test red, confirming the assertion is actually exercised. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../falco/anvil/FalcoAnvilLoaderIntegrationTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 10bb7ab..cfeeb29 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 @@ -821,8 +821,13 @@ void testWithoutAnyPolicyALegacyChunkIsNotChecked(Env env) throws Exception { .versionPolicy(null) .build(this.worldRoot, OVERWORLD)) { Instance instance = env.createEmptyInstance(loader); + Chunk chunk = loader.loadChunk(instance, 12, 12); - assertNotNull(loader.loadChunk(instance, 12, 12)); + assertNotNull(chunk); + // The claim of this test's own javadoc: not merely that a chunk comes back, but that it + // is the chunk of air the legacy Level layout decodes to when sections is absent and no + // policy caught it. + assertEquals(Block.AIR, blockAt(chunk, 0, 40, 0)); } } From 67f58ba19ca96198701374a6b9e5f3ae82cb9649 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 16:26:56 +0200 Subject: [PATCH 17/18] test(anvil): assert the shipped default's name is absent from the refusal message testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered only checked that the two foreign provider names appear in the refusal message, never that the shipped default's name does not -- the other half of the rule the spec states: a default that stepped aside for a foreign provider is not one of the competing candidates and should not be named as one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../onelitefeather/falco/anvil/ServiceResolutionTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java index abb2737..8b1784f 100644 --- a/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java +++ b/falco-anvil/src/test/java/net/onelitefeather/falco/anvil/ServiceResolutionTest.java @@ -3,6 +3,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -166,5 +167,10 @@ void testTwoForeignProvidersAreStillRefusedEvenWithAShippedDefaultRegistered() { assertTrue(failure.getMessage().contains("FirstForeignProvider"), failure.getMessage()); assertTrue(failure.getMessage().contains("SecondForeignProvider"), failure.getMessage()); + // The other half of the rule the spec states: the shipped default stepped aside because a + // foreign provider was found, so it is not one of the competing opinions the message lists. + // Naming it here too would send the reader chasing a class that was never actually a + // candidate. + assertFalse(failure.getMessage().contains("AnotherShippedDefault"), failure.getMessage()); } } From f41530b1af017c0662c3e70a19a2d17282c4a492 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Tue, 4 Aug 2026 16:27:12 +0200 Subject: [PATCH 18/18] docs(spec): pull the UnknownEntryPolicy signature in the design doc to what shipped The design section still declared int onUnknownBlock(...) / int onUnknownBiome(...) and said "returning an id substitutes it," but the interface that actually shipped returns String, not int. The registry lookup that turns a name into an id belongs in the one adapter that already owns it -- the resolver -- not duplicated into every UnknownEntryPolicy implementation, which is why the signature changed during implementation. The plan's Result section already documents this as superseded; only the design section still showed the old shape. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jtJ4GUtmyCSHkiGY1CvgR --- .../2026-08-04-anvil-extension-points-design.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md index 08d58df..d37dd91 100644 --- a/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md +++ b/docs/superpowers/specs/2026-08-04-anvil-extension-points-design.md @@ -74,15 +74,19 @@ number, because the layout check does not rest on a version at all. ```java public interface UnknownEntryPolicy { - int onUnknownBlock(String name, @Nullable CompoundBinaryTag properties) throws ChunkDataException; + String onUnknownBlock(String name, @Nullable CompoundBinaryTag properties); - int onUnknownBiome(String name) throws ChunkDataException; + String onUnknownBiome(String name); } ``` Consulted by `BlockPaletteResolver` and `BiomePaletteResolver` where they substitute today. Returning -an id substitutes it; throwing fails the chunk. The built-in implementation returns air and plains and -keeps the existing counting, so behaviour without a provider is byte-for-byte what it is now. +a name substitutes it; throwing `AnvilChunkException` fails the chunk. Built as `String` rather than +`int` as this section originally specified: the id belongs to the registry lookup, and the project's +architecture rule keeps that lookup in exactly one adapter — the resolver that already owns it for the +entry it could not otherwise decode — not duplicated into every implementation of this interface. The +built-in implementation returns `"minecraft:air"` and `"minecraft:plains"` and keeps the existing +counting, so behaviour without a provider is byte-for-byte what it is now. This is the seam `falco-migration` needs: a converter installs a policy that throws, because on the upgrade path an unmappable block means the mapping data is incomplete and must be seen.