feat(world): merge chunk light after ready, not as a pipeline stage

engine-tests/src/test/java/org/terasology/engine/integrationenvironment/LateLightMergerMteTest.java

@@ -0,0 +1,94 @@
+// Copyright 2026 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.engine.integrationenvironment;
+
+import org.joml.Vector3f;
+import org.joml.Vector3fc;
+import org.joml.Vector3i;
+import org.junit.jupiter.api.Test;
+import org.terasology.engine.entitySystem.entity.EntityManager;
+import org.terasology.engine.entitySystem.entity.EntityRef;
+import org.terasology.engine.entitySystem.entity.internal.EntityScope;
+import org.terasology.engine.integrationenvironment.jupiter.IntegrationEnvironment;
+import org.terasology.engine.logic.location.LocationComponent;
+import org.terasology.engine.network.NetworkMode;
+import org.terasology.engine.world.block.BlockRegionc;
+import org.terasology.engine.world.chunks.ChunkProvider;
+import org.terasology.engine.world.chunks.Chunks;
+import org.terasology.engine.world.chunks.localChunkProvider.RelevanceSystem;
+import org.terasology.unittest.worlds.DummyWorldGenerator;
+
+import java.util.stream.StreamSupport;
+
+/**
+ * Pins down the central behaviour change of {@code LateLightMerger}: a chunk becomes ready as soon
+ * as its own generation finishes, not only once its whole 3x3x3 neighbourhood does too.
+ * <p>
+ * Under the pipeline stage this replaced, a chunk whose neighbours were never requested could only
+ * become ready via {@code ChunkProcessingPipeline}'s idle-skip timeout - two consecutive 5s-idle
+ * polls, so ~10s at best (see {@code POLL_INTERVAL_MS} / {@code IDLE_POLLS_BEFORE_SKIP} there). Both
+ * tests below bound completion well under that, on relevance requests deliberately built so the
+ * requested chunk(s)' own neighbours are never themselves requested. That makes them fail loudly
+ * (timeout) rather than merely slowly against the old, pipeline-stage merge - see the task/PR notes
+ * for the actual before/after timings observed when checking that.
+ * <p>
+ * Deliberately not asserted here: that a requested chunk's neighbours stay unloaded. They usually do,
+ * but {@code RelevanceSystem.addRelevanceEntity}'s own {@code .sorted()} pass over a
+ * {@code BlockRegion}'s iterator can request one extra, wrong position - a pre-existing aliasing bug
+ * (the iterator hands out a reused, mutable {@code Vector3i} that a later {@code hasNext()} call can
+ * mutate out from under a caller that buffers rather than immediately consumes it), unrelated to
+ * light merging. {@code RelevanceSystem.updateRelevance()}'s follow-up pass - which uses the
+ * defensive-copying {@code ChunkRelevanceRegion.getNeededChunks()} instead - still requests the
+ * correct position(s) a tick later, so it doesn't affect these tests' timing, but it does mean a
+ * "neighbours were never loaded" assertion is not reliable and was left out rather than pinned to
+ * today's incidental behaviour of an unrelated bug.
+ *
+ * @see org.terasology.engine.world.chunks.LateLightMerger
+ */
+@IntegrationEnvironment(networkMode = NetworkMode.LISTEN_SERVER)
+class LateLightMergerMteTest {
+
+    /**
+     * Comfortably above what one dummy-world chunk takes to generate, comfortably below the ~10s
+     * ChunkProcessingPipeline idle-skip the old pipeline-stage merge needed whenever a chunk's
+     * neighbours were never requested.
+     */
+    private static final long READY_TIMEOUT_MS = 8000;
+
+    @Test
+    void chunkBecomesReadyWithoutNeighbourhoodLoaded(EntityManager entityManager, RelevanceSystem relevanceSystem,
+            MainLoop mainLoop, ChunkProvider chunkProvider) {
+        // Far from spawn (a fixed (0,0,0) for DummyWorldGenerator) and from the other test below, so
+        // nothing else ever requests this position or its neighbours.
+        Vector3fc center = new Vector3f(200_000, DummyWorldGenerator.SURFACE_HEIGHT, 200_000);
+        Vector3i chunkPos = Chunks.toChunkPos(center, new Vector3i());
+
+        EntityRef entity = entityManager.create(new LocationComponent(center));
+        entity.setScope(EntityScope.GLOBAL);
+        // distance (1,1,1) requests relevance for exactly this one chunk - unlike
+        // ChunkRegionFuture, no margin, so its neighbours are never deliberately requested.
+        relevanceSystem.addRelevanceEntity(entity, new Vector3i(1, 1, 1), null);
+
+        mainLoop.awaitUntil(READY_TIMEOUT_MS, "an isolated chunk (no neighbours requested) to become ready",
+                () -> chunkProvider.isChunkReady(chunkPos));
+    }
+
+    @Test
+    void relevanceRegionBecomesFullyReadyWithoutMargin(EntityManager entityManager, RelevanceSystem relevanceSystem,
+            MainLoop mainLoop, ChunkProvider chunkProvider) {
+        // ChunkRegionFuture.REQUIRED_CHUNK_MARGIN pads every relevance request by one extra shell of
+        // chunks, specifically so the requested region's own outer shell has its neighbourhood
+        // requested too (see its FIXME comment). Going straight to RelevanceSystem instead of through
+        // ChunkRegionFuture, with no padding at all, tests whether that padding is still needed now
+        // that readiness no longer waits on the neighbourhood.
+        Vector3fc center = new Vector3f(300_000, DummyWorldGenerator.SURFACE_HEIGHT, 300_000);
+
+        EntityRef entity = entityManager.create(new LocationComponent(center));
+        entity.setScope(EntityScope.GLOBAL);
+        BlockRegionc region = relevanceSystem.addRelevanceEntity(entity, new Vector3i(3, 3, 3), null);
+
+        mainLoop.awaitUntil(READY_TIMEOUT_MS, "every chunk in an unpadded 3x3x3 relevance region to become ready",
+                () -> StreamSupport.stream(region.spliterator(), false).allMatch(chunkProvider::isChunkReady));
+    }
+}

