diff --git a/src/main/java/net/minecraftforge/fml/common/FMLContainer.java b/src/main/java/net/minecraftforge/fml/common/FMLContainer.java index e62741a33..c400684cb 100644 --- a/src/main/java/net/minecraftforge/fml/common/FMLContainer.java +++ b/src/main/java/net/minecraftforge/fml/common/FMLContainer.java @@ -147,6 +147,16 @@ public final class FMLContainer extends DummyModContainer implements WorldAccess } data.setTag("aliases", aliases); + NBTTagList overrides = new NBTTagList(); + for (Entry entry : e.getValue().overrides.entrySet()) + { + NBTTagCompound tag = new NBTTagCompound(); + tag.setString("K", entry.getKey().toString()); + tag.setString("V", entry.getValue().toString()); + aliases.appendTag(tag); + } + data.setTag("overrides", overrides); + int[] blocked = new int[e.getValue().blocked.size()]; int idx = 0; for (Integer i : e.getValue().blocked) diff --git a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeMessage.java b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeMessage.java index efac5f2d0..88f95a26b 100644 --- a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeMessage.java +++ b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeMessage.java @@ -184,12 +184,14 @@ public abstract class FMLHandshakeMessage { this.name = name; this.ids = entry.ids; this.dummied = entry.dummied; + this.overrides = entry.overrides; } private boolean hasMore; private ResourceLocation name; private Map ids; private Set dummied; + private Map overrides; @Override public void fromBytes(ByteBuf buffer) @@ -206,13 +208,20 @@ public abstract class FMLHandshakeMessage { } length = ByteBufUtils.readVarInt(buffer, 3); - dummied = Sets.newHashSet(); for (int i = 0; i < length; i++) { dummied.add(new ResourceLocation(ByteBufUtils.readUTF8String(buffer))); } + + length = ByteBufUtils.readVarInt(buffer, 3); + overrides = Maps.newHashMap(); + + for (int i = 0; i < length; i++) + { + overrides.put(new ResourceLocation(ByteBufUtils.readUTF8String(buffer)), ByteBufUtils.readUTF8String(buffer)); + } } @Override @@ -233,6 +242,13 @@ public abstract class FMLHandshakeMessage { { ByteBufUtils.writeUTF8String(buffer, entry.toString()); } + + ByteBufUtils.writeVarInt(buffer, overrides.size(), 3); + for (Entry entry: overrides.entrySet()) + { + ByteBufUtils.writeUTF8String(buffer, entry.getKey().toString()); + ByteBufUtils.writeUTF8String(buffer, entry.getValue().toString()); + } } public Map getIdMap() diff --git a/src/main/java/net/minecraftforge/registries/ForgeRegistry.java b/src/main/java/net/minecraftforge/registries/ForgeRegistry.java index 89a5113d5..c0ed23ea1 100644 --- a/src/main/java/net/minecraftforge/registries/ForgeRegistry.java +++ b/src/main/java/net/minecraftforge/registries/ForgeRegistry.java @@ -1,6 +1,5 @@ package net.minecraftforge.registries; -import java.lang.ref.WeakReference; import java.util.BitSet; import java.util.Collection; import java.util.Collections; @@ -25,11 +24,14 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Sets; - import net.minecraft.util.ResourceLocation; import net.minecraftforge.event.RegistryEvent; import net.minecraftforge.event.RegistryEvent.MissingMappings; +import net.minecraftforge.fml.common.FMLContainer; import net.minecraftforge.fml.common.FMLLog; +import net.minecraftforge.fml.common.InjectedModContainer; +import net.minecraftforge.fml.common.Loader; +import net.minecraftforge.fml.common.ModContainer; import net.minecraftforge.fml.relauncher.ReflectionHelper; public class ForgeRegistry> implements IForgeRegistryInternal, IForgeRegistryModifiable @@ -48,7 +50,8 @@ public class ForgeRegistry> implements IForgeRe private final BitSet availabilityMap; private final Set dummies = Sets.newHashSet(); private final Set blocked = Sets.newHashSet(); - private final Multimap> overrides = ArrayListMultimap.create(); + private final Multimap overrides = ArrayListMultimap.create(); + private final BiMap override_owners = HashBiMap.create(); private final DummyFactory dummyFactory; private final boolean isDelegated; private final int min; @@ -78,7 +81,6 @@ public class ForgeRegistry> implements IForgeRe this.create.onCreate(this, stage); } - @Override public void register(V value) { @@ -227,6 +229,13 @@ public class ForgeRegistry> implements IForgeRe } int add(int id, V value) + { + ModContainer mc = Loader.instance().activeModContainer(); + String owner = mc == null || (mc instanceof InjectedModContainer && ((InjectedModContainer)mc).wrappedContainer instanceof FMLContainer) ? null : mc.getModId().toLowerCase(); + return add(id, value, owner); + } + + int add(int id, V value, String owner) { ResourceLocation key = value == null ? null : value.getRegistryName(); Preconditions.checkNotNull(key, "Can't use a null-name for the registry, object %s.", value); @@ -249,7 +258,12 @@ public class ForgeRegistry> implements IForgeRe { if (!this.allowOverrides) throw new IllegalArgumentException(String.format("The name %s has been registered twice, for %s and %s.", key, getRaw(key), value)); + if (owner == null) + throw new IllegalStateException(String.format("Could not determine owner for the override on %s. Value: %s", key, value)); + this.override_owners.put(new OverrideOwner(owner, key), value); idToUse = this.getID(oldEntry); + if (!this.override_owners.containsValue(oldEntry)) + this.override_owners.put(new OverrideOwner(key.getResourceDomain(), key), oldEntry); } Integer foundId = this.ids.inverse().get(value); //Is this ever possible to trigger with otherThing being different? @@ -278,7 +292,7 @@ public class ForgeRegistry> implements IForgeRe getDelegate(value).setName(key); if (oldEntry != null) { - this.overrides.put(key, new WeakReference(oldEntry)); + this.overrides.put(key, oldEntry); if (this.stage == RegistryManager.ACTIVE) getDelegate(oldEntry).changeReference(value); } @@ -348,16 +362,8 @@ public class ForgeRegistry> implements IForgeRe for (V value : this) getDelegate(value).changeReference(value); - Iterator>> itr = this.overrides.entries().iterator(); - while (itr.hasNext()) - { - Entry> next = itr.next(); - V value = next.getValue().get(); - if (value == null) - itr.remove(); - else - getDelegate(value).changeReference(value); - } + for (V value: this.overrides.values()) + getDelegate(value).changeReference(value); } V getDefault() @@ -437,17 +443,43 @@ public class ForgeRegistry> implements IForgeRe this.names.clear(); this.availabilityMap.clear(0, this.availabilityMap.length()); this.defaultValue = null; + this.override_owners.clear(); + this.override_owners.putAll(from.override_owners); boolean errored = false; for (Entry entry : from.names.entrySet()) { + List overrides = Lists.newArrayList(from.overrides.get(entry.getKey())); int id = from.getID(entry.getKey()); - int realId = add(id, entry.getValue()); - if (id != realId && id != -1) + if (overrides.isEmpty()) { - FMLLog.warning("Registered object did not get ID it asked for. Name: {} Type: {} Expected: {} Got: {}", entry.getKey(), this.getRegistrySuperType().getName(), id, realId); - errored = true; + int realId = add(id, entry.getValue()); + if (id != realId && id != -1) + { + FMLLog.warning("Registered object did not get ID it asked for. Name: %s Type: %s Expected: %s Got: %s", entry.getKey(), this.getRegistrySuperType().getName(), id, realId); + errored = true; + } + } + else + { + overrides.add(entry.getValue()); + for (V value : overrides) + { + OverrideOwner owner = from.override_owners.inverse().get(value); + if (owner == null) + { + FMLLog.warning("Registered override did not have an associated owner object. Name: %s Type: %s Value: %s", entry.getKey(), this.getRegistrySuperType().getName(), value); + errored = true; + continue; + } + int realId = add(id, value, owner.owner); + if (id != realId && id != -1) + { + FMLLog.warning("Registered object did not get ID it asked for. Name: %s Type: %s Expected: %s Got: %s", entry.getKey(), this.getRegistrySuperType().getName(), id, realId); + errored = true; + } + } } } @@ -540,11 +572,11 @@ public class ForgeRegistry> implements IForgeRe Collections.sort(ids); - FMLLog.finer("Registry Name : {}", name); + FMLLog.finer("Registry Name : %s", name); ids.forEach(id -> FMLLog.finer("Registry: %d %s %s", id, getKey(getValue(id)), getValue(id))); } - public void loadIds(Map ids, Map missing, Map remapped, ForgeRegistry old, ResourceLocation name) + public void loadIds(Map ids, Map overrides, Map missing, Map remapped, ForgeRegistry old, ResourceLocation name) { for (Map.Entry entry : ids.entrySet()) { @@ -566,12 +598,35 @@ public class ForgeRegistry> implements IForgeRe V obj = old.getRaw(itemName); Preconditions.checkState(obj != null, "objectKey has an ID but no object. Reflection/ASM hackery? Registry bug?"); - // Register all overrides so new registry has the full list so we can update delegates later - old.overrides.get(obj.getRegistryName()).stream() - .filter(e -> e.get() != null) - .forEach(e -> add(newId, e.get())); + String primaryName = null; + if (!overrides.containsKey(itemName) && old.overrides.containsKey(itemName)) + { + obj = old.overrides.get(itemName).iterator().next(); //Get the first one in the list, Which should be the first one registered + primaryName = old.override_owners.inverse().get(obj).owner; + } + else + primaryName = overrides.get(itemName); - add(newId, obj); + for (V value : old.overrides.get(itemName)) + { + OverrideOwner owner = old.override_owners.inverse().get(value); + if (owner == null) + { + FMLLog.warning("Registered override did not have an associated owner object. Name: %s Type: %s Value: %s", entry.getKey(), this.getRegistrySuperType().getName(), value); + continue; + } + + if (primaryName.equals(owner.owner)) + continue; + + int realId = add(newId, value, owner.owner); + if (newId != realId) + FMLLog.warning("Registered object did not get ID it asked for. Name: %s Type: %s Expected: %s Got: %s", entry.getKey(), this.getRegistrySuperType().getName(), newId, realId); + } + + int realId = add(newId, obj, primaryName == null ? itemName.getResourceDomain() : primaryName); + if (realId != newId) + FMLLog.warning("Registered object did not get ID it asked for. Name: %s Type: %s Expected: %s Got: %s", entry.getKey(), this.getRegistrySuperType().getName(), newId, realId); } } @@ -603,6 +658,18 @@ public class ForgeRegistry> implements IForgeRe this.aliases.forEach((from, to) -> ret.aliases.put(from, to)); this.blocked.forEach(id -> ret.blocked.add(id)); this.dummies.forEach(name -> ret.dummied.add(name)); + ret.overrides.putAll(getOverrideOwners()); + return ret; + } + + Map getOverrideOwners() + { + Map ret = Maps.newHashMap(); + for (ResourceLocation key : this.overrides.keySet()) + { + V obj = this.names.get(key); + ret.put(key, this.override_owners.inverse().get(obj).owner); + } return ret; } @@ -612,6 +679,7 @@ public class ForgeRegistry> implements IForgeRe public final Map aliases = Maps.newHashMap(); public final Set blocked = Sets.newHashSet(); public final Set dummied = Sets.newHashSet(); + public final Map overrides = Maps.newHashMap(); } public MissingMappings getMissingEvent(ResourceLocation name, Map map) @@ -679,4 +747,32 @@ public class ForgeRegistry> implements IForgeRe if (failed.isEmpty() && ignored > 0) FMLLog.fine("There were %d missing mappings that have been ignored", ignored); } + + private static class OverrideOwner + { + final String owner; + final ResourceLocation key; + private OverrideOwner(String owner, ResourceLocation key) + { + this.owner = owner; + this.key = key; + } + + public boolean equals(Object o) + { + if (this == o) + return true; + + if (!(o instanceof OverrideOwner)) + return false; + + OverrideOwner oo = (OverrideOwner)o; + return this.owner.equals(oo.owner) && this.key.equals(oo.key); + } + + public int hashCode() + { + return 31 * this.key.hashCode() + this.owner.hashCode(); + } + } } diff --git a/src/main/java/net/minecraftforge/registries/GameData.java b/src/main/java/net/minecraftforge/registries/GameData.java index 95cdd8771..6eac6055e 100644 --- a/src/main/java/net/minecraftforge/registries/GameData.java +++ b/src/main/java/net/minecraftforge/registries/GameData.java @@ -230,6 +230,7 @@ public class GameData FMLLog.warning("Can't revert to frozen GameData state without freezing first."); return; } + RegistryManager.ACTIVE.registries.forEach((name, reg) -> reg.resetDelegates()); FMLLog.fine("Reverting to frozen data state."); for (Map.Entry>> r : RegistryManager.ACTIVE.registries.entrySet()) @@ -626,7 +627,7 @@ public class GameData snap.blocked.forEach(id -> _new.block(id)); // Load current dummies BEFORE the snapshot is loaded so that add() will remove from the list. snap.dummied.forEach(key -> _new.addDummy(key)); - _new.loadIds(snap.ids, missing, remaps, active, name); + _new.loadIds(snap.ids, snap.overrides, missing, remaps, active, name); } //Another bouncer for generic reasons @@ -645,7 +646,7 @@ public class GameData ForgeRegistry newRegistry = STAGING.getRegistry(name, RegistryManager.FROZEN); Map _new = Maps.newHashMap(); frozen.getKeys().stream().filter(key -> !newRegistry.containsKey(key)).forEach(key -> _new.put(key, frozen.getID(key))); - newRegistry.loadIds(_new, Maps.newLinkedHashMap(), remaps, frozen, name); + newRegistry.loadIds(_new, frozen.getOverrideOwners(), Maps.newLinkedHashMap(), remaps, frozen, name); } public static void fireCreateRegistryEvents() diff --git a/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionInjectionTest.java b/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionInjectionTest.java index e1b905805..68b3a1f59 100644 --- a/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionInjectionTest.java +++ b/src/test/java/net/minecraftforge/fml/common/registry/SubstitutionInjectionTest.java @@ -58,10 +58,6 @@ public class SubstitutionInjectionTest @Test public void testSubstitutionInjection() throws Exception { - //TODO: Decide exactly how I want to deal with subs, this test doesn't really do anything right now - if (true == Boolean.valueOf("true").booleanValue()) - return; - final ForgeRegistry blockRegistry = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry(Block.class); final ForgeRegistry itemRegistry = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry(Item.class); @@ -111,7 +107,7 @@ public class SubstitutionInjectionTest assertEquals("ObjectHolder didn't apply - Blocks and registry", currDirt, fnd); assertEquals("Got my dirt substitute - registry", vanilDirt, fnd); dirtitem = (ItemBlock) itemRegistry.getValue(MC_DIRT); - assertEquals("ItemBlock points at my block", toSub, dirtitem.getBlock()); + assertEquals("ItemBlock points at my block", vanilDirt, dirtitem.getBlock()); // TEST 3: Does the substitute get restored when reverting to frozen state? The substitute should be found in the registry again GameData.revertToFrozen(); @@ -133,7 +129,7 @@ public class SubstitutionInjectionTest assertEquals("ObjectHolder didn't apply - Blocks and registry", currDirt, fnd); assertEquals("Got my dirt substitute - registry", vanilDirt, fnd); dirtitem = (ItemBlock) itemRegistry.getValue(MC_DIRT); - assertEquals("ItemBlock points at my block", toSub, dirtitem.getBlock()); + assertEquals("ItemBlock points at my block", vanilDirt, dirtitem.getBlock()); // TEST 3 repeat: Does the substitute get restored when reverting to frozen state? The substitute should be found in the registry again GameData.revertToFrozen();