Starlight 1.0.0-RC1

- Completely rewrite how lighting is loaded in for clients
 Attempts to fix #25
 and #40
 The old hook was vulnerable to mods doing unexpected things, especially
 serverside. The old method also relied on unreliable ordering of events
 inside packet handling. New method just hooks directly into the
 packet handling methods to avoid conflicting behavior with mods and
 to create more reliable hooks.

- Completely rewrites internal scheduling of light tasks
 Old scheduling was very vulnerable to block updates completely
 overloading everything. The old scheduling was also incapable of
 indicating to the chunk system whether block updates or other
 light modifying tasks were pending for a chunk. The new
 scheduling now groups tasks by chunk (light update, needs lighting,
 edge checks) and simply drains tasks by chunks. In the event
 that there are a lot of block updates, they no longer pile up
 in a queue since they are just grouped onto their chunk, which
 will eliminate duplicate updates. And since it's a FIFO by chunk,
 the maximum latency for a light update to happen is simply the
 number of chunks queued. The new queue also tells the chunk
 system to not unload chunks until any lighting task is fully complete
 for that chunk and its 1 radius neighbours. This will eliminate
 problems where the light engine was never able to complete tasks
 before server shutdown. This also officially kills light suppression.
 Before you ask, no I'm not going to re-add lighting _bugs_ to my light
 engine. I want lighting to work, which means I need to fix it when
 it clearly doesn't work.

- Mark the vanilla light engine fields in LevelLightEngine as null
 This will break any mod expecting them to be non-null. But since
 we replace the light engine, it's better this way since it forces
 an explicit break rather than a silent break.

gradle.properties

@@ -6,7 +6,7 @@ minecraft_version=1.16.5
 yarn_mappings=1.16.5+build.6
 loader_version=0.11.3
 # Mod Properties
-mod_version=0.0.3
+mod_version=1.0.0-RC1
 maven_group=ca.spottedleaf.starlight
 archives_base_name=starlight
 # Dependencies

src/main/java/ca/spottedleaf/starlight/common/light/SWMRNibbleArray.java

@@ -42,6 +42,16 @@ private static void freeBytes(final byte[] bytes) {
         WORKING_BYTES_POOL.get().addFirst(bytes);
     }
 
+    public static SWMRNibbleArray fromVanilla(final DataLayer nibble) {
+        if (nibble == null) {
+            return new SWMRNibbleArray(null, true);
+        } else if (nibble.isEmpty()) {
+            return new SWMRNibbleArray();
+        } else {
+            return new SWMRNibbleArray(nibble.getData().clone()); // make sure we don't write to the parameter later
+        }
+    }
+
     protected int stateUpdating;
     protected volatile int stateVisible;
 

src/main/java/ca/spottedleaf/starlight/common/light/StarLightEngine.java

@@ -102,7 +102,7 @@ public AxisDirection getOpposite() {
     // index = x + (z * 5) + (y * 25)
     protected final boolean[] notifyUpdateCache;
 
-    // always initialsed during start of lighting. no index is null.
+    // always initialsed during start of lighting.
     // index = x + (z * 5)
     protected final ChunkAccess[] chunkCache = new ChunkAccess[5 * 5];
 

src/main/java/ca/spottedleaf/starlight/common/light/StarLightInterface.java

@@ -4,9 +4,9 @@
 import ca.spottedleaf.starlight.common.util.CoordinateUtils;
 import ca.spottedleaf.starlight.common.util.WorldUtil;
 import ca.spottedleaf.starlight.common.world.ExtendedWorld;
-import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
-import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap;
 import it.unimi.dsi.fastutil.shorts.ShortCollection;
+import it.unimi.dsi.fastutil.shorts.ShortOpenHashSet;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.SectionPos;
 import net.minecraft.server.level.ServerLevel;
@@ -19,9 +19,11 @@
 import net.minecraft.world.level.chunk.LightChunkGetter;
 import net.minecraft.world.level.lighting.LayerLightEventListener;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
 import java.util.function.IntConsumer;
 
@@ -38,7 +40,7 @@ public final class StarLightInterface {
     protected final ArrayDeque<SkyStarLightEngine> cachedSkyPropagators;
     protected final ArrayDeque<BlockStarLightEngine> cachedBlockPropagators;
 
-    protected final Long2ObjectOpenHashMap<ChunkChanges> changedBlocks = new Long2ObjectOpenHashMap<>();
+    protected final LightQueue lightQueue = new LightQueue(this);
 
     protected final LayerLightEventListener skyReader;
     protected final LayerLightEventListener blockReader;
@@ -232,9 +234,7 @@ public ChunkAccess getAnyChunkNow(final int chunkX, final int chunkZ) {
     }
 
     public boolean hasUpdates() {
-        synchronized (this) {
-            return !this.changedBlocks.isEmpty();
-        }
+        return !this.lightQueue.isEmpty();
     }
 
     public Level getWorld() {
@@ -293,33 +293,20 @@ protected final void releaseBlockLightEngine(final BlockStarLightEngine engine)
         }
     }
 
-    public void blockChange(BlockPos pos) {
+    public CompletableFuture<Void> blockChange(final BlockPos pos) {
         if (this.world == null || pos.getY() < WorldUtil.getMinBlockY(this.world) || pos.getY() > WorldUtil.getMaxBlockY(this.world)) { // empty world
-            return;
+            return null;
         }
 
-        pos = pos.immutable();
-        synchronized (this.changedBlocks) {
-            this.changedBlocks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), (final long keyInMap) -> {
-                return new ChunkChanges();
-            }).changedPositions.add(pos);
-        }
+        return this.lightQueue.queueBlockChange(pos);
     }
 
