Merge branch '1.7.10' - fix up some oredictionary and substitution related issues

src/main/java/net/minecraftforge/fml/common/Loader.java

@@ -979,14 +979,7 @@ public class Loader
 
     public void fireRemapEvent(Map<String, Integer[]> remapBlocks, Map<String, Integer[]> remapItems)
     {
-        if (remapBlocks.isEmpty() && remapItems.isEmpty())
+	modController.propogateStateMessage(new FMLModIdMappingEvent(remapBlocks, remapItems));
-        {
-            FMLLog.finer("Skipping remap event - no remaps occured");
-        }
-        else
-        {
-            modController.propogateStateMessage(new FMLModIdMappingEvent(remapBlocks, remapItems));
-        }
     }
 
     public void runtimeDisableMod(String modId)

src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java

@@ -10,8 +10,10 @@ import java.util.Map;
 import java.util.Set;
 
 import org.apache.commons.lang3.Validate;
+import org.apache.logging.log4j.Level;
 
 import net.minecraft.block.Block;
+import net.minecraft.item.Item;
 import net.minecraft.item.ItemBanner;
 import net.minecraft.item.ItemBlock;
 import net.minecraft.util.ObjectIntIdentityMap;
@@ -20,6 +22,7 @@ import net.minecraft.util.RegistryNamespacedDefaultedByKey;
 import net.minecraft.util.ResourceLocation;
 import net.minecraftforge.fml.common.FMLLog;
 import net.minecraftforge.fml.common.functions.GenericIterableFactory;
+import net.minecraftforge.fml.common.registry.RegistryDelegate.Delegate;
 
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
@@ -54,6 +57,7 @@ public class FMLControlledNamespacedRegistry<I> extends RegistryNamespacedDefaul
         {
             int id = getId(obj);
             Object name = getNameForObject(obj);
+            boolean isSubstituted = activeSubstitutions.containsKey(name);
 
             // name lookup failed -> obj is not in the obj<->name map
             if (name == null) throw new IllegalStateException(String.format("Registry entry for %s %s, id %d, doesn't yield a name.", type, obj, id));
@@ -63,19 +67,21 @@ public class FMLControlledNamespacedRegistry<I> extends RegistryNamespacedDefaul
             if (loc == null && nameS == null) throw new IllegalStateException(String.format("Registry entry for %s %s name is invalid, must be a String or ResourceLocation %s", type, obj, name));
 
             // id lookup failed -> obj is not in the obj<->id map
-            if (id < 0) throw new IllegalStateException(String.format("Registry entry for %s %s, name %s, doesn't yield an id.", type, obj, name));
+            if (!isSubstituted && id < 0) throw new IllegalStateException(String.format("Registry entry for %s %s, name %s, doesn't yield an id.", type, obj, name));
             // id is too high
             if (id > maxId) throw new IllegalStateException(String.format("Registry entry for %s %s, name %s uses the too large id %d.", type, obj, name));
             // empty name
             if (name.toString().isEmpty()) throw new IllegalStateException(String.format("Registry entry for %s %s, id %d, yields an empty name.", type, obj, id));
             // non-prefixed name
             if (name.toString().indexOf(':') == -1) throw new IllegalStateException(String.format("Registry entry for %s %s, id %d, has the non-prefixed name %s.", type, obj, id, name));
+            // the rest of the tests don't really work for substituted items or blocks
+            if (isSubstituted) continue;
             // id -> obj lookup is inconsistent
             if (getRaw(id) != obj) throw new IllegalStateException(String.format("Registry entry for id %d, name %s, doesn't yield the expected %s %s.", id, name, type, obj));
             // name -> obj lookup is inconsistent
-            if (!(activeSubstitutions.containsKey(name) || activeSubstitutions.containsValue(name)) && getRaw(nameS) != obj ) throw new IllegalStateException(String.format("Registry entry for name %s, id %d, doesn't yield the expected %s %s.", name, id, type, obj));
+            if (getRaw(nameS) != obj ) throw new IllegalStateException(String.format("Registry entry for name %s, id %d, doesn't yield the expected %s %s.", name, id, type, obj));
             // name -> id lookup is inconsistent
-            if (!(activeSubstitutions.containsKey(name) || activeSubstitutions.containsValue(name)) && getId(nameS) != id) throw new IllegalStateException(String.format("Registry entry for name %s doesn't yield the expected id %d.", name, id));
+            if (getId(nameS) != id) throw new IllegalStateException(String.format("Registry entry for name %s doesn't yield the expected id %d.", name, id));
             // id isn't marked as unavailable
             if (!availabilityMap.get(id)) throw new IllegalStateException(String.format("Registry entry for %s %s, id %d, name %s, marked as empty.", type, obj, id, name));
             // entry is blocked, thus should be empty
@@ -510,6 +516,19 @@ public class FMLControlledNamespacedRegistry<I> extends RegistryNamespacedDefaul
     {
         if (getPersistentSubstitutions().containsKey(nameToReplace))
         {
+            I original = getRaw(nameToReplace);
+            if (superType == Item.class) {
+                Item sub = (Item) getPersistentSubstitutions().get(nameToReplace);
+                if (original == null) {
+                    // When we're activated from the server side, we need to set the delegate on the original instance to
+                    // point to us. Go to the "default state" registry to get it
+                    original = (I)GameData.getItemRegistry().getRaw(nameToReplace);
+                }
+                FMLLog.log(Level.DEBUG, "Replacing %s with %s (name %s)", original, sub, nameToReplace);
+                Delegate<Item> delegate = (Delegate<Item>)((Item)original).delegate;
+                delegate.changeReference(sub);
+                ((Delegate<Item>)sub.delegate).setName(nameToReplace);
+            }
             activeSubstitutions.put(nameToReplace, getPersistentSubstitutions().get(nameToReplace));
         }
     }
@@ -537,6 +556,7 @@ public class FMLControlledNamespacedRegistry<I> extends RegistryNamespacedDefaul
             FMLLog.severe("The substitute %s for %s is registered into the game independently. This won't work", replacement.getClass().getName(), nameToReplace);
             throw new IllegalArgumentException("The object substitution is already registered. This won't work");
         }
