From ac5c7124deab6972bc0f932044a88e11af896196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Zeutzheim?= Date: Mon, 31 Aug 2015 03:39:33 +0900 Subject: [PATCH 1/5] Fix possible crash in EventBus There is currently no way to check if an event handler has been registered or not. But when trying to unregister a not-registered event handler, Minecraft crashes with a NullPointerException. This is a simple fix to prevent such crashes. --- .../main/java/cpw/mods/fml/common/eventhandler/EventBus.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fml/src/main/java/cpw/mods/fml/common/eventhandler/EventBus.java b/fml/src/main/java/cpw/mods/fml/common/eventhandler/EventBus.java index d4570b0ef..49bcf0d5e 100644 --- a/fml/src/main/java/cpw/mods/fml/common/eventhandler/EventBus.java +++ b/fml/src/main/java/cpw/mods/fml/common/eventhandler/EventBus.java @@ -121,6 +121,8 @@ public class EventBus implements IEventExceptionHandler public void unregister(Object object) { ArrayList list = listeners.remove(object); + if (list == null) + return; for (IEventListener listener : list) { ListenerList.unregisterAll(busID, listener); From 945d3887d280cd9d2d198f2b073b27bda048c44d Mon Sep 17 00:00:00 2001 From: cpw Date: Thu, 29 Oct 2015 08:46:23 -0400 Subject: [PATCH 2/5] Fix substitutions for recipes and oredict recipes. Should mean that substitutions start working properly. --- .../FMLControlledNamespacedRegistry.java | 43 ++++++++++++++++--- .../mods/fml/common/registry/GameData.java | 1 + .../minecraftforge/oredict/OreDictionary.java | 7 ++- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java b/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java index ef6a1031f..ab5e9a576 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java @@ -9,7 +9,10 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.logging.log4j.Level; + import net.minecraft.block.Block; +import net.minecraft.item.Item; import net.minecraft.item.ItemBlock; import net.minecraft.util.ObjectIntIdentityMap; import net.minecraft.util.RegistryNamespaced; @@ -21,6 +24,7 @@ import com.google.common.collect.Iterators; import cpw.mods.fml.common.FMLLog; import cpw.mods.fml.common.functions.GenericIterableFactory; +import cpw.mods.fml.common.registry.RegistryDelegate.Delegate; public class FMLControlledNamespacedRegistry extends RegistryNamespaced { public static final boolean DEBUG = Boolean.parseBoolean(System.getProperty("fml.debugRegistryEntries", "false")); @@ -51,9 +55,10 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { { int id = getId(obj); String name = getNameForObject(obj); + boolean isSubstituted = activeSubstitutions.containsKey(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)); // name lookup failed -> obj is not in the obj<->name map @@ -62,12 +67,14 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { if (name.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.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(name) != 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(name) != 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(name) != id) throw new IllegalStateException(String.format("Registry entry for name %s doesn't yield the expected id %d.", name, id)); + if (getId(name) != 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 @@ -105,7 +112,8 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { for (I thing : registry.typeSafeIterable()) { - addObjectRaw(registry.getId(thing), registry.getNameForObject(thing), thing); + int id = registry.getId(thing); + addObjectRaw(id, registry.getNameForObject(thing), thing); } this.activeSubstitutions.putAll(registry.activeSubstitutions); } @@ -239,9 +247,9 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { * @return */ @SuppressWarnings("unchecked") - private I cast(Object obj) + private I cast(Object obj) { - return (I)(obj); + return (I)(obj); } /** * Get the object identified by the specified name. @@ -475,6 +483,19 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { { 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)); } } @@ -502,6 +523,7 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { 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); } @@ -529,4 +551,13 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { { return Iterators.concat(super.iterator(),getPersistentSubstitutions().values().iterator()); } + + // ONLY CALLED ON ITEM registry + void resetSubstitutionDelegates() + { + for (I item: typeSafeIterable()) { + Delegate delegate = (Delegate)((Item)item).delegate; + delegate.changeReference((Item)item); + } + } } diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java index aa7ec6c3d..13698082b 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java @@ -439,6 +439,7 @@ public class GameData { getMain().iBlockRegistry.dump(); getMain().iItemRegistry.dump(); + getMain().iItemRegistry.resetSubstitutionDelegates(); GameData newData = new GameData(); for (int id : blockedIds) diff --git a/src/main/java/net/minecraftforge/oredict/OreDictionary.java b/src/main/java/net/minecraftforge/oredict/OreDictionary.java index 5b5006092..e18053e0d 100644 --- a/src/main/java/net/minecraftforge/oredict/OreDictionary.java +++ b/src/main/java/net/minecraftforge/oredict/OreDictionary.java @@ -29,6 +29,7 @@ import com.google.common.collect.Maps; import cpw.mods.fml.common.FMLLog; import cpw.mods.fml.common.eventhandler.Event; +import cpw.mods.fml.common.registry.GameData; public class OreDictionary { @@ -493,7 +494,8 @@ public class OreDictionary } 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 + int hash = GameData.getItemRegistry().getId(ore.getItem().delegate.name()); if (ore.getItemDamage() != WILDCARD_VALUE) { hash |= ((ore.getItemDamage() + 1) << 16); // +1 so 0 is significant @@ -537,7 +539,8 @@ 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 + int hash = GameData.getItemRegistry().getId(ore.getItem().delegate.name()); if (ore.getItemDamage() != WILDCARD_VALUE) { hash |= ((ore.getItemDamage() + 1) << 16); // +1 so meta 0 is significant From a92f2a263b8d163cab2d4212f74d4f571d8269c6 Mon Sep 17 00:00:00 2001 From: cpw Date: Thu, 29 Oct 2015 12:46:12 -0400 Subject: [PATCH 3/5] OreDictionary will warn if there's an invalid ore being registered now, rather than just using -1 and doing weird things with the list as a result. --- .../minecraftforge/oredict/OreDictionary.java | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/minecraftforge/oredict/OreDictionary.java b/src/main/java/net/minecraftforge/oredict/OreDictionary.java index e18053e0d..229351890 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; @@ -28,6 +30,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import cpw.mods.fml.common.FMLLog; +import cpw.mods.fml.common.Loader; import cpw.mods.fml.common.eventhandler.Event; import cpw.mods.fml.common.registry.GameData; @@ -494,8 +497,22 @@ public class OreDictionary } int oreID = getOreID(name); - // HACK: use the registry name's ID. It is unique and it knows about substitutions - int hash = GameData.getItemRegistry().getId(ore.getItem().delegate.name()); + // 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 @@ -539,8 +556,18 @@ public class OreDictionary if (ores == null) continue; for (ItemStack ore : ores) { - // HACK: use the registry name's ID. It is unique and it knows about substitutions - int hash = GameData.getItemRegistry().getId(ore.getItem().delegate.name()); + // 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 From c474da04b3cae97c05f2f49b0a1f4bfb57763f8c Mon Sep 17 00:00:00 2001 From: cpw Date: Mon, 9 Nov 2015 15:20:53 -0500 Subject: [PATCH 4/5] Two more corner cases in the oredictionary. Should work for all cases now. --- .../minecraftforge/oredict/OreDictionary.java | 55 ++++++++++++++----- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/minecraftforge/oredict/OreDictionary.java b/src/main/java/net/minecraftforge/oredict/OreDictionary.java index 229351890..50dd6fc79 100644 --- a/src/main/java/net/minecraftforge/oredict/OreDictionary.java +++ b/src/main/java/net/minecraftforge/oredict/OreDictionary.java @@ -1,6 +1,7 @@ package net.minecraftforge.oredict; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -301,7 +302,20 @@ public class OreDictionary { if (stack == null || stack.getItem() == null) return -1; - 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 -1; + } + else + { + id = GameData.getItemRegistry().getId(registryName); + } List ids = stackToId.get(id); //Try the wildcard first if (ids == null || ids.size() == 0) { @@ -323,7 +337,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)); @@ -354,39 +381,39 @@ public class OreDictionary /** * Retrieves the List of items that are registered to this ore type at this instant. * If the flag is TRUE, then it will create the list as empty if it did not exist. - * + * * This option should be used by modders who are doing blanket scans in postInit. * It greatly reduces clutter in the OreDictionary is the responsible and proper * way to use the dictionary in a large number of cases. - * + * * The other function above is utilized in OreRecipe and is required for the * operation of that code. - * + * * @param name The ore name, directly calls getOreID if the flag is TRUE * @param alwaysCreateEntry Flag - should a new entry be created if empty * @return An arraylist containing ItemStacks registered for this ore */ public static List getOres(String name, boolean alwaysCreateEntry) { - if (alwaysCreateEntry) { - return getOres(getOreID(name)); - } - return nameToId.get(name) != null ? getOres(getOreID(name)) : EMPTY_LIST; + if (alwaysCreateEntry) { + return getOres(getOreID(name)); + } + return nameToId.get(name) != null ? getOres(getOreID(name)) : EMPTY_LIST; } /** * Returns whether or not an oreName exists in the dictionary. * This function can be used to safely query the Ore Dictionary without * adding needless clutter to the underlying map structure. - * + * * Please use this when possible and appropriate. - * + * * @param name The ore name * @return Whether or not that name is in the Ore Dictionary. */ public static boolean doesOreNameExist(String name) { - return nameToId.get(name) != null; + return nameToId.get(name) != null; } /** @@ -492,8 +519,8 @@ public class OreDictionary if (name == null || name.isEmpty() || "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. + 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); From aa8eaf2b286e809146b7faf4e59ce801a40eab9b Mon Sep 17 00:00:00 2001 From: cpw Date: Tue, 10 Nov 2015 16:40:27 -0500 Subject: [PATCH 5/5] Fix firing the remap event. It always fires now, and additionally fires when the registry reverts to frozen. Most mods refer to the gameregistry for ids they care about, so this shouldn't affect anything significantly, but if your mod was dependent on their being content in the remap event, and only acting on that content, empty content means it's "reverted to frozen" state - the state at the start of the game. --- fml/src/main/java/cpw/mods/fml/common/Loader.java | 9 +-------- .../main/java/cpw/mods/fml/common/registry/GameData.java | 3 +++ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/Loader.java b/fml/src/main/java/cpw/mods/fml/common/Loader.java index 0f46e8ca9..85b700218 100644 --- a/fml/src/main/java/cpw/mods/fml/common/Loader.java +++ b/fml/src/main/java/cpw/mods/fml/common/Loader.java @@ -981,14 +981,7 @@ public class Loader public void fireRemapEvent(Map remaps) { - if (remaps.isEmpty()) - { - FMLLog.finer("Skipping remap event - no remaps occured"); - } - else - { - modController.propogateStateMessage(new FMLModIdMappingEvent(remaps)); - } + modController.propogateStateMessage(new FMLModIdMappingEvent(remaps)); } public void runtimeDisableMod(String modId) diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java index 13698082b..00a03764c 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java @@ -39,6 +39,7 @@ import com.google.common.collect.HashBasedTable; import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -735,6 +736,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()); // the id mapping has reverted, ensure we sync up the object holders ObjectHolderRegistry.INSTANCE.applyObjectHolders(); }