Implement override tracking and syncing.

Overrides MUST be registered within a tracked event so we know what mod it came from.
This will allow servers/saves to select which mod 'wins' and becomes to active entry.
This should also mean that when connecting to a vanilla server things will revert.

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<ResourceLocation, String> 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)

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<ResourceLocation, Integer> ids;
         private Set<ResourceLocation> dummied;
+        private Map<ResourceLocation, String> 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<ResourceLocation, String> entry: overrides.entrySet())
+            {
+                ByteBufUtils.writeUTF8String(buffer, entry.getKey().toString());
+                ByteBufUtils.writeUTF8String(buffer, entry.getValue().toString());
+            }
         }
 
         public Map<ResourceLocation, Integer> getIdMap()

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<V extends IForgeRegistryEntry<V>> implements IForgeRegistryInternal<V>, IForgeRegistryModifiable<V>
@@ -48,7 +50,8 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> implements IForgeRe
     private final BitSet availabilityMap;
     private final Set<ResourceLocation> dummies = Sets.newHashSet();
     private final Set<Integer> blocked = Sets.newHashSet();
-    private final Multimap<ResourceLocation, WeakReference<V>> overrides = ArrayListMultimap.create();
+    private final Multimap<ResourceLocation, V> overrides = ArrayListMultimap.create();
+    private final BiMap<OverrideOwner, V> override_owners = HashBiMap.create();
     private final DummyFactory<V> dummyFactory;
     private final boolean isDelegated;
     private final int min;
@@ -78,7 +81,6 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> implements IForgeRe
             this.create.onCreate(this, stage);
     }
 
-
     @Override
     public void register(V value)
     {
@@ -227,6 +229,13 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> 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<V extends IForgeRegistryEntry<V>> 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<V extends IForgeRegistryEntry<V>> implements IForgeRe
             getDelegate(value).setName(key);
             if (oldEntry != null)
             {
-                this.overrides.put(key, new WeakReference<V>(oldEntry));
+                this.overrides.put(key, oldEntry);
                 if (this.stage == RegistryManager.ACTIVE)
                     getDelegate(oldEntry).changeReference(value);
             }
@@ -348,16 +362,8 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> implements IForgeRe
         for (V value : this)
             getDelegate(value).changeReference(value);
 
-        Iterator<Entry<ResourceLocation, WeakReference<V>>> itr = this.overrides.entries().iterator();
+        for (V value: this.overrides.values())
-        while (itr.hasNext())
+            getDelegate(value).changeReference(value);
-        {
-            Entry<ResourceLocation, WeakReference<V>> next = itr.next();
-            V value = next.getValue().get();
-            if (value == null)
-                itr.remove();
-            else
-                getDelegate(value).changeReference(value);
-        }
     }
 
     V getDefault()
@@ -437,17 +443,43 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> 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<ResourceLocation, V> entry : from.names.entrySet())
         {
+            List<V> overrides = Lists.newArrayList(from.overrides.get(entry.getKey()));
             int id = from.getID(entry.getKey());
-            int realId = add(id, entry.getValue());
+            if (overrides.isEmpty())
-            if (id != realId && id != -1)
             {
-                FMLLog.warning("Registered object did not get ID it asked for. Name: {} Type: {} Expected: {} Got: {}", entry.getKey(), this.getRegistrySuperType().getName(), id, realId);
+                int realId = add(id, entry.getValue());
-                errored = true;
+                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<V extends IForgeRegistryEntry<V>> 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<ResourceLocation, Integer> ids, Map<ResourceLocation, Integer> missing, Map<ResourceLocation, Integer[]> remapped, ForgeRegistry<V> old, ResourceLocation name)
+    public void loadIds(Map<ResourceLocation, Integer> ids, Map<ResourceLocation, String> overrides, Map<ResourceLocation, Integer> missing, Map<ResourceLocation, Integer[]> remapped, ForgeRegistry<V> old, ResourceLocation name)
     {
         for (Map.Entry<ResourceLocation, Integer> entry : ids.entrySet())
         {
@@ -566,12 +598,35 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> 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
+            String primaryName = null;
-            old.overrides.get(obj.getRegistryName()).stream()
+            if (!overrides.containsKey(itemName) && old.overrides.containsKey(itemName))
-                .filter(e -> e.get() != null)
+            {
-                .forEach(e -> add(newId, e.get()));
+                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<V extends IForgeRegistryEntry<V>> 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<ResourceLocation, String> getOverrideOwners()
+    {
+        Map<ResourceLocation, String> 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<V extends IForgeRegistryEntry<V>> implements IForgeRe
         public final Map<ResourceLocation, ResourceLocation> aliases = Maps.newHashMap();
         public final Set<Integer> blocked = Sets.newHashSet();
         public final Set<ResourceLocation> dummied = Sets.newHashSet();
+        public final Map<ResourceLocation, String> overrides = Maps.newHashMap();
     }
 
     public MissingMappings<?> getMissingEvent(ResourceLocation name, Map<ResourceLocation, Integer> map)
@@ -679,4 +747,32 @@ public class ForgeRegistry<V extends IForgeRegistryEntry<V>> 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();
+        }
+    }
 }

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<ResourceLocation, ForgeRegistry<? extends IForgeRegistryEntry<?>>> 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<T> newRegistry = STAGING.getRegistry(name, RegistryManager.FROZEN);
         Map<ResourceLocation, Integer> _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()

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<Block> blockRegistry = (ForgeRegistry<Block>)RegistryManager.ACTIVE.getRegistry(Block.class);
         final ForgeRegistry<Item> itemRegistry = (ForgeRegistry<Item>)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();