+        FMLLog.log(Level.DEBUG, "Adding substitution %s with %s (name %s)", original, replacement, nameToReplace);
         getPersistentSubstitutions().put(nameToReplace, replacement);
     }
 
@@ -555,9 +575,28 @@ public class FMLControlledNamespacedRegistry<I> extends RegistryNamespacedDefaul
         if (this.optionalDefaultKey != null)
             Validate.notNull(this.optionalDefaultObject);
     }
+    /*
+     * This iterator is used by some regular MC methods to visit all blocks, we need to include substitutions
+     * Compare #typeSafeIterable()
+     */
+    @SuppressWarnings("unchecked")
+    @Override
+    public Iterator<I> iterator()
+    {
+        return Iterators.concat(super.iterator(),getPersistentSubstitutions().values().iterator());
+    }
+
 
 
     FMLControlledNamespacedRegistry<I> makeShallowCopy() {
         return new FMLControlledNamespacedRegistry<I>(optionalDefaultKey, maxId, minId, superType);
     }
+    // ONLY CALLED ON ITEM registry
+    void resetSubstitutionDelegates()
+    {
+        for (I item: typeSafeIterable()) {
+            Delegate<Item> delegate = (Delegate<Item>)((Item)item).delegate;
+            delegate.changeReference((Item)item);
+        }
+    }
 }

src/main/java/net/minecraftforge/fml/common/registry/GameData.java

@@ -351,6 +351,8 @@ public class GameData {
         getMain().iBlockRegistry.dump();
         getMain().iItemRegistry.dump();
 
+        getMain().iItemRegistry.resetSubstitutionDelegates();
+
         GameDataSnapshot.Entry blocks = snapshot.entries.get("fml:blocks");
         GameDataSnapshot.Entry items = snapshot.entries.get("fml:items");
 
@@ -655,6 +657,8 @@ public class GameData {
 
             getMain().set(frozen);
         }
+        // the id mapping has reverted, fire remap events for those that care about id changes
+        Loader.instance().fireRemapEvent(ImmutableMap.<String,Integer[]>of(), ImmutableMap.<String,Integer[]>of());
         // the id mapping has reverted, ensure we sync up the object holders
         ObjectHolderRegistry.INSTANCE.applyObjectHolders();
     }

src/main/java/net/minecraftforge/oredict/OreDictionary.java

@@ -13,6 +13,8 @@ import java.util.RandomAccess;
 import java.util.Map.Entry;
 import java.util.Set;
 
+import org.apache.logging.log4j.Level;
+
 import net.minecraft.block.Block;
 import net.minecraft.init.Blocks;
 import net.minecraft.init.Items;