engine-tests/src/test/java/org/terasology/engine/world/chunks/LateLightMergerTest.java

@@ -0,0 +1,109 @@
+// Copyright 2026 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+package org.terasology.engine.world.chunks;
+
+import com.google.common.collect.Maps;
+import org.joml.Vector3i;
+import org.joml.Vector3ic;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.terasology.engine.TerasologyTestingEnvironment;
+import org.terasology.engine.registry.CoreRegistry;
+import org.terasology.engine.world.block.internal.BlockManagerImpl;
+import org.terasology.engine.world.block.tiles.NullWorldAtlas;
+import org.terasology.engine.world.chunks.blockdata.ExtraBlockDataManager;
+import org.terasology.engine.world.chunks.internal.ChunkImpl;
+import org.terasology.engine.world.propagation.light.LightMerger;
+import org.terasology.gestalt.assets.management.AssetManager;
+
+import java.util.Map;
+
+import static com.google.common.truth.Truth.assertThat;
+
+/**
+ * {@link LateLightMerger} is driven directly here rather than through an {@code IntegrationEnvironment}
+ * - its constructor takes a plain {@code Map<Vector3ic, Chunk>}, so the bookkeeping can be tested
+ * without a running engine. Real {@link ChunkImpl} chunks (rather than a bare stub) are used because
+ * {@link LightMerger#merge} does genuine light propagation and needs real block/light storage - see
+ * {@code BetweenChunkPropagationTest} for the same pattern.
+ */
+@Tag("TteTest")
+class LateLightMergerTest extends TerasologyTestingEnvironment {
+
+    /** Generous, so a slow machine cannot make the drain give up mid-test. */
+    private static final int TICK_BUDGET_MS = 10_000;
+
+    private BlockManagerImpl blockManager;
+    private ExtraBlockDataManager extraDataManager;
+
+    @BeforeEach
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        blockManager = new BlockManagerImpl(new NullWorldAtlas(), CoreRegistry.get(AssetManager.class), true);
+        extraDataManager = new ExtraBlockDataManager();
+    }
+
+    private Chunk createChunkAt(Vector3ic pos) {
+        return new ChunkImpl(new Vector3i(pos), blockManager, extraDataManager);
+    }
+
+    /**
+     * Covers a bug found and fixed in review, with no prior coverage: {@link LateLightMerger#mergeAt}
+     * re-checks the neighbourhood at merge time, not just at queue time in {@link
+     * LateLightMerger#chunkReady}. A position that loses a neighbour in between must go back into
+     * {@code needsMerging} rather than being dropped - it is queued from neither bookkeeping set
+     * otherwise, and only a chunk becoming ready ever re-queues anything, so it would stay unmerged
+     * forever even once the neighbour comes back.
+     */
+    @Test
+    void positionRequeuedWhenNeighbourGoesMissingBeforeMergeRuns() {
+        Vector3ic center = new Vector3i(0, 0, 0);
+        Vector3ic missingNeighbour = new Vector3i(1, 0, 0);
+
+        Map<Vector3ic, Chunk> chunkCache = Maps.newHashMap();
+        for (Vector3ic pos : LightMerger.requiredChunks(center)) {
+            chunkCache.put(new Vector3i(pos), createChunkAt(pos));
+        }
+
+        // The merge only writes - and so only dirties - where light actually moves, so an entirely
+        // uniform neighbourhood would merge to no observable effect at all. Light the face of the
+        // +X neighbour that abuts the center chunk, giving the merge something to propagate inwards.
+        Chunk litNeighbour = chunkCache.get(missingNeighbour);
+        for (int y = 0; y < 4; y++) {
+            for (int z = 0; z < 4; z++) {
+                litNeighbour.setLight(0, y, z, (byte) 15);
+            }
+        }
+        // ChunkImpl starts dirty (it still needs its first mesh); clear that so isDirty() below is a
+        // clean signal for "the merge wrote here", not construction noise.
+        chunkCache.values().forEach(chunk -> chunk.setDirty(false));
+
+        LateLightMerger merger = new LateLightMerger(chunkCache);
+
+        // Full neighbourhood already present, so this discovers it and queues center for merging.
+        merger.chunkReady(center);
+
+        // A neighbour unloads before the merge actually runs - checkForUnload() runs every tick in
+        // both providers, ahead of processPending().
+        Chunk removedNeighbour = chunkCache.remove(missingNeighbour);
+        merger.chunkUnloaded(missingNeighbour);
+
+        merger.processPending(System.currentTimeMillis(), TICK_BUDGET_MS);
+
+        // mergeAt() must have found the hole and backed off rather than merging with it.
+        assertThat(chunkCache.get(center).isDirty()).isFalse();
+
+        // The neighbour reloads. Nothing but a chunkReady() call ever re-discovers a completed
+        // neighbourhood - if mergeAt() had dropped center instead of requeuing it, this would never
+        // recover it and the assertions below would fail.
+        chunkCache.put(new Vector3i(missingNeighbour), removedNeighbour);
+        merger.chunkReady(missingNeighbour);
+        merger.processPending(System.currentTimeMillis(), TICK_BUDGET_MS);
+
+        // The seeded light has now propagated into the center chunk, which is both the proof that
+        // mergeAt() ran for it and the reason ChunkMeshWorker will re-mesh it.
+        assertThat(chunkCache.get(center).isDirty()).isTrue();
+    }
+}

