feat: give the three modules a fluent construction API by TheMeinerLP · Pull Request #16 · OneLiteFeatherNET/Falco

TheMeinerLP · 2026-08-01T21:31:45Z

Proposed changes

Builders for falco-anvil, falco-light and falco-instance, and the change that finally lets the
three run as one stack. Written from
Research: Fluent API, following
its sequence; three of its conclusions did not survive contact with the code and are listed below.

FalcoInstance instance = FalcoInstance.builder(DimensionType.OVERWORLD)
        .chunkLoader(FalcoAnvilLoader.builder().compressionLevel(9).build(root, OVERWORLD))
        .chunkSupplier(scheduler.supplier())
        .chunkLifecycle(c -> ((FalcoLightingChunk) c).markLoaded(),
                        c -> ((FalcoLightingChunk) c).markUnloaded())
        .ownsLoader(true)
        .registerAndShutdownWith(instanceManager, schedulerManager);

The blocker is gone

FalcoInstance accepted only FalcoChunk, because Chunk#onLoad() and Chunk#unload() are
protected and a package can drive them only on a type it defines itself. FalcoLightingChunk
extends DynamicChunk, so the two modules could not be combined — both demo stacks run on
InstanceContainer for that reason.

setChunkLifecycle hands the two hooks to whoever owns the chunk type. Neither module learns about
the other: falco-instance sees a Consumer<Chunk>, falco-light sees its own class, and the cast
is written by the caller. That is what keeps the three module isolation rules green, and it is why
this shape beat an interface both modules would have to name.

Without a configured lifecycle nothing changes, and the check stays in completeLoad ahead of
publishChunk. That order is load bearing: moving it into the lifecycle call would let a foreign
chunk enter the chunk map and get a partition before anything threw.

Two mechanisms, checked against their own absence

Rather than asserting that they work:

Immutable builders. One field from final to mutable → sharedStateIsSafelyPublished turns
red, because a class holding a java.util.concurrent field must publish every field safely. A
classic setter builder would fail CI.
The lifecycle. Removing the setChunkLifecycle call from the integration test brings back
exactly the old FalcoInstanceException.

Where this departs from the research page