-    public void sectionChange(final SectionPos pos, final boolean newEmptyValue) {
+    public CompletableFuture<Void> sectionChange(final SectionPos pos, final boolean newEmptyValue) {
         if (this.world == null) { // empty world
-            return;
+            return null;
         }
 
-        synchronized (this.changedBlocks) {
-            final ChunkChanges changes = this.changedBlocks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), (final long keyInMap) -> {
-                return new ChunkChanges();
-            });
-            if (changes.changedSectionSet == null) {
-                changes.changedSectionSet = new Boolean[this.maxSection - this.minSection + 1];
-            }
-            changes.changedSectionSet[pos.getY() - this.minSection] = Boolean.valueOf(newEmptyValue);
-        }
+        return this.lightQueue.queueSectionChange(pos, newEmptyValue);
     }
 
     public void forceLoadInChunk(final ChunkAccess chunk, final Boolean[] emptySections) {
@@ -443,53 +430,156 @@ public void checkBlockEdges(final int chunkX, final int chunkZ, final ShortColle
         }
     }
 
+    public void scheduleChunkLight(final ChunkPos pos, final Runnable run) {
+        this.lightQueue.queueChunkLighting(pos, run);
+    }
+
+    public void removeChunkTasks(final ChunkPos pos) {
+        this.lightQueue.removeChunk(pos);
+    }
+
     public void propagateChanges() {
-        synchronized (this.changedBlocks) {
-            if (this.changedBlocks.isEmpty()) {
-                return;
-            }
+        if (this.lightQueue.isEmpty()) {
+            return;
         }
+
         final SkyStarLightEngine skyEngine = this.getSkyLightEngine();
         final BlockStarLightEngine blockEngine = this.getBlockLightEngine();
 
         try {
-            // TODO be smarter about this in the future
-            final Long2ObjectOpenHashMap<ChunkChanges> changedBlocks;
-            synchronized (this.changedBlocks) {
-                changedBlocks = this.changedBlocks.clone();
-                this.changedBlocks.clear();
-            }
-
-            for (final Iterator<Long2ObjectMap.Entry<ChunkChanges>> iterator = changedBlocks.long2ObjectEntrySet().fastIterator(); iterator.hasNext();) {
-                final Long2ObjectMap.Entry<ChunkChanges> entry = iterator.next();
-                final long coordinate = entry.getLongKey();
-                final ChunkChanges changes = entry.getValue();
-                final Set<BlockPos> positions = changes.changedPositions;
-                final Boolean[] sectionChanges = changes.changedSectionSet;
+            LightQueue.ChunkTasks task;
+            while ((task = this.lightQueue.removeFirstTask()) != null) {
+                if (task.lightTasks != null) {
+                    for (final Runnable run : task.lightTasks) {
+                        run.run();
+                    }
+                }
 
+                final long coordinate = task.chunkCoordinate;
                 final int chunkX = CoordinateUtils.getChunkX(coordinate);
                 final int chunkZ = CoordinateUtils.getChunkZ(coordinate);
 
-                if (skyEngine != null) {
+                final Set<BlockPos> positions = task.changedPositions;
+                final Boolean[] sectionChanges = task.changedSectionSet;
+
+                if (skyEngine != null && (!positions.isEmpty() || sectionChanges != null)) {
                     skyEngine.blocksChangedInChunk(this.lightAccess, chunkX, chunkZ, positions, sectionChanges);
                 }
-                if (blockEngine != null) {
+                if (blockEngine != null && (!positions.isEmpty() || sectionChanges != null)) {
                     blockEngine.blocksChangedInChunk(this.lightAccess, chunkX, chunkZ, positions, sectionChanges);
                 }
+
+                if (skyEngine != null && task.queuedEdgeChecksSky != null) {
+                    skyEngine.checkChunkEdges(this.lightAccess, chunkX, chunkZ, task.queuedEdgeChecksSky);
+                }
+                if (blockEngine != null && task.queuedEdgeChecksBlock != null) {
+                    blockEngine.checkChunkEdges(this.lightAccess, chunkX, chunkZ, task.queuedEdgeChecksBlock);
+                }
+
+                task.onComplete.complete(null);
             }
         } finally {
             this.releaseSkyLightEngine(skyEngine);
             this.releaseBlockLightEngine(blockEngine);
         }
     }
 
-    protected static final class ChunkChanges {
+    protected static final class LightQueue {
+
+        protected final Long2ObjectLinkedOpenHashMap<ChunkTasks> chunkTasks = new Long2ObjectLinkedOpenHashMap<>();
+        protected final StarLightInterface manager;
+
+        public LightQueue(final StarLightInterface manager) {
+            this.manager = manager;
+        }
+
+        public synchronized boolean isEmpty() {
+            return this.chunkTasks.isEmpty();
+        }
+
+        public synchronized CompletableFuture<Void> queueBlockChange(final BlockPos pos) {
+            final ChunkTasks tasks = this.chunkTasks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), ChunkTasks::new);
+            tasks.changedPositions.add(pos.immutable());
+            return tasks.onComplete;
+        }
+
+        public synchronized CompletableFuture<Void> queueSectionChange(final SectionPos pos, final boolean newEmptyValue) {
+            final ChunkTasks tasks = this.chunkTasks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), ChunkTasks::new);
+
+            if (tasks.changedSectionSet == null) {
+                tasks.changedSectionSet = new Boolean[this.manager.maxSection - this.manager.minSection + 1];
+            }
+            tasks.changedSectionSet[pos.getY() - this.manager.minSection] = Boolean.valueOf(newEmptyValue);
+
+            return tasks.onComplete;
+        }
+
+        public synchronized CompletableFuture<Void> queueChunkLighting(final ChunkPos pos, final Runnable lightTask) {
+            final ChunkTasks tasks = this.chunkTasks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), ChunkTasks::new);
+            if (tasks.lightTasks == null) {
+                tasks.lightTasks = new ArrayList<>();
+            }
+            tasks.lightTasks.add(lightTask);
+
+            return tasks.onComplete;
+        }
+
+        public synchronized CompletableFuture<Void> queueChunkSkylightEdgeCheck(final SectionPos pos, final ShortCollection sections) {
+            final ChunkTasks tasks = this.chunkTasks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), ChunkTasks::new);
+
+            ShortOpenHashSet queuedEdges = tasks.queuedEdgeChecksSky;
+            if (queuedEdges == null) {
+                queuedEdges = tasks.queuedEdgeChecksSky = new ShortOpenHashSet();
+            }
+            queuedEdges.addAll(sections);
 