@@ -29,7 +31,9 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
 import net.minecraftforge.fml.common.FMLLog;
+import net.minecraftforge.fml.common.Loader;
 import net.minecraftforge.fml.common.eventhandler.Event;
+import net.minecraftforge.fml.common.registry.GameData;
 
 public class OreDictionary
 {
@@ -324,7 +328,20 @@ public class OreDictionary
 
         Set<Integer> set = new HashSet<Integer>();
 
-        int id = Item.getIdFromItem(stack.getItem());
+        // HACK: use the registry name's ID. It is unique and it knows about substitutions. Fallback to a -1 value (what Item.getIDForItem would have returned) in the case where the registry is not aware of the item yet
+        // IT should be noted that -1 will fail the gate further down, if an entry already exists with value -1 for this name. This is what is broken and being warned about.
+        // APPARENTLY it's quite common to do this. OreDictionary should be considered alongside Recipes - you can't make them properly until you've registered with the game.
+        String registryName = stack.getItem().delegate.name();
+        int id;
+        if (registryName == null)
+        {
+            FMLLog.log(Level.DEBUG, "Attempted to find the oreIDs for an unregistered object (%s). This won't work very well.", stack);
+            return new int[0];
+        }
+        else
+        {
+            id = GameData.getItemRegistry().getId(registryName);
+        }
         List<Integer> ids = stackToId.get(id);
         if (ids != null) set.addAll(ids);
         ids = stackToId.get(id | ((stack.getItemDamage() + 1) << 16));
@@ -428,9 +445,29 @@ public class OreDictionary
     private static void registerOreImpl(String name, ItemStack ore)
     {
         if ("Unknown".equals(name)) return; //prevent bad IDs.
+        if (ore == null || ore.getItem() == null)
+        {
+            FMLLog.bigWarning("Invalid registration attempt for an Ore Dictionary item with name %s has occurred. The registration has been denied to prevent crashes. The mod responsible for the registration needs to correct this.", name);
+            return; //prevent bad ItemStacks.
+        }
 
         int oreID = getOreID(name);
-        int hash = Item.getIdFromItem(ore.getItem());
+        // HACK: use the registry name's ID. It is unique and it knows about substitutions. Fallback to a -1 value (what Item.getIDForItem would have returned) in the case where the registry is not aware of the item yet
+        // IT should be noted that -1 will fail the gate further down, if an entry already exists with value -1 for this name. This is what is broken and being warned about.
+        // APPARENTLY it's quite common to do this. OreDictionary should be considered alongside Recipes - you can't make them properly until you've registered with the game.
+        String registryName = ore.getItem().delegate.name();
+        int hash;
+        if (registryName == null)
+        {
+            FMLLog.bigWarning("A broken ore dictionary registration with name %s has occurred. It adds an item (type: %s) which is currently unknown to the game registry. This dictionary item can only support a single value when"
+                    + " registered with ores like this, and NO I am not going to turn this spam off. Just register your ore dictionary entries after the GameRegistry.\n"
+                    + "TO USERS: YES this is a BUG in the mod "+Loader.instance().activeModContainer().getName()+" report it to them!", name, ore.getItem().getClass());
+            hash = -1;
+        }
+        else
+        {
+            hash = GameData.getItemRegistry().getId(registryName);
+        }
         if (ore.getItemDamage() != WILDCARD_VALUE)
         {
             hash |= ((ore.getItemDamage() + 1) << 16); // +1 so 0 is significant
@@ -474,7 +511,18 @@ public class OreDictionary
             if (ores == null) continue;
             for (ItemStack ore : ores)
             {
-                int hash = Item.getIdFromItem(ore.getItem());
+                // HACK: use the registry name's ID. It is unique and it knows about substitutions
+                String name = ore.getItem().delegate.name();
+                int hash;
+                if (name == null)
+                {
+                    FMLLog.log(Level.DEBUG, "Defaulting unregistered ore dictionary entry for ore dictionary %s: type %s to -1", getOreName(id), ore.getItem().getClass());
+                    hash = -1;
+                }
+                else
+                {
+                    hash = GameData.getItemRegistry().getId(name);
+                }
                 if (ore.getItemDamage() != WILDCARD_VALUE)
                 {
                     hash |= ((ore.getItemDamage() + 1) << 16); // +1 so meta 0 is significant