Question 13 was not a defect. The copy of a lighting chunk does not lose its light — the
sections are cloned with it — and losing the scheduler binding is correct, since a copy goes into
another instance and a bound copy would turn the first block change into an
IllegalStateException. A test and a javadoc paragraph replace the planned bugfix.
The anvil builder is immutable, against the page's design. Three builders with two semantics is
a trap: the same line means two different things, and the variant that silently does nothing is the
one nobody notices. The page's argument for mutability (reuse across dimensions) does not hold —
an immutable builder is reused identically.
ChunkLightArea.remove(ChunkArea) was unnecessary (already merged in fix(light): forget chunks that left the instance #14):
computeIncrementally drops the kept light itself.

Note on merging

Eight commits, deliberately separate. A squash merge collapses them into this title. One of them
is refactor(anvil)! with a BREAKING CHANGE footer, which Release Please would read as a major
bump — the affected type, FalcoAnvilLoader.Builder, has never been in a release, so that bump would
be for nothing. Either Rebase and merge and drop the ! first, or squash under this feat: title,
which loses the footer and is the honest outcome here. Say which and I will adjust.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Performance change (behaviour unchanged, cost changed)
Documentation Update (if none of the other choices apply)

The breaking-change box is deliberately unticked: the only ! commit touches a type introduced
earlier in this same branch. No released API changes. All existing constructors and setters are
untouched.

Checklist

I have read the CONTRIBUTING.md
The title of this pull request is a Conventional Commit — see the note on merging
I have added tests that prove my fix is effective or that my feature works, and I wrote them before the
implementation and watched them fail for the right reason
Tests are package-private, named test<What><Expectation>, and use plain JUnit assertions
Every new or changed class and method carries Javadoc with @param / @return / @throws
I have not written @NotNull; the package @NotNullByDefault covers it
./gradlew build is green locally — 656 tests, 0 failures, including all 39 architecture rules
I have added necessary documentation (if appropriate)

falco-demo gained testImplementation(libs.cyano): the integration test needs a real Minestom
environment, and this is the only module that may know all three modules at once.

Not done: a third ServerStack entry. That would be an extension of the demo rather than a
follow-up, and it would work against its purpose — the comparison holds only while the two servers
differ in the loader and the chunk type and in nothing else. The note that claimed the combination
was impossible is corrected instead.

Concurrency

I have stated below which state the change adds or shares, and what guards it
Any new mutable state is either confined to one call or explicitly documented as thread-safe
I have added or extended a *ConcurrencyTest where the change touches shared state

What is shared, and how is it guarded? The builders are immutable and confined to the thread that
configures them; every field is final, which is what sharedStateIsSafelyPublished demands of the
light builder in particular, since it holds an Executor.

On the instances, three fields are new and all three are volatile, for the reason chunkSupplier
and chunkLoader already are (#11): they are caller objects written by a public setter and read on
the load path from another thread — chunkLoaded, chunkUnloaded, saveOnShutdown and ownsLoader.
The lifecycle pair is read through a local so the two halves of one decision cannot come from two
different configurations.

FalcoAnvilLoader and ChunkLightScheduler both resolve their failure sink per failure rather
than capturing it, so a loader or scheduler built before MinecraftServer.init() does not die in its
own error path on a null pointer that hides the real cause.

No *ConcurrencyTest was extended: the builders add no sharing, and the lifecycle consumers are read
on the same path and under the same publication guarantee as the supplier next to them.

🤖 Generated with Claude Code

Six values the constructors set for themselves were unreachable from outside: the compression level, the diagnostics, both palette resolvers, the save parallelism and the data version. PaletteEntryResolver is the sharpest case - a published, documented, experimental interface that SectionCodec takes on all four of its methods, which no consumer could get into the loader. ChunkCompression.compress says in its own javadoc that a caller who writes a world once and reads it often should pass a higher level; that caller could not. The two constructors stay and are unchanged for callers. The three parameter one now delegates through the builder, which is what keeps its bound check in one place. Both required values sit in build(Path, Key) rather than in slots, and there is deliberately no copy() or toBuilder(): the loader stores neither the world root nor the key, and reconstructing them would be wrong on a legacy world, where the overworld and the nether resolve to the same directory and would write over each other. Three slots check immediately instead of in build(). A compression level outside the supported range does not fail construction - saveChunk catches the exception and swallows it with a log line, so the world would silently stop being written. exceptionHandler resolves its default per error rather than capturing it. MinecraftServer.getExceptionManager() reads a field only init() sets, so a loader used by a tool without a server process would die in its own error path on a null pointer that hides the actual cause. The diagnostics default to a new instance per build() so two loaders from one builder do not share the throttles of their warnings, and the default resolvers are created from whichever diagnostics are effective, so the order of the setter calls does not matter. A resolver of your own counts past the loader's diagnostics; that cannot be checked, so it is stated on the slot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The kept-chunk count was reachable only through the four parameter constructor, so a caller who wanted to change it had to name an executor and an area size as well - and could not name the default executor, which was private until #14. Worse, maxAreaSize and maxCachedChunks are both int and stand next to each other there: swapped, it compiles and runs with a silent misconfiguration. Three things become configurable that were not reachable at all: whether a pass computes sky light, where the failure of an area is reported, and what observes a finished area. The builder is immutable - every slot returns a new builder. That is not a style choice. The builder has to hold an Executor for its slot, and falco-archunit selects any class declaring a field from java.util.concurrent and then requires every field of it to be final, volatile or synchronized-written. A classic setter builder writes its fields from ordinary slot methods and fails that rule; this was verified by making a single field non-final, which turns sharedStateIsSafelyPublished red with exactly that message. Final fields pass, and a builder that cannot be changed under a caller is the better object anyway. The executor is the one value not resolved until build(), so a builder that is never built starts no threads, and two schedulers from one builder each get their own bound. SkyLight is a three-state, not a flag per kind of light: only the block light pass yields the written chunks, and entries leave the dirty set through those, so a scheduler without a block light pass would never clear its dirty set. That configuration is deliberately not representable. onFailure resolves its default per failure rather than capturing it, the same shape the anvil builder uses, and for the same reason: a scheduler built before MinecraftServer.init() would otherwise die in its own error path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Question 13 of the fluent API research called the missing copy override a defect, on the reasoning that a copy returns a bare DynamicChunk and loses light and lifecycle hooks. Checked against the code, it is not one, and the test says why. The light is not lost: the inherited implementation clones the sections and the light data goes with them, so the copy carries what it was made from as a snapshot. The binding is lost, and that is the point - Minestom copies a chunk into another instance, a scheduler serves exactly one, and a copy that kept the binding would turn the first block change placed into it into an IllegalStateException. This is the opposite of FalcoChunk, which does override copy so FalcoInstance can still unload the copy. The two look inconsistent and are not, so the class javadoc now states it rather than leaving the next reader to rediscover it and "fix" the difference away. No production behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Chunk#onLoad() and Chunk#unload() are protected, so an instance implementation in another module cannot drive them. That is one half of why FalcoInstance and FalcoLightingChunk cannot be used together today: the instance accepts only FalcoChunk, which offers exactly these two methods, while a lighting chunk extends DynamicChunk and offers neither. markLoaded() and markUnloaded() are word for word what FalcoChunk already has. Nothing else changes, and nothing in falco-light calls them - they exist for a caller that owns both modules, which keeps the module boundary intact: falco-light still knows nothing about falco-instance. The test drives the observable effect rather than the flag. A freshly built DynamicChunk already reports isLoaded() == true, so what markLoaded proves is that onLoad ran: the chunk reports itself to its scheduler, which is the line that keeps a world that is only ever read from staying black. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

FalcoInstance accepted only FalcoChunk, and the reason was structural rather than a preference: Chunk#onLoad() and Chunk#unload() are protected, so this package can drive them on a type it defines itself and on nothing else. A lighting chunk extends DynamicChunk and offers neither, so the two modules could not be combined - both demo stacks run on InstanceContainer for that reason, and ServerStack argues it at length. setChunkLifecycle hands the two hooks to whoever owns the chunk type. The instance then manages any DynamicChunk. Neither module learns about the other: falco-instance sees a Consumer<Chunk>, falco-light sees its own class, and the cast that connects them is written by the caller. That is what keeps the three module isolation rules of falco-archunit green, and it is why this shape was chosen over an interface both modules would have to name. Without a configured lifecycle nothing changes: the check for FalcoChunk runs where it ran before, in completeLoad and ahead of publishChunk. That order is load bearing - moving the check into the lifecycle call would let a foreign chunk enter the chunk map and get a partition before anything threw, and testAForeignChunkSupplierIsRejected would find it through getChunk afterwards. The proof is three tests in falco-demo, the only module allowed to know all three. Removing the setChunkLifecycle call from the first one brings back exactly the old FalcoInstanceException, which is what says the test is testing something. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Setting a world up takes seven independent statements today plus a shutdown task, and exactly one of them is the one whose absence shows up days later: the one that saves. registerAndShutdownWith makes it part of the expression in which the instance first becomes reachable, so "instance built, shutdown forgotten" stops being a path a caller can take. shutdown(InstanceManager) fixes the order that costs data if it is wrong. saveChunksToStorage takes a snapshot of the chunk map and unregister empties that map, so a save after the unregister writes nothing and reports success. Saving therefore comes first, and a failed save stops the shutdown rather than carrying on: the chunks stay in memory and the instance stays registered, so a second attempt can still succeed. Carrying on would drop exactly the chunks whose saving just failed. ownsLoader defaults to false because a loader is usually shared - the overworld and the nether of one world are two instances on one loader, and the first to shut down must not close it under the second. saveOnShutdown defaults to true, because saving a world nobody changed costs time while not saving one that was changed costs the changes. The builder is immutable, like the one in falco-light, so the two read the same way. The existing constructors and setters are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The three builders of the project behaved differently: the light and the instance builder return a new builder from every slot, this one mutated and returned itself. That is a trap rather than a preference - the same line written against two of them means two different things, and the variant that silently does nothing is the one nobody notices. The research page had argued for a mutable builder here on the grounds that one builder produces several loaders for several dimensions. That argument does not hold: an immutable builder is reused exactly the same way, since build(Path, Key) takes the values that differ per loader anyway. BREAKING CHANGE: FalcoAnvilLoader.Builder slots return a new builder instead of the same one. Chained calls are unaffected; a caller who relied on mutating a builder through a stored reference has to keep the returned value. The type was never published in a release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ServerStack argued at length that FalcoInstance cannot be combined with FalcoLightingChunk, and its note() repeated it to whoever reads the server log. That was true when it was written and stopped being true with setChunkLifecycle. The decision itself does not change: the demo keeps both stacks on an InstanceContainer, because the comparison is worth something only while the two servers differ in the loader and the chunk type and in nothing else. Adding the instance to the Falco side alone would make them differ in three. Only the reason is now the real one. The test pinned the old reason through the word "FalcoChunk" and follows the new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-01T21:35:35Z

Test results

189 files 189 suites 4m 35s ⏱️
653 tests 653 ✅ 0 💤 0 ❌
1 968 runs 1 968 ✅ 0 💤 0 ❌

Results for commit 498c758.

The build had nothing that notices when a published signature changes. The architecture rules say so in their own javadoc - "the build has no japicmp, no revapi and no Error Prone, so nothing but these rules notices when a type slips into repo.onelitefeather.dev without its stability marker" - and with three builders added in #16 the gap is worth closing before the next release rather than after it. checkApiCompatibility runs for the three published library modules, compares their jar against apiBaselineVersion from gradle.properties, and is wired into check. falco-bom is excluded: a java-platform has no classes. Two traps were found by trying to break it on purpose rather than by trusting the configuration, and both would have shipped a task that passes whatever happens: newClasspath.from(tasks.named<Jar>("jar")) hands japicmp the task, not its archive, so it compared the baseline against itself. It needs flatMap { it.archiveFile }. Worse, Gradle substitutes an external dependency with a project of the same group and name from the same build - regardless of version. The baseline net.onelitefeather:falco-anvil:0.3.0 therefore resolved to project ':falco-anvil', and japicmp compared the local jar with the local jar and reported "No changes" for a removed public method. Resolving it through a detached configuration keeps it out of the project graph and fixes it. A doFirst guard now fails the task if the baseline ever resolves to this build's own output again, because the failure mode of both traps was a green check that verified nothing. Verified by making regionDirectory() package-private: japicmp reports "MODIFIED METHOD: PACKAGE_PROTECTED (<- PUBLIC)" and fails the build, with the release version as well as with -Psnapshot. Against 0.3.0 the current main is clean in all three modules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…20) The build had nothing that notices when a published signature changes. The architecture rules say so in their own javadoc - "the build has no japicmp, no revapi and no Error Prone, so nothing but these rules notices when a type slips into repo.onelitefeather.dev without its stability marker" - and with three builders added in #16 the gap is worth closing before the next release rather than after it. checkApiCompatibility runs for the three published library modules, compares their jar against apiBaselineVersion from gradle.properties, and is wired into check. falco-bom is excluded: a java-platform has no classes. Two traps were found by trying to break it on purpose rather than by trusting the configuration, and both would have shipped a task that passes whatever happens: newClasspath.from(tasks.named<Jar>("jar")) hands japicmp the task, not its archive, so it compared the baseline against itself. It needs flatMap { it.archiveFile }. Worse, Gradle substitutes an external dependency with a project of the same group and name from the same build - regardless of version. The baseline net.onelitefeather:falco-anvil:0.3.0 therefore resolved to project ':falco-anvil', and japicmp compared the local jar with the local jar and reported "No changes" for a removed public method. Resolving it through a detached configuration keeps it out of the project graph and fixes it. A doFirst guard now fails the task if the baseline ever resolves to this build's own output again, because the failure mode of both traps was a green check that verified nothing. Verified by making regionDirectory() package-private: japicmp reports "MODIFIED METHOD: PACKAGE_PROTECTED (<- PUBLIC)" and fails the build, with the release version as well as with -Psnapshot. Against 0.3.0 the current main is clean in all three modules. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

TheMeinerLP and others added 8 commits August 1, 2026 21:53

TheMeinerLP requested a review from a team as a code owner August 1, 2026 21:31

TheMeinerLP merged commit 86f098b into main Aug 1, 2026
8 checks passed

github-actions Bot mentioned this pull request Aug 1, 2026

chore(main): release 1.0.0 #9

Open

This was referenced Aug 1, 2026

test(anvil): cover the two builder slots that were only asserted by symmetry #19

Merged

ci: check binary compatibility against the last release with japicmp #20

Merged

TheMeinerLP deleted the feat/fluent-construction-api branch August 2, 2026 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: give the three modules a fluent construction API - #16

feat: give the three modules a fluent construction API#16
TheMeinerLP merged 8 commits into
mainfrom
feat/fluent-construction-api

TheMeinerLP commented Aug 1, 2026

Uh oh!

github-actions Bot commented Aug 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

TheMeinerLP commented Aug 1, 2026

Proposed changes

The blocker is gone

Two mechanisms, checked against their own absence

Where this departs from the research page

Note on merging

Types of changes

Checklist

Concurrency

Uh oh!

github-actions Bot commented Aug 1, 2026

Test results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant