diff --git a/src/main/java/net/minecraftforge/registries/ForgeRegistry.java b/src/main/java/net/minecraftforge/registries/ForgeRegistry.java index b54a8d8f0..89a5113d5 100644 --- a/src/main/java/net/minecraftforge/registries/ForgeRegistry.java +++ b/src/main/java/net/minecraftforge/registries/ForgeRegistry.java @@ -199,6 +199,15 @@ public class ForgeRegistry> implements IForgeRe { return getID(this.names.get(name)); } + private int getIDRaw(V value) + { + Integer ret = this.ids.inverse().get(value); + return ret == null ? -1 : ret.intValue(); + } + private int getIDRaw(ResourceLocation name) + { + return getIDRaw(this.names.get(name)); + } public V getValue(int id) { @@ -411,6 +420,8 @@ public class ForgeRegistry> implements IForgeRe if (from.superType != this.superType) throw new IllegalArgumentException("Attempted to copy to incompatible registry: " + name + " " + from.superType + " -> " + this.superType); + this.isFrozen = false; + if (this.clear != null) this.clear.onClear(this, stage); @@ -421,14 +432,11 @@ public class ForgeRegistry> implements IForgeRe */ this.aliases.clear(); from.aliases.forEach((f, t) -> this.addAlias(f, t)); - this.dummies.clear(); - from.dummies.forEach(dummy -> this.addDummy(dummy)); this.ids.clear(); this.names.clear(); this.availabilityMap.clear(0, this.availabilityMap.length()); this.defaultValue = null; - this.isFrozen = false; boolean errored = false; @@ -443,6 +451,10 @@ public class ForgeRegistry> implements IForgeRe } } + //Needs to be below add so that dummies are persisted + this.dummies.clear(); + from.dummies.forEach(dummy -> this.addDummy(dummy)); + if (errored) throw new RuntimeException("One of more entry values did not copy to the correct id. Check log for details!"); } @@ -538,7 +550,7 @@ public class ForgeRegistry> implements IForgeRe { ResourceLocation itemName = entry.getKey(); int newId = entry.getValue(); - int currId = old.getID(itemName); + int currId = old.getIDRaw(itemName); if (currId == -1) { @@ -572,9 +584,12 @@ public class ForgeRegistry> implements IForgeRe if (DEBUG) FMLLog.finer("Registry Dummy Add: %s %d -> %s", key, id, dummy); + //It was blocked before so we need to unset the blocking map + this.availabilityMap.clear(id); + int realId = this.add(id, dummy); if (realId != id) - FMLLog.warning("Registered object did not get ID it asked for. Name: {} Type: {} Expected: {} Got: {}", key, dummy.getRegistryType().getName(), id, realId); + FMLLog.warning("Registered object did not get ID it asked for. Name: %s Type: %s Expected: %s Got: %s", key, dummy.getRegistryType().getName(), id, realId); this.dummies.add(key); return true; diff --git a/src/main/java/net/minecraftforge/registries/GameData.java b/src/main/java/net/minecraftforge/registries/GameData.java index 02f885948..95cdd8771 100644 --- a/src/main/java/net/minecraftforge/registries/GameData.java +++ b/src/main/java/net/minecraftforge/registries/GameData.java @@ -325,7 +325,14 @@ public class GameData @Override public Block createDummy(ResourceLocation key) { - return new Block(Material.AIR).setUnlocalizedName("air").setRegistryName(key); + return new BlockDummyAir().setUnlocalizedName("air").setRegistryName(key); + } + private static class BlockDummyAir extends Block //A named class so DummyBlockReplacementTest can detect if its a dummy + { + private BlockDummyAir() + { + super(Material.AIR); + } } } diff --git a/src/main/java/net/minecraftforge/registries/RegistryManager.java b/src/main/java/net/minecraftforge/registries/RegistryManager.java index 8c3c8fdb3..bf1cbe190 100644 --- a/src/main/java/net/minecraftforge/registries/RegistryManager.java +++ b/src/main/java/net/minecraftforge/registries/RegistryManager.java @@ -84,7 +84,7 @@ public class RegistryManager if (!overlappedTypes.isEmpty()) { Class foundType = overlappedTypes.iterator().next(); - FMLLog.severe("Found existing registry of type %1s named %2s, you cannot create a new registry (%3s) with type %4s, as %4s has a parent of that type", foundType, superTypes.get(foundType), name, type); + FMLLog.severe("Found existing registry of type %s named %s, you cannot create a new registry (%s) with type %s, as %s has a parent of that type", foundType, superTypes.get(foundType), name, type, type); throw new IllegalArgumentException("Duplicate registry parent type found - you can only have one registry for a particular super type"); } ForgeRegistry reg = new ForgeRegistry(type, defaultKey, min, max, create, add, clear, this, allowOverrides, isModifiable, dummyFactory); diff --git a/src/test/java/net/minecraftforge/fml/common/registry/DummyBlockReplacementTest.java b/src/test/java/net/minecraftforge/fml/common/registry/DummyBlockReplacementTest.java index 00add3111..8f338c2c1 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/DummyBlockReplacementTest.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/DummyBlockReplacementTest.java @@ -70,9 +70,9 @@ public class DummyBlockReplacementTest fnd = blockRegistry.getValue(myDirt); assertNotEquals("Did not find my block", fnd, testDirtBlock); - assertTrue("Found a dummy air block", fnd.getClass().getName().endsWith("BlockDummyAir")); + assertTrue("Did not find a dummy air block", fnd.getClass().getName().endsWith("BlockDummyAir")); final Set dummied = RegistryManager.ACTIVE.takeSnapshot(false).get(GameData.BLOCKS).dummied; - assertTrue("Found my block in the dummy list", dummied.contains(myDirt)); + assertTrue("Did not find my block in the dummy list", dummied.contains(myDirt)); GameData.revertToFrozen(); ObjectHolderRegistry.INSTANCE.applyObjectHolders(); @@ -80,6 +80,7 @@ public class DummyBlockReplacementTest assertNotEquals("Did not find my block", fnd, testDirtBlock); assertEquals("Found a default air block", fnd, Blocks.AIR); + ((ForgeRegistry)RegistryManager.ACTIVE.getRegistry(Block.class)).unfreeze(); RegistryManager.ACTIVE.getRegistry(Block.class).register(testDirtBlock.setRegistryName(myDirt)); fnd = blockRegistry.getValue(myDirt); assertEquals("Found my block", fnd, testDirtBlock); 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 ad5dc798c..81c073bff 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/ForgeTestRunner.java @@ -76,6 +76,7 @@ public class ForgeTestRunner extends Runner { try { + System.setProperty("forge.disableVanillaGameData", "false"); innerRunnerClass.getMethod("run", RunNotifier.class).invoke(innerRunner, notifier); } catch (Exception e) diff --git a/src/test/java/net/minecraftforge/fml/common/registry/FreezingTests.java b/src/test/java/net/minecraftforge/fml/common/registry/FreezingTests.java index 750a79e8d..a9ced43cd 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/FreezingTests.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/FreezingTests.java @@ -43,9 +43,13 @@ public class FreezingTests { setRegistryName(name); } + @Override + public String toString() + { + return this.getRegistryName().toString(); + } } - public static IForgeRegistry registry; public static ResourceLocation resloc = new ResourceLocation("fmltest:test"); @BeforeClass @@ -53,27 +57,33 @@ public class FreezingTests { Loader.instance(); System.setProperty("fml.queryResult", "confirm"); + System.setProperty("fml.doNotBackup", "true"); System.setProperty("forge.disableVanillaGameData", "true"); - registry = new RegistryBuilder().setName(resloc).setType(RTest.class).setIDRange(0, 255).create(); + + new RegistryBuilder().setName(resloc).setType(RTest.class).setIDRange(0, 255).create(); new RegistryBuilder().setName(GameData.BLOCKS).setType(Block.class).setIDRange(0, 255).create(); new RegistryBuilder().setName(GameData.ITEMS).setType(Item.class).setIDRange(0, 255).create(); + r1 = new RTest("test1"); r2 = new RTest("test2"); r3 = new RTest("test3"); r4 = new RTest("test4"); r5 = new RTest("test5"); r6 = new RTest("test6"); + ss = Maps.newHashMap(); ss.put(GameData.BLOCKS, new ForgeRegistry.Snapshot()); ss.put(GameData.ITEMS, new ForgeRegistry.Snapshot()); + RegistryManager.ACTIVE.getRegistry(RTest.class).register(r1); RegistryManager.ACTIVE.getRegistry(RTest.class).register(r2); RegistryManager.ACTIVE.getRegistry(RTest.class).register(r3); RegistryManager.ACTIVE.getRegistry(RTest.class).register(r4); ss.put(resloc, ((ForgeRegistry)RegistryManager.ACTIVE.getRegistry(RTest.class)).makeSnapshot()); + RegistryManager.ACTIVE.clean(); RegistryManager.FROZEN.clean(); - registry = new RegistryBuilder().setName(resloc).setType(RTest.class).setIDRange(0, 255).create(); + new RegistryBuilder().setName(resloc).setType(RTest.class).setIDRange(0, 255).create(); new RegistryBuilder().setName(GameData.BLOCKS).setType(Block.class).setIDRange(0, 255).create(); new RegistryBuilder().setName(GameData.ITEMS).setType(Item.class).setIDRange(0, 255).create(); } @@ -81,30 +91,43 @@ public class FreezingTests @Test public void testFreezeCycle() { - RegistryManager.ACTIVE.getRegistry(RTest.class).register(r6); - RegistryManager.ACTIVE.getRegistry(RTest.class).register(r5); - RegistryManager.ACTIVE.getRegistry(RTest.class).register(r4); - RegistryManager.ACTIVE.getRegistry(RTest.class).register(r3); - ForgeRegistry r = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry(r3.getRegistryType()); - int r3id = r.getID(r3); + ResourceLocation name = new ResourceLocation("test3"); + ForgeRegistry active = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry(RTest.class); + active.register(r6); + active.register(r5); + active.register(r4); + active.register(r3); + + int r3id = active.getID(r3); GameData.freezeData(); - assertEquals("Frozen object is the same", r3, RegistryManager.FROZEN.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - assertEquals("Active object is the same", r3, RegistryManager.ACTIVE.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - GameData.injectSnapshot(ss, false, false); - assertNotEquals("IDs don't match", r3id, r.getID(r3)); - assertEquals("Frozen object is the same", r3, RegistryManager.FROZEN.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - assertEquals("Active object is the same", r3, RegistryManager.ACTIVE.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); + + //Frozen data should be the same, as there are no replacement + ForgeRegistry frozen = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry(RTest.class); + assertEquals("Frozen object not the same", r3, frozen.getValue(name)); + assertEquals("Active object not the same", r3, active.getValue(name)); + + // r3 is in the snapshot, so the ID SHOULD change to whats in the snapshot. + GameData.injectSnapshot(ss, false, true); //Unlike the old system we die on missing mappings for custom registries. So we need to tell it to continue loading as if we're local + assertNotEquals("IDs match", r3id, active.getID(r3)); + assertEquals("Frozen object not the same", r3, frozen.getValue(name)); + assertEquals("Active object not the same", r3, active.getValue(name)); + + // Frozen has the original ID GameData.revertToFrozen(); - assertEquals("IDs match", r3id, r.getID(r3)); - assertEquals("Frozen object is the same", r3, RegistryManager.FROZEN.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - assertEquals("Active object is the same", r3, RegistryManager.ACTIVE.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); + assertEquals("IDs don't match", r3id, active.getID(r3)); + assertEquals("Frozen object not the same", r3, frozen.getValue(name)); + assertEquals("Active object not the same", r3, active.getValue(name)); + + // Make sure we have snapshot ID again GameData.injectSnapshot(ss, true, true); - assertNotEquals("IDs don't match", r3id, r.getID(r3)); - assertEquals("Frozen object is the same", r3, RegistryManager.FROZEN.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - assertEquals("Active object is the same", r3, RegistryManager.ACTIVE.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); + assertNotEquals("IDs match", r3id, active.getID(r3)); + assertEquals("Frozen object not the same", r3, frozen.getValue(name)); + assertEquals("Active object not the same", r3, active.getValue(name)); + + //And back to the frozen ID GameData.revertToFrozen(); - assertEquals("IDs match", r3id, r.getID(r3)); - assertEquals("Frozen object is the same", r3, RegistryManager.FROZEN.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); - assertEquals("Active object is the same", r3, RegistryManager.ACTIVE.getRegistry(RTest.class).getValue(new ResourceLocation("test3"))); + assertEquals("IDs don't match", r3id, active.getID(r3)); + assertEquals("Frozen object not the same", r3, frozen.getValue(name)); + assertEquals("Active object not the same", r3, active.getValue(name)); } } diff --git a/src/test/java/net/minecraftforge/fml/common/registry/ItemBlockSubstitutionRemoveRestoreTest.java b/src/test/java/net/minecraftforge/fml/common/registry/ItemBlockSubstitutionRemoveRestoreTest.java index 03a1dffda..e1c78bf83 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/ItemBlockSubstitutionRemoveRestoreTest.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/ItemBlockSubstitutionRemoveRestoreTest.java @@ -86,11 +86,11 @@ public class ItemBlockSubstitutionRemoveRestoreTest snapshot.get(GameData.ITEMS).substitutions.clear(); GameData.injectSnapshot(snapshot, false, false); ObjectHolderRegistry.INSTANCE.applyObjectHolders(); - */ dirtitem = (ItemBlock) itemRegistry.getValue(myDirt); assertEquals("ItemBlock points at vanilla block", originalDirt, dirtitem); assertNotEquals("ItemBlock points at my block", myDirtInstance, dirtitem); + */ // TEST 3: Does the substitute get restored when reverting to frozen state? The substitute should be found in the registry again GameData.revertToFrozen(); diff --git a/src/test/java/net/minecraftforge/fml/common/registry/VanillaRegistryTests.java b/src/test/java/net/minecraftforge/fml/common/registry/VanillaRegistryTests.java index 8f7589fca..33787dc2a 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/VanillaRegistryTests.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/VanillaRegistryTests.java @@ -18,6 +18,8 @@ import org.junit.runner.RunWith; import static org.junit.Assert.*; +import java.lang.reflect.Field; + /** * Vanilla registry tests */ @@ -42,7 +44,7 @@ public class VanillaRegistryTests // Our lookups find the same stuff vanilla sees final IForgeRegistry blocks = RegistryManager.ACTIVE.getRegistry(Block.class); - assertEquals("We have the right blocks for a block", blocks, Block.REGISTRY); + assertEquals("We have a different block registry then vanilla", blocks, getDelegate(Block.REGISTRY)); // We can look up stuff through our APIs Block bl = blocks.getValue(new ResourceLocation("minecraft:air")); @@ -54,7 +56,7 @@ public class VanillaRegistryTests // Our lookups find the same stuff vanilla sees final IForgeRegistry items = RegistryManager.ACTIVE.getRegistry(Item.class); - assertEquals("We have the right items for an item", items, Item.REGISTRY); + assertEquals("We have a different item registry then vanilla", items, getDelegate(Item.REGISTRY)); // We can look up stuff through our APIs Item it = items.getValue(new ResourceLocation("minecraft:bed")); @@ -65,6 +67,20 @@ public class VanillaRegistryTests assertEquals("We got nothing (items) when we asked for cheese", null, none); } + private Object getDelegate(Object obj) + { + try + { + Field f = obj.getClass().getDeclaredField("delegate"); + f.setAccessible(true); + return f.get(obj); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + } + @Test public void testRegistration() { @@ -74,7 +90,7 @@ public class VanillaRegistryTests assertNotNull("Registered my block", myBlock); // Our lookups find the same stuff vanilla sees - assertEquals("We have the right blocks for a block", blocks, Block.REGISTRY); + assertEquals("We have a different block registry then vanilla", blocks, getDelegate(Block.REGISTRY)); Block found = blocks.getValue(new ResourceLocation("minecraft:testy")); assertEquals("Registry lookup works", myBlock, found);