-        // note: on the main thread, empty section changes are queued before block changes. This means we don't need
-        // to worry about cases where a block change is called inside an empty chunk section, according to the "emptiness" map per chunk,
-        // for example.
-        public final Set<BlockPos> changedPositions = new HashSet<>();
+            return tasks.onComplete;
+        }
+
+        public synchronized CompletableFuture<Void> queueChunkBlocklightEdgeCheck(final SectionPos pos, final ShortCollection sections) {
+            final ChunkTasks tasks = this.chunkTasks.computeIfAbsent(CoordinateUtils.getChunkKey(pos), ChunkTasks::new);
+
+            ShortOpenHashSet queuedEdges = tasks.queuedEdgeChecksBlock;
+            if (queuedEdges == null) {
+                queuedEdges = tasks.queuedEdgeChecksBlock = new ShortOpenHashSet();
+            }
+            queuedEdges.addAll(sections);
 
-        public Boolean[] changedSectionSet;
+            return tasks.onComplete;
+        }
+
+        public void removeChunk(final ChunkPos pos) {
+            final ChunkTasks tasks;
+            synchronized (this) {
+                tasks = this.chunkTasks.remove(CoordinateUtils.getChunkKey(pos));
+            }
+            if (tasks != null) {
+                tasks.onComplete.complete(null);
+            }
+        }
+
+        public synchronized ChunkTasks removeFirstTask() {
+            if (this.chunkTasks.isEmpty()) {
+                return null;
+            }
+            return this.chunkTasks.removeFirst();
+        }
+
+        protected static final class ChunkTasks {
+
+            public final Set<BlockPos> changedPositions = new HashSet<>();
+            public Boolean[] changedSectionSet;
+            public ShortOpenHashSet queuedEdgeChecksSky;
+            public ShortOpenHashSet queuedEdgeChecksBlock;
+            public List<Runnable> lightTasks;
+
+            public final CompletableFuture<Void> onComplete = new CompletableFuture<>();
+
+            public final long chunkCoordinate;
+
+            public ChunkTasks(final long chunkCoordinate) {
+                this.chunkCoordinate = chunkCoordinate;
+            }
+        }
     }
 }

src/main/java/ca/spottedleaf/starlight/common/light/StarLightLightingProvider.java

@@ -1,7 +1,21 @@
 package ca.spottedleaf.starlight.common.light;
 
+import net.minecraft.core.SectionPos;
+import net.minecraft.world.level.ChunkPos;
+import net.minecraft.world.level.LightLayer;
+import net.minecraft.world.level.chunk.DataLayer;
+import net.minecraft.world.level.chunk.LevelChunk;
+import org.jetbrains.annotations.Nullable;
+
 public interface StarLightLightingProvider {
 
     public StarLightInterface getLightEngine();
 
+    public void clientUpdateLight(final LightLayer lightType, final SectionPos pos,
+                            final @Nullable DataLayer nibble, final boolean trustEdges);
+
+    public void clientRemoveLightData(final ChunkPos chunkPos);
+
+    public void clientChunkLoad(final ChunkPos pos, final LevelChunk chunk);
+
 }