engine-tests/src/test/java/org/terasology/engine/world/propagation/LocalChunkViewTest.java

@@ -0,0 +1,99 @@
+// Copyright 2026 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+package org.terasology.engine.world.propagation;
+
+import org.joml.Vector3i;
+import org.joml.Vector3ic;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.terasology.engine.TerasologyTestingEnvironment;
+import org.terasology.engine.registry.CoreRegistry;
+import org.terasology.engine.world.block.internal.BlockManagerImpl;
+import org.terasology.engine.world.block.tiles.NullWorldAtlas;
+import org.terasology.engine.world.chunks.Chunk;
+import org.terasology.engine.world.chunks.Chunks;
+import org.terasology.engine.world.chunks.blockdata.ExtraBlockDataManager;
+import org.terasology.engine.world.chunks.internal.ChunkImpl;
+import org.terasology.engine.world.propagation.light.LightMerger;
+import org.terasology.engine.world.propagation.light.LightPropagationRules;
+import org.terasology.gestalt.assets.management.AssetManager;
+
+import java.util.Arrays;
+import java.util.Comparator;
+
+import static com.google.common.truth.Truth.assertThat;
+
+@Tag("TteTest")
+class LocalChunkViewTest extends TerasologyTestingEnvironment {
+
+    private BlockManagerImpl blockManager;
+    private ExtraBlockDataManager extraDataManager;
+
+    @BeforeEach
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        blockManager = new BlockManagerImpl(new NullWorldAtlas(), CoreRegistry.get(AssetManager.class), true);
+        extraDataManager = new ExtraBlockDataManager();
+    }
+
+    /**
+     * Builds the neighbourhood exactly as {@code LightMerger.merge} does - sorted by x, then y, then z -
+     * since that sort is what defines the array order this view has to agree with.
+     */
+    private Chunk[] sortedNeighbourhoodAround(Vector3ic centre) {
+        Chunk[] chunks = LightMerger.requiredChunks(centre).stream()
+                .map(p -> (Chunk) new ChunkImpl(new Vector3i(p), blockManager, extraDataManager))
+                .toArray(Chunk[]::new);
+        Arrays.sort(chunks, Comparator.<Chunk>comparingInt(c -> c.getPosition().x())
+                .thenComparingInt(c -> c.getPosition().y())
+                .thenComparing(c -> c.getPosition().z()));
+        return chunks;
+    }
+
+    /**
+     * A write must land in the chunk that actually contains the position.
+     * <p>
+     * This did not hold: the view indexed the array with x varying fastest while the sort makes z vary
+     * fastest, so x and z were transposed and a write aimed at the +X neighbour landed in the +Z one.
+     * It went unnoticed because reads used the same wrong mapping - so a read-back check passes either
+     * way - and because the centre chunk is invariant under the swap. Hence asserting on the chunks
+     * themselves rather than on what the view returns.
+     */
+    @Test
+    void writesLandInTheChunkContainingThePosition() {
+        Chunk[] chunks = sortedNeighbourhoodAround(new Vector3i(0, 0, 0));
+        LocalChunkView view = new LocalChunkView(chunks, new LightPropagationRules());
+
+        for (Chunk expected : chunks) {
+            Vector3ic chunkPos = expected.getPosition();
+            // First block of that chunk, in world coordinates.
+            Vector3ic blockPos = new Vector3i(
+                    chunkPos.x() * Chunks.SIZE_X,
+                    chunkPos.y() * Chunks.SIZE_Y,
+                    chunkPos.z() * Chunks.SIZE_Z);
+
+            Arrays.stream(chunks).forEach(c -> c.setLight(0, 0, 0, (byte) 0));
+            view.setValueAt(blockPos, (byte) 15);
+
+            assertThat(expected.getLight(0, 0, 0)).isEqualTo((byte) 15);
+        }
+    }
+
+    /** A position outside the 3x3x3 must not alias onto a chunk that happens to sit at that index. */
+    @Test
+    void positionsOutsideTheViewAreUnavailable() {
+        Chunk[] chunks = sortedNeighbourhoodAround(new Vector3i(0, 0, 0));
+        LocalChunkView view = new LocalChunkView(chunks, new LightPropagationRules());
+
+        // Three chunks along +X: outside the view, but a flat index would wrap into it.
+        Vector3ic outside = new Vector3i(3 * Chunks.SIZE_X, 0, 0);
+
+        assertThat(view.getValueAt(outside)).isEqualTo(PropagatorWorldView.UNAVAILABLE);
+        assertThat(view.getBlockAt(outside)).isNull();
+
+        view.setValueAt(outside, (byte) 15);
+        Arrays.stream(chunks).forEach(c -> assertThat(c.getLight(0, 0, 0)).isEqualTo((byte) 0));
+    }
+}