Cleanup some of the game tests to use more specific types and avoid using mutable to capture intermediary results

pupnewfster · pupnewfster · commit 5fa4404b29fd · 2024-04-15T12:54:56.000-05:00
diff --git a/src/gameTest/java/mekanism/common/tests/network/InventoryNetworkTest.java b/src/gameTest/java/mekanism/common/tests/network/InventoryNetworkTest.java
@@ -281,8 +281,8 @@ public static void colorIsNotPriority(final DynamicTest test) {
     @GameTest(template = UPGRADEABLE, setupTicks = SETUP_TICKS, timeoutTicks = TIMEOUT_TICKS)
     @TestHolder(description = "Tests that newly pulled items will go to the destination that had its path upgraded, "
                               + "but any items that were already en-route will continue to the destination they had already calculated.")
-    public static void upgradeFurtherPath(final DynamicTest test) {
-        test.onGameTest(helper -> helper.startSequence()
+    public static void upgradeFurtherPath(final ExtendedGameTestHelper helper) {
+        helper.startSequence()
               //Wait a few seconds for it to pull some items out, and upgrade the transporter to the further destination
               .thenExecuteAfter(4 * SharedConstants.TICKS_PER_SECOND, () -> applyAlloyUpgrade(helper, new BlockPos(9, 1, 0), AlloyTier.INFUSED))
               //Wait a few seconds for transferring to happen then validate stuff
@@ -293,16 +293,15 @@ public static void upgradeFurtherPath(final DynamicTest test) {
                   GameTestUtils.validateContainerHas(helper, new BlockPos(8, 1, 0), 0, new ItemStack(Items.STONE, 12));
                   //Validate start is also empty
                   helper.assertContainerEmpty(3, 1, 0);
-              }).thenSucceed()
-        );
+              }).thenSucceed();
     }
 
     @GameTest(template = UPGRADEABLE, setupTicks = SETUP_TICKS, timeoutTicks = TIMEOUT_TICKS)
     @TestHolder(description = "Tests that newly pulled items will go to the destination that had its path upgraded, "
                               + "but any items that were already en-route will continue to the destination they had "
                               + "already calculated as the new destination is slightly \"closer\".")
-    public static void upgradeFurtherOverlapping(final DynamicTest test) {
-        test.onGameTest(helper -> helper.startSequence()
+    public static void upgradeFurtherOverlapping(final ExtendedGameTestHelper helper) {
+        helper.startSequence()
               //Wait a few seconds for it to pull some items out, and upgrade the transporter to the further destination
               .thenExecuteAfter(4 * SharedConstants.TICKS_PER_SECOND, () -> applyAlloyUpgrade(helper, new BlockPos(6, 1, 2), AlloyTier.INFUSED))
               //Wait a few seconds for transferring to happen then validate stuff
@@ -313,14 +312,13 @@ public static void upgradeFurtherOverlapping(final DynamicTest test) {
                   GameTestUtils.validateContainerHas(helper, new BlockPos(8, 1, 0), 0, new ItemStack(Items.STONE, 12));
                   //Validate start is also empty
                   helper.assertContainerEmpty(3, 1, 0);
-              }).thenSucceed()
-        );
+              }).thenSucceed();
     }
 
     @GameTest(template = UPGRADEABLE, setupTicks = SETUP_TICKS, timeoutTicks = TIMEOUT_TICKS)
     @TestHolder(description = "Tests that all items pre- and post-upgrade will go to the original destination.")
