address PR review comments

Author: CursedFlames

src/main/java/io/github/opencubicchunks/cubicchunks/mixin/core/client/renderer/MixinViewArea.java

@@ -36,7 +36,6 @@ private void cc_onSetViewDistance(int renderDistanceChunks, CallbackInfo ci) {
         if (!((CanBeCubic) level).cc_isCubic()) return;
         ci.cancel();
         int i = renderDistanceChunks * 2 + 1;
-        i *= CubicConstants.DIAMETER_IN_SECTIONS; // TODO is this correct?
         this.sectionGridSizeX = i;
         this.sectionGridSizeY = i;
         this.sectionGridSizeZ = i;

src/main/java/io/github/opencubicchunks/cubicchunks/mixin/core/client/renderer/chunk/MixinSectionRenderDispatcher$RenderSection.java

@@ -40,6 +40,7 @@ private void cc_onDoesChunkExistAt(long sectionPosLong, CallbackInfoReturnable<B
         if (!((CanBeCubic) ((SectionRenderDispatcherAccess) this$0).cc_getLevel()).cc_isCubic()) {
             return;
         }
+        // TODO (P2) lighting: also check the cubic equivalent of LevelLightEngine.lightOnInColumn here (see vanilla doesChunkExistAt method) - sections currently sometimes fail to render due to this missing check
         cir.setReturnValue(((CubicLevel) ((SectionRenderDispatcherAccess) this$0).cc_getLevel()).cc_getCube(Coords.sectionToCube(SectionPos.x(sectionPosLong)), Coords.sectionToCube(SectionPos.y(sectionPosLong)), Coords.sectionToCube(SectionPos.z(sectionPosLong)), ChunkStatus.FULL, false) != null);
     }
 
@@ -54,7 +55,6 @@ private void cc_onHasAllNeighbors(CallbackInfoReturnable<Boolean> cir) {
             for (int dy = -1; dy <= 1; dy++) {
                 for (int dz = -1; dz <= 1; dz++) {
                     if (dx == 0 && dy == 0 && dz == 0) continue;
-                    // TODO this checks neighboring sections rather than neighboring cubes; not sure if we should be checking neighboring cubes instead?
                     if (!this.doesChunkExistAt(SectionPos.offset(this.sectionNode, dx, dy, dz))) {
                         cir.setReturnValue(false);
                         return;

src/main/java/io/github/opencubicchunks/cubicchunks/mixin/core/common/server/level/MixinChunkGenerationTask.java

@@ -275,8 +275,9 @@ private void cc_onScheduleLayer(ChunkStatus status, boolean needsGeneration, Cal
 
     @Inject(method = "scheduleChunkInLayer", at = @At("HEAD"), cancellable = true)
     private void cc_onScheduleChunkInLayer(ChunkStatus status, boolean needsGeneration, GenerationChunkHolder chunk, CallbackInfoReturnable<Boolean> cir) {
-        var cloPos = ((GenerationCloHolder) chunk).cc_getCloPos();
-        if (cloPos != null && cloPos.isCube()) cir.setReturnValue(cc_scheduleCubeInLayer(status, needsGeneration, chunk));
+        if (((GenerationCloHolder) chunk).cc_getCubePos() != null) {
+            cir.setReturnValue(cc_scheduleCubeInLayer(status, needsGeneration, chunk));
+        }
     }
 
     @TransformFromMethod(useRedirectSets = ChunkToCubeSet.class, owner = @Ref(ChunkGenerationTask.class), value = @MethodSig("scheduleChunkInLayer(Lnet/minecraft/world/level/chunk/status/ChunkStatus;ZLnet/minecraft/server/level/GenerationChunkHolder;)Z"))

src/main/java/io/github/opencubicchunks/cubicchunks/mixin/core/common/server/level/MixinChunkHolder.java

@@ -27,23 +27,24 @@
 import io.github.opencubicchunks.cubicchunks.world.level.chunklike.LevelClo;
 import io.github.opencubicchunks.cubicchunks.world.level.cube.CubeAccess;
 import io.github.opencubicchunks.cubicchunks.world.level.cube.LevelCube;
-import it.unimi.dsi.fastutil.shorts.ShortOpenHashSet;
 import it.unimi.dsi.fastutil.shorts.ShortSet;
 import net.minecraft.core.BlockPos;
-import net.minecraft.core.SectionPos;
 import net.minecraft.server.level.ChunkHolder;
 import net.minecraft.server.level.ChunkMap;
 import net.minecraft.server.level.FullChunkStatus;
 import net.minecraft.server.level.GenerationChunkHolder;
 import net.minecraft.world.level.ChunkPos;
+import net.minecraft.world.level.LevelHeightAccessor;
 import net.minecraft.world.level.LightLayer;
 import net.minecraft.world.level.chunk.LevelChunk;
 import net.minecraft.world.level.chunk.status.ChunkStatus;
+import org.spongepowered.asm.mixin.Dynamic;
 import org.spongepowered.asm.mixin.Final;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.Redirect;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 /**
@@ -93,27 +94,23 @@ public MixinChunkHolder() {
         return (LevelClo) getChunkToSend();
     }
 
-    // TODO dasm + mixin?
     @Inject(method = "blockChanged", at = @At("HEAD"), cancellable = true)
     public void cc_onBlockChanged(BlockPos pos, CallbackInfoReturnable<Boolean> cir) {
         if (cc_cubePos != null) {
-            LevelCube levelCube = this.cc_getTickingCube();
-            if (levelCube == null) {
-                cir.setReturnValue(false);
-            } else {
-                boolean alreadyHadChangedSections = this.hasChangedSections;
-                int sectionIndex = Coords.sectionToIndex(Coords.blockToSection(pos.getX()), Coords.blockToSection(pos.getY()), Coords.blockToSection(pos.getZ()));
-                if (this.changedBlocksPerSection[sectionIndex] == null) {
-                    this.hasChangedSections = true;
-                    this.changedBlocksPerSection[sectionIndex] = new ShortOpenHashSet();
-                }
-
-                this.changedBlocksPerSection[sectionIndex].add(SectionPos.sectionRelativePos(pos));
-                cir.setReturnValue(!alreadyHadChangedSections);
-            }
+            cir.setReturnValue(cc_blockChanged(pos));
         }
     }
 
+    // region [cc_blockChanged dasm + mixin]
+    @TransformFromMethod(@MethodSig("blockChanged(Lnet/minecraft/core/BlockPos;)Z"))
+    private native boolean cc_blockChanged(BlockPos pos);
+
+    @Dynamic @Redirect(method = "cc_dasm$cc_blockChanged", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/LevelHeightAccessor;getSectionIndex(I)I"))
+    private int cc_onBlockChanged_sectionIndex(LevelHeightAccessor instance, int y, BlockPos pos) {
+        return Coords.sectionToIndex(Coords.blockToSection(pos.getX()), Coords.blockToSection(pos.getY()), Coords.blockToSection(pos.getZ()));
+    }
+    // endregion
+
     @Inject(method = "sectionLightChanged", at = @At("HEAD"), cancellable = true)
     public void cc_onSectionLightChanged(LightLayer lightLayer, int sectionY, CallbackInfoReturnable<Boolean> cir) {
         if (cc_cubePos != null) {

src/main/java/io/github/opencubicchunks/cubicchunks/mixin/core/common/server/level/MixinChunkMap.java

@@ -102,20 +102,17 @@ public abstract class MixinChunkMap extends MixinChunkStorage implements Generat
     @Shadow @Final ServerLevel level;
     @Shadow @Final private ChunkMap.DistanceManager distanceManager;
 
-    private static CompletableFuture<ChunkResult<CloAccess>> UNLOADED_CHUNK_FUTURE = CompletableFuture.completedFuture(
-        ChunkResult.error("Unloaded chunk")
-    );
-
     @Shadow @Final private static CompletableFuture<ChunkResult<List<CloAccess>>> UNLOADED_CHUNK_LIST_FUTURE;
     @Shadow @Final private static ChunkResult<List<CloAccess>> UNLOADED_CHUNK_LIST_RESULT;
 
     @Shadow private static double euclideanDistanceSquared(ChunkPos chunkPos, Vec3 pos) {
         throw new IllegalStateException();
     }
 
+    // TODO this one being on GlobalSet is a bit jank
     @AddFieldToSets(sets = GlobalSet.class, owner = @Ref(ChunkMap.class), field = @FieldSig(type = @Ref(ChunkProgressListener.class), name = "progressListener"))
     private CloProgressListener cc_progressListener;
-    @AddFieldToSets(sets = GlobalSet.class, owner = @Ref(ChunkMap.class), field = @FieldSig(type = @Ref(ChunkStatusUpdateListener.class), name = "chunkStatusListener"))
+    @AddFieldToSets(sets = ChunkToCloSet.class, owner = @Ref(ChunkMap.class), field = @FieldSig(type = @Ref(ChunkStatusUpdateListener.class), name = "chunkStatusListener"))
     private CloStatusUpdateListener cc_cloStatusListener;
 
     // TODO once we can target non-return locations in constructors, do this when the vanilla field is set
@@ -259,12 +256,12 @@ private void cc_onGetChunkRangeFuture(ChunkHolder cloHolder, int radius, IntFunc
 
     //region [cc_updateCubeScheduling dasm + mixin]
     @AddTransformToSets(ChunkToCubeSet.class) @TransformFromMethod(useRedirectSets = ChunkToCubeSet.class, value = @MethodSig("updateChunkScheduling(JILnet/minecraft/server/level/ChunkHolder;I)Lnet/minecraft/server/level/ChunkHolder;"))
-    @Nullable public native ChunkHolder cc_updateCubeScheduling(long chunkPos, int newLevel, @Nullable ChunkHolder holder, int oldLevel);
+    @Nullable public native ChunkHolder cc_updateCubeScheduling(long cubePos, int newLevel, @Nullable ChunkHolder holder, int oldLevel);
 
     @Inject(method = "updateChunkScheduling", at = @At("HEAD"), cancellable = true)
-    private void cc_onUpdateChunkScheduling(long chunkPos, int newLevel, ChunkHolder holder, int oldLevel, CallbackInfoReturnable<ChunkHolder> cir) {
-        if (((CanBeCubic) level).cc_isCubic() && CloPos.isCube(chunkPos)) {
-            cir.setReturnValue(cc_updateCubeScheduling(chunkPos, newLevel, holder, oldLevel));
+    private void cc_onUpdateChunkScheduling(long cloPos, int newLevel, ChunkHolder holder, int oldLevel, CallbackInfoReturnable<ChunkHolder> cir) {
+        if (((CanBeCubic) level).cc_isCubic() && CloPos.isCube(cloPos)) {
+            cir.setReturnValue(cc_updateCubeScheduling(cloPos, newLevel, holder, oldLevel));
         }
     }
     //endregion

src/main/java/io/github/opencubicchunks/cubicchunks/server/level/GenerationCloHolder.java

@@ -1,10 +1,15 @@
 package io.github.opencubicchunks.cubicchunks.server.level;
 
+import javax.annotation.Nullable;
+
+import io.github.opencubicchunks.cc_core.api.CubePos;
 import io.github.opencubicchunks.cc_core.world.level.CloPos;
 import io.github.opencubicchunks.cubicchunks.world.level.cube.ImposterProtoCube;
 
 public interface GenerationCloHolder {
     CloPos cc_getCloPos();
 
+    @Nullable CubePos cc_getCubePos();
+
     void cc_replaceProtoCube(ImposterProtoCube cube);
 }

src/main/java/io/github/opencubicchunks/cubicchunks/world/level/cube/LevelCube.java

@@ -185,7 +185,7 @@ public void setUnsavedListener(LevelCube.UnsavedListener unsavedListener) {
     @Override public native TickContainerAccess<Fluid> getFluidTicks();
 
     @TransformFromMethod(value = @MethodSig("getTicksForSerialization(J)Lnet/minecraft/world/level/chunk/ChunkAccess$PackedTicks;"), owner = @Ref(LevelChunk.class))
-    @Override public native ChunkAccess.PackedTicks getTicksForSerialization(long todoNameThis);
+    @Override public native ChunkAccess.PackedTicks getTicksForSerialization(long gameTime);
 
     // TODO should this actually be dasm'd?
     @TransformFromMethod(value = @MethodSig("getListenerRegistry(I)Lnet/minecraft/world/level/gameevent/GameEventListenerRegistry;"), owner = @Ref(LevelChunk.class))