Fixes #4733: Fix a crash bug when an external machine tries draining fluids from a fluid interface without a filter. (#4738)

Also adds debug logging to the locatable registry to better debug this issues in the future.

src/main/java/appeng/core/AppEng.java

@@ -90,11 +90,11 @@ public final class AppEng {
         ModLoadingContext.get().registerConfig(ModConfig.Type.CLIENT, AEConfig.CLIENT_SPEC);
         ModLoadingContext.get().registerConfig(ModConfig.Type.COMMON, AEConfig.COMMON_SPEC);
 
-        proxy = DistExecutor.safeRunForDist(() -> ClientHelper::new, () -> ServerHelper::new);
-
         CreativeTab.init();
         new FacadeItemGroup(); // This call has a side-effect (adding it to the creative screen)
 
+        proxy = DistExecutor.safeRunForDist(() -> ClientHelper::new, () -> ServerHelper::new);
+
         IEventBus modEventBus = FMLJavaModLoadingContext.get().getModEventBus();
         registration = new Registration();
         modEventBus.addGenericListener(Block.class, registration::registerBlocks);

src/main/java/appeng/core/features/registries/LocatableRegistry.java

@@ -28,6 +28,7 @@ import appeng.api.events.LocatableEventAnnounce;
 import appeng.api.events.LocatableEventAnnounce.LocatableEvent;
 import appeng.api.features.ILocatable;
 import appeng.api.features.ILocatableRegistry;
+import appeng.core.AELog;
 import appeng.util.Platform;
 
 public final class LocatableRegistry implements ILocatableRegistry {
@@ -45,8 +46,10 @@ public final class LocatableRegistry implements ILocatableRegistry {
         }
 
         if (e.change == LocatableEvent.REGISTER) {
+            AELog.debug("Registering locatable %s: %s", e.target.getLocatableSerial(), e.target);
             this.set.put(e.target.getLocatableSerial(), e.target);
         } else if (e.change == LocatableEvent.UNREGISTER) {
+            AELog.debug("Unregistering locatable %s: %s", e.target.getLocatableSerial(), e.target);
             this.set.remove(e.target.getLocatableSerial());
         }
     }

src/main/java/appeng/fluids/parts/FluidImportBusPart.java

@@ -96,16 +96,15 @@ public class FluidImportBusPart extends SharedFluidBusPart {
         }
 
         final TileEntity te = this.getConnectedTE();
-        LazyOptional<IFluidHandler> fhOpt = LazyOptional.empty();
         if (te != null) {
-            te.getCapability(CapabilityFluidHandler.FLUID_HANDLER_CAPABILITY, this.getSide().getFacing().getOpposite());
-        }
-        if (fhOpt.isPresent()) {
-            try {
-                final IFluidHandler fh = fhOpt.orElse(null);
-                final IMEMonitor<IAEFluidStack> inv = this.getProxy().getStorage().getInventory(this.getChannel());
+            LazyOptional<IFluidHandler> fhOpt = te.getCapability(CapabilityFluidHandler.FLUID_HANDLER_CAPABILITY,
+                    this.getSide().getFacing().getOpposite());
+
+            if (fhOpt.isPresent()) {
+                try {
+                    final IFluidHandler fh = fhOpt.orElseThrow(() -> new IllegalStateException());
+                    final IMEMonitor<IAEFluidStack> inv = this.getProxy().getStorage().getInventory(this.getChannel());
 
-                if (fh != null) {
                     final FluidStack fluidStack = fh.drain(this.calculateAmountToSend(), FluidAction.SIMULATE);
 
                     if (this.filterEnabled() && !this.isInFilter(fluidStack)) {
@@ -128,12 +127,11 @@ public class FluidImportBusPart extends SharedFluidBusPart {
                     }
 
                     return TickRateModulation.IDLE;
+                } catch (GridAccessException e) {
+                    // skip
                 }
-            } catch (GridAccessException e) {
-                e.printStackTrace();
             }
         }
-
         return TickRateModulation.SLEEP;
     }
 

src/main/java/appeng/fluids/util/AEFluidInventory.java

@@ -13,6 +13,14 @@ import appeng.core.AELog;
 import appeng.util.Platform;
 
 public class AEFluidInventory implements IAEFluidTank {
+
+    /**
+     * While this may seem redundant, it helps since this class heavily mixes AE
+     * fluids stacks, which use null to represent "nothing", and Minecraft's
+     * FluidStack, which uses #isEmpty() to represent nothing.
+     */
+    private static final IAEFluidStack EMPTY_AE_FLUIDSTACK = null;
+
     private final IAEFluidStack[] fluids;
     private final IAEFluidInventory handler;
     private final int capacity;
@@ -31,13 +39,13 @@ public class AEFluidInventory implements IAEFluidTank {
     public void setFluidInSlot(final int slot, final IAEFluidStack fluid) {
         if (slot >= 0 && slot < this.getSlots()) {
             if (Objects.equals(this.fluids[slot], fluid)) {
-                if (fluid != null && fluid.getStackSize() != this.fluids[slot].getStackSize()) {
+                if (fluid != EMPTY_AE_FLUIDSTACK && fluid.getStackSize() != this.fluids[slot].getStackSize()) {
                     this.fluids[slot].setStackSize(Math.min(fluid.getStackSize(), this.capacity));
                     this.onContentChanged(slot);
                 }
             } else {
-                if (fluid == null) {
-                    this.fluids[slot] = null;
+                if (fluid == EMPTY_AE_FLUIDSTACK) {
+                    this.fluids[slot] = EMPTY_AE_FLUIDSTACK;
                 } else {
                     this.fluids[slot] = fluid.copy();
                     this.fluids[slot].setStackSize(Math.min(fluid.getStackSize(), this.capacity));
@@ -59,7 +67,7 @@ public class AEFluidInventory implements IAEFluidTank {
         if (slot >= 0 && slot < this.getSlots()) {
             return this.fluids[slot];
         }
-        return null;
+        return EMPTY_AE_FLUIDSTACK;
     }
 
     @Override
@@ -78,12 +86,12 @@ public class AEFluidInventory implements IAEFluidTank {
         if (tank < 0 || tank >= fluids.length) {
             return FluidStack.EMPTY;
         }
-        return fluids[tank] == null ? FluidStack.EMPTY : fluids[tank].getFluidStack();
+        return fluids[tank] == EMPTY_AE_FLUIDSTACK ? FluidStack.EMPTY : fluids[tank].getFluidStack();
     }
 
     @Override
     public int getTankCapacity(int tank) {
-        return Math.min(this.capacity, Integer.MAX_VALUE);
+        return this.capacity;
     }
 
     @Override
@@ -98,20 +106,20 @@ public class AEFluidInventory implements IAEFluidTank {
 
         final IAEFluidStack fluid = this.fluids[slot];
 
-        if (fluid != null && !fluid.getFluidStack().equals(resource)) {
+        if (fluid != EMPTY_AE_FLUIDSTACK && !fluid.getFluidStack().equals(resource)) {
             return 0;
         }
 
         int amountToStore = this.capacity;
 
-        if (fluid != null) {
+        if (fluid != EMPTY_AE_FLUIDSTACK) {
             amountToStore -= fluid.getStackSize();
         }
 
         amountToStore = Math.min(amountToStore, resource.getAmount());
 
         if (doFill) {
-            if (fluid == null) {
+            if (fluid == EMPTY_AE_FLUIDSTACK) {
                 this.setFluidInSlot(slot, AEFluidStack.fromFluidStack(resource));
             } else {
                 fluid.setStackSize(fluid.getStackSize() + amountToStore);
@@ -141,14 +149,14 @@ public class AEFluidInventory implements IAEFluidTank {
 
         int amountToStore = this.capacity;
 
-        if (fluid != null) {
+        if (fluid != EMPTY_AE_FLUIDSTACK) {
             amountToStore -= fluid.getStackSize();
         }
 
         amountToStore = Math.min(amountToStore, resource.getAmount());
 
         if (action == FluidAction.EXECUTE) {
-            if (fluid == null) {
+            if (fluid == EMPTY_AE_FLUIDSTACK) {
                 this.setFluidInSlot(slot, AEFluidStack.fromFluidStack(resource));
             } else {
                 fluid.setStackSize(fluid.getStackSize() + amountToStore);
@@ -170,7 +178,7 @@ public class AEFluidInventory implements IAEFluidTank {
         FluidStack totalDrained = FluidStack.EMPTY;
         for (int slot = 0; slot < this.getSlots(); ++slot) {
             FluidStack drain = this.drain(slot, resource, action == FluidAction.EXECUTE);
-            if (drain != null) {
+            if (!drain.isEmpty()) {
                 if (totalDrained.isEmpty()) {
                     totalDrained = drain;
                 } else {
@@ -205,7 +213,7 @@ public class AEFluidInventory implements IAEFluidTank {
                 FluidStack copy = totalDrained.copy();
                 copy.setAmount(toDrain);
                 FluidStack drain = this.drain(slot, copy, action == FluidAction.EXECUTE);
-                if (drain != null) {
+                if (!drain.isEmpty()) {
                     totalDrained.setAmount(totalDrained.getAmount() + drain.getAmount());
                     toDrain -= drain.getAmount();
                 }
@@ -220,7 +228,7 @@ public class AEFluidInventory implements IAEFluidTank {
 
     private int indexOfFluid(FluidStack resource) {
         for (int slot = 0; slot < fluids.length; slot++) {
-            if (fluids[slot] != null && fluids[slot].getFluidStack().isFluidEqual(resource)) {
+            if (fluids[slot] != EMPTY_AE_FLUIDSTACK && fluids[slot].getFluidStack().isFluidEqual(resource)) {
                 return slot;
             }
         }
@@ -229,7 +237,7 @@ public class AEFluidInventory implements IAEFluidTank {
 
     private int indexOfEmptySlot() {
         for (int slot = 0; slot < fluids.length; slot++) {
-            if (fluids[slot] == null) {
+            if (fluids[slot] == EMPTY_AE_FLUIDSTACK) {
                 return slot;
             }
         }
@@ -238,16 +246,16 @@ public class AEFluidInventory implements IAEFluidTank {
 
     public FluidStack drain(final int slot, final FluidStack resource, final boolean doDrain) {
         final IAEFluidStack fluid = this.fluids[slot];
-        if (resource.isEmpty() || fluid == null || !fluid.getFluidStack().equals(resource)) {
-            return null;
+        if (resource.isEmpty() || fluid == EMPTY_AE_FLUIDSTACK || !fluid.getFluidStack().equals(resource)) {
+            return FluidStack.EMPTY;
         }
         return this.drain(slot, resource.getAmount(), doDrain);
     }
 
     public FluidStack drain(final int slot, final int maxDrain, boolean doDrain) {
         final IAEFluidStack fluid = this.fluids[slot];
-        if (fluid == null || maxDrain <= 0) {
-            return null;
+        if (fluid == EMPTY_AE_FLUIDSTACK || maxDrain <= 0) {
+            return FluidStack.EMPTY;
         }
 
         int drained = maxDrain;
@@ -259,7 +267,7 @@ public class AEFluidInventory implements IAEFluidTank {
         if (doDrain) {
             fluid.setStackSize(fluid.getStackSize() - drained);
             if (fluid.getStackSize() <= 0) {
-                this.fluids[slot] = null;
+                this.fluids[slot] = EMPTY_AE_FLUIDSTACK;
             }
             this.onContentChanged(slot);
         }
@@ -277,7 +285,7 @@ public class AEFluidInventory implements IAEFluidTank {
             try {
                 final CompoundNBT c = new CompoundNBT();
 
-                if (this.fluids[x] != null) {
+                if (this.fluids[x] != EMPTY_AE_FLUIDSTACK) {
                     this.fluids[x].writeToNBT(c);
                 }
 

src/main/java/appeng/me/cluster/implementations/QuantumCluster.java

@@ -22,7 +22,6 @@ import java.util.Iterator;
 
 import net.minecraft.tileentity.TileEntity;
 import net.minecraft.util.math.BlockPos;
-import net.minecraft.util.math.ChunkPos;
 import net.minecraft.world.World;
 import net.minecraft.world.dimension.DimensionType;
 import net.minecraftforge.common.MinecraftForge;
@@ -160,13 +159,17 @@ public class QuantumCluster implements ILocatable, IAECluster {
         if (qc != null) {
             final World theWorld = qc.center.getWorld();
             if (!qc.isDestroyed) {
-                ChunkPos cPos = new ChunkPos(qc.center.getPos());
-                if (theWorld.getChunkProvider().isChunkLoaded(cPos)) {
+                // In future versions, we might actually want to delay the entire registration
+                // until the center
+                // tile begins ticking normally.
+                if (theWorld.isBlockLoaded(qc.center.getPos())) {
                     final DimensionType id = theWorld.dimension.getType();
                     final World cur = theWorld.getServer().getWorld(id);
 
                     final TileEntity te = theWorld.getTileEntity(qc.center.getPos());
                     return te != qc.center || theWorld != cur;
+                } else {
+                    AELog.warn("Found a registered QNB with serial %s whose chunk seems to be unloaded: %s", qe, qc);
                 }
             }
         }
@@ -277,4 +280,17 @@ public class QuantumCluster implements ILocatable, IAECluster {
     private void setRing(final QuantumBridgeTileEntity[] ring) {
         this.Ring = ring;
     }
+
+    @Override
+    public String toString() {
+        if (center == null) {
+            return "QuantumCluster{no-center}";
+        }
+
+        World world = center.getWorld();
+        BlockPos pos = center.getPos();
+
+        return "QuantumCluster{" + world + "," + pos + "}";
+    }
+
 }

src/main/java/appeng/tile/AEBaseTileEntity.java

@@ -253,9 +253,11 @@ public class AEBaseTileEntity extends TileEntity implements IOrientable, ICommon
         if (this.renderFragment > 0) {
             this.renderFragment |= 1;
         } else {
+            // Clearing the cached model-data is always harmless regardless of status
+            this.requestModelDataUpdate();
+
             // TODO: Optimize Network Load
-            if (this.world != null) {
-                this.requestModelDataUpdate();
+            if (this.world != null && !this.isRemoved() && !notLoaded()) {
 
                 boolean alreadyUpdated = false;
                 // Let the block update it's own state with our internal state changes
@@ -367,7 +369,6 @@ public class AEBaseTileEntity extends TileEntity implements IOrientable, ICommon
      * null means nothing to store...
      *
      * @param from source of settings
-     *
      * @return compound of source
      */
     public CompoundNBT downloadSettings(final SettingsFrom from) {