-    public static void upgradeExisting(final DynamicTest test) {
-        test.onGameTest(helper -> helper.startSequence()
+    public static void upgradeExisting(final ExtendedGameTestHelper helper) {
+        helper.startSequence()
               //Wait a few seconds for it to pull some items out, and upgrade the transporter to the further destination
               .thenExecuteAfter(4 * SharedConstants.TICKS_PER_SECOND, () -> applyAlloyUpgrade(helper, new BlockPos(3, 1, 2), AlloyTier.INFUSED))
               //Wait a few seconds for transferring to happen then validate stuff
@@ -331,8 +329,7 @@ public static void upgradeExisting(final DynamicTest test) {
                   helper.assertContainerEmpty(8, 1, 0);
                   //Validate start is also empty
                   helper.assertContainerEmpty(3, 1, 0);
-              }).thenSucceed()
-        );
+              }).thenSucceed();
     }
 
     private static void applyAlloyUpgrade(ExtendedGameTestHelper helper, BlockPos relativePos, AlloyTier tier) {
diff --git a/src/gameTest/java/mekanism/common/tests/network/TransmitterNetworkTest.java b/src/gameTest/java/mekanism/common/tests/network/TransmitterNetworkTest.java
@@ -14,16 +14,12 @@
 import net.minecraft.gametest.framework.GameTest;
 import net.minecraft.world.level.ChunkPos;
 import net.minecraft.world.level.block.Blocks;
-import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.levelgen.structure.templatesystem.StructureTemplate;
-import net.neoforged.testframework.DynamicTest;
 import net.neoforged.testframework.annotation.ForEachTest;
 import net.neoforged.testframework.annotation.RegisterStructureTemplate;
 import net.neoforged.testframework.annotation.TestHolder;
 import net.neoforged.testframework.gametest.ExtendedGameTestHelper;
 import net.neoforged.testframework.gametest.StructureTemplateBuilder;
-import org.apache.commons.lang3.mutable.MutableInt;
-import org.apache.commons.lang3.mutable.MutableObject;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -42,8 +38,8 @@ public class TransmitterNetworkTest {
 
     @GameTest(template = STRAIGHT_CABLE, setupTicks = SETUP_TICKS, batch = "1")
     @TestHolder(description = "Tests that reloading intermediary chunks does not cause a network to break.")
-    public static void reloadIntermediary(final DynamicTest test) {
-        test.onGameTest(helper -> GameTestUtils.succeedIfAfterReload(helper, new ChunkPos(1, 0), new MatchingNetworkValidator(helper)));
+    public static void reloadIntermediary(final ExtendedGameTestHelper helper) {
+        GameTestUtils.succeedIfAfterReload(helper, new ChunkPos(1, 0), new MatchingNetworkValidator(helper));
     }
 
     /**
@@ -53,33 +49,27 @@ public static void reloadIntermediary(final DynamicTest test) {
     @GameTest(template = STRAIGHT_CABLE, setupTicks = SETUP_TICKS, batch = "2")
     @TestHolder(description = "Tests that when part of a network becomes inaccessible but still loaded, "
                               + "we are able to properly remove the transmitters and then recover when the chunk becomes accessible again.")
-    public static void inaccessibleNotUnloaded(final DynamicTest test) {
-        test.onGameTest(helper -> {
-            ChunkPos relativeChunk = new ChunkPos(1, 0);
-            BlockPos relativeTargetTransmitter = new BlockPos(0, 1, 0);
-            MutableInt lastLevel = new MutableInt();
-            //This is used
-            MutableObject<BlockState> lastState = new MutableObject<>(Blocks.AIR.defaultBlockState());
-            helper.startSequence()
-                  .thenWaitUntil(() -> lastLevel.setValue(GameTestUtils.setChunkLoadLevel(helper, relativeChunk, GameTestUtils.INACCESSIBLE_LEVEL)))
-                  //Wait 5 ticks in case anything needs more time to process after the chunk unloads
-                  //Force a rebuild of the network by breaking one transmitter
-                  .thenExecuteAfter(5, () -> {
-                      //Update the last state, so we can set it again afterwards
-                      lastState.setValue(helper.getBlockState(relativeTargetTransmitter));
-                      helper.setBlock(relativeTargetTransmitter, Blocks.AIR);
-                  })
-                  //Wait 5 ticks to ensure it has time to process everything (expected to only take two ticks)
-                  //Set the block back to what it was before (the transmitter)
-                  .thenExecuteAfter(5, () -> helper.setBlock(relativeTargetTransmitter, lastState.getValue()))
-                  //Wait 5 ticks to ensure it has time to process everything (expected to only take two ticks)
-                  //Set the chunk level back to what it was before (aka loading it fully again)
-                  .thenExecuteAfter(5, () -> GameTestUtils.setChunkLoadLevel(helper, relativeChunk, lastLevel.getValue()))
-                  //Wait 5 ticks in case anything needs more time to process after the chunk loads
-                  .thenIdle(5)
-                  .thenWaitUntil(0, new MatchingNetworkValidator(helper))
-                  .thenSucceed();
-        });
+    public static void inaccessibleNotUnloaded(final ExtendedGameTestHelper helper) {
+        ChunkPos relativeChunk = new ChunkPos(1, 0);
+        BlockPos relativeTargetTransmitter = new BlockPos(0, 1, 0);
+        helper.startSequence()
+              .thenMap(() -> GameTestUtils.setChunkLoadLevel(helper, relativeChunk, GameTestUtils.INACCESSIBLE_LEVEL))
+              //Wait 5 ticks in case anything needs more time to process after the chunk unloads
+              .thenIdle(5)
+              .thenSequence(sequence -> sequence.thenMap(() -> helper.getBlockState(relativeTargetTransmitter))
+                    //Force a rebuild of the network by breaking one transmitter
+                    .thenExecute(() -> helper.setBlock(relativeTargetTransmitter, Blocks.AIR))
+                    //Wait 5 ticks to ensure it has time to process everything (expected to only take two ticks)
+                    //Set the block back to what it was before (the transmitter)
+                    .thenExecuteAfter(5, state -> helper.setBlock(relativeTargetTransmitter, state))
+              )
+              //Wait 5 ticks to ensure it has time to process everything (expected to only take two ticks)
+              //Set the chunk level back to what it was before (aka loading it fully again)
+              .thenExecuteAfter(5, level -> GameTestUtils.setChunkLoadLevel(helper, relativeChunk, level))
+              //Wait 5 ticks in case anything needs more time to process after the chunk loads
+              .thenIdle(5)
+              .thenWaitUntil(0, new MatchingNetworkValidator(helper))
+              .thenSucceed();
     }
 
     private static void forEachTransmitter(ExtendedGameTestHelper helper, TriConsumer<TileEntityTransmitter, Transmitter<?, ?, ?>, BlockPos> consumer) {
diff --git a/src/gameTest/java/mekanism/common/tests/util/GameTestUtils.java b/src/gameTest/java/mekanism/common/tests/util/GameTestUtils.java
@@ -14,7 +14,6 @@
 import net.minecraft.world.level.block.entity.BlockEntity;
 import net.neoforged.neoforge.items.IItemHandler;
 import net.neoforged.testframework.gametest.ExtendedGameTestHelper;
-import org.apache.commons.lang3.mutable.MutableInt;
 import org.jetbrains.annotations.Nullable;
 
 public class GameTestUtils {
@@ -24,22 +23,26 @@ public class GameTestUtils {
 
     static final boolean DEBUG_CHUNK_LOADING = false;
 
-    public static void succeedIfAfterUnload(ExtendedGameTestHelper helper, ChunkPos relativePos, Runnable criteria) {
-        helper.startSequence()
-              .thenWaitUntil(() -> unloadChunk(helper, relativePos))
-              //Wait 5 ticks in case anything needs more time to process after the chunk unloads
-              .thenIdle(5)
-              .thenWaitUntil(0, criteria)
-              .thenSucceed();
-    }
-
     public static void succeedIfAfterReload(ExtendedGameTestHelper helper, ChunkPos relativePos, Runnable afterReload) {
-        MutableInt lastLevel = new MutableInt(UNLOAD_LEVEL);
         helper.startSequence()
-              .thenWaitUntil(() -> unloadChunk(helper, relativePos, lastLevel))
+              .thenMap(() -> unloadChunk(helper, relativePos))
+              .thenWaitUntil(() -> {
+                  ChunkPos absolutePos = absolutePos(helper, relativePos);
+                  long absPos = absolutePos.toLong();
+                  if (GameTestEventListeners.watchedChunks.containsKey(absPos)) {
+                      //Watched chunk
+                      if (GameTestEventListeners.watchedChunks.get(absPos)) {
+                          //Is loaded, but we are watching, throw exception so that we keep waiting for it to unload
+                          fail(helper, "Chunk has not been marked as unloaded yet", absolutePos, relativePos);
+                      } else {
+                          //Remove the watch on the chunk now that it has been unloaded
+                          GameTestEventListeners.watchedChunks.remove(absPos);
+                      }
+                  }
+              })
               //Wait 5 ticks in case anything needs more time to process after the chunk unloads
               .thenIdle(5)
-              .thenWaitUntil(() -> loadChunk(helper, relativePos, lastLevel))
+              .thenWaitUntil(level -> loadChunk(helper, relativePos, level))
               //Wait 5 ticks in case anything needs more time to process after the chunk loads
               .thenIdle(5)
               .thenWaitUntil(0, afterReload)
@@ -48,23 +51,11 @@ public static void succeedIfAfterReload(ExtendedGameTestHelper helper, ChunkPos
 
     //TODO - GameTest: Can we make unloads not cause the game to crash if a player tries to run a test that uses them instead of using game test server?
     // Most likely the answer is no
-    private static void unloadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos) {
-        unloadChunk(helper, relativePos, new MutableInt());
-    }
-
-    private static void unloadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos, MutableInt levelMemory) {
+    private static int unloadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos) {
+        int level = UNLOAD_LEVEL;
         ChunkPos absolutePos = absolutePos(helper, relativePos);
         long absPos = absolutePos.toLong();
-        if (GameTestEventListeners.watchedChunks.containsKey(absPos)) {
-            //Watched chunk
-            if (GameTestEventListeners.watchedChunks.get(absPos)) {
-                //Is loaded, but we are watching, throw exception so that we keep waiting for it to unload
-                fail(helper, "Chunk has not been marked as unloaded yet", absolutePos, relativePos);
-            } else {
-                //Remove the watch on the chunk now that it has been unloaded
-                GameTestEventListeners.watchedChunks.remove(absPos);
-            }
-        } else if (WorldUtils.isChunkLoaded(helper.getLevel(), absolutePos)) {
+        if (WorldUtils.isChunkLoaded(helper.getLevel(), absolutePos)) {
             //If the chunk isn't watched and is loaded we want to try and unload it
             ChunkMap chunkMap = helper.getLevel().getChunkSource().chunkMap;
             DistanceManager distanceManager = chunkMap.getDistanceManager();
@@ -77,25 +68,25 @@ private static void unloadChunk(ExtendedGameTestHelper helper, ChunkPos relative
                     Mekanism.logger.info("Trying to unload chunk at: {}, {}", absolutePos.x, absolutePos.z);
                 }
                 //If it is currently loaded, queue it for unload
-                levelMemory.setValue(holder.getTicketLevel());
-                distanceManager.updateChunkScheduling(absPos, UNLOAD_LEVEL, holder, holder.getTicketLevel());
+                level = holder.getTicketLevel();
+                distanceManager.updateChunkScheduling(absPos, UNLOAD_LEVEL, holder, level);
                 //And then unload it
                 chunkMap.processUnloads(ConstantPredicates.ALWAYS_TRUE);
-                fail(helper, "Chunk queued for unloading", absolutePos, relativePos);
             } else if (DEBUG_CHUNK_LOADING) {
                 //Note: Even with debug logging enabled odds are this case isn't even possible due to the earlier check to skip if unloaded
                 Mekanism.logger.info("Trying to unload already unloaded chunk at: {}, {}", absolutePos.x, absolutePos.z);
             }
         } else if (DEBUG_CHUNK_LOADING) {
             Mekanism.logger.info("Chunk at: {}, {} is already unloaded", absolutePos.x, absolutePos.z);
         }
+        return level;
     }
 
     private static void loadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos) {
-        loadChunk(helper, relativePos, new MutableInt(ChunkMap.FORCED_TICKET_LEVEL));
+        loadChunk(helper, relativePos, ChunkMap.FORCED_TICKET_LEVEL);
     }
 
-    private static void loadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos, MutableInt levelMemory) {
+    private static void loadChunk(ExtendedGameTestHelper helper, ChunkPos relativePos, int level) {
         ChunkPos absolutePos = absolutePos(helper, relativePos);
         long absPos = absolutePos.toLong();
         if (GameTestEventListeners.watchedChunks.containsKey(absPos)) {
@@ -119,7 +110,7 @@ private static void loadChunk(ExtendedGameTestHelper helper, ChunkPos relativePo
                     Mekanism.logger.info("Trying to load chunk at: {}, {}", absolutePos.x, absolutePos.z);
                 }
                 //Load the chunk to the level it was unloaded at
-                holder = distanceManager.updateChunkScheduling(absPos, levelMemory.getValue(), null, UNLOAD_LEVEL);
+                holder = distanceManager.updateChunkScheduling(absPos, level, null, UNLOAD_LEVEL);
                 if (holder == null) {//Should never happen unless start value was unloaded
                     fail(helper, "Error loading chunk", absolutePos, relativePos);
                 } else {
