Fix #6990: Crash when moving controller with SCS (#7067)

- The root problem was that the controllers block entities are removed by
Spatial IO before the blocks are, so we must ensure that the controller
BE never gets queried by neighbors, otherwise a new "ghost" BE appears
which causes the crash.
- Also consolidated some ticking checks to make them all point to a
Platform method.

src/main/java/appeng/block/networking/ControllerBlock.java

@@ -39,6 +39,7 @@
 
 import appeng.block.AEBaseEntityBlock;
 import appeng.blockentity.networking.ControllerBlockEntity;
+import appeng.core.definitions.AEBlocks;
 import appeng.menu.MenuOpener;
 import appeng.menu.locator.MenuLocators;
 import appeng.menu.me.networktool.NetworkStatusMenu;
@@ -116,12 +117,9 @@ private BlockState getControllerType(BlockState baseState, LevelAccessor level,
         int z = pos.getZ();
 
         // Detect whether controllers are on both sides of the x, y, and z axes
-        final boolean xx = this.getBlockEntity(level, x - 1, y, z) != null
-                && this.getBlockEntity(level, x + 1, y, z) != null;
-        final boolean yy = this.getBlockEntity(level, x, y - 1, z) != null
-                && this.getBlockEntity(level, x, y + 1, z) != null;
-        final boolean zz = this.getBlockEntity(level, x, y, z - 1) != null
-                && this.getBlockEntity(level, x, y, z + 1) != null;
+        final boolean xx = isController(level, x - 1, y, z) && isController(level, x + 1, y, z);
+        final boolean yy = isController(level, x, y - 1, z) && isController(level, x, y + 1, z);
+        final boolean zz = isController(level, x, y, z - 1) && isController(level, x, y, z + 1);
 
         if (xx && !yy && !zz) {
             type = ControllerRenderType.column_x;
@@ -144,6 +142,13 @@ private BlockState getControllerType(BlockState baseState, LevelAccessor level,
         return baseState.setValue(CONTROLLER_TYPE, type);
     }
 
+    private static boolean isController(LevelAccessor level, int x, int y, int z) {
+        // Do NOT query block entity:
+        // - in Spatial IO movement, block entity might have been removed but block might still be there
+        // - if we call getBlockEntity a new block entity will be loaded even though it has already been removed (bad!)
+        return level.getBlockState(new BlockPos(x, y, z)).is(AEBlocks.CONTROLLER.block());
+    }
+
     @Override
     public InteractionResult onActivated(Level level, BlockPos pos, Player player, InteractionHand hand,
             @org.jetbrains.annotations.Nullable ItemStack heldItem, BlockHitResult hit) {
@@ -156,13 +161,4 @@ public InteractionResult onActivated(Level level, BlockPos pos, Player player, I
         }
         return InteractionResult.FAIL;
     }
-
-    @Override
-    public void neighborChanged(BlockState state, Level level, BlockPos pos, Block blockIn, BlockPos fromPos,
-            boolean isMoving) {
-        final ControllerBlockEntity tc = this.getBlockEntity(level, pos);
-        if (tc != null) {
-            tc.updateState();
-        }
-    }
 }

src/main/java/appeng/hooks/ticking/TickHandler.java

@@ -348,7 +348,7 @@ private void readyBlockEntities(ServerLevel level) {
         for (long packedChunkPos : workSet) {
             // Readies all of our block entities in this chunk as soon as it can tick BEs
             // The following test is equivalent to ServerLevel#isPositionTickingWithEntitiesLoaded
-            if (level.shouldTickBlocksAt(packedChunkPos)) {
+            if (Platform.areBlockEntitiesTicking(level, packedChunkPos)) {
                 // Take the currently waiting block entities for this chunk and ready them all. Should more block
                 // entities be added to this chunk while we're working on it, a new list will be added automatically and
                 // we'll work on this chunk again next tick.

src/main/java/appeng/server/testplots/SpatialTestPlots.java

@@ -0,0 +1,52 @@
+package appeng.server.testplots;
+
+import net.minecraft.core.BlockPos;
+import net.minecraft.core.Direction;
+
+import appeng.core.definitions.AEBlocks;
+import appeng.core.definitions.AEItems;
+import appeng.server.testworld.PlotBuilder;
+
+public final class SpatialTestPlots {
+    private SpatialTestPlots() {
+    }
+
+    /**
+     * Validates that controllers inside of SCSs do not cause crashes. Regression test for
+     * <a href="https://github.com/AppliedEnergistics/Applied-Energistics-2/issues/6990">issue 6990</a>.
+     */
+    @TestPlot("controller_inside_scs")
+    public static void controllerInsideScs(PlotBuilder plot) {
+        // Outer network
+        plot.creativeEnergyCell("0 0 0");
+        plot.block("[1,10] 0 0", AEBlocks.SPATIAL_PYLON);
+        plot.block("0 [1,10] 0", AEBlocks.SPATIAL_PYLON);
+        plot.block("0 0 [1,10]", AEBlocks.SPATIAL_PYLON);
+        plot.blockEntity("-1 0 0", AEBlocks.SPATIAL_IO_PORT, port -> {
+            port.getInternalInventory().insertItem(0, AEItems.SPATIAL_CELL128.stack(), false);
+        });
+        var leverPos = plot.leverOn(new BlockPos(-1, 0, 0), Direction.WEST);
+
+        // Inner network
+        plot.creativeEnergyCell("[4,6] 3 [4,6]");
+        for (int x = -1; x <= 1; ++x) {
+            for (int y = -1; y <= 1; ++y) {
+                for (int z = -1; z <= 1; ++z) {
+                    boolean edge = Math.abs(x) + Math.abs(y) + Math.abs(z) >= 2;
+                    if (edge) {
+                        plot.block(new BlockPos(5 + x, 5 + y, 5 + z), AEBlocks.CONTROLLER);
+                    }
+                }
+            }
+        }
+
+        // Woosh!
+        plot.test(helper -> {
+            helper.startSequence()
+                    .thenIdle(5)
+                    .thenExecute(() -> helper.pullLever(leverPos))
+                    .thenIdle(5)
+                    .thenSucceed();
+        });
+    }
+}

src/main/java/appeng/server/testplots/TestPlots.java

@@ -86,7 +86,8 @@ public final class TestPlots {
                 TestPlots.class,
                 AutoCraftingTestPlots.class,
                 P2PTestPlots.class,
-                PatternProviderLockModePlots.class));
+                PatternProviderLockModePlots.class,
+                SpatialTestPlots.class));
     }
 
     private TestPlots() {

src/main/java/appeng/util/Platform.java

@@ -527,24 +527,24 @@ public static boolean isSortOrderAvailable(SortOrder order) {
      */
     @Nullable
     public static BlockEntity getTickingBlockEntity(@Nullable Level level, BlockPos pos) {
-        if (!(level instanceof ServerLevel serverLevel)) {
-            return null;
-        }
-
-        // Note: chunk could be in ticking range but not loaded, this checks for both!
-        if (!serverLevel.getChunkSource().isPositionTicking(ChunkPos.asLong(pos))) {
+        if (!areBlockEntitiesTicking(level, pos)) {
             return null;
         }
 
-        return serverLevel.getBlockEntity(pos);
+        return level.getBlockEntity(pos);
     }
 
     /**
      * Checks that the chunk at the given position in the given level is in a state where block entities would tick.
-     * Vanilla does this check in {@link Level#tickBlockEntities}
+     * This means that it must both be fully loaded, and close enough to a ticking ticket.
      */
     public static boolean areBlockEntitiesTicking(@Nullable Level level, BlockPos pos) {
-        return level instanceof ServerLevel serverLevel && serverLevel.shouldTickBlocksAt(ChunkPos.asLong(pos));
+        return areBlockEntitiesTicking(level, ChunkPos.asLong(pos));
+    }
+
+    public static boolean areBlockEntitiesTicking(@Nullable Level level, long chunkPos) {
+        // isPositionTicking checks both that the chunk is loaded, and that it's in ticking range...
+        return level instanceof ServerLevel serverLevel && serverLevel.getChunkSource().isPositionTicking(chunkPos);
     }
 
     public static Transaction openOrJoinTx() {