From 7d4bf619feaaff993ff786e598fafb05fd8cd974 Mon Sep 17 00:00:00 2001 From: cpw Date: Wed, 20 Jul 2016 16:03:56 -0400 Subject: [PATCH] More tests. Substitutions now work, and a fix is included. *sigh* Closes too many bugs to count. --- .../fml/common/LoadController.java | 4 + .../net/minecraftforge/fml/common/Loader.java | 16 +++- .../FMLControlledNamespacedRegistry.java | 15 +++- .../fml/common/registry/ForgeTestRunner.java | 5 +- .../common/registry/RegistryTestSuite.java | 13 ++++ .../common/registry/SubstitutionTests.java | 75 +++++++++++++++++++ 6 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 src/test/java/net/minecraftforge/fml/common/registry/RegistryTestSuite.java create mode 100644 src/test/java/net/minecraftforge/fml/common/registry/SubstitutionTests.java diff --git a/src/main/java/net/minecraftforge/fml/common/LoadController.java b/src/main/java/net/minecraftforge/fml/common/LoadController.java index 99df66907f..7422eec039 100644 --- a/src/main/java/net/minecraftforge/fml/common/LoadController.java +++ b/src/main/java/net/minecraftforge/fml/common/LoadController.java @@ -199,6 +199,10 @@ public class LoadController return activeContainer != null ? activeContainer : findActiveContainerFromStack(); } + void forceActiveContainer(ModContainer container) + { + activeContainer = container; + } @Subscribe public void propogateStateMessage(FMLEvent stateEvent) { diff --git a/src/main/java/net/minecraftforge/fml/common/Loader.java b/src/main/java/net/minecraftforge/fml/common/Loader.java index 982c38973c..82bcd17dce 100644 --- a/src/main/java/net/minecraftforge/fml/common/Loader.java +++ b/src/main/java/net/minecraftforge/fml/common/Loader.java @@ -38,6 +38,7 @@ import net.minecraftforge.common.capabilities.CapabilityManager; import net.minecraftforge.fml.common.LoaderState.ModState; import net.minecraftforge.fml.common.ModContainer.Disableable; import net.minecraftforge.fml.common.ProgressManager.ProgressBar; +import net.minecraftforge.fml.common.discovery.ASMDataTable; import net.minecraftforge.fml.common.discovery.ModDiscoverer; import net.minecraftforge.fml.common.event.FMLInterModComms; import net.minecraftforge.fml.common.event.FMLLoadEvent; @@ -488,11 +489,24 @@ public class Loader return instance().mods != null ? ImmutableList.copyOf(instance().mods) : ImmutableList.of(); } + /** + * Used to setup a testharness with a single dummy mod instance for use with various testing hooks + * @param dummycontainer A dummy container that will be returned as "active" for all queries + */ + public void setupTestHarness(ModContainer dummycontainer) + { + modController = new LoadController(this); + mods = Lists.newArrayList(dummycontainer); + modController.transition(LoaderState.LOADING, false); + modController.transition(LoaderState.CONSTRUCTING, false); + ObjectHolderRegistry.INSTANCE.findObjectHolders(new ASMDataTable()); + modController.forceActiveContainer(dummycontainer); + } /** * Called from the hook to start mod loading. We trigger the * {@link #identifyMods()} and Constructing, Preinitalization, and Initalization phases here. Finally, * the mod list is frozen completely and is consider immutable from then on. - * @param injectedModContainers + * @param injectedModContainers containers to inject */ public void loadMods(List injectedModContainers) { diff --git a/src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java b/src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java index c6c2d4b7bc..5ccad594f9 100644 --- a/src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java +++ b/src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java @@ -65,6 +65,10 @@ public class FMLControlledNamespacedRegistry> e * Persistent substitutions are the mechanism to allow mods to override specific behaviours with new behaviours. */ private final BiMap persistentSubstitutions = HashBiMap.create(); + /** + * Substitution originals - these are the originals that are being substituted + */ + private final BiMap substitutionOriginals = HashBiMap.create(); /** * This is the current active substitution set for a particular world. It will change as worlds come and go. */ @@ -602,6 +606,11 @@ public class FMLControlledNamespacedRegistry> e I sub = getPersistentSubstitutions().get(nameToReplace); getExistingDelegate(original).changeReference(sub); activeSubstitutions.put(nameToReplace, sub); + int id = getIDForObjectBypass(original); + // force the new object into the existing map + addObjectRaw(id, nameToReplace, sub); + // Track the original in the substitution originals collection + substitutionOriginals.put(nameToReplace, original); return original; } return null; @@ -747,7 +756,11 @@ public class FMLControlledNamespacedRegistry> e remappedIds.put(itemName, new Integer[] {currId, newId}); } I obj = currentRegistry.getRaw(itemName); - + // If we have an object in the originals set, we use that for initial adding - substitute activation will readd the substitute if neceessary later + if (currentRegistry.substitutionOriginals.containsKey(itemName)) + { + obj = currentRegistry.substitutionOriginals.get(itemName); + } add(newId, itemName, obj); } } diff --git a/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java b/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java index cc4fbb053f..8302867abb 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java @@ -13,9 +13,8 @@ import org.junit.runners.JUnit4; import org.junit.runners.model.InitializationError; /** - * Uses {@code ResettingClassLoader} to load the test class, meaning the - * {@code Quarantine} annotation can be used to ensure certain classes are - * loaded separately. + * Uses {@code ResettingClassLoader} to load the test class. Minecraft and Forge + * classes are loaded using the separate class loader. * * Use of a separate class loader allows classes to be reloaded for each test * class, which is handy when you're testing frameworks that make use of static diff --git a/src/test/java/net/minecraftforge/fml/common/registry/RegistryTestSuite.java b/src/test/java/net/minecraftforge/fml/common/registry/RegistryTestSuite.java new file mode 100644 index 0000000000..d3512ae16a --- /dev/null +++ b/src/test/java/net/minecraftforge/fml/common/registry/RegistryTestSuite.java @@ -0,0 +1,13 @@ +package net.minecraftforge.fml.common.registry; + +import org.junit.runner.RunWith; +import org.junit.runners.Suite; + +/** + * Run the full suite of tests + */ +@RunWith(Suite.class) +@Suite.SuiteClasses({VanillaRegistryTests.class, FreezingTests.class, SubstitutionTests.class}) +public class RegistryTestSuite +{ +} diff --git a/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionTests.java b/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionTests.java new file mode 100644 index 0000000000..76c663a097 --- /dev/null +++ b/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionTests.java @@ -0,0 +1,75 @@ +package net.minecraftforge.fml.common.registry; + +import net.minecraft.block.Block; +import net.minecraft.block.BlockDirt; +import net.minecraft.init.Blocks; +import net.minecraft.init.Bootstrap; +import net.minecraft.util.ResourceLocation; +import net.minecraftforge.fml.common.DummyModContainer; +import net.minecraftforge.fml.common.Loader; +import net.minecraftforge.fml.common.ModMetadata; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +/** + * Substitution test harness - tests that substitutions behave correctly + */ +@RunWith(ForgeTestRunner.class) +public class SubstitutionTests +{ + private ResourceLocation myDirt = new ResourceLocation("minecraft:dirt"); + private BlockDirt toSub = new BlockDirt() { + + }; + + @BeforeClass + public static void setup() + { + Loader.instance(); + Bootstrap.register(); + Loader.instance().setupTestHarness(new DummyModContainer(new ModMetadata() {{ + modId = "test"; + }})); + } + + @Test + public void testSubstitution() throws Exception + { + GameRegistry.addSubstitutionAlias("minecraft:dirt", GameRegistry.Type.BLOCK, toSub); + PersistentRegistryManager.freezeData(); + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); + + final FMLControlledNamespacedRegistry blockRegistry = (FMLControlledNamespacedRegistry)PersistentRegistryManager.findRegistryByType(Block.class); + + // TEST 1: Does my substitute take effect? The substitute should be found in the registry + Block fnd = blockRegistry.getValue(myDirt); + Block currDirt = Blocks.DIRT; + assertEquals("Got my dirt substitute - Blocks", toSub, currDirt); + assertEquals("Got my dirt substitute - Blocks and registry", currDirt, fnd); + assertEquals("Got my dirt substitute - registry", toSub, fnd); + + // TEST 2: Does the substitute get removed when told by remote operation? The substitute should NOT be found in the registry + final PersistentRegistryManager.GameDataSnapshot snapshot = PersistentRegistryManager.takeSnapshot(); + snapshot.entries.get(PersistentRegistryManager.BLOCKS).substitutions.clear(); + PersistentRegistryManager.injectSnapshot(snapshot, false, false); + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); + fnd = blockRegistry.getValue(myDirt); + currDirt = Blocks.DIRT; + assertNotEquals("Got my dirt substitute - Blocks", toSub, currDirt); + assertEquals("Got my dirt substitute - Blocks and registry", currDirt, fnd); + assertNotEquals("Got my dirt substitute - registry", toSub, fnd); + + // TEST 3: Does the substitute get restored when reverting to frozen state? The substitute should be found in the registry again + PersistentRegistryManager.revertToFrozen(); + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); + fnd = blockRegistry.getValue(myDirt); + currDirt = Blocks.DIRT; + assertEquals("Got my dirt substitute - Blocks", toSub, currDirt); + assertEquals("Got my dirt substitute - Blocks and registry", currDirt, fnd); + assertEquals("Got my dirt substitute - registry", toSub, fnd); + } +}