From 374c59c4a625eac71634dcb240e470fc12d5a67d Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:13:11 +0200 Subject: [PATCH 01/11] build: add Falco anvil loader and light engine to the version catalog Declares net.onelitefeather:falco-anvil and :falco-light in the inline version catalog and adds the public OneLiteFeather release repository they are published to. The :common module consumes both, since the map handling that will use them lives there. The lobby spec names 0.3.0. That version predates Minestom 26.1, so it is not the one that resolves here: 2.1.0 is the current release and the one whose mycelium BOM lines up with the Minestom version the aonyx BOM pins. Pulling it in moves Minestom from 2026.06.05-26.1.2 to 2026.06.20-26.1.2, because falco brings mycelium-bom 1.7.2 where aonyx-bom 0.8.0 brings 1.7.1. --- common/build.gradle.kts | 5 +++++ settings.gradle.kts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/common/build.gradle.kts b/common/build.gradle.kts index 4b836698..3b4eb261 100644 --- a/common/build.gradle.kts +++ b/common/build.gradle.kts @@ -10,6 +10,9 @@ dependencies { implementation(libs.togglz) implementation(libs.aves) implementation(libs.adventure.minimessage) + // Falco replaces Minestom's AnvilLoader and light engine (US-1.01 - US-1.03). + implementation(libs.falco.anvil) + implementation(libs.falco.light) // No CloudNet here anymore: anything touching the CloudNet bridge lives in the // :bridge extension; common only talks to it through the JDK-typed @@ -17,6 +20,8 @@ dependencies { testImplementation(platform(libs.aonyx.bom)) testImplementation(libs.minestom) + testImplementation(libs.falco.anvil) + testImplementation(libs.falco.light) testImplementation(libs.cyano) testImplementation(libs.aves) testImplementation(libs.junit.api) diff --git a/settings.gradle.kts b/settings.gradle.kts index 10ce3ec6..bb1ab748 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -11,6 +11,12 @@ dependencyResolutionManagement { url = uri("https://repo.onelitefeather.dev/onelitefeather-proxy") } maven("https://central.sonatype.com/repository/maven-snapshots/") + // Falco (chunk loader and light engine) is published to the public + // OneLiteFeather release repository. + maven { + name = "OneLiteFeatherReleases" + url = uri("https://repo.onelitefeather.dev/releases") + } maven("https://repository.derklaro.dev/snapshots/") maven("https://repository.derklaro.dev/releases/") maven { @@ -36,6 +42,12 @@ dependencyResolutionManagement { version("cloudnet", "4.0.0-RC17-SNAPSHOT") version("butterfly", "1.0.23") + // Falco: the OneLiteFeather chunk loader and light engine. 0.3.0 is the + // version the lobby spec names, but it predates Minestom 26.1; 2.1.0 is + // the current release and the first that resolves against the Minestom + // version the aonyx BOM pins. + version("falco", "2.1.0") + version("luckperms", "5.6-SNAPSHOT") version("togglz", "4.6.2") @@ -56,6 +68,10 @@ dependencyResolutionManagement { library("adventure.minimessage", "net.kyori", "adventure-text-minimessage").withoutVersion() library("butterfly-minestom", "net.onelitefeather", "butterfly-minestom").versionRef("butterfly") + // Falco + library("falco-anvil", "net.onelitefeather", "falco-anvil").versionRef("falco") + library("falco-light", "net.onelitefeather", "falco-light").versionRef("falco") + library("togglz", "org.togglz", "togglz-core").versionRef("togglz") library("caffeine", "com.github.ben-manes.caffeine", "caffeine").versionRef("caffeine") library("tomcat-annotations-api", "org.apache.tomcat", "annotations-api").versionRef("tomcat-annotations-api") From 69bc2bd4b92aeefc9c03c70ac7b9374f23c6eabc Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:26:23 +0200 Subject: [PATCH 02/11] refactor(map): declare the map package non-null by default Adds the package-info the OLF Minestom standard asks for (OLF-L3-01) and removes the hand-placed @NotNull annotations it makes redundant, starting with MapEntry, which also gains the type and method javadoc of OLF-L4-01. The remaining classes of the package follow in the commits that change them. --- .../titan/common/map/MapEntry.java | 24 +++++++++++++-- .../titan/common/map/package-info.java | 29 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 common/src/main/java/net/onelitefeather/titan/common/map/package-info.java diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/MapEntry.java b/common/src/main/java/net/onelitefeather/titan/common/map/MapEntry.java index e8d00907..658e1b50 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/map/MapEntry.java +++ b/common/src/main/java/net/onelitefeather/titan/common/map/MapEntry.java @@ -17,18 +17,36 @@ package net.onelitefeather.titan.common.map; import net.onelitefeather.titan.common.config.AppConfig; -import org.jetbrains.annotations.NotNull; import java.nio.file.Files; import java.nio.file.Path; -public record MapEntry(@NotNull Path path) { +/** + * The {@link MapEntry} record points at one world directory below {@code worlds} and answers + * whether that directory carries map data. + * + * @param path the root directory of the world + * @author theEvilReaper + * @version 1.0.0 + * @since 1.0.0 + */ +public record MapEntry(Path path) { + /** + * Checks whether the world directory carries the map data file. + * + * @return true if the file exists, otherwise false + */ public boolean hasMapFile() { return Files.exists(path.resolve(AppConfig.MAP_FILE_NAME)); } - public @NotNull Path getMapFile() { + /** + * Gets the map data file of this world, whether it exists or not. + * + * @return the path of the map data file + */ + public Path getMapFile() { return path.resolve(AppConfig.MAP_FILE_NAME); } } diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/package-info.java b/common/src/main/java/net/onelitefeather/titan/common/map/package-info.java new file mode 100644 index 00000000..5f3b8370 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/map/package-info.java @@ -0,0 +1,29 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +/** + * Holds the lobby worlds: which one is selected, where its map data lives and which engine serves + * its chunks. + * + * @author theEvilReaper + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +@NotNullByDefault +package net.onelitefeather.titan.common.map; + +import org.jetbrains.annotations.NotNullByDefault; From 8106576677332f0df0cfe203138c3a15232cdcf2 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:26:36 +0200 Subject: [PATCH 03/11] feat(map): serve the lobby world through Falco (US-1.01, US-1.02, US-1.03) Replaces Minestom's AnvilLoader with Falco's FalcoAnvilLoader and computes the block light of every loaded chunk with falco-light's ChunkLightService instead of LightingChunk.relight. Why the loader matters beyond it being ours: Minestom's reports a chunk it cannot read as absent, which makes the server generate a fresh one and overwrite the built world on the next save. Falco's reports the failure (US-1.02). The loader is handed the world root rather than its region directory - it resolves dimensions///region itself and falls back to a plain region/ for a world in the older layout, which is the layout the lobby worlds are in. It also holds region files open for as long as it lives, so it is created once per world root rather than on every read of the map data, and MapProvider became AutoCloseable so the app shutdown can close it. Lighting keeps LightingChunk as the chunk supplier. Falco writes its result through Light#set, which clears the update flag of the section, so Minestom does not recompute the block light - it stays responsible for sending the light and for the sky pass. The cached packets are dropped and a resend is scheduled after the calculation, because the load event is dispatched after the loading future completes and the chunk may already be on its way to a player by then. Not used here: ChunkLightScheduler with its own chunk supplier, which would be the fuller replacement. In falco-light 2.1.0 its FalcoLightingChunk extends FalcoChunk from falco-instance, a module the artefact neither bundles nor declares, so that route does not link. --- .../net/onelitefeather/titan/app/Titan.java | 4 +- .../titan/common/map/MapProvider.java | 172 +++++++++++++++--- .../map/MapProviderIntegrationTest.java | 100 ++++++++++ 3 files changed, 254 insertions(+), 22 deletions(-) create mode 100644 common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java diff --git a/app/src/main/java/net/onelitefeather/titan/app/Titan.java b/app/src/main/java/net/onelitefeather/titan/app/Titan.java index 42a23911..80a56dc6 100644 --- a/app/src/main/java/net/onelitefeather/titan/app/Titan.java +++ b/app/src/main/java/net/onelitefeather/titan/app/Titan.java @@ -71,7 +71,9 @@ public void initialize() { } public void terminate() { - + // The Falco chunk loader keeps region files open for as long as it lives, so the shutdown + // is where they are flushed and closed. + this.mapProvider.close(); } private void initCommands() { diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java b/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java index 55eb6eca..fb36c77e 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java +++ b/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java @@ -24,17 +24,19 @@ import net.minestom.server.instance.Clock; import net.minestom.server.instance.InstanceContainer; import net.minestom.server.instance.LightingChunk; -import net.minestom.server.instance.anvil.AnvilLoader; import net.minestom.server.utils.chunk.ChunkUtils; +import net.onelitefeather.falco.anvil.FalcoAnvilLoader; +import net.onelitefeather.falco.light.ChunkLightService; import net.onelitefeather.titan.common.config.AppConfig; import net.theevilreaper.aves.file.GsonFileHandler; import net.theevilreaper.aves.file.gson.PositionGsonAdapter; import net.theevilreaper.aves.map.BaseMap; -import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.UnmodifiableView; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Path; import java.util.Collections; import java.util.List; @@ -43,47 +45,96 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -public final class MapProvider { +/** + * The {@link MapProvider} class owns the lobby instance: it picks the world the + * {@link MapPool} selected, reads the map data next to it and wires the engine that serves the + * chunks of that world. + *

+ * Chunks are read by Falco's {@link FalcoAnvilLoader} instead of Minestom's {@code AnvilLoader}. + * The difference that matters for a built lobby is what happens when a chunk cannot be read: the + * Falco loader reports the failure, while the Minestom one reports the chunk as absent, which makes + * the server generate a fresh chunk and overwrite the built one on the next save. + *

+ *

+ * Block light is computed by Falco as well: every chunk that arrives is handed to a + * {@link ChunkLightService}, which lights it from the chunks around it rather than on its own, so + * the lobby does not end up with a dark line every sixteen blocks. + *

+ * + * @author theEvilReaper + * @author TheMeinerLP + * @version 1.1.0 + * @since 1.0.0 + */ +public final class MapProvider implements AutoCloseable { private static final Logger LOGGER = LoggerFactory.getLogger(MapProvider.class); private static final String MAP_PATH = "worlds"; private final GsonFileHandler fileHandler; private final MapPool mapPool; private final Gson gson; + private final ChunkLightService lightService; private InstanceContainer instance; private LobbyMap activeLobby; + private @Nullable FalcoAnvilLoader chunkLoader; + private @Nullable Path chunkLoaderRoot; - private MapProvider(@NotNull Path path, @NotNull InstanceContainer instance, Function, List> filterMaps) { + private MapProvider(Path path, InstanceContainer instance, Function, List> filterMaps) { this.mapPool = new MapPool(path.resolve(MAP_PATH), filterMaps); this.instance = instance; - // "Exploration" lighting: relight each chunk as it is loaded so regions - // light up while players explore into new map sections (anvil chunks - // otherwise stay dark until a block update triggers a relight). - this.instance.eventNode().addListener(InstanceChunkLoadEvent.class, event -> LightingChunk.relight(event.getInstance(), List.of(event.getChunk()))); + // Use LightingChunk so the world is actually lit: it computes and sends sky/block light. + // Plain DynamicChunks send no light, leaving the lobby pitch black. Must be set before any + // chunk is loaded by the chunk loader. + this.instance.setChunkSupplier(LightingChunk::new); + // "Exploration" lighting: light each chunk as it is loaded so regions light up while + // players explore into new map sections (anvil chunks otherwise stay dark until a block + // update triggers a relight). Falco computes it (US-1.03); writing the result clears the + // update flag of the section, so Minestom does not recompute what was just calculated. + this.lightService = new ChunkLightService(); + this.instance.eventNode().addListener(InstanceChunkLoadEvent.class, this::lightLoadedChunk); var typeAdapter = new PositionGsonAdapter(); this.gson = new Gson().newBuilder().registerTypeAdapter(Pos.class, typeAdapter).registerTypeAdapter(Vec.class, typeAdapter).create(); this.fileHandler = new GsonFileHandler(this.gson); this.loadMapData(); } - private MapProvider(@NotNull Path path, @NotNull InstanceContainer instance) { + private MapProvider(Path path, InstanceContainer instance) { this(path, instance, MapProvider::defaultFilter); } + /** + * Lights a chunk that has just been loaded, together with the ring around it. + *

+ * The cached packets of the chunk are dropped afterwards and its neighbourhood is scheduled for + * a resend, because the chunk may already have been sent by the time this runs: the load event + * is dispatched after the loading future completes. + *

+ * + * @param event the event of the chunk that was loaded + */ + private void lightLoadedChunk(InstanceChunkLoadEvent event) { + this.lightService.calculateWithNeighbours(event.getInstance(), event.getChunkX(), event.getChunkZ()); + if (event.getChunk() instanceof LightingChunk lightingChunk) { + lightingChunk.invalidate(); + lightingChunk.invalidateResendDelay(); + } + } + private static List defaultFilter(Stream pathStream) { return pathStream.map(MapEntry::new).filter(MapEntry::hasMapFile).collect(Collectors.toList()); } - public void saveMap(@NotNull BaseMap baseMap) { + /** + * Writes the given map next to the world it belongs to and reads it back. + * + * @param baseMap the map to store + */ + public void saveMap(BaseMap baseMap) { this.fileHandler.save(this.mapPool.getMapEntry().path().resolve(AppConfig.MAP_FILE_NAME), baseMap instanceof LobbyMap gameMap ? gameMap : baseMap); loadMapData(); } private void loadMapData() { var lobbyData = this.fileHandler.load(this.mapPool.getMapEntry().path().resolve(AppConfig.MAP_FILE_NAME), LobbyMap.class); - // Use LightingChunk so the world is actually lit: it computes and sends - // sky/block light. Plain DynamicChunks send no light, leaving the lobby - // pitch black. Must be set before any chunk is loaded by the AnvilLoader. - this.instance.setChunkSupplier(LightingChunk::new); // Freeze the lobby at midday so it stays bright; otherwise the default // day/night cycle keeps advancing and the world renders dark. this.instance.setTime(6000); @@ -91,7 +142,7 @@ private void loadMapData() { if (clock != null) { clock.rate(0.0f); } - this.instance.setChunkLoader(new AnvilLoader(mapPool.getMapEntry().path())); + this.installChunkLoader(this.mapPool.getMapEntry().path()); try { this.activeLobby = lobbyData.orElse(LobbyMap.lobbyMapBuilder().build()); @@ -104,30 +155,109 @@ private void loadMapData() { } - private void loadChunk(@NotNull InstanceContainer instance, @NotNull T pos) { + /** + * Points the instance at the world below the given root, unless it already reads from there. + *

+ * The root is the world directory itself and not its {@code region} directory: the Falco loader + * resolves {@code dimensions///region} below it and falls back to a plain + * {@code region} for a world in the older layout. + *

+ *

+ * A loader holds open region files, so the one it replaces is closed here. Reusing the loader + * for an unchanged root matters for {@link #saveMap(BaseMap)}, which reads the map data back + * and + * would otherwise drop every open region file on each call. + *

+ * + * @param worldRoot the root directory of the world to read + */ + private void installChunkLoader(Path worldRoot) { + if (worldRoot.equals(this.chunkLoaderRoot)) { + return; + } + this.closeChunkLoader(); + FalcoAnvilLoader loader = new FalcoAnvilLoader(worldRoot, this.instance.getDimensionType().key()); + this.chunkLoader = loader; + this.chunkLoaderRoot = worldRoot; + this.instance.setChunkLoader(loader); + } + + private void closeChunkLoader() { + FalcoAnvilLoader loader = this.chunkLoader; + this.chunkLoader = null; + this.chunkLoaderRoot = null; + if (loader == null) { + return; + } + try { + loader.close(); + } catch (IOException exception) { + LOGGER.error("Unable to close the chunk loader of the lobby world", exception); + } + } + + /** + * Closes the chunk loader and with it every region file the lobby still holds open. + */ + @Override + public void close() { + this.closeChunkLoader(); + } + + private void loadChunk(InstanceContainer instance, T pos) { if (!ChunkUtils.isLoaded(instance, pos)) { instance.loadChunk(pos); } } + /** + * Gets the instance the lobby world is served from. + * + * @return the lobby instance + */ public InstanceContainer getInstance() { return instance; } - public @NotNull LobbyMap getActiveLobby() { + /** + * Gets the map data of the world that is currently active. + * + * @return the active lobby map + */ + public LobbyMap getActiveLobby() { return activeLobby; } - public @NotNull - @UnmodifiableView List getAvailableMaps() { + /** + * Gets every world the pool found below {@code worlds}. + * + * @return an unmodifiable list with all available maps + */ + public @UnmodifiableView List getAvailableMaps() { return Collections.unmodifiableList(this.mapPool.getAvailableMaps()); } - public static MapProvider create(@NotNull Path path, @NotNull InstanceContainer instance) { + /** + * Creates a provider that reads its worlds below the given path. + * + * @param path the directory that holds the {@code worlds} directory + * @param instance the instance the lobby world is served from + * @return a new provider + */ + public static MapProvider create(Path path, InstanceContainer instance) { return new MapProvider(path, instance); } - public static MapProvider create(@NotNull Path path, @NotNull InstanceContainer instance, Function, List> filterMaps) { + /** + * Creates a provider that reads its worlds below the given path and keeps the entries the given + * filter accepts. + * + * @param path the directory that holds the {@code worlds} directory + * @param instance the instance the lobby world is served from + * @param filterMaps the filter that decides which directories count as a world + * @return a new provider + */ + public static MapProvider create(Path path, InstanceContainer instance, Function, List> filterMaps) { return new MapProvider(path, instance, filterMaps); } } diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java new file mode 100644 index 00000000..27f02eca --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java @@ -0,0 +1,100 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.map; + +import net.minestom.server.instance.Chunk; +import net.minestom.server.instance.Instance; +import net.minestom.server.instance.InstanceContainer; +import net.minestom.server.instance.LightingChunk; +import net.minestom.testing.Env; +import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.falco.anvil.FalcoAnvilLoader; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@ExtendWith(MicrotusExtension.class) +class MapProviderIntegrationTest { + + private static List allDirectories(Stream paths) { + return paths.map(MapEntry::new).collect(Collectors.toList()); + } + + @AfterEach + void clearProperty() { + System.clearProperty(MapPool.LOBBY_MAP_PROPERTY); + } + + private static MapProvider provider(Env env, Path root, String world) throws IOException { + // A world in the layout the lobby worlds actually use: one directory per season below + // worlds/, with the region files directly inside it (US-1.06). + Files.createDirectories(root.resolve("worlds").resolve(world).resolve("region")); + Instance instance = env.createEmptyInstance(); + return MapProvider.create(root, (InstanceContainer) instance, MapProviderIntegrationTest::allDirectories); + } + + @Test + @DisplayName("The provider serves the world through Falco's Anvil loader (US-1.01)") + void testTheProviderInstallsTheFalcoLoader(Env env, @TempDir Path root) throws IOException { + try (MapProvider provider = provider(env, root, "world")) { + InstanceContainer instance = provider.getInstance(); + + FalcoAnvilLoader loader = assertInstanceOf(FalcoAnvilLoader.class, instance.getChunkLoader()); + // The loader is handed the world root and resolves the region directory itself. A world + // without a dimensions/ directory keeps the older layout, which is the one the lobby + // worlds are in. + assertEquals(root.resolve("worlds").resolve("world").resolve("region"), loader.regionDirectory(), "the loader resolves the region directory below the world root"); + assertTrue(loader.legacyLayout(), "a world without a dimensions directory keeps the older layout"); + } + } + + @Test + @DisplayName("The chunks of the lobby carry light") + void testTheLobbyChunksCarryLight(Env env, @TempDir Path root) throws IOException { + try (MapProvider provider = provider(env, root, "world")) { + InstanceContainer instance = provider.getInstance(); + + Chunk chunk = instance.getChunkSupplier().createChunk(instance, 0, 0); + assertInstanceOf(LightingChunk.class, chunk); + } + } + + @Test + @DisplayName("Closing the provider closes the chunk loader") + void testClosingTheProviderClosesTheLoader(Env env, @TempDir Path root) throws IOException { + MapProvider provider = provider(env, root, "world"); + FalcoAnvilLoader loader = (FalcoAnvilLoader) provider.getInstance().getChunkLoader(); + + provider.close(); + + assertThrows(IllegalStateException.class, () -> loader.loadChunk(provider.getInstance(), 0, 0), "a closed loader refuses further work instead of reporting the chunk as absent"); + } +} From c443713be117ce214a9c00182e825e27cc26f8ab Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:26:46 +0200 Subject: [PATCH 04/11] fix(map): always honour TITAN_LOBBY_MAP and fall back instead of failing Two defects in one method. peekMap took the only entry without ever reading the property when exactly one world was present, so a machine with one world behaved differently from production (US-1.05). With more than one world it threw a bare NoSuchElementException when the named world was absent, which turned a typo into a lobby that does not start and a stack trace that names neither the world that was searched for nor the ones that are there (US-1.04). The property is now read regardless of how many worlds exist. A named world that is not present is reported with both halves that make the typo obvious - the searched name and the found names - and the pool falls back to the default world. Only a pool without any world at all is still fatal. The property is read per instance rather than into a static field, which is what makes any of this testable at all, and the fallback is observable through isRequestedMapSelected() rather than only in the log. --- .../titan/common/map/MapPool.java | 136 ++++++++++++-- .../titan/common/map/MapPoolTest.java | 167 ++++++++++++++++++ 2 files changed, 288 insertions(+), 15 deletions(-) create mode 100644 common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java b/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java index 9f3f7668..031cbf41 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java +++ b/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java @@ -18,7 +18,6 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.utils.validate.Check; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.UnmodifiableView; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,23 +28,43 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; /** * The map pool is responsible for managing the available maps. It will load all * maps data from the given path and store them. It would not load the map - * itself over a {@link net.minestom.server.instance.anvil.AnvilLoader} - * instance. This behavior is handled by another class. + * itself over a chunk loader instance. This behavior is handled by another class. + *

+ * Which of the maps is the active one is decided by the system property + * {@value #LOBBY_MAP_PROPERTY}, and it is decided by that property alone: the amount of worlds + * below the map directory does not change how the property is read. A world that is named there but + * is not present is a typo and not a reason to refuse the start, so it is reported by name, next to + * the names that were actually found, and the pool falls back to {@value #DEFAULT_MAP_NAME}. + *

* * @author theEvilReaper - * @version 1.0.0 + * @author TheMeinerLP + * @version 1.1.0 * @since 1.0.0 **/ public final class MapPool { private static final Logger LOGGER = LoggerFactory.getLogger(MapPool.class); - private static final String LOBBY_MAP_NAME = System.getProperty("TITAN_LOBBY_MAP", "world"); + /** + * The system property that names the world the lobby starts with. + */ + public static final String LOBBY_MAP_PROPERTY = "TITAN_LOBBY_MAP"; + + /** + * The name of the world the pool falls back to when the named one is absent. + */ + public static final String DEFAULT_MAP_NAME = "world"; + + private final String requestedMapName; + private boolean requestedMapSelected; private List referenceList; private MapEntry selectedMap; private final Function, List> filterMaps; @@ -54,22 +73,87 @@ public final class MapPool { * Creates a new instance of the map pool. It will load all maps from the given * path. * - * @param path - * the path where the maps are stored + * @param path the path where the maps are stored + * @param filterMaps the filter that decides which directories count as a world */ - public MapPool(@NotNull Path path, @NotNull Function, List> filterMaps) { + public MapPool(Path path, Function, List> filterMaps) { this.filterMaps = filterMaps; + // Read per instance rather than once per class: a static field would freeze the value at + // class load, which is both untestable and a source of surprises when the property is set + // from code rather than the command line. + this.requestedMapName = System.getProperty(LOBBY_MAP_PROPERTY, DEFAULT_MAP_NAME); this.referenceList = loadMapsEntries(path); this.peekMap(); } + /** + * Selects the world the lobby starts with. + *

+ * The property wins whenever the world it names exists. When it does not, the searched name and + * the found names are logged together — the two halves that make a typo obvious — and the + * default world is used instead. Only a pool without any world at all is fatal, because there + * is + * then nothing left to start with. + *

+ */ private void peekMap() { Check.argCondition(this.referenceList.isEmpty(), "The map list is empty"); - if (this.referenceList.size() == 1) { - this.selectedMap = this.referenceList.getFirst(); + + Optional requested = findMap(this.requestedMapName); + if (requested.isPresent()) { + this.selectedMap = requested.get(); + this.requestedMapSelected = true; return; } - this.selectedMap = this.referenceList.stream().filter(mapEntry -> mapEntry.path().getFileName().toString().equalsIgnoreCase(LOBBY_MAP_NAME)).findFirst().orElseThrow(); + + LOGGER.warn(describeMissingMap(this.requestedMapName, availableMapNames())); + + Optional fallback = findMap(DEFAULT_MAP_NAME); + if (fallback.isPresent()) { + this.selectedMap = fallback.get(); + return; + } + + // Neither the named world nor the default one is there. Refusing to start would leave the + // lobby down over a naming question, so the first world that was found is used and the + // situation is reported loudly enough to be fixed. + this.selectedMap = this.referenceList.getFirst(); + LOGGER.warn("The default world '{}' does not exist either. Falling back to '{}'.", DEFAULT_MAP_NAME, this.selectedMap.path().getFileName().toString()); + } + + /** + * Builds the warning that reports a world which the property named but which is not there. + *

+ * The message is built rather than formatted into the log call so that the rule behind it — + * both the searched name and the found ones have to appear — is one a test can hold the code + * to. + *

+ * + * @param requested the name of the world that was searched for + * @param found the names of the worlds that are present + * @return the warning to log + */ + static String describeMissingMap(String requested, List found) { + return "The world '" + requested + "' named by the system property " + LOBBY_MAP_PROPERTY + " does not exist. Found worlds: " + String.join(", ", found) + ". Falling back to the default world '" + DEFAULT_MAP_NAME + "'."; + } + + /** + * Looks up a world by the name of its directory, ignoring case. + * + * @param name the name of the world directory + * @return the entry of that world, or empty when no world carries the name + */ + private Optional findMap(String name) { + return this.referenceList.stream().filter(mapEntry -> mapEntry.path().getFileName().toString().equalsIgnoreCase(name)).findFirst(); + } + + /** + * Gets the names of every world the pool found, for a log line that has to name them. + * + * @return the names of the found worlds + */ + List availableMapNames() { + return this.referenceList.stream().map(mapEntry -> mapEntry.path().getFileName().toString()).collect(Collectors.toList()); } /** @@ -80,7 +164,7 @@ private void peekMap() { * the path where the maps are stored * @return a list with all available maps */ - private @NotNull List loadMapsEntries(@NotNull Path path) { + private List loadMapsEntries(Path path) { List mapEntries = new ArrayList<>(); try (Stream stream = Files.list(path)) { mapEntries = this.filterMaps.apply(stream.filter(Files::isDirectory)); @@ -91,12 +175,35 @@ private void peekMap() { return mapEntries; } + /** + * Gets the name of the world the system property asked for, which is + * {@value #DEFAULT_MAP_NAME} when the property is not set. + * + * @return the name of the requested world + */ + public String getRequestedMapName() { + return this.requestedMapName; + } + + /** + * Answers whether the world the property named is the one that was selected. + *

+ * A false here is the fallback case: the named world was not found and the pool went on with + * another one rather than refusing to start. + *

+ * + * @return true if the requested world was found, otherwise false + */ + public boolean isRequestedMapSelected() { + return this.requestedMapSelected; + } + /** * Gets the selected map entry. * * @return the selected map entry */ - public @NotNull MapEntry getMapEntry() { + public MapEntry getMapEntry() { return this.selectedMap; } @@ -114,8 +221,7 @@ public void clear() { * * @return an unmodifiable list with all available maps */ - public @NotNull - @UnmodifiableView List getAvailableMaps() { + public @UnmodifiableView List getAvailableMaps() { return Collections.unmodifiableList(this.referenceList); } } diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java new file mode 100644 index 00000000..f433cfdd --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java @@ -0,0 +1,167 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.map; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MapPoolTest { + + private static List allDirectories(Stream paths) { + return paths.map(MapEntry::new).collect(Collectors.toList()); + } + + private static Path worlds(Path root, String... names) throws IOException { + Path worlds = Files.createDirectories(root.resolve("worlds")); + for (String name : names) { + Files.createDirectories(worlds.resolve(name)); + } + return worlds; + } + + private static String selectedName(Path worlds) { + return new MapPool(worlds, MapPoolTest::allDirectories).getMapEntry().path().getFileName().toString(); + } + + @AfterEach + void clearProperty() { + System.clearProperty(MapPool.LOBBY_MAP_PROPERTY); + } + + @Test + @DisplayName("The property selects the named world when several worlds exist") + void testThePropertySelectsTheNamedWorld(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "world", "halloween", "winter"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + + assertEquals("halloween", selectedName(worlds)); + } + + @Test + @DisplayName("The property is evaluated even when only one world exists (US-1.05)") + void testThePropertyIsEvaluatedForASingleWorld(@TempDir Path root) throws IOException { + // The old implementation short-circuited here and took the only entry without ever looking + // at the property, which made a single-world machine behave differently from production. + Path worlds = worlds(root, "halloween"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + MapPool pool = new MapPool(worlds, MapPoolTest::allDirectories); + + assertEquals("halloween", pool.getMapEntry().path().getFileName().toString()); + assertTrue(pool.isRequestedMapSelected(), "the only world is the requested one"); + } + + @Test + @DisplayName("A single world that the property does not name is reported as a fallback (US-1.05)") + void testASingleUnrequestedWorldIsAFallback(@TempDir Path root) throws IOException { + // Same selection as before the fix, but no longer a silent one: the property was read and + // the world it named was not there. + Path worlds = worlds(root, "winter"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + MapPool pool = new MapPool(worlds, MapPoolTest::allDirectories); + + assertEquals("winter", pool.getMapEntry().path().getFileName().toString()); + assertEquals("halloween", pool.getRequestedMapName()); + assertFalse(pool.isRequestedMapSelected(), "the requested world does not exist"); + } + + @Test + @DisplayName("A missing named world falls back to the default world instead of failing (US-1.04)") + void testAMissingNamedWorldFallsBackToTheDefaultWorld(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "world", "winter"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloewen"); + + assertEquals(MapPool.DEFAULT_MAP_NAME, selectedName(worlds)); + } + + @Test + @DisplayName("A missing named world falls back even when the default world is the only one") + void testAMissingNamedWorldFallsBackWithASingleWorld(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "world"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + + assertEquals(MapPool.DEFAULT_MAP_NAME, selectedName(worlds)); + } + + @Test + @DisplayName("Without the property the default world is selected") + void testWithoutThePropertyTheDefaultWorldIsSelected(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "winter", "world"); + + assertEquals(MapPool.DEFAULT_MAP_NAME, selectedName(worlds)); + } + + @Test + @DisplayName("The world name is matched ignoring case") + void testTheWorldNameIsMatchedIgnoringCase(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "world", "Halloween"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + + assertEquals("Halloween", selectedName(worlds)); + } + + @Test + @DisplayName("Without the named and the default world the first found world is used") + void testWithoutTheNamedAndTheDefaultWorldTheFirstWorldIsUsed(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "winter"); + System.setProperty(MapPool.LOBBY_MAP_PROPERTY, "halloween"); + + assertEquals("winter", selectedName(worlds)); + } + + @Test + @DisplayName("An empty map directory is still fatal") + void testAnEmptyMapDirectoryIsFatal(@TempDir Path root) throws IOException { + Path worlds = worlds(root); + + assertThrows(IllegalArgumentException.class, () -> selectedName(worlds)); + } + + @Test + @DisplayName("The pool reports every world it found") + void testThePoolReportsEveryWorldItFound(@TempDir Path root) throws IOException { + Path worlds = worlds(root, "world", "halloween"); + MapPool pool = new MapPool(worlds, MapPoolTest::allDirectories); + + assertEquals(2, pool.getAvailableMaps().size()); + assertTrue(pool.availableMapNames().containsAll(List.of("world", "halloween"))); + } + + @Test + @DisplayName("The warning names the searched world and the found ones (US-1.04)") + void testTheWarningNamesTheSearchedAndTheFoundWorlds() { + String message = MapPool.describeMissingMap("halloewen", List.of("world", "winter")); + + assertTrue(message.contains("halloewen"), message); + assertTrue(message.contains("world"), message); + assertTrue(message.contains("winter"), message); + assertTrue(message.contains(MapPool.LOBBY_MAP_PROPERTY), message); + } +} From e4f6bb259fb4448330666451be78c4129e2c9675 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:28:36 +0200 Subject: [PATCH 05/11] docs: record the stage 1 world layout and selection behaviour (US-1.06) Adds lobby-world-selection.md, which describes what US-1.06 already required and the code already did: one directory per world below worlds/, named by TITAN_LOBBY_MAP, with the region files resolved inside it. Nothing changed for that story - it is verified and written down. The same document carries the parts of stage 1 that did change: the fallback when the named world is absent, and which engine serves the chunks. exploration-lighting.md is brought level with the lighting path that is actually in the code now, and the stage 1 rows of the spec are marked umgesetzt with a note on the three points where the result deviates from what the spec assumed. --- docs/exploration-lighting.md | 19 +++++-- docs/lobby-world-selection.md | 94 ++++++++++++++++++++++++++++++++ docs/spec-lobby-saison-events.md | 36 +++++++++--- 3 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 docs/lobby-world-selection.md diff --git a/docs/exploration-lighting.md b/docs/exploration-lighting.md index 1b828317..01922627 100644 --- a/docs/exploration-lighting.md +++ b/docs/exploration-lighting.md @@ -14,7 +14,7 @@ There are two distinct behaviours, and they must not be confused: 1. **Lobby = complete relight (now).** The whole lobby is lit. Maps are large (~18–20k populated chunks each), so we do **not** pre-load and relight every - chunk at boot. Instead `MapProvider` relights each chunk as it is loaded, so + chunk at boot. Instead `MapProvider` lights each chunk as it is loaded, so everything is lit by the time it is visible — no dark areas, no visible reveal. 2. **Special maps = deliberate darkening + per-player reveal (later).** Some @@ -27,9 +27,17 @@ There are two distinct behaviours, and they must not be confused: `MapProvider` does an **instance-wide** relight: - The world instance uses `LightingChunk` (`instance.setChunkSupplier(LightingChunk::new)`). -- An `InstanceChunkLoadEvent` listener relights every chunk as it is loaded - (`LightingChunk.relight(instance, List.of(chunk))`), because anvil chunks - otherwise stay dark until a block update triggers a relight. +- An `InstanceChunkLoadEvent` listener lights every chunk as it is loaded, + because anvil chunks otherwise stay dark until a block update triggers a + relight. Since US-1.03 the block light comes from `falco-light`'s + `ChunkLightService.calculateWithNeighbours(...)` instead of + `LightingChunk.relight(...)`. Falco writes its result through `Light#set`, + which clears the update flag of the section, so Minestom does not recompute + it; the cached packets of the chunk are dropped afterwards and a resend is + scheduled, because the load event is dispatched after the loading future + completes. +- `LightingChunk` stays the chunk type: it is what sends the light, and it still + owns the sky pass, which `calculateWithNeighbours` does not cover. - Lobby time is frozen at midday (`setTime(6000)` + `setTimeRate(0)`) so lit chunks render bright. @@ -102,4 +110,5 @@ per-player we override the light **on the wire**, per player: 3. Add per-player discovered-chunk tracking on `TitanPlayer`. 4. Implement the dark-light override on chunk send + reveal `UpdateLightPacket` on discovery (only for maps with the dark/reveal flag set). -5. Keep the instance-level `LightingChunk` relight as the real-light source. +5. Keep the instance-level lighting (Falco block light, `LightingChunk` sky + light and sending) as the real-light source. diff --git a/docs/lobby-world-selection.md b/docs/lobby-world-selection.md new file mode 100644 index 00000000..d105607f --- /dev/null +++ b/docs/lobby-world-selection.md @@ -0,0 +1,94 @@ +# Lobby worlds: layout and selection + +How the lobby decides which world it serves, and what a world directory has to +look like for it to be servable. Covers US-1.01 to US-1.06 of +[`spec-lobby-saison-events.md`](spec-lobby-saison-events.md). + +## One directory per world (US-1.06) + +Every world is a directory below `worlds/`, next to the process: + +``` +worlds/ + world/ <- the default world, used when nothing else is selected + halloween/ + winter/ +``` + +The name of the directory is the name of the world. Nothing in the code knows +these names — adding a season means adding a directory and setting one property, +never a code change. + +A directory only counts as a world if it carries the map data file +(`AppConfig.MAP_FILE_NAME`, `map.json`) — that is what `:app` filters on. The +`:setup` module accepts every directory, because that is where a new world gets +its map data in the first place. + +Inside the directory, Falco's `FalcoAnvilLoader` looks for the region files in +this order: + +1. `worlds//dimensions///region/` — the 26.1 layout, + with the dimension of the instance filled in (`minecraft/overworld` for the + lobby) +2. `worlds//region/` — the older layout, used when the directory above + does not exist + +The lobby worlds are in the older layout, so the fallback is the path that is +actually taken today. `level.dat` is not read; a directory holding only region +files is enough. Upgrading a world to a newer Minecraft version is a separate +job, described in [`world-conversion.md`](world-conversion.md). + +## Selecting the active world (US-1.04, US-1.05) + +The active world is named by the system property `TITAN_LOBBY_MAP`. It defaults +to `world`: + +```bash +java -DTITAN_LOBBY_MAP=halloween -jar app-titan.jar +``` + +The `:setup` module wires `-DTITAN_LOBBY_MAP=halloween` into its +`applicationDefaultJvmArgs`, so a setup run edits the Halloween world unless it +is told otherwise. + +The property is evaluated no matter how many worlds are present. That is worth +stating because it used to not be true: with exactly one world below `worlds/`, +the old code took that world and never looked at the property, which made a +machine with one world behave differently from a machine with three. + +**A world that is named but not present is not fatal.** The lobby logs a warning +that carries both halves needed to spot a typo — the name that was searched for +and the names that were found — and starts with the default world `world` +instead: + +``` +The world 'halloewen' named by the system property TITAN_LOBBY_MAP does not +exist. Found worlds: world, winter. Falling back to the default world 'world'. +``` + +If the default world is missing as well, the first world that was found is used +and a second warning says so. Only a `worlds/` directory without any world at +all stops the start, because there is then nothing left to serve. + +Whether the requested world was the one that got selected is readable from +`MapPool#isRequestedMapSelected()`, so the fallback is observable from code and +not only from the log. + +## Which engine serves the chunks (US-1.01, US-1.02, US-1.03) + +`MapProvider` installs `net.onelitefeather.falco.anvil.FalcoAnvilLoader` as the +chunk loader of the lobby instance, not Minestom's `AnvilLoader`. The reason is +narrower than "it is ours": Minestom's loader reports a chunk it cannot read as +absent, the server then generates a fresh chunk in its place, and the next save +writes that over the built world. Falco's loader reports the failure instead. + +The loader keeps region files open for as long as it lives, so it is created +once per world root and closed on shutdown through `MapProvider#close()`. + +Block light is computed by `falco-light`'s `ChunkLightService`, which lights a +loaded chunk together with the ring around it. `LightingChunk` stays the chunk +type of the instance: writing the light clears the update flag of the section, +so Minestom does not recompute what Falco just calculated, and it keeps doing +the part Falco is not asked for here — sending the light and the sky pass. See +[`exploration-lighting.md`](exploration-lighting.md) for what this is the +foundation of. diff --git a/docs/spec-lobby-saison-events.md b/docs/spec-lobby-saison-events.md index d9cee2af..71948649 100644 --- a/docs/spec-lobby-saison-events.md +++ b/docs/spec-lobby-saison-events.md @@ -138,12 +138,30 @@ und 3. | ID | Story | Akzeptanzkriterium (EARS) | Schnittstelle | Priorität | Status | |---|---|---|---|---|---| -| US-1.01 | Als Betreiber möchte ich Welten über Falco laden, damit wir unsere eigene Engine nutzen und Ladefehler nicht als „Chunk fehlt" durchgehen. | When eine Instanz erzeugt wird, shall die Lobby einen `FalcoAnvilLoader` als `ChunkLoader` setzen statt Minestoms `AnvilLoader`. | `net.onelitefeather.falco.anvil.FalcoAnvilLoader(Path, Key)` | Must | offen | -| US-1.02 | Als Betreiber möchte ich, dass ein Lesefehler den Chunk nicht stillschweigend neu generiert, damit gebaute Welten nicht überschrieben werden. | If ein Chunk nicht gelesen werden kann, then shall der Ladevorgang eine Ausnahme werfen und der Chunk shall nicht neu generiert werden. | `AnvilFault`, `ChunkDataException` | Must | offen | -| US-1.03 | Als Betreiber möchte ich Licht über `falco-light` steuern, damit die Lobby vollständig ausgeleuchtet ist und die Tageszeit später korrekt wirkt. | When ein Chunk geladen wird, shall die Lobby dessen Licht über `ChunkLightService` berechnen. | `ChunkLightService`, `ChunkLightScheduler` | Must | offen | -| US-1.04 | Als Betreiber möchte ich bei falsch gesetzter Welt-Property eine verständliche Meldung, damit ein Tippfehler kein Rätsel ist. | If die über `TITAN_LOBBY_MAP` benannte Welt nicht existiert, then shall die Lobby den gesuchten Namen und die gefundenen Welten protokollieren und mit der Standardwelt starten. | `MapPool.peekMap()` | Must | offen | -| US-1.05 | Als Entwickler möchte ich, dass die Welt-Property immer gilt, damit sich lokal und in Produktion nichts unterschiedlich verhält. | The Welt-Auswahl shall die Property unabhängig von der Anzahl vorhandener Welten auswerten. | `MapPool.peekMap()` | Must | offen | -| US-1.06 | Als Betreiber möchte ich Welten pro Saison als eigenes Verzeichnis ablegen, damit der Wechsel ohne Codeänderung möglich ist. | The Lobby shall die aktive Welt aus einem Verzeichnis unter `worlds/` laden, dessen Name konfigurierbar ist. | `worlds//` | Must | offen | +| US-1.01 | Als Betreiber möchte ich Welten über Falco laden, damit wir unsere eigene Engine nutzen und Ladefehler nicht als „Chunk fehlt" durchgehen. | When eine Instanz erzeugt wird, shall die Lobby einen `FalcoAnvilLoader` als `ChunkLoader` setzen statt Minestoms `AnvilLoader`. | `net.onelitefeather.falco.anvil.FalcoAnvilLoader(Path, Key)` | Must | umgesetzt | +| US-1.02 | Als Betreiber möchte ich, dass ein Lesefehler den Chunk nicht stillschweigend neu generiert, damit gebaute Welten nicht überschrieben werden. | If ein Chunk nicht gelesen werden kann, then shall der Ladevorgang eine Ausnahme werfen und der Chunk shall nicht neu generiert werden. | `AnvilFault`, `ChunkDataException` | Must | umgesetzt | +| US-1.03 | Als Betreiber möchte ich Licht über `falco-light` steuern, damit die Lobby vollständig ausgeleuchtet ist und die Tageszeit später korrekt wirkt. | When ein Chunk geladen wird, shall die Lobby dessen Licht über `ChunkLightService` berechnen. | `ChunkLightService`, `ChunkLightScheduler` | Must | umgesetzt | +| US-1.04 | Als Betreiber möchte ich bei falsch gesetzter Welt-Property eine verständliche Meldung, damit ein Tippfehler kein Rätsel ist. | If die über `TITAN_LOBBY_MAP` benannte Welt nicht existiert, then shall die Lobby den gesuchten Namen und die gefundenen Welten protokollieren und mit der Standardwelt starten. | `MapPool.peekMap()` | Must | umgesetzt | +| US-1.05 | Als Entwickler möchte ich, dass die Welt-Property immer gilt, damit sich lokal und in Produktion nichts unterschiedlich verhält. | The Welt-Auswahl shall die Property unabhängig von der Anzahl vorhandener Welten auswerten. | `MapPool.peekMap()` | Must | umgesetzt | +| US-1.06 | Als Betreiber möchte ich Welten pro Saison als eigenes Verzeichnis ablegen, damit der Wechsel ohne Codeänderung möglich ist. | The Lobby shall die aktive Welt aus einem Verzeichnis unter `worlds/` laden, dessen Name konfigurierbar ist. | `worlds//` | Must | umgesetzt | + +**Umsetzungsstand Stufe 1 (28.08.2026).** US-1.01 bis US-1.06 sind umgesetzt; +Layout und Auswahl der Welten sind in +[`lobby-world-selection.md`](lobby-world-selection.md) dokumentiert. Drei Punkte, +die von dieser Spec abweichen und beim Review bekannt sein sollten: + +- **Falco-Version.** Die Spec nennt 0.3.0. Aufgelöst wird **2.1.0** — 0.3.0 + stammt aus der Zeit vor Minestom 26.1. Falco bringt `mycelium-bom` 1.7.2 mit, + `aonyx-bom` 0.8.0 bringt 1.7.1, wodurch Minestom von `2026.06.05-26.1.2` auf + `2026.06.20-26.1.2` steigt (gleiche Protokollversion). +- **Licht.** Der Blocklichtpfad läuft über `ChunkLightService`; `LightingChunk` + bleibt der Chunktyp, weil er das Licht versendet und den Himmelspass hält. + Der vollständigere Weg über `ChunkLightScheduler.supplier()` ist **nicht** + gangbar: dessen `FalcoLightingChunk` erbt in falco-light 2.1.0 von + `FalcoChunk` aus `falco-instance`, einem Modul, das das Artefakt weder + mitliefert noch deklariert. Nachziehen, sobald Falco das behoben hat. +- **US-1.06** war bereits erfüllt und ist nur verifiziert und dokumentiert + worden, nicht geändert. ### Stufe 2 — Jahreszeiten und Echtzeit-Tageszeit @@ -348,9 +366,9 @@ bekommen den Zeitpunkt übergeben, statt selbst auf die Uhr zu sehen. Die - [x] Die Lizenzfrage Falco ↔ Titan ist entschieden: Titan steht unter AGPL-3.0 (21.08.2026). - [ ] Die Zustimmung der Mitautoren zum Lizenzwechsel liegt schriftlich vor und ist im Repository abgelegt. -- [ ] Die Lobby lädt Welten über `FalcoAnvilLoader`; Minestoms `AnvilLoader` wird nicht mehr verwendet. -- [ ] Ein Lesefehler an einem Chunk führt zu einer Ausnahme, nicht zu einem neu generierten Chunk. -- [ ] Ein falsch gesetztes `TITAN_LOBBY_MAP` startet die Lobby mit der Standardwelt und protokolliert den gesuchten Namen. +- [x] Die Lobby lädt Welten über `FalcoAnvilLoader`; Minestoms `AnvilLoader` wird nicht mehr verwendet. +- [x] Ein Lesefehler an einem Chunk führt zu einer Ausnahme, nicht zu einem neu generierten Chunk. +- [x] Ein falsch gesetztes `TITAN_LOBBY_MAP` startet die Lobby mit der Standardwelt und protokolliert den gesuchten Namen. - [ ] Die Tageszeit der Lobby entspricht der Uhrzeit in Berlin, auch über eine Sommerzeitumstellung hinweg. - [ ] Die Zeitsteuerung ist mit einer festen `Clock` testbar; ein Test prüft Winter im Sommer. - [ ] Ein Feature lässt sich nacheinander auf intern, lite und ga stellen, ohne dass Code geändert wird. From 56ab7b69613207db75a9ac73c39b87015568a63a Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:27:55 +0200 Subject: [PATCH 06/11] build: declare falco-instance so the light scheduler can be loaded falco-light 2.1.0 declares no dependency on falco-instance, and its ChunkLightScheduler cannot be loaded without one: the class carries the lambda body of supplier(), which returns a FalcoLightingChunk, and the verifier resolves that type and its FalcoChunk supertype while linking the scheduler rather than when the lambda runs. Probed with the published artifacts on a bare classpath: FAIL net.onelitefeather.falco.light.ChunkLightScheduler -> java.lang.NoClassDefFoundError: net/onelitefeather/falco/instance/FalcoChunk Adding falco-instance of the same release makes the class load. Nothing in Titan names a type from it, so it is a runtime dependency only. --- common/build.gradle.kts | 5 +++++ settings.gradle.kts | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/common/build.gradle.kts b/common/build.gradle.kts index 3b4eb261..12ae8338 100644 --- a/common/build.gradle.kts +++ b/common/build.gradle.kts @@ -13,6 +13,10 @@ dependencies { // Falco replaces Minestom's AnvilLoader and light engine (US-1.01 - US-1.03). implementation(libs.falco.anvil) implementation(libs.falco.light) + // Runtime only for us: nothing here names a falco-instance type, but linking + // ChunkLightScheduler resolves FalcoLightingChunk and its FalcoChunk supertype. See the + // version catalog for the details. + runtimeOnly(libs.falco.instance) // No CloudNet here anymore: anything touching the CloudNet bridge lives in the // :bridge extension; common only talks to it through the JDK-typed @@ -22,6 +26,7 @@ dependencies { testImplementation(libs.minestom) testImplementation(libs.falco.anvil) testImplementation(libs.falco.light) + testRuntimeOnly(libs.falco.instance) testImplementation(libs.cyano) testImplementation(libs.aves) testImplementation(libs.junit.api) diff --git a/settings.gradle.kts b/settings.gradle.kts index bb1ab748..8866e7bf 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -71,6 +71,13 @@ dependencyResolutionManagement { // Falco library("falco-anvil", "net.onelitefeather", "falco-anvil").versionRef("falco") library("falco-light", "net.onelitefeather", "falco-light").versionRef("falco") + // falco-instance is not used directly, but falco-light's ChunkLightScheduler cannot be + // loaded without it: the class carries the lambda body of supplier(), which returns a + // FalcoLightingChunk, and the verifier resolves that type - and its FalcoChunk + // supertype, which lives here - while linking the scheduler, not when the lambda runs. + // Without this line the very first `new ChunkLightScheduler(...)` dies with a + // NoClassDefFoundError. falco-light declares no dependency on it, so we do. + library("falco-instance", "net.onelitefeather", "falco-instance").versionRef("falco") library("togglz", "org.togglz", "togglz-core").versionRef("togglz") library("caffeine", "com.github.ben-manes.caffeine", "caffeine").versionRef("caffeine") From 529607a8ce2189f9c77f8c8712dd0cd292c748a7 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:28:07 +0200 Subject: [PATCH 07/11] fix(map): light the lobby through Falco's scheduler instead of per chunk Three defects, one cause: the light of a chunk was computed once, inline, from whatever neighbourhood happened to be loaded, and written through Light#set, which clears the update flag of the section. Nothing ever looks at such a section again. - A chunk that loaded before its neighbours was lit without them and was never corrected when they arrived, leaving a permanent dark strip along the border - the very thing the class comment claimed to prevent. - FalcoAnvilLoader#supportsParallelLoading() is true, so the load event runs on a virtual thread and adjacent chunks light each other's neighbourhoods concurrently. Falco's own javadoc forbids that: the result is a seam, never an error, and permanent. - Nothing computed sky light at all. A fresh Light reports itself as valid, so LightingChunk skips the sky pass, and a section whose region file carries no SkyLight array stayed at level zero for good. ChunkLightScheduler answers all three. A loaded chunk is only marked, together with the eight around it, so a chunk is lit again when its neighbours turn up; the scheduler groups marks into areas that do not overlap and discards a result whose chunk changed underneath it; and it runs the sky pass itself. LightingChunk stays the chunk type, because sending the light is what it is for - the resend timer is armed from the completion callback. The tests load chunks out of a real region file, which is the only path on which these defects exist: a generated chunk goes through Chunk#onGenerate(), which invalidates its sections, and hides all three. --- .../titan/common/map/MapProvider.java | 186 +++++++++++-- .../map/MapProviderIntegrationTest.java | 15 +- .../common/map/MapProviderLightingTest.java | 249 ++++++++++++++++++ 3 files changed, 413 insertions(+), 37 deletions(-) create mode 100644 common/src/test/java/net/onelitefeather/titan/common/map/MapProviderLightingTest.java diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java b/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java index fb36c77e..8b7bf0f4 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java +++ b/common/src/main/java/net/onelitefeather/titan/common/map/MapProvider.java @@ -20,17 +20,25 @@ import net.minestom.server.coordinate.Point; import net.minestom.server.coordinate.Pos; import net.minestom.server.coordinate.Vec; +import net.minestom.server.event.EventListener; import net.minestom.server.event.instance.InstanceChunkLoadEvent; +import net.minestom.server.event.instance.InstanceTickEvent; +import net.minestom.server.event.trait.InstanceEvent; +import net.minestom.server.instance.ChunkLoader; import net.minestom.server.instance.Clock; import net.minestom.server.instance.InstanceContainer; import net.minestom.server.instance.LightingChunk; +import net.minestom.server.event.EventNode; import net.minestom.server.utils.chunk.ChunkUtils; import net.onelitefeather.falco.anvil.FalcoAnvilLoader; +import net.onelitefeather.falco.light.ChunkArea; +import net.onelitefeather.falco.light.ChunkLightScheduler; import net.onelitefeather.falco.light.ChunkLightService; import net.onelitefeather.titan.common.config.AppConfig; import net.theevilreaper.aves.file.GsonFileHandler; import net.theevilreaper.aves.file.gson.PositionGsonAdapter; import net.theevilreaper.aves.map.BaseMap; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.UnmodifiableView; import org.slf4j.Logger; @@ -41,6 +49,7 @@ import java.util.Collections; import java.util.List; import java.util.NoSuchElementException; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -56,14 +65,34 @@ * the server generate a fresh chunk and overwrite the built one on the next save. *

*

- * Block light is computed by Falco as well: every chunk that arrives is handed to a - * {@link ChunkLightService}, which lights it from the chunks around it rather than on its own, so - * the lobby does not end up with a dark line every sixteen blocks. + * Light is scheduled, not computed on the spot. A chunk that arrives is only marked + * as needing light; a {@link ChunkLightScheduler} collects every mark of a tick, groups the marked + * chunks into areas which do not overlap and computes each area off the tick thread. Two properties + * follow from that, and both of them are the reason this is not done inline: + *

+ *
    + *
  • A chunk is lit again when its neighbours arrive. Marking a chunk marks the eight + * around it as well, so the chunk that loaded first — and was lit with nothing beside it — is + * corrected as soon as its neighbour shows up. Lighting a chunk inline cannot do that: the write + * goes through {@code Light#set}, which clears the update flag of the section, so neither Falco + * nor Minestom would ever look at that chunk again and the border would stay dark for good.
    • + *
  • Two chunks are never lit against each other. The Falco loader loads in parallel, so + * several adjacent chunks arriving at once is the normal case rather than a corner one, and + * lighting them from two threads reads chunks another thread is writing. The result of that is a + * seam, it is never an error, and — again because the update flag is cleared — it is + * permanent.
    • + *
+ *

+ * The scheduler owns the sky pass too. A section which arrives without a {@code SkyLight} array in + * its NBT is not recomputed by Minestom, because a fresh {@code Light} reports itself as + * valid, so a lobby that leaves the sky to {@code LightingChunk} is a lobby whose sky light is + * whatever the region file happened to carry. {@link LightingChunk} stays the chunk type for what + * it does do: sending the light to the players. *

* * @author theEvilReaper * @author TheMeinerLP - * @version 1.1.0 + * @version 1.2.0 * @since 1.0.0 */ public final class MapProvider implements AutoCloseable { @@ -72,29 +101,45 @@ public final class MapProvider implements AutoCloseable { private final GsonFileHandler fileHandler; private final MapPool mapPool; private final Gson gson; - private final ChunkLightService lightService; - private InstanceContainer instance; + private final ChunkLightScheduler lightScheduler; + private final EventListener chunkLoadListener; + private final EventListener tickListener; + /** + * Counts the light passes. The scheduler only needs a value that differs from the one it saw + * last, and the tick event of Minestom carries a duration rather than a timestamp. + */ + private final AtomicLong lightPass = new AtomicLong(); + private final InstanceContainer instance; private LobbyMap activeLobby; private @Nullable FalcoAnvilLoader chunkLoader; private @Nullable Path chunkLoaderRoot; + private volatile boolean closed; private MapProvider(Path path, InstanceContainer instance, Function, List> filterMaps) { this.mapPool = new MapPool(path.resolve(MAP_PATH), filterMaps); this.instance = instance; - // Use LightingChunk so the world is actually lit: it computes and sends sky/block light. - // Plain DynamicChunks send no light, leaving the lobby pitch black. Must be set before any - // chunk is loaded by the chunk loader. + // Use LightingChunk so the world is actually lit: it is what sends sky and block light to + // the players. Plain DynamicChunks send none, leaving the lobby pitch black. Must be set + // before any chunk is loaded by the chunk loader. this.instance.setChunkSupplier(LightingChunk::new); - // "Exploration" lighting: light each chunk as it is loaded so regions light up while - // players explore into new map sections (anvil chunks otherwise stay dark until a block - // update triggers a relight). Falco computes it (US-1.03); writing the result clears the - // update flag of the section, so Minestom does not recompute what was just calculated. - this.lightService = new ChunkLightService(); - this.instance.eventNode().addListener(InstanceChunkLoadEvent.class, this::lightLoadedChunk); + // The scheduler is built per instance, as its contract requires, and it is what keeps the + // light of the lobby correct while chunks keep arriving; see the class comment. + this.lightScheduler = ChunkLightScheduler.builder(new ChunkLightService()).skyLight(ChunkLightScheduler.SkyLight.FROM_DIMENSION).onAreaCompleted(this::resendLight).build(); + this.chunkLoadListener = EventListener.of(InstanceChunkLoadEvent.class, this::markLoadedChunk); + this.tickListener = EventListener.of(InstanceTickEvent.class, this::runLightPass); + this.instance.eventNode().addListener(this.chunkLoadListener); + this.instance.eventNode().addListener(this.tickListener); var typeAdapter = new PositionGsonAdapter(); this.gson = new Gson().newBuilder().registerTypeAdapter(Pos.class, typeAdapter).registerTypeAdapter(Vec.class, typeAdapter).create(); this.fileHandler = new GsonFileHandler(this.gson); - this.loadMapData(); + try { + this.loadMapData(); + } catch (RuntimeException | Error failure) { + // The chunk loader is installed by loadMapData and holds open region files. A provider + // whose constructor threw is never closed by anybody, so it closes itself here. + this.close(); + throw failure; + } } private MapProvider(Path path, InstanceContainer instance) { @@ -102,20 +147,54 @@ private MapProvider(Path path, InstanceContainer instance) { } /** - * Lights a chunk that has just been loaded, together with the ring around it. + * Marks a chunk that has just been loaded as needing light. *

- * The cached packets of the chunk are dropped afterwards and its neighbourhood is scheduled for - * a resend, because the chunk may already have been sent by the time this runs: the load event - * is dispatched after the loading future completes. + * This runs on the thread that loaded the chunk — a virtual thread, since the Falco loader + * loads in parallel — and does no work beyond the mark, which is the point: the light itself is + * computed by the scheduler, once per tick, over areas that do not overlap. *

* * @param event the event of the chunk that was loaded */ - private void lightLoadedChunk(InstanceChunkLoadEvent event) { - this.lightService.calculateWithNeighbours(event.getInstance(), event.getChunkX(), event.getChunkZ()); - if (event.getChunk() instanceof LightingChunk lightingChunk) { - lightingChunk.invalidate(); - lightingChunk.invalidateResendDelay(); + private void markLoadedChunk(InstanceChunkLoadEvent event) { + if (this.closed) { + return; + } + // markChanged rather than markDirty: nothing is known about which blocks arrived, so the + // light kept for the chunk is dropped, and the eight chunks around it are marked as well + // because the new blocks reach into them. + this.lightScheduler.markChanged(event.getInstance(), event.getChunkX(), event.getChunkZ()); + } + + /** + * Runs the light pass of one tick. + * + * @param event the tick event of the lobby instance + */ + private void runLightPass(InstanceTickEvent event) { + if (this.closed) { + return; + } + this.lightScheduler.onTick(event.getInstance(), this.lightPass.incrementAndGet()); + } + + /** + * Sends the light of the chunks a pass rewrote to the players who already hold them. + *

+ * The scheduler drops the cached packets of every chunk it wrote, which covers everybody who + * receives the chunk afterwards. It cannot cover a player who is already looking at it, because + * a {@link LightingChunk} does not announce its own light; that is what the resend timer of + * Minestom is for, and this is where it is armed. + *

+ * + * @param claimed the chunks the pass took + * @param written the chunks it actually wrote + */ + private void resendLight(List claimed, List written) { + for (ChunkArea position : written) { + if (this.instance.getChunk(position.x(), position.z()) instanceof LightingChunk chunk) { + chunk.invalidateResendDelay(); + } } } @@ -127,13 +206,16 @@ private static List defaultFilter(Stream pathStream) { * Writes the given map next to the world it belongs to and reads it back. * * @param baseMap the map to store + * @throws IllegalStateException if the provider is closed */ public void saveMap(BaseMap baseMap) { + ensureOpen(); this.fileHandler.save(this.mapPool.getMapEntry().path().resolve(AppConfig.MAP_FILE_NAME), baseMap instanceof LobbyMap gameMap ? gameMap : baseMap); loadMapData(); } private void loadMapData() { + ensureOpen(); var lobbyData = this.fileHandler.load(this.mapPool.getMapEntry().path().resolve(AppConfig.MAP_FILE_NAME), LobbyMap.class); // Freeze the lobby at midday so it stays bright; otherwise the default // day/night cycle keeps advancing and the world renders dark. @@ -196,11 +278,40 @@ private void closeChunkLoader() { } } + private void ensureOpen() { + if (this.closed) { + throw new IllegalStateException("The map provider of the lobby world is closed"); + } + } + /** * Closes the chunk loader and with it every region file the lobby still holds open. + *

+ * Unwiring comes before closing, and that order is the whole point of this method. A closed + * loader that is still the loader of the instance refuses every chunk a player walks into with + * an {@link IllegalStateException}, and the listeners would keep marking chunks for a light + * engine nobody is going to drive again. The instance is handed the no-op loader instead, which + * reports a chunk as absent rather than throwing — nothing is saved after this point, so + * nothing built can be overwritten by it. + *

+ *

+ * Closing twice is allowed and does nothing the second time. Everything the provider still + * offers after this point is read-only; {@link #saveMap(BaseMap)} refuses, rather than building + * a fresh loader and reopening the files that were just closed. + *

*/ @Override public void close() { + if (this.closed) { + return; + } + this.closed = true; + EventNode node = this.instance.eventNode(); + if (node != null) { + node.removeListener(this.chunkLoadListener); + node.removeListener(this.tickListener); + } + this.instance.setChunkLoader(ChunkLoader.noop()); this.closeChunkLoader(); } @@ -228,6 +339,31 @@ public LobbyMap getActiveLobby() { return activeLobby; } + /** + * Answers whether this provider has been closed. + * + * @return true if the provider is closed, otherwise false + */ + public boolean isClosed() { + return this.closed; + } + + /** + * Answers whether the given chunk is still waiting for its light. + *

+ * The light of a chunk is written a tick or more after the chunk itself arrives, so a test that + * reads a light level has to know when to look. Nothing in production asks this. + *

+ * + * @param chunkX the chunk x coordinate + * @param chunkZ the chunk z coordinate + * @return true if the chunk is still marked as needing light, otherwise false + */ + @ApiStatus.Internal + public boolean isLightPending(int chunkX, int chunkZ) { + return this.lightScheduler.isDirty(chunkX, chunkZ); + } + /** * Gets every world the pool found below {@code worlds}. * diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java index 27f02eca..b6839dd2 100644 --- a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java @@ -16,10 +16,8 @@ */ package net.onelitefeather.titan.common.map; -import net.minestom.server.instance.Chunk; import net.minestom.server.instance.Instance; import net.minestom.server.instance.InstanceContainer; -import net.minestom.server.instance.LightingChunk; import net.minestom.testing.Env; import net.minestom.testing.extension.MicrotusExtension; import net.onelitefeather.falco.anvil.FalcoAnvilLoader; @@ -76,16 +74,9 @@ void testTheProviderInstallsTheFalcoLoader(Env env, @TempDir Path root) throws I } } - @Test - @DisplayName("The chunks of the lobby carry light") - void testTheLobbyChunksCarryLight(Env env, @TempDir Path root) throws IOException { - try (MapProvider provider = provider(env, root, "world")) { - InstanceContainer instance = provider.getInstance(); - - Chunk chunk = instance.getChunkSupplier().createChunk(instance, 0, 0); - assertInstanceOf(LightingChunk.class, chunk); - } - } + // What the lobby does about light is asserted in MapProviderLightingTest, by loading chunks out + // of a region file and reading light levels back. The test that used to stand here asked the + // chunk supplier for a chunk and checked its type, which every one of the light defects passed. @Test @DisplayName("Closing the provider closes the chunk loader") diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderLightingTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderLightingTest.java new file mode 100644 index 00000000..1cb810b2 --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderLightingTest.java @@ -0,0 +1,249 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.map; + +import net.minestom.server.coordinate.CoordConversion; +import net.minestom.server.instance.Chunk; +import net.minestom.server.instance.Instance; +import net.minestom.server.instance.InstanceContainer; +import net.minestom.server.instance.block.Block; +import net.minestom.server.instance.generator.Generator; +import net.minestom.testing.Env; +import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.falco.anvil.FalcoAnvilLoader; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Holds the lobby to the light it promises. + *

+ * Every test here reads a real light level out of a chunk the provider loaded from a real region + * file. That is the point: a chunk which comes from the loader is never handed to + * {@code Chunk#onGenerate()}, so Minestom does not invalidate its sections on the way in, and a + * light bug that only shows up on that path is invisible to any test which generates its world. + *

+ */ +@ExtendWith(MicrotusExtension.class) +class MapProviderLightingTest { + + /** + * The largest amount of time a light pass may take before the test gives up on it. + */ + private static final Duration SETTLE = Duration.ofSeconds(15); + + /** + * The height of the stone floor every test world carries. + */ + private static final int FLOOR_TOP = 1; + + /** + * The block coordinates of the lamp which stands in the chunk east of the first one. + */ + private static final int LAMP_X = 16; + private static final int LAMP_Y = 5; + private static final int LAMP_Z = 8; + + /** + * The block coordinates of the lamp which stands in the far corner of the western chunk, out of + * reach of the border. + */ + private static final int CORNER_LAMP_X = 0; + private static final int CORNER_LAMP_Z = 0; + + private static List allDirectories(Stream paths) { + return paths.map(MapEntry::new).collect(Collectors.toList()); + } + + @AfterEach + void clearProperty() { + System.clearProperty(MapPool.LOBBY_MAP_PROPERTY); + } + + /** + * Writes a world to disk and returns the root of it. + *

+ * The chunks are generated in a throwaway instance whose chunks are plain {@code DynamicChunk}s + * and are then written through a Falco loader. Nothing computes light on the way, so the + * sections of the region file carry neither a {@code BlockLight} nor a {@code SkyLight} array — + * which is exactly the state a world built in a world editor arrives in. + *

+ * + * @param env the test environment which owns the throwaway instance + * @param root the directory which holds the {@code worlds} directory + * @param world the name of the world directory + * @param generator the generator which shapes the written chunks + * @param chunks the chunks to write, as pairs of chunk coordinates + * @return the root directory of the written world + */ + private static Path writeWorld(Env env, Path root, String world, Generator generator, int[][] chunks) throws IOException { + Path worldRoot = root.resolve("worlds").resolve(world); + Files.createDirectories(worldRoot.resolve("region")); + + Instance source = env.createEmptyInstance(); + source.setGenerator(generator); + List written = new ArrayList<>(chunks.length); + for (int[] position : chunks) { + written.add(source.loadChunk(position[0], position[1]).join()); + } + try (FalcoAnvilLoader writer = new FalcoAnvilLoader(worldRoot, source.getDimensionType().key())) { + for (Chunk chunk : written) { + writer.saveChunk(chunk); + } + } + env.destroyInstance(source); + return worldRoot; + } + + /** + * Builds the shape every test world uses: a stone floor and two glowstone blocks. + *

+ * The first lamp sits in the far corner of the western chunk and is what tells a test that the + * light of that chunk has been written at all. It is far enough from the border — more than + * fifteen blocks of travel — that it contributes nothing there, so the level at the border is a + * statement about the second lamp alone, which stands one block east of that border in the + * chunk next door. + *

+ * + * @return the generator of the test world + */ + private static Generator floorWithALampOnEachSideOfTheBorder() { + return unit -> { + unit.modifier().fillHeight(0, FLOOR_TOP, Block.STONE); + int startX = unit.absoluteStart().blockX(); + int startZ = unit.absoluteStart().blockZ(); + + if (startX == 0 && startZ == 0) { + unit.modifier().setBlock(CORNER_LAMP_X, LAMP_Y, CORNER_LAMP_Z, Block.GLOWSTONE); + } + if (startX == LAMP_X && startZ == 0) { + unit.modifier().setBlock(LAMP_X, LAMP_Y, LAMP_Z, Block.GLOWSTONE); + } + }; + } + + private static MapProvider provider(Env env, Path root) { + Instance instance = env.createEmptyInstance(); + return MapProvider.create(root, (InstanceContainer) instance, MapProviderLightingTest::allDirectories); + } + + private static int blockLightAt(Instance instance, int x, int y, int z) { + return sectionOf(instance, x, y, z).blockLight().getLevel(x & 15, y & 15, z & 15); + } + + private static int skyLightAt(Instance instance, int x, int y, int z) { + return sectionOf(instance, x, y, z).skyLight().getLevel(x & 15, y & 15, z & 15); + } + + private static net.minestom.server.instance.Section sectionOf(Instance instance, int x, int y, int z) { + Chunk chunk = instance.getChunk(CoordConversion.globalToChunk(x), CoordConversion.globalToChunk(z)); + assertTrue(chunk != null, "the chunk holding " + x + "/" + y + "/" + z + " is loaded"); + return chunk.getSection(CoordConversion.globalToChunk(y)); + } + + @Test + @DisplayName("A chunk that was lit before its neighbour arrived is lit again (D1)") + void testAChunkIsRelitWhenItsNeighbourArrives(Env env, @TempDir Path root) throws IOException { + writeWorld(env, root, "world", floorWithALampOnEachSideOfTheBorder(), new int[][]{{0, 0}, {1, 0}}); + + try (MapProvider provider = provider(env, root)) { + InstanceContainer instance = provider.getInstance(); + + // The western chunk arrives alone, with nothing east of it. Its corner lamp is what + // makes "the light of this chunk has been written" observable at all. + instance.loadChunk(0, 0).join(); + assertTrue(env.tickWhile(() -> blockLightAt(instance, CORNER_LAMP_X + 1, LAMP_Y, CORNER_LAMP_Z) == 0, SETTLE), "the chunk is lit from its own lamp"); + assertNull(instance.getChunk(1, 0), "the chunk east of the border is not loaded yet"); + assertEquals(0, blockLightAt(instance, LAMP_X - 1, LAMP_Y, LAMP_Z), "the border is dark while the chunk behind it is missing"); + + // The eastern chunk arrives afterwards. Its lamp stands one block across the border, so + // a level has to appear on the western side — which happens only if the chunk that was + // already lit is lit a second time. + instance.loadChunk(1, 0).join(); + env.tickWhile(() -> blockLightAt(instance, LAMP_X - 1, LAMP_Y, LAMP_Z) == 0, SETTLE); + + assertEquals(14, blockLightAt(instance, LAMP_X - 1, LAMP_Y, LAMP_Z), "the lamp of the chunk that arrived later reaches across the border"); + } + } + + @Test + @DisplayName("A chunk whose region file carries no sky light still gets some (D3)") + void testSkyLightIsComputedForAWorldWithout(Env env, @TempDir Path root) throws IOException { + Path worldRoot = writeWorld(env, root, "world", floorWithALampOnEachSideOfTheBorder(), new int[][]{{0, 0}}); + + // First the premise of the test: the world on disk really does carry no sky light, so a + // lobby which only reads it and never computes any is a lobby in the dark. + Instance bare = env.createEmptyInstance(); + try (FalcoAnvilLoader reader = new FalcoAnvilLoader(worldRoot, bare.getDimensionType().key())) { + Chunk raw = reader.loadChunk(bare, 0, 0); + assertTrue(raw != null, "the written chunk is readable"); + assertEquals(0, raw.getSection(0).skyLight().getLevel(8, 10, 8), "the region file carries no sky light for the section above the floor"); + } + env.destroyInstance(bare); + + try (MapProvider provider = provider(env, root)) { + InstanceContainer instance = provider.getInstance(); + instance.loadChunk(0, 0).join(); + + assertTrue(env.tickWhile(() -> skyLightAt(instance, 8, 10, 8) == 0, SETTLE), "the lobby computes the sky light the region file does not carry"); + assertEquals(15, skyLightAt(instance, 8, 10, 8), "open sky is fully lit"); + assertEquals(0, skyLightAt(instance, 8, 0, 8), "the block below the floor sees no sky"); + } + } + + @Test + @DisplayName("Chunks that arrive together are lit without a seam between them (D2)") + void testChunksThatArriveTogetherHaveNoSeam(Env env, @TempDir Path root) throws IOException { + int[][] area = {{0, 0}, {1, 0}, {0, 1}, {1, 1}}; + writeWorld(env, root, "world", floorWithALampOnEachSideOfTheBorder(), area); + + try (MapProvider provider = provider(env, root)) { + InstanceContainer instance = provider.getInstance(); + + // Falco's loader loads in parallel, so this is the normal case rather than a contrived + // one: four adjacent chunks, four virtual threads, overlapping neighbourhoods. + List> loading = new ArrayList<>(area.length); + for (int[] position : area) { + loading.add(instance.loadChunk(position[0], position[1])); + } + loading.forEach(CompletableFuture::join); + + env.tickWhile(() -> blockLightAt(instance, LAMP_X - 1, LAMP_Y, LAMP_Z) == 0 || blockLightAt(instance, LAMP_X + 1, LAMP_Y, LAMP_Z) == 0, SETTLE); + + int inside = blockLightAt(instance, LAMP_X + 1, LAMP_Y, LAMP_Z); + int across = blockLightAt(instance, LAMP_X - 1, LAMP_Y, LAMP_Z); + assertEquals(14, inside, "the chunk that carries the lamp is lit by it"); + assertEquals(inside, across, "the border is not a seam: both sides of the lamp are equally bright"); + } + } +} From d4a6dd7714f2a10b9a99993775ada66c9c170c0b Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:29:17 +0200 Subject: [PATCH 08/11] fix(map): make closing the provider final, and close it in :setup too close() nulled the loader root but left the closed loader wired into the instance and the listeners on its event node. Two things fell out of that: a player who moved while the server was stopping hit the IllegalStateException of a loader that had been closed underneath the instance, and saveMap() failed the same-root check, built a fresh loader and reopened exactly the region files the shutdown had just closed. The provider now carries a closed flag, unwires listeners and loader before closing, hands the instance the no-op loader and refuses to save afterwards. A failure inside the constructor closes the loader it had already installed rather than leaking it. :setup creates a MapProvider and never closed it, which is backwards: it is the module that mutates worlds, and FalcoAnvilLoader#close() is what flushes the region handles. It now registers a shutdown task like :app does. --- .../map/MapProviderIntegrationTest.java | 58 +++++++++++++++++++ .../net/onelitefeather/titan/setup/Titan.java | 11 ++++ 2 files changed, 69 insertions(+) diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java index b6839dd2..451e6dc2 100644 --- a/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapProviderIntegrationTest.java @@ -16,6 +16,7 @@ */ package net.onelitefeather.titan.common.map; +import net.minestom.server.instance.ChunkLoader; import net.minestom.server.instance.Instance; import net.minestom.server.instance.InstanceContainer; import net.minestom.testing.Env; @@ -34,7 +35,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -88,4 +93,57 @@ void testClosingTheProviderClosesTheLoader(Env env, @TempDir Path root) throws I assertThrows(IllegalStateException.class, () -> loader.loadChunk(provider.getInstance(), 0, 0), "a closed loader refuses further work instead of reporting the chunk as absent"); } + + @Test + @DisplayName("Closing the provider takes the closed loader off the instance") + void testClosingTheProviderUnwiresTheLoader(Env env, @TempDir Path root) throws IOException { + MapProvider provider = provider(env, root, "world"); + InstanceContainer instance = provider.getInstance(); + ChunkLoader closed = instance.getChunkLoader(); + + provider.close(); + + assertNotSame(closed, instance.getChunkLoader(), "the closed loader is not the loader of the instance any more"); + // A player who is still walking around while the server shuts down must not fall over the + // IllegalStateException of a loader that was closed underneath the instance. + assertDoesNotThrow(() -> instance.loadChunk(0, 0).join(), "a chunk request after the shutdown is answered rather than thrown at"); + } + + @Test + @DisplayName("A closed provider refuses to save instead of reopening the world") + void testAClosedProviderRefusesToSave(Env env, @TempDir Path root) throws IOException { + MapProvider provider = provider(env, root, "world"); + ChunkLoader closed = provider.getInstance().getChunkLoader(); + + provider.close(); + + // Saving reads the map data back, which used to reinstall a chunk loader: the same-root + // check compares against a field that close() had just nulled, so it built a fresh loader + // and reopened the region files the shutdown had closed. + assertThrows(IllegalStateException.class, () -> provider.saveMap(LobbyMap.lobbyMapBuilder().name("late").build())); + assertNotSame(closed, provider.getInstance().getChunkLoader(), "no new loader was installed"); + assertSame(ChunkLoader.noop(), provider.getInstance().getChunkLoader(), "the instance keeps the no-op loader the shutdown left it with"); + } + + @Test + @DisplayName("A closed provider stops asking for light") + void testAClosedProviderStopsSchedulingLight(Env env, @TempDir Path root) throws IOException { + MapProvider provider = provider(env, root, "world"); + provider.close(); + + provider.getInstance().loadChunk(0, 0).join(); + + assertFalse(provider.isLightPending(0, 0), "the chunk load listener is gone with the provider"); + } + + @Test + @DisplayName("Closing twice is harmless") + void testClosingTwiceIsHarmless(Env env, @TempDir Path root) throws IOException { + MapProvider provider = provider(env, root, "world"); + + provider.close(); + + assertDoesNotThrow(provider::close); + assertTrue(provider.isClosed()); + } } diff --git a/setup/src/main/java/net/onelitefeather/titan/setup/Titan.java b/setup/src/main/java/net/onelitefeather/titan/setup/Titan.java index 5a08cb7d..1a67ea50 100644 --- a/setup/src/main/java/net/onelitefeather/titan/setup/Titan.java +++ b/setup/src/main/java/net/onelitefeather/titan/setup/Titan.java @@ -52,6 +52,17 @@ private Titan() { initCommands(); initListeners(); + // The setup server is the one that changes worlds, so it is the one that must not drop its + // region handles unflushed. The Falco chunk loader keeps them open for as long as it lives + // and MapProvider#close() is the only thing that closes them. + MinecraftServer.getSchedulerManager().buildShutdownTask(this::terminate); + } + + /** + * Closes the map provider and with it every region file the setup server holds open. + */ + public void terminate() { + this.mapProvider.close(); } private void initListeners() { From 20c2776280123290bd56bb93d713d83b313daa3a Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:30:26 +0200 Subject: [PATCH 09/11] fix(map): report the world the pool really fell back to describeMissingMap announced the default world unconditionally, including in the case where the default world is missing as well and peekMap goes on with the first world it found. The message now names the world that was actually taken and says why, and the second warning that used to follow it is gone with it. clear() also set the entry list to null under an @NotNullByDefault package, which turned every later read of the pool into a NullPointerException instead of an empty list. The list is emptied instead, and it is copied on the way in because one of the two filters hands back an immutable list. --- .../titan/common/map/MapPool.java | 60 ++++++++++++------- .../titan/common/map/MapPoolTest.java | 29 ++++++++- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java b/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java index 031cbf41..52131dc7 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java +++ b/common/src/main/java/net/onelitefeather/titan/common/map/MapPool.java @@ -65,7 +65,7 @@ public final class MapPool { private final String requestedMapName; private boolean requestedMapSelected; - private List referenceList; + private final List referenceList; private MapEntry selectedMap; private final Function, List> filterMaps; @@ -82,7 +82,9 @@ public MapPool(Path path, Function, List> filterMaps) { // class load, which is both untestable and a source of surprises when the property is set // from code rather than the command line. this.requestedMapName = System.getProperty(LOBBY_MAP_PROPERTY, DEFAULT_MAP_NAME); - this.referenceList = loadMapsEntries(path); + // Copied because the filter decides what it returns and one of them hands back an immutable + // list, which the pool has to be able to empty. + this.referenceList = new ArrayList<>(loadMapsEntries(path)); this.peekMap(); } @@ -106,35 +108,45 @@ private void peekMap() { return; } - LOGGER.warn(describeMissingMap(this.requestedMapName, availableMapNames())); - - Optional fallback = findMap(DEFAULT_MAP_NAME); - if (fallback.isPresent()) { - this.selectedMap = fallback.get(); - return; - } + // Neither refusing to start nor picking blindly: the default world first, and if that is + // not there either the first world that was found, because leaving the lobby down over a + // naming question would be the worse outcome. The warning is written afterwards so that it + // can name the world that was actually taken rather than the one the rule prefers. + this.selectedMap = findMap(DEFAULT_MAP_NAME).orElseGet(this.referenceList::getFirst); + LOGGER.warn(describeMissingMap(this.requestedMapName, availableMapNames(), selectedMapName())); + } - // Neither the named world nor the default one is there. Refusing to start would leave the - // lobby down over a naming question, so the first world that was found is used and the - // situation is reported loudly enough to be fixed. - this.selectedMap = this.referenceList.getFirst(); - LOGGER.warn("The default world '{}' does not exist either. Falling back to '{}'.", DEFAULT_MAP_NAME, this.selectedMap.path().getFileName().toString()); + /** + * Gets the name of the world directory that was selected. + * + * @return the name of the selected world + */ + private String selectedMapName() { + return this.selectedMap.path().getFileName().toString(); } /** * Builds the warning that reports a world which the property named but which is not there. *

- * The message is built rather than formatted into the log call so that the rule behind it — - * both the searched name and the found ones have to appear — is one a test can hold the code - * to. + * The message is built rather than formatted into the log call so that the rules behind it — + * the searched name, the found ones and the world that was taken instead all have to appear — + * are ones a test can hold the code to. + *

+ *

+ * The fallback is named rather than assumed. The message used to announce the default world + * unconditionally, including in the case where the default world was missing as well and the + * code went on with the first world it had found: it said one thing and did another, which is + * the one failure mode a warning of this kind must not have. *

* * @param requested the name of the world that was searched for * @param found the names of the worlds that are present + * @param fallback the name of the world that was selected instead * @return the warning to log */ - static String describeMissingMap(String requested, List found) { - return "The world '" + requested + "' named by the system property " + LOBBY_MAP_PROPERTY + " does not exist. Found worlds: " + String.join(", ", found) + ". Falling back to the default world '" + DEFAULT_MAP_NAME + "'."; + static String describeMissingMap(String requested, List found, String fallback) { + String target = DEFAULT_MAP_NAME.equalsIgnoreCase(fallback) ? "the default world '" + fallback + "'" : "the world '" + fallback + "', because the default world '" + DEFAULT_MAP_NAME + "' is not there either"; + return "The world '" + requested + "' named by the system property " + LOBBY_MAP_PROPERTY + " does not exist. Found worlds: " + String.join(", ", found) + ". Falling back to " + target + "."; } /** @@ -208,12 +220,16 @@ public MapEntry getMapEntry() { } /** - * Removes the selected map from the list. If the list is empty it will throw an - * exception. + * Forgets every world the pool found. + *

+ * The list is emptied rather than replaced by null. This package is + * {@code @NotNullByDefault}, so a null there was a promise the class broke against itself, and + * it turned every later call of {@link #getAvailableMaps()} into a + * {@link NullPointerException} instead of an empty list. + *

*/ public void clear() { this.referenceList.clear(); - this.referenceList = null; } /** diff --git a/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java b/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java index f433cfdd..ad442d73 100644 --- a/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java +++ b/common/src/test/java/net/onelitefeather/titan/common/map/MapPoolTest.java @@ -157,11 +157,38 @@ void testThePoolReportsEveryWorldItFound(@TempDir Path root) throws IOException @Test @DisplayName("The warning names the searched world and the found ones (US-1.04)") void testTheWarningNamesTheSearchedAndTheFoundWorlds() { - String message = MapPool.describeMissingMap("halloewen", List.of("world", "winter")); + String message = MapPool.describeMissingMap("halloewen", List.of("world", "winter"), MapPool.DEFAULT_MAP_NAME); assertTrue(message.contains("halloewen"), message); assertTrue(message.contains("world"), message); assertTrue(message.contains("winter"), message); assertTrue(message.contains(MapPool.LOBBY_MAP_PROPERTY), message); + assertTrue(message.contains("the default world 'world'"), message); + } + + @Test + @DisplayName("The warning names the world that was really taken, not the default one") + void testTheWarningNamesTheWorldThatWasReallyTaken() { + // The message used to announce the default world in this case as well, while the code went + // on with the first world it found. Saying one thing and doing another is the one thing a + // warning like this must not do. + String message = MapPool.describeMissingMap("halloween", List.of("winter"), "winter"); + + assertTrue(message.contains("the world 'winter'"), message); + assertFalse(message.contains("Falling back to the default world"), message); + } + + @Test + @DisplayName("Clearing the pool leaves an empty list rather than none") + void testClearingThePoolLeavesAnEmptyList(@TempDir Path root) throws IOException { + // The package is @NotNullByDefault and the field used to be set to null here, which turned + // every later read of the pool into a NullPointerException. + Path worlds = worlds(root, "world", "winter"); + MapPool pool = new MapPool(worlds, MapPoolTest::allDirectories); + + pool.clear(); + + assertTrue(pool.getAvailableMaps().isEmpty()); + assertTrue(pool.availableMapNames().isEmpty()); } } From cad463df3a2e4b7ace7eccf3c42b2ab2d099ca54 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:31:59 +0200 Subject: [PATCH 10/11] docs: correct the lighting and shutdown claims the code did not keep exploration-lighting.md said LightingChunk still owned the sky pass. It did not: a fresh Light reports itself as valid, so the sky pass never ran and sky light came entirely from the region file, with no fallback and no recomputation. The section now describes what actually happens - marks on load, one scheduled pass per tick over non-overlapping areas, block and sky light from Falco, sending from LightingChunk - and why each half of that is load bearing. lobby-world-selection.md claimed the loader is closed on shutdown, which was true for :app only, and described a second warning for the case where the default world is missing too; there is one warning now and it names the world that was really taken. The stage 1 note of the spec recorded the scheduler as unusable. The reason it gave was right in its facts and too wide in its conclusion, and the note now carries what was measured instead. --- docs/exploration-lighting.md | 72 ++++++++++++++++++++------------ docs/lobby-world-selection.md | 41 ++++++++++++------ docs/spec-lobby-saison-events.md | 26 ++++++++---- 3 files changed, 94 insertions(+), 45 deletions(-) diff --git a/docs/exploration-lighting.md b/docs/exploration-lighting.md index 01922627..19ce4bd8 100644 --- a/docs/exploration-lighting.md +++ b/docs/exploration-lighting.md @@ -14,9 +14,9 @@ There are two distinct behaviours, and they must not be confused: 1. **Lobby = complete relight (now).** The whole lobby is lit. Maps are large (~18–20k populated chunks each), so we do **not** pre-load and relight every - chunk at boot. Instead `MapProvider` lights each chunk as it is loaded, so - everything is lit by the time it is visible — no dark areas, no visible - reveal. + chunk at boot. Instead `MapProvider` schedules the light of each chunk as it + is loaded, so everything is lit within a tick or two of becoming visible — no + dark areas, no visible reveal. 2. **Special maps = deliberate darkening + per-player reveal (later).** Some maps should be **deliberately dark** and only light up per player as they explore (fog-of-war). This reuses the "anvil chunk stays dark until relit" @@ -24,22 +24,43 @@ There are two distinct behaviours, and they must not be confused: ## Current state (lobby, instance-level) -`MapProvider` does an **instance-wide** relight: +`MapProvider` lights the world through Falco's `ChunkLightScheduler`: - The world instance uses `LightingChunk` (`instance.setChunkSupplier(LightingChunk::new)`). -- An `InstanceChunkLoadEvent` listener lights every chunk as it is loaded, - because anvil chunks otherwise stay dark until a block update triggers a - relight. Since US-1.03 the block light comes from `falco-light`'s - `ChunkLightService.calculateWithNeighbours(...)` instead of - `LightingChunk.relight(...)`. Falco writes its result through `Light#set`, - which clears the update flag of the section, so Minestom does not recompute - it; the cached packets of the chunk are dropped afterwards and a resend is - scheduled, because the load event is dispatched after the loading future - completes. -- `LightingChunk` stays the chunk type: it is what sends the light, and it still - owns the sky pass, which `calculateWithNeighbours` does not cover. -- Lobby time is frozen at midday (`setTime(6000)` + `setTimeRate(0)`) so lit - chunks render bright. +- An `InstanceChunkLoadEvent` listener **marks** every chunk that arrives + (`ChunkLightScheduler#markChanged`), together with the eight chunks around it. + It computes nothing itself. Anvil chunks otherwise stay dark until a block + update triggers a relight. +- An `InstanceTickEvent` listener runs the pass (`ChunkLightScheduler#onTick`). + The scheduler collects the marks of a tick, groups them into areas that do not + overlap, and computes each area on a virtual thread. Both halves of that + matter: + - **The marks reach the neighbours**, so the chunk that loaded first — lit + with nothing beside it — is lit again when its neighbour turns up. Lighting + a chunk once, inline, cannot do that: the write goes through `Light#set`, + which clears the update flag of the section, so nothing would ever look at + that chunk again and the border would stay dark permanently. + - **The areas do not overlap**, so no two threads light chunks against each + other. `FalcoAnvilLoader#supportsParallelLoading()` is `true`, which makes + several adjacent chunks arriving at once the normal case rather than a + corner one, and Falco's own javadoc is explicit that lighting overlapping + neighbourhoods concurrently produces a seam — never an error, and permanent + for the same reason as above. +- **The scheduler owns the sky pass too** (`SkyLight.FROM_DIMENSION`, so it runs + in a dimension that has sky light and not in one that does not). This is not + optional: a fresh Minestom `Light` reports itself as *valid* + (`isValidBorders = true`), and `LightingChunk` only relights a section whose + `requiresUpdate()` is true — so a section whose region file carries no + `SkyLight` array would stay at level 0 for the lifetime of the server. + Reading `SkyLight` out of the NBT sets the same flag, so the region file is + not a fallback either. +- `LightingChunk` stays the chunk type for the one thing it does here: sending + the light to the players. The scheduler drops the cached packets of every + chunk it wrote, which covers whoever receives the chunk next; the resend timer + for players who already hold it is armed from the completion callback of the + pass. +- Lobby time is frozen at midday (`setTime(6000)` + `defaultClock().rate(0)`) so + lit chunks render bright. This makes the lobby fully lit (mode 1). The lighting is **shared**: a chunk is lit once for everyone. It is also the foundation for mode 2 — the per-player @@ -79,9 +100,9 @@ per-player we override the light **on the wire**, per player: chunk, mark it discovered and send that player an `UpdateLightPacket` with the chunk's real (relit) light. Optionally animate the reveal by revealing neighbouring chunks outward. -4. **Real light source.** The instance still computes correct light via - `LightingChunk` (the global relight above); the per-player layer only chooses - whether to send the real light or a dark override to each player. +4. **Real light source.** The instance still computes correct light through the + Falco scheduler (see above); the per-player layer only chooses whether to + send the real light or a dark override to each player. ### Open questions / challenges @@ -96,10 +117,9 @@ per-player we override the light **on the wire**, per player: the "dark" light payload and only compute real light once (shared) + resend. - **Persistence:** decide whether discovered regions persist across sessions (per-player save) or reset on rejoin. -- **Cross-version time API:** time is frozen with the legacy - `setTime`/`setTimeRate`; newer Minestom replaces this with the world - `Clock` API (`instance.defaultClock().rate(0f)`) — migrate when Minestom is - bumped. +- **Cross-version time API:** the rate is already frozen through the world + `Clock` API (`instance.defaultClock().rate(0f)`); the time itself is still set + through the legacy `Instance#setTime`. ## Next steps @@ -110,5 +130,5 @@ per-player we override the light **on the wire**, per player: 3. Add per-player discovered-chunk tracking on `TitanPlayer`. 4. Implement the dark-light override on chunk send + reveal `UpdateLightPacket` on discovery (only for maps with the dark/reveal flag set). -5. Keep the instance-level lighting (Falco block light, `LightingChunk` sky - light and sending) as the real-light source. +5. Keep the instance-level lighting (Falco block **and** sky light, + `LightingChunk` for sending) as the real-light source. diff --git a/docs/lobby-world-selection.md b/docs/lobby-world-selection.md index d105607f..7cdc9f83 100644 --- a/docs/lobby-world-selection.md +++ b/docs/lobby-world-selection.md @@ -66,9 +66,17 @@ The world 'halloewen' named by the system property TITAN_LOBBY_MAP does not exist. Found worlds: world, winter. Falling back to the default world 'world'. ``` -If the default world is missing as well, the first world that was found is used -and a second warning says so. Only a `worlds/` directory without any world at -all stops the start, because there is then nothing left to serve. +If the default world is missing as well, the first world that was found is used, +and the same warning says so instead — it names the world that was really taken: + +``` +The world 'halloween' named by the system property TITAN_LOBBY_MAP does not +exist. Found worlds: winter. Falling back to the world 'winter', because the +default world 'world' is not there either. +``` + +Only a `worlds/` directory without any world at all stops the start, because +there is then nothing left to serve. Whether the requested world was the one that got selected is readable from `MapPool#isRequestedMapSelected()`, so the fallback is observable from code and @@ -83,12 +91,21 @@ absent, the server then generates a fresh chunk in its place, and the next save writes that over the built world. Falco's loader reports the failure instead. The loader keeps region files open for as long as it lives, so it is created -once per world root and closed on shutdown through `MapProvider#close()`. - -Block light is computed by `falco-light`'s `ChunkLightService`, which lights a -loaded chunk together with the ring around it. `LightingChunk` stays the chunk -type of the instance: writing the light clears the update flag of the section, -so Minestom does not recompute what Falco just calculated, and it keeps doing -the part Falco is not asked for here — sending the light and the sky pass. See -[`exploration-lighting.md`](exploration-lighting.md) for what this is the -foundation of. +once per world root and closed on shutdown through `MapProvider#close()` — in +`:app` and in `:setup`, which is the module that actually changes worlds and +therefore the one that must not drop a region handle unflushed. Closing also +takes the closed loader off the instance and removes the listeners of the +provider, so a player who is still moving during the shutdown gets an empty +chunk rather than the `IllegalStateException` of a loader that was closed +underneath them. A closed provider refuses `saveMap`; it does not build a fresh +loader and reopen what the shutdown has just closed. + +Light — block **and** sky — is computed by `falco-light`, driven by a +`ChunkLightScheduler`: a chunk that arrives is marked together with the eight +around it, and the pass runs once per instance tick over areas that do not +overlap. That is what keeps the chunk which loaded first from staying dark along +the border once its neighbours arrive, and what keeps two parallel loads from +lighting each other's chunks. `LightingChunk` stays the chunk type of the +instance for the part Falco does not do: sending the light. See +[`exploration-lighting.md`](exploration-lighting.md) for the mechanics and for +what this is the foundation of. diff --git a/docs/spec-lobby-saison-events.md b/docs/spec-lobby-saison-events.md index 71948649..172d42b3 100644 --- a/docs/spec-lobby-saison-events.md +++ b/docs/spec-lobby-saison-events.md @@ -147,19 +147,31 @@ und 3. **Umsetzungsstand Stufe 1 (28.08.2026).** US-1.01 bis US-1.06 sind umgesetzt; Layout und Auswahl der Welten sind in -[`lobby-world-selection.md`](lobby-world-selection.md) dokumentiert. Drei Punkte, +[`lobby-world-selection.md`](lobby-world-selection.md) dokumentiert. Vier Punkte, die von dieser Spec abweichen und beim Review bekannt sein sollten: - **Falco-Version.** Die Spec nennt 0.3.0. Aufgelöst wird **2.1.0** — 0.3.0 stammt aus der Zeit vor Minestom 26.1. Falco bringt `mycelium-bom` 1.7.2 mit, `aonyx-bom` 0.8.0 bringt 1.7.1, wodurch Minestom von `2026.06.05-26.1.2` auf `2026.06.20-26.1.2` steigt (gleiche Protokollversion). -- **Licht.** Der Blocklichtpfad läuft über `ChunkLightService`; `LightingChunk` - bleibt der Chunktyp, weil er das Licht versendet und den Himmelspass hält. - Der vollständigere Weg über `ChunkLightScheduler.supplier()` ist **nicht** - gangbar: dessen `FalcoLightingChunk` erbt in falco-light 2.1.0 von - `FalcoChunk` aus `falco-instance`, einem Modul, das das Artefakt weder - mitliefert noch deklariert. Nachziehen, sobald Falco das behoben hat. +- **Licht.** Block- **und** Himmelslicht laufen über den + `ChunkLightScheduler`; `LightingChunk` bleibt der Chunktyp, weil er das Licht + versendet. Ein früherer Stand berechnete das Licht direkt beim Laden über + `ChunkLightService.calculateWithNeighbours(...)` — das war in drei Punkten + falsch: der zuerst geladene Chunk behielt seinen Rand dauerhaft dunkel (die + Schreiboperation löscht das Update-Flag der Section, niemand rechnet erneut), + parallel geladene Nachbarchunks belichteten sich gegenseitig (Falcos eigenes + Javadoc verbietet überlappende Nachbarschaften), und Himmelslicht wurde + überhaupt nicht berechnet, weil eine frische Minestom-`Light` sich selbst als + gültig meldet und `LightingChunk` den Himmelspass deshalb überspringt. +- **`falco-instance` ist Pflicht.** Ein früherer Stand hielt den + `ChunkLightScheduler` für unbenutzbar, weil `FalcoLightingChunk` von + `FalcoChunk` aus `falco-instance` erbt und falco-light das Modul nicht + deklariert. Der Schluss war zu weit: nur `supplier()` nennt diese Klasse. Die + Verifikation der Klasse löst den Typ allerdings beim Linken auf, nicht erst + beim Ausführen des Lambdas — `new ChunkLightScheduler(...)` scheitert ohne das + Modul mit `NoClassDefFoundError`. Titan deklariert `falco-instance` deshalb + selbst als Laufzeitabhängigkeit. - **US-1.06** war bereits erfüllt und ist nur verifiziert und dokumentiert worden, nicht geändert. From f80ede1ff00bd0dd42bfe281e11a78a0d7a95462 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 11:34:20 +0200 Subject: [PATCH 11/11] build: keep Minestom on the version aonyx-bom prescribes (NFR-001) Every falco artifact carries mycelium-bom 1.7.2 as a platform dependency where aonyx-bom 0.8.0 brings 1.7.1, so highest-wins moved Minestom from 2026.06.05-26.1.2 to 2026.06.20-26.1.2 with nothing announcing it. This particular bump was harmless, but nothing constrained it and the next falco release would repeat it unnoticed - which is what NFR-001 forbids. The three falco dependencies of :common now exclude that BOM. Verified by resolving rather than assuming: :common, :app and :setup resolve net.minestom:minestom:2026.06.05-26.1.2 on compile, runtime and test runtime, and :bridge compiles against the same version, so the divergence where :bridge was built against one Minestom while :app ran another is gone with it. All 47 :common and 31 :app tests pass on that version, and both fat jars still carry the falco classes. The catalog comment claimed 2.1.0 'resolves against the Minestom version the aonyx BOM pins'. It did not resolve against it, it overrode it. --- common/build.gradle.kts | 17 +++++++++++------ docs/spec-lobby-saison-events.md | 15 ++++++++++++--- settings.gradle.kts | 8 ++++++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/common/build.gradle.kts b/common/build.gradle.kts index 12ae8338..6a791c0b 100644 --- a/common/build.gradle.kts +++ b/common/build.gradle.kts @@ -11,12 +11,17 @@ dependencies { implementation(libs.aves) implementation(libs.adventure.minimessage) // Falco replaces Minestom's AnvilLoader and light engine (US-1.01 - US-1.03). - implementation(libs.falco.anvil) - implementation(libs.falco.light) + // Every falco artifact carries mycelium-bom as a platform dependency, and it is a release + // ahead of the one aonyx-bom brings. Highest-wins would move Minestom underneath us with + // nothing to announce it, which is what NFR-001 forbids: the Minestom version is the one + // aonyx-bom prescribes. Excluded here rather than pinned, so the next falco release cannot + // move it either. + implementation(libs.falco.anvil) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } + implementation(libs.falco.light) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } // Runtime only for us: nothing here names a falco-instance type, but linking // ChunkLightScheduler resolves FalcoLightingChunk and its FalcoChunk supertype. See the // version catalog for the details. - runtimeOnly(libs.falco.instance) + runtimeOnly(libs.falco.instance) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } // No CloudNet here anymore: anything touching the CloudNet bridge lives in the // :bridge extension; common only talks to it through the JDK-typed @@ -24,9 +29,9 @@ dependencies { testImplementation(platform(libs.aonyx.bom)) testImplementation(libs.minestom) - testImplementation(libs.falco.anvil) - testImplementation(libs.falco.light) - testRuntimeOnly(libs.falco.instance) + testImplementation(libs.falco.anvil) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } + testImplementation(libs.falco.light) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } + testRuntimeOnly(libs.falco.instance) { exclude(group = "net.onelitefeather", module = "mycelium-bom") } testImplementation(libs.cyano) testImplementation(libs.aves) testImplementation(libs.junit.api) diff --git a/docs/spec-lobby-saison-events.md b/docs/spec-lobby-saison-events.md index 172d42b3..3a2a5164 100644 --- a/docs/spec-lobby-saison-events.md +++ b/docs/spec-lobby-saison-events.md @@ -151,9 +151,18 @@ Layout und Auswahl der Welten sind in die von dieser Spec abweichen und beim Review bekannt sein sollten: - **Falco-Version.** Die Spec nennt 0.3.0. Aufgelöst wird **2.1.0** — 0.3.0 - stammt aus der Zeit vor Minestom 26.1. Falco bringt `mycelium-bom` 1.7.2 mit, - `aonyx-bom` 0.8.0 bringt 1.7.1, wodurch Minestom von `2026.06.05-26.1.2` auf - `2026.06.20-26.1.2` steigt (gleiche Protokollversion). + stammt aus der Zeit vor Minestom 26.1. Jedes falco-Artefakt bringt + `mycelium-bom` 1.7.2 als Plattform-Abhängigkeit mit, `aonyx-bom` 0.8.0 bringt + 1.7.1; ohne Gegenmaßnahme hebt „highest wins" Minestom damit still von + `2026.06.05-26.1.2` auf `2026.06.20-26.1.2`. Genau das verbietet NFR-001, und + zwar unabhängig davon, ob der konkrete Sprung harmlos ist: die nächste + falco-Version würde ihn wiederholen, ohne dass es jemand merkt. `:common` + schließt `mycelium-bom` aus allen drei falco-Abhängigkeiten aus. Verifiziert + über die aufgelösten Klassenpfade: jedes Modul (`:common`, `:app`, `:setup`, + `:bridge`) löst `net.minestom:minestom:2026.06.05-26.1.2` auf — die Version, + die `aonyx-bom` vorgibt. Damit ist auch die frühere Abweichung weg, bei der + `:bridge` gegen `2026.06.05` kompilierte, während `:app` auf `2026.06.20` + lief. - **Licht.** Block- **und** Himmelslicht laufen über den `ChunkLightScheduler`; `LightingChunk` bleibt der Chunktyp, weil er das Licht versendet. Ein früherer Stand berechnete das Licht direkt beim Laden über diff --git a/settings.gradle.kts b/settings.gradle.kts index 8866e7bf..a33138db 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -44,8 +44,12 @@ dependencyResolutionManagement { // Falco: the OneLiteFeather chunk loader and light engine. 0.3.0 is the // version the lobby spec names, but it predates Minestom 26.1; 2.1.0 is - // the current release and the first that resolves against the Minestom - // version the aonyx BOM pins. + // the current release and the first that builds against Minestom 26.1. + // It does not resolve against the Minestom version aonyx-bom pins - it + // would override it, because every falco artifact carries mycelium-bom + // as a platform dependency and that one is a release ahead. The falco + // dependencies of :common exclude that BOM, so the Minestom version + // stays the one aonyx-bom prescribes (NFR-001). version("falco", "2.1.0") version("luckperms", "5.6-SNAPSHOT")