diff --git a/src/main/java/net/minecraftforge/fml/common/Loader.java b/src/main/java/net/minecraftforge/fml/common/Loader.java index 55efa62e8..f2e35b869 100644 --- a/src/main/java/net/minecraftforge/fml/common/Loader.java +++ b/src/main/java/net/minecraftforge/fml/common/Loader.java @@ -979,14 +979,7 @@ public class Loader public void fireRemapEvent(Map remapBlocks, Map remapItems) { - if (remapBlocks.isEmpty() && remapItems.isEmpty()) - { - FMLLog.finer("Skipping remap event - no remaps occured"); - } - else - { - modController.propogateStateMessage(new FMLModIdMappingEvent(remapBlocks, remapItems)); - } + modController.propogateStateMessage(new FMLModIdMappingEvent(remapBlocks, remapItems)); } public void runtimeDisableMod(String modId) 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 db7bed7a2..81a3c914b 100644 --- a/src/main/java/net/minecraftforge/fml/common/registry/FMLControlledNamespacedRegistry.java +++ b/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 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 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 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 delegate = (Delegate)((Item)original).delegate; + delegate.changeReference(sub); + ((Delegate)sub.delegate).setName(nameToReplace); + } activeSubstitutions.put(nameToReplace, getPersistentSubstitutions().get(nameToReplace)); } } @@ -537,6 +556,7 @@ public class FMLControlledNamespacedRegistry 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 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 iterator() + { + return Iterators.concat(super.iterator(),getPersistentSubstitutions().values().iterator()); + } + FMLControlledNamespacedRegistry makeShallowCopy() { return new FMLControlledNamespacedRegistry(optionalDefaultKey, maxId, minId, superType); } + // ONLY CALLED ON ITEM registry + void resetSubstitutionDelegates() + { + for (I item: typeSafeIterable()) { + Delegate delegate = (Delegate)((Item)item).delegate; + delegate.changeReference((Item)item); + } + } } diff --git a/src/main/java/net/minecraftforge/fml/common/registry/GameData.java b/src/main/java/net/minecraftforge/fml/common/registry/GameData.java index 9745f1257..0d278bc1d 100644 --- a/src/main/java/net/minecraftforge/fml/common/registry/GameData.java +++ b/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.of(), ImmutableMap.of()); // the id mapping has reverted, ensure we sync up the object holders ObjectHolderRegistry.INSTANCE.applyObjectHolders(); } diff --git a/src/main/java/net/minecraftforge/oredict/OreDictionary.java b/src/main/java/net/minecraftforge/oredict/OreDictionary.java index 0f9799925..3a7de810f 100644 --- a/src/main/java/net/minecraftforge/oredict/OreDictionary.java +++ b/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 set = new HashSet(); - 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 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