perf: grounded singularity builds its pull box and queries entities once per tick

Closes TDLM/thaumicadditions#21

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

dependencies.gradle

@@ -4,6 +4,7 @@ repositories {
 
 dependencies {
     // Thaumcraft 4 full dev jar (api + common impl; the sources import thaumcraft.common.* and thaumcraft.client.*)
+    testImplementation("junit:junit:4.13.2")
     api("uk.potatolab:Thaumcraft4:4.2.3.5-20261028.155248-g1a600e9")
     // cofh.api.energy.IEnergyReceiver is used by TileThaumicGenerator (the CoFH API lives in CoFHLib, not CoFHCore)
     compileOnly("uk.potatolab:CoFHLib-1.7.10-Legacy:1.7.10R3.0.3-20261028.145717-gbf0ac89")

src/main/java/com/pengu/thaumcraft/additions/entity/EntitySingularity.java

@@ -67,10 +67,18 @@ public class EntitySingularity extends EntityAlumentum {
             List l;
             List l2;
             int i;
+            // position and radius do not change inside the loop, so the box is built once and the entity lists are
+            // only queried again after a round that could have added or removed entities (alumentum, block breaking)
+            AxisAlignedBB ent = AxisAlignedBB.getBoundingBox(this.posX - (double)this.radius, this.posY - (double)this.radius, this.posZ - (double)this.radius, this.posX + (double)this.radius, this.posY + (double)this.radius, this.posZ + (double)this.radius);
+            l = this.worldObj.getEntitiesWithinAABB(Entity.class, ent);
+            l2 = this.worldObj.getEntitiesWithinAABB(EntityLivingBase.class, ent);
+            boolean entitiesChanged = false;
             for(p = 0; p < 128; ++p) {
-               AxisAlignedBB ent = AxisAlignedBB.getBoundingBox(this.posX - (double)this.radius, this.posY - (double)this.radius, this.posZ - (double)this.radius, this.posX + (double)this.radius, this.posY + (double)this.radius, this.posZ + (double)this.radius);
-               l = this.worldObj.getEntitiesWithinAABB(Entity.class, ent);
-               l2 = this.worldObj.getEntitiesWithinAABB(EntityLivingBase.class, ent);
+               if(entitiesChanged) {
+                  l = this.worldObj.getEntitiesWithinAABB(Entity.class, ent);
+                  l2 = this.worldObj.getEntitiesWithinAABB(EntityLivingBase.class, ent);
+                  entitiesChanged = false;
+               }
 
                double ry;
                double ey;
@@ -103,6 +111,7 @@ public class EntitySingularity extends EntityAlumentum {
                      var24.posZ = ez;
                      var24.inGround = true;
                      this.worldObj.spawnEntityInWorld(var24);
+                     entitiesChanged = true;
                   }
                }
 
@@ -122,23 +131,6 @@ public class EntitySingularity extends EntityAlumentum {
                }
 
                int var22;
-               for(i = 0; i < 16; ++i) {
-                  for(var22 = 0; var22 < l2.size(); ++var22) {
-                     double rz = ((EntityLivingBase)l2.get(var22)).posX;
-                     double ey1 = ((EntityLivingBase)l2.get(var22)).posY;
-                     double ez1 = ((EntityLivingBase)l2.get(var22)).posZ;
-                     double motionX1 = ((EntityLivingBase)l2.get(var22)).motionX;
-                     double motionY1 = ((EntityLivingBase)l2.get(var22)).motionY;
-                     double motionZ1 = ((EntityLivingBase)l2.get(var22)).motionZ;
-                     motionX1 = this.posX - rz;
-                     motionY1 = this.posY - ey1 + (double)(this.ticksExisted < 400?(float)this.ticksExisted / 400.0F * 4.0F:4.0F);
-                     motionZ1 = this.posZ - ez1;
-                     ((EntityLivingBase)l2.get(var22)).motionX = motionX1 / 10.0D;
-                     ((EntityLivingBase)l2.get(var22)).motionY = motionY1 / 10.0D;
-                     ((EntityLivingBase)l2.get(var22)).motionZ = motionZ1 / 10.0D;
-                  }
-               }
-
                i = (int)this.posX;
                var22 = (int)this.posY;
                int var23 = (int)this.posZ;
@@ -153,6 +145,7 @@ public class EntitySingularity extends EntityAlumentum {
                   }
 
                   this.worldObj.setBlockToAir(i, var22, var23);
+                  entitiesChanged = true;
                }
             }
 

src/main/java/com/pengu/thaumcraft/additions/piping/old/tile/TileOldReinforcedTank.java

@@ -14,20 +14,11 @@ public class TileOldReinforcedTank extends TileTankBasic {
    }
 
    public void updateEntity() {
-      if(!(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic) && !(this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 0) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 0);
-      }
-
-      if(!(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 1) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 1);
-      }
-
-      if(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic && !(this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 2) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 2);
-      }
-
-      if(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 3) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 3);
+      boolean above = this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic;
+      boolean below = this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic;
+      int wantedMeta = TileTankBasic.connectionMeta(above, below);
+      if(this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != wantedMeta) {
+         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, wantedMeta);
       }
 
       boolean ext = this.worldObj.getTileEntity(this.xCoord, this.yCoord + 1, this.zCoord) instanceof TileOldReinforcedTank && this.vis + this.taint < this.capacity;

