perf: hyper node creator rewrites the node only when it no longer holds the wanted aspects

Closes TDLM/thaumicadditions#16

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

src/main/java/com/pengu/thaumcraft/additions/blocks/BlockCreativeNode.java

@@ -40,6 +40,11 @@ public class BlockCreativeNode extends BlockContainer {
    }
 
    public AxisAlignedBB getCollisionBoundingBoxFromPool(World w, int x, int y, int z) {
+      if(!w.isRemote) {
+         // rendering never happens on the server, so the stack walk below can only find nothing
+         return AxisAlignedBB.getBoundingBox(0.0D, 0.0D, 0.0D, 0.0D, 0.0D, 0.0D);
+      }
+
       StackTraceElement[] var5 = Thread.currentThread().getStackTrace();
       int var6 = var5.length;
 

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

@@ -35,6 +35,7 @@ public class TileCreativeNode extends TileVisNode implements INode, IWandable {
    public Color targetColor = new Color(16777215);
    public Color color = new Color(16777215);
    public boolean hasUpdatedOnce;
+   private boolean locationRegistered;
 
 
    public void updateEntity() {
@@ -44,12 +45,30 @@ public class TileCreativeNode extends TileVisNode implements INode, IWandable {
 
       if(!this.hasUpdatedOnce) {
          this.hasUpdatedOnce = !this.hasUpdatedOnce;
-      } else {
+      } else if(!this.locationRegistered) {
          this.generateId();
       }
 
    }
 
+   public void invalidate() {
+      this.unregisterLocation();
+      super.invalidate();
+   }
+
+   public void onChunkUnload() {
+      this.unregisterLocation();
+      super.onChunkUnload();
+   }
+
+   private void unregisterLocation() {
+      if(this.id != null && TileNode.locations != null) {
+         TileNode.locations.remove(this.id);
+      }
+
+      this.locationRegistered = false;
+   }
+
    public AspectList getAspects() {
       return (new AspectList()).add(Aspect.AIR, 134217).add(Aspect.WATER, 134217).add(Aspect.FIRE, 134217).add(Aspect.ORDER, 134217).add(Aspect.ENTROPY, 134217).add(Aspect.EARTH, 134217);
    }
@@ -232,6 +251,7 @@ public class TileCreativeNode extends TileVisNode implements INode, IWandable {
          t.add(Integer.valueOf(this.yCoord));
          t.add(Integer.valueOf(this.zCoord));
          TileNode.locations.put(this.id, t);
+         this.locationRegistered = true;
       }
 
       return this.id;

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

@@ -35,25 +35,17 @@ public class TileHyperNodeCreator extends TileThaumcraft implements IInventory,
          }
       }
 
-      TileNode var5 = this.worldObj.getTileEntity(this.xCoord, this.yCoord + 1, this.zCoord) != null && this.worldObj.getTileEntity(this.xCoord, this.yCoord + 1, this.zCoord) instanceof TileNode?(TileNode)this.worldObj.getTileEntity(this.xCoord, this.yCoord + 1, this.zCoord):null;
+      net.minecraft.tileentity.TileEntity above = this.worldObj.getTileEntity(this.xCoord, this.yCoord + 1, this.zCoord);
+      TileNode var5 = above instanceof TileNode?(TileNode)above:null;
       this.node = var5 != null;
       if(this.node) {
-         AspectList list = new AspectList();
-
-         int corner;
-         for(corner = 0; corner < Aspect.getPrimalAspects().size(); ++corner) {
-            list.add((Aspect)Aspect.getPrimalAspects().get(corner), totalItems);
+         // the node is only rewritten when it no longer holds the wanted amounts (drained, or the stock changed)
+         if(!holdsExactly(var5.getAspects(), totalItems, this.primalsOnly)) {
+            var5.setAspects(wantedAspects(totalItems, this.primalsOnly));
          }
 
-         if(!this.primalsOnly) {
-            for(corner = 0; corner < Aspect.getCompoundAspects().size(); ++corner) {
-               list.add((Aspect)Aspect.getCompoundAspects().get(corner), totalItems);
-            }
-         }
-
-         var5.setAspects(list);
          if(this.worldObj.rand.nextBoolean()) {
-            corner = this.worldObj.rand.nextInt(8);
+            int corner = this.worldObj.rand.nextInt(8);
             if(corner == 0) {
                Thaumcraft.proxy.nodeBolt(this.worldObj, (float)this.xCoord, (float)this.yCoord + 2.0F, (float)this.zCoord, (float)this.xCoord + 0.5F, (float)this.yCoord + 1.5F, (float)this.zCoord + 0.5F);
             }
@@ -90,6 +82,52 @@ public class TileHyperNodeCreator extends TileThaumcraft implements IInventory,
 
    }
 
+   /** The aspect list a node above gets: every primal (and unless primalsOnly every compound) at {@code total}. */
+   public static AspectList wantedAspects(int total, boolean primalsOnly) {
+      AspectList list = new AspectList();
+
+      int i;
+      for(i = 0; i < Aspect.getPrimalAspects().size(); ++i) {
+         list.add((Aspect)Aspect.getPrimalAspects().get(i), total);
+      }
+
+      if(!primalsOnly) {
+         for(i = 0; i < Aspect.getCompoundAspects().size(); ++i) {
+            list.add((Aspect)Aspect.getCompoundAspects().get(i), total);
+         }
+      }
+
+      return list;
+   }
+
+   /** Whether {@code current} already equals {@link #wantedAspects(int, boolean)} without building it. */
+   public static boolean holdsExactly(AspectList current, int total, boolean primalsOnly) {
+      if(current == null) {
+         return false;
+      }
+
+      int expected = Aspect.getPrimalAspects().size() + (primalsOnly?0:Aspect.getCompoundAspects().size());
+      return current.size() == expected && allAmounts(current, total, primalsOnly);
+   }
+
+   private static boolean allAmounts(AspectList current, int total, boolean primalsOnly) {
+      for(int i = 0; i < Aspect.getPrimalAspects().size(); ++i) {
+         if(current.getAmount((Aspect)Aspect.getPrimalAspects().get(i)) != total) {
+            return false;
+         }
+      }
+
+      if(!primalsOnly) {
+         for(int i = 0; i < Aspect.getCompoundAspects().size(); ++i) {
+            if(current.getAmount((Aspect)Aspect.getCompoundAspects().get(i)) != total) {
+               return false;
+            }
+         }
+      }
+
+      return true;
+   }
+
    public static boolean areItemStacksEqual(ItemStack s1, ItemStack s2) {
       return s1 == null && s2 == null?true:s1 != null && s2 != null && s1.getItem() == s2.getItem() && s1.getItemDamage() == s2.getItemDamage();
    }

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

@@ -0,0 +1,80 @@
+package com.pengu.thaumcraft.additions.tileentity;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.HashMap;
+import net.minecraft.world.World;
+import net.minecraft.world.WorldProviderSurface;
+import net.minecraft.world.WorldServer;
+import org.junit.Before;
+import org.junit.Test;
+import thaumcraft.common.tiles.TileNode;
+
+/** TDLM/thaumicadditions#17: a creative node registers its location once, not every tick, and unregisters on removal. */
+public class CreativeNodeRegistryTest {
+
+    private TileCreativeNode node;
+
+    @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 {
+        if (TileNode.locations == null) {
+            Field f = TileNode.class.getDeclaredField("locations");
+            f.setAccessible(true);
+            f.set(null, new HashMap<String, ArrayList>());
+        }
+        TileNode.locations.clear();
+        World world = allocate(FixtureWorld.class);
+        Field provider = World.class.getDeclaredField("provider");
+        provider.setAccessible(true);
+        provider.set(world, new WorldProviderSurface());
+        this.node = allocate(TileCreativeNode.class);
+        this.node.setWorldObj(world);
+        this.node.xCoord = 3;
+        this.node.yCoord = 70;
+        this.node.zCoord = 5;
+    }
+
+    @Test
+    public void locationIsPutOnceAcrossManyTicks() {
+        this.node.updateEntity(); // first tick only marks the node as updated
+        this.node.updateEntity();
+        ArrayList first = TileNode.locations.get("0:3:70:5");
+        assertNotNull(first);
+        for (int i = 0; i < 1000; i++) {
+            this.node.updateEntity();
+        }
+        assertSame("no re-registration on later ticks", first, TileNode.locations.get("0:3:70:5"));
+        assertEquals(1, TileNode.locations.size());
+    }
+
+    @Test
+    public void unloadingRemovesTheLocationSoItCanBeRegisteredAgain() {
+        this.node.updateEntity();
+        this.node.updateEntity();
+        assertNotNull(TileNode.locations.get("0:3:70:5"));
+        this.node.onChunkUnload();
+        assertNull(TileNode.locations.get("0:3:70:5"));
+        this.node.updateEntity();
+        assertNotNull(TileNode.locations.get("0:3:70:5"));
+    }
+
+    public static class FixtureWorld extends WorldServer {
+
+        FixtureWorld() {
+            super(null, null, "node", 0, null, null);
+        }
+    }
+}

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

@@ -0,0 +1,47 @@
+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;
+import thaumcraft.api.aspects.Aspect;
+import thaumcraft.api.aspects.AspectList;
+
+/** TDLM/thaumicadditions#16: the node is only rewritten when it no longer holds what the creator wants. */
+public class HyperNodeCreatorAspectsTest {
+
+    @Test
+    public void aFreshlyBuiltListIsRecognisedAsCurrent() {
+        for (boolean primalsOnly : new boolean[] { false, true }) {
+            for (int total : new int[] { 0, 1, 64, 5000 }) {
+                AspectList list = TileHyperNodeCreator.wantedAspects(total, primalsOnly);
+                assertTrue("total " + total + " primalsOnly " + primalsOnly,
+                    TileHyperNodeCreator.holdsExactly(list, total, primalsOnly));
+            }
+        }
+    }
+
+    @Test
+    public void wantedListHasTheSameShapeAsBefore() {
+        AspectList all = TileHyperNodeCreator.wantedAspects(7, false);
+        assertEquals(Aspect.getPrimalAspects().size() + Aspect.getCompoundAspects().size(), all.size());
+        assertEquals(7, all.getAmount(Aspect.getPrimalAspects().get(0)));
+        assertEquals(Aspect.getPrimalAspects().size(), TileHyperNodeCreator.wantedAspects(7, true).size());
+    }
+
+    @Test
+    public void aDrainedOrChangedNodeIsRewritten() {
+        AspectList list = TileHyperNodeCreator.wantedAspects(10, false);
+        list.reduce(Aspect.getPrimalAspects().get(0), 3);
+        assertFalse(TileHyperNodeCreator.holdsExactly(list, 10, false));
+        assertFalse(TileHyperNodeCreator.holdsExactly(TileHyperNodeCreator.wantedAspects(10, false), 11, false));
+        assertFalse(TileHyperNodeCreator.holdsExactly(TileHyperNodeCreator.wantedAspects(10, true), 10, false));
+        assertFalse(TileHyperNodeCreator.holdsExactly(null, 10, false));
+    }
+
+    @Test
+    public void primalsOnlyDoesNotMatchAnUnrelatedFullList() {
+        assertFalse(TileHyperNodeCreator.holdsExactly(TileHyperNodeCreator.wantedAspects(10, false), 10, true));
+    }
+}