src/main/java/com/pengu/thaumcraft/additions/tileentity/TileTaintedCrops.java

@@ -2,11 +2,8 @@ package com.pengu.thaumcraft.additions.tileentity;
 
 import com.pengu.thaumcraft.additions.init.All;
 import com.pengu.thaumcraft.additions.utils.Util;
-import java.lang.reflect.Method;
 import net.minecraft.init.Blocks;
-import net.minecraft.item.ItemStack;
 import net.minecraft.nbt.NBTTagCompound;
-import net.minecraft.world.World;
 import net.minecraftforge.common.IPlantable;
 import net.minecraftforge.common.util.ForgeDirection;
 import thaumcraft.api.TileThaumcraft;
@@ -17,24 +14,29 @@ public class TileTaintedCrops extends TileThaumcraft {
    public int tick = 0;
 
 
+   /** Ticks of growth needed per stage; the taint biome grows faster. */
+   public static int growthThreshold(boolean taintBiome) {
+      return 4000 - (taintBiome?450:0);
+   }
+
+   /** Growth only counts while the crop is not yet fully grown. */
+   public static boolean countsGrowth(int meta) {
+      return meta < 4;
+   }
+
    public void updateEntity() {
-      if(!this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord).canSustainPlant(this.worldObj, this.xCoord, this.yCoord - 1, this.zCoord, ForgeDirection.UP, (IPlantable)All.B_taint_crops) && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) != Blocks.grass && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) != Blocks.dirt) {
+      net.minecraft.block.Block soil = this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord);
+      if(!soil.canSustainPlant(this.worldObj, this.xCoord, this.yCoord - 1, this.zCoord, ForgeDirection.UP, (IPlantable)All.B_taint_crops) && soil != Blocks.grass && soil != Blocks.dirt) {
+         // the crop block is gone after this, so there is nothing left to drop
          this.worldObj.setBlockToAir(this.xCoord, this.yCoord, this.zCoord);
-
-         try {
-            Method m = this.worldObj.getBlock(this.xCoord, this.yCoord, this.zCoord).getClass().getDeclaredMethod("dropBlockAsItem", new Class[]{World.class, Integer.class, Integer.class, Integer.class, ItemStack.class});
-            m.setAccessible(true);
-            m.invoke(this.worldObj.getBlock(this.xCoord, this.yCoord, this.zCoord), new Object[]{this.worldObj, Integer.valueOf(this.xCoord), Integer.valueOf(this.yCoord), Integer.valueOf(this.zCoord), null});
-         } catch (Throwable var2) {
-            ;
-         }
-
       } else {
-         if(this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) < 4) {
+         int meta = this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord);
+         if(countsGrowth(meta)) {
             ++this.tick;
          }
 
-         if(this.tick >= 4000 - (this.worldObj.getBiomeGenForCoords(this.xCoord, this.zCoord).biomeID == Config.biomeTaintID?450:0)) {
+         // the lowest possible threshold is 3550, so the biome is only looked up once it matters
+         if(this.tick >= growthThreshold(true) && this.tick >= growthThreshold(this.worldObj.getBiomeGenForCoords(this.xCoord, this.zCoord).biomeID == Config.biomeTaintID)) {
             Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) + 1);
          }
 

src/main/java/com/pengu/thaumcraft/additions/tileentity/TileTankBasic.java

@@ -23,21 +23,17 @@ public class TileTankBasic extends TileThaumcraft implements ITubable {
       this.capacity = 1005;
    }
 
+   /** Block metadata for a tank by which neighbours are tanks: 0 none, 1 only below, 2 only above, 3 both. */
+   public static int connectionMeta(boolean above, boolean below) {
+      return (above?2:0) | (below?1:0);
+   }
+
    public void updateEntity() {
-      if(!(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic) && !(this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 0) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 0);
-      }
-
-      if(!(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 1) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 1);
-      }
-
-      if(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic && !(this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic) && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 2) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 2);
-      }
-
-      if(this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic && this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != 3) {
-         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, 3);
+      boolean above = this.worldObj.getBlock(this.xCoord, this.yCoord + 1, this.zCoord) instanceof BlockTankBasic;
+      boolean below = this.worldObj.getBlock(this.xCoord, this.yCoord - 1, this.zCoord) instanceof BlockTankBasic;
+      int wantedMeta = TileTankBasic.connectionMeta(above, below);
+      if(this.worldObj.getBlockMetadata(this.xCoord, this.yCoord, this.zCoord) != wantedMeta) {
+         Util.setBlockMetadata(this.worldObj, this.xCoord, this.yCoord, this.zCoord, wantedMeta);
       }
 
       if(!this.worldObj.isRemote) {

src/test/java/com/pengu/thaumcraft/additions/entity/EntitySingularityQueryTest.java

@@ -0,0 +1,159 @@
+package com.pengu.thaumcraft.additions.entity;
+
+import static org.junit.Assert.assertEquals;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import net.minecraft.block.Block;
+import net.minecraft.entity.Entity;
+import net.minecraft.entity.EntityLivingBase;
+import net.minecraft.item.ItemStack;
+import net.minecraft.util.AxisAlignedBB;
+import net.minecraft.world.World;
+import net.minecraft.world.WorldProviderSurface;
+import net.minecraft.world.WorldServer;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import thaumcraft.common.CommonProxy;
+import thaumcraft.common.Thaumcraft;
+
+/** TDLM/thaumicadditions#21: a grounded singularity queries its pull box once per tick, not 256 times. */
+public class EntitySingularityQueryTest {
+
+    private CommonProxy originalProxy;
+    private QueryWorld world;
+    private EntitySingularity singularity;
+    private Entity bystander;
+    private Mob mob;
+
+    @SuppressWarnings("unchecked")
+    private static <T> T allocate(Class<T> type) throws Exception {
+        Constructor<?> ctor = sun.reflect.ReflectionFactory.getReflectionFactory()
+            .newConstructorForSerialization(type, Object.class.getDeclaredConstructor());
+        return (T) ctor.newInstance();
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        this.originalProxy = Thaumcraft.proxy;
+        Thaumcraft.proxy = new CommonProxy();
+        this.world = allocate(QueryWorld.class);
+        this.world.rand = new Random(3);
+        this.world.all = new ArrayList<Entity>();
+        this.world.living = new ArrayList<Entity>();
+        Field provider = World.class.getDeclaredField("provider");
+        provider.setAccessible(true);
+        provider.set(this.world, new WorldProviderSurface());
+
+        this.singularity = allocate(EntitySingularity.class);
+        this.singularity.worldObj = this.world;
+        this.singularity.posX = 8;
+        this.singularity.posY = 64;
+        this.singularity.posZ = 8;
+        this.singularity.radius = 10;
+        Field ground = net.minecraft.entity.projectile.EntityThrowable.class.getDeclaredField("inGround");
+        ground.setAccessible(true);
+        ground.setBoolean(this.singularity, true);
+
+        this.bystander = allocate(Plain.class);
+        this.bystander.posX = 12;
+        this.bystander.posY = 66;
+        this.bystander.posZ = 5;
+        this.mob = allocate(Mob.class);
+        this.mob.posX = 4;
+        this.mob.posY = 63;
+        this.mob.posZ = 11;
+        this.world.all.add(this.bystander);
+        this.world.all.add(this.mob);
+        this.world.living.add(this.mob);
+    }
+
+    @After
+    public void tearDown() {
+        Thaumcraft.proxy = this.originalProxy;
+    }
+
+    @Test
+    public void thePullBoxIsQueriedOncePerTickAndEntitiesAreStillPulled() {
+        this.singularity.onUpdate();
+
+        assertEquals("entity queries over the pull box", 2, this.world.pullQueries);
+        double lift = 1 / 400.0F * 4.0F;
+        assertEquals((8 - 12) / 10.0D, this.bystander.motionX, 1e-9);
+        assertEquals((64 - 66 + lift) / 10.0D, this.bystander.motionY, 1e-9);
+        assertEquals((8 - 5) / 10.0D, this.bystander.motionZ, 1e-9);
+        assertEquals((8 - 4) / 10.0D, this.mob.motionX, 1e-9);
+        assertEquals((64 - 63 + lift) / 10.0D, this.mob.motionY, 1e-9);
+        assertEquals((8 - 11) / 10.0D, this.mob.motionZ, 1e-9);
+    }
+
+    public static class Plain extends Entity {
+
+        public Plain() {
+            super(null);
+        }
+
+        @Override
+        protected void entityInit() {}
+
+        @Override
+        protected void readEntityFromNBT(net.minecraft.nbt.NBTTagCompound nbt) {}
+
+        @Override
+        protected void writeEntityToNBT(net.minecraft.nbt.NBTTagCompound nbt) {}
+    }
+
+    public static class Mob extends EntityLivingBase {
+
+        public Mob() {
+            super(null);
+        }
+
+        @Override
+        public ItemStack getHeldItem() {
+            return null;
+        }
+
+        @Override
+        public ItemStack getEquipmentInSlot(int slot) {
+            return null;
+        }
+
+        @Override
+        public void setCurrentItemOrArmor(int slot, ItemStack stack) {}
+
+        @Override
+        public ItemStack[] getLastActiveItems() {
+            return new ItemStack[0];
+        }
+    }
+
+    public static class QueryWorld extends WorldServer {
+
+        List<Entity> all;
+        List<Entity> living;
+        int pullQueries;
+
+        QueryWorld() {
+            super(null, null, "singularity", 0, null, null);
+        }
+
+        @Override
+        public List getEntitiesWithinAABB(Class type, AxisAlignedBB box) {
+            if (box.maxX - box.minX < 3.0D) {
+                return new ArrayList();
+            }
+            this.pullQueries++;
+            return new ArrayList(type == EntityLivingBase.class ? this.living : this.all);
+        }
+
+        @Override
+        public Block getBlock(int x, int y, int z) {
+            return null; // equals Blocks.air, which is unset without a Minecraft bootstrap
+        }
+    }
+}

src/test/java/com/pengu/thaumcraft/additions/tileentity/TankAndCropLogicTest.java

@@ -0,0 +1,32 @@
+package com.pengu.thaumcraft.additions.tileentity;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+/** TDLM/thaumicadditions#20 and #22: the lookups were hoisted without changing what the tiles decide. */
+public class TankAndCropLogicTest {
+
+    @Test
+    public void tankMetadataMatchesTheOriginalFourBranches() {
+        assertEquals(0, TileTankBasic.connectionMeta(false, false));
+        assertEquals(1, TileTankBasic.connectionMeta(false, true));
+        assertEquals(2, TileTankBasic.connectionMeta(true, false));
+        assertEquals(3, TileTankBasic.connectionMeta(true, true));
+    }
+
+    @Test
+    public void cropThresholdIsLowerInTheTaintBiome() {
+        assertEquals(4000, TileTaintedCrops.growthThreshold(false));
+        assertEquals(3550, TileTaintedCrops.growthThreshold(true));
+    }
+
+    @Test
+    public void cropOnlyCountsGrowthBelowStageFour() {
+        assertTrue(TileTaintedCrops.countsGrowth(0));
+        assertTrue(TileTaintedCrops.countsGrowth(3));
+        assertFalse(TileTaintedCrops.countsGrowth(4));
+    }
+}