Registry: Repair mismatched ItemBlocks as well

Fix a few misc issues

fml/patches/minecraft/net/minecraft/world/storage/SaveHandler.java.patch

@@ -37,7 +37,7 @@
              }
          }
  
-+        FMLCommonHandler.instance().confirmBackupLevelDatUse();
++        FMLCommonHandler.instance().confirmBackupLevelDatUse(this);
          file1 = new File(this.field_75770_b, "level.dat_old");
  
          if (file1.exists())

fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java

@@ -55,7 +55,7 @@ import net.minecraft.network.ServerStatusResponse;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.util.ResourceLocation;
 import net.minecraft.world.WorldSettings;
-import net.minecraft.world.storage.ISaveFormat;
+import net.minecraft.world.storage.SaveFormatOld;
 
 import org.apache.logging.log4j.Level;
 import org.lwjgl.input.Mouse;
@@ -87,8 +87,6 @@ import cpw.mods.fml.common.ModMetadata;
 import cpw.mods.fml.common.ObfuscationReflectionHelper;
 import cpw.mods.fml.common.StartupQuery;
 import cpw.mods.fml.common.WrongMinecraftVersionException;
-import cpw.mods.fml.common.event.FMLMissingMappingsEvent;
-import cpw.mods.fml.common.event.FMLMissingMappingsEvent.Action;
 import cpw.mods.fml.common.eventhandler.EventBus;
 import cpw.mods.fml.common.network.FMLNetworkEvent;
 import cpw.mods.fml.common.registry.GameData;
@@ -496,9 +494,9 @@ public class FMLClientHandler implements IFMLSidedHandler
     }
 
     @Override
-    public ISaveFormat getSaveFormat()
+    public File getSavesDirectory()
     {
-        return client.getSaveLoader();
+        return ((SaveFormatOld) client.getSaveLoader()).savesDirectory;
     }
 
     @Override

fml/src/main/java/cpw/mods/fml/common/FMLCommonHandler.java

@@ -12,6 +12,8 @@
 
 package cpw.mods.fml.common;
 
+import java.io.File;
+import java.lang.ref.WeakReference;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -28,7 +30,6 @@ import net.minecraft.network.INetHandler;
 import net.minecraft.network.NetworkManager;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.world.World;
-import net.minecraft.world.storage.ISaveFormat;
 import net.minecraft.world.storage.SaveHandler;
 import net.minecraft.world.storage.WorldInfo;
 
@@ -44,7 +45,6 @@ import com.google.common.collect.MapMaker;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
-import cpw.mods.fml.common.event.FMLMissingMappingsEvent;
 import cpw.mods.fml.common.eventhandler.EventBus;
 import cpw.mods.fml.common.gameevent.InputEvent;
 import cpw.mods.fml.common.gameevent.PlayerEvent;
@@ -84,6 +84,7 @@ public class FMLCommonHandler
     private List<String> brandingsNoMC;
     private List<ICrashCallable> crashCallables = Lists.newArrayList(Loader.instance().getCallableCrashInformation());
     private Set<SaveHandler> handlerSet = Sets.newSetFromMap(new MapMaker().weakKeys().<SaveHandler,Boolean>makeMap());
+    private WeakReference<SaveHandler> handlerToCheck;
     private EventBus eventBus = new EventBus();
     /**
      * The FML event bus. Subscribe here for FML related events
@@ -284,8 +285,8 @@ public class FMLCommonHandler
         Loader.instance().serverStopping();
     }
 
-    public ISaveFormat getSaveFormat() {
+    public File getSavesDirectory() {
-        return sidedDelegate.getSaveFormat();
+        return sidedDelegate.getSavesDirectory();
     }
 
     public MinecraftServer getMinecraftServerInstance()
@@ -385,6 +386,7 @@ public class FMLCommonHandler
             return;
         }
         handlerSet.add(handler);
+        handlerToCheck = new WeakReference<SaveHandler>(handler); // for confirmBackupLevelDatUse
         Map<String,NBTBase> additionalProperties = Maps.newHashMap();
         worldInfo.setAdditionalProperties(additionalProperties);
         for (ModContainer mc : Loader.instance().getModList())
@@ -400,22 +402,12 @@ public class FMLCommonHandler
         }
     }
 
-    public void confirmBackupLevelDatUse()
+    public void confirmBackupLevelDatUse(SaveHandler handler)
     {
-        // ignore invocations from the world ctor, those are always preceded by another invocation
+        if (handlerToCheck == null || handlerToCheck.get() != handler) {
-        for (StackTraceElement e : Thread.currentThread().getStackTrace()) {
+            // only run if the save has been initially loaded
-            try
+            handlerToCheck = null;
-            {
+            return;
-                if (e.getMethodName().equals("<init>") &&
-                        Class.forName(e.getClassName()) == World.class)
-                {
-                    return;
-                }
-            }
-            catch (ClassNotFoundException e1)
-            {
-                // nothing
-            }
         }
 
         String text = "Forge Mod Loader detected that the backup level.dat is being used.\n\n" +

fml/src/main/java/cpw/mods/fml/common/FMLContainer.java

@@ -16,9 +16,11 @@ import java.io.File;
 import java.security.cert.Certificate;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import net.minecraft.item.Item;
 import net.minecraft.nbt.NBTBase;
@@ -217,15 +219,20 @@ public class FMLContainer extends DummyModContainer implements WorldAccessContai
                 dataList.put(dataTag.getString("K"), dataTag.getInteger("V"));
             }
 
+            Set<Integer> blockedIds = new HashSet<Integer>();
+
             if (!tag.hasKey("BlockedItemIds")) // no blocked id info -> old 1.7 save
             {
                 // old early 1.7 save potentially affected by the registry mapping bug
                 // fix the ids the best we can...
-                GameData.fixBrokenIds(dataList);
+                GameData.fixBrokenIds(dataList, blockedIds);
             }
 
             // blocked ids
-            int[] blockedIds = tag.getIntArray("BlockedItemIds");
+            for (int id : tag.getIntArray("BlockedItemIds"))
+            {
+                blockedIds.add(id);
+            }
             // block aliases
             Map<String, String> blockAliases = new HashMap<String, String>();
             list = tag.getTagList("BlockAliases", 10);

fml/src/main/java/cpw/mods/fml/common/IFMLSidedHandler.java

@@ -12,14 +12,13 @@
 
 package cpw.mods.fml.common;
 
+import java.io.File;
 import java.util.List;
 import java.util.Set;
 
 import net.minecraft.network.INetHandler;
 import net.minecraft.network.NetworkManager;
 import net.minecraft.server.MinecraftServer;
-import net.minecraft.world.storage.ISaveFormat;
-import cpw.mods.fml.common.event.FMLMissingMappingsEvent;
 import cpw.mods.fml.common.eventhandler.EventBus;
 import cpw.mods.fml.relauncher.Side;
 
@@ -39,7 +38,7 @@ public interface IFMLSidedHandler
 
     void finishServerLoading();
 
-    ISaveFormat getSaveFormat();
+    File getSavesDirectory();
 
     MinecraftServer getServer();
 

fml/src/main/java/cpw/mods/fml/common/LoadController.java

@@ -130,7 +130,7 @@ public class LoadController
             FMLLog.severe("Fatal errors were detected during the transition from %s to %s. Loading cannot continue", oldState, desiredState);
             StringBuilder sb = new StringBuilder();
             printModStates(sb);
-            FMLLog.severe(sb.toString());
+            FMLLog.severe("%s", sb.toString());
             if (errors.size()>0)
             {
                 FMLLog.severe("The following problems were captured during this phase");

fml/src/main/java/cpw/mods/fml/common/ZipperUtil.java

@@ -80,7 +80,7 @@ public class ZipperUtil {
 
     public static void backupWorld(String dirName, String saveName) throws IOException
     {
-        File dstFolder = FMLCommonHandler.instance().getSaveFormat().getSaveLoader(dirName, false).getWorldDirectory().getParentFile();
+        File dstFolder = FMLCommonHandler.instance().getSavesDirectory();
         File zip = new File(dstFolder, String.format("%s-%2$tY%2$tm%2$td-%2$tH%2$tM%2$tS.zip", saveName, System.currentTimeMillis()));
 
         try

fml/src/main/java/cpw/mods/fml/common/registry/GameData.java

@@ -14,9 +14,11 @@ package cpw.mods.fml.common.registry;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.BitSet;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -229,7 +231,7 @@ public class GameData {
      *
      * @param dataList List containing the IDs to fix
      */
-    public static void fixBrokenIds(Map<String, Integer> dataList)
+    public static void fixBrokenIds(Map<String, Integer> dataList, Set<Integer> blockedIds)
     {
         BitSet availabilityMap = new BitSet(32000);
 
@@ -237,6 +239,7 @@ public class GameData {
         for (Entry<String, Integer> entry : dataList.entrySet())
         {
             String itemName = entry.getKey();
+            String realName = itemName.substring(1);
 
             if (itemName.charAt(0) == '\u0001') // is a block
             {
@@ -244,7 +247,8 @@ public class GameData {
             }
         }
 
-        Set<String> itemsToAllocate = new HashSet<String>();
+        Set<Integer> newBlockedIds = new HashSet<Integer>();
+        Set<String> itemsToRemove = new HashSet<String>();
         Map<String, Integer> itemsToRelocate = new HashMap<String, Integer>();
 
         // check all ids occupied by items
@@ -255,40 +259,76 @@ public class GameData {
             if (itemName.charAt(0) != '\u0001') // is an item
             {
                 int oldId = entry.getValue();
+                String realName = itemName.substring(1);
+                String blockName = '\u0001' + realName;
+                Item item = getMain().iItemRegistry.getRaw(realName);
+                boolean blockThisId = false; // block oldId unless it's used by a block
 
-                if (availabilityMap.get(oldId)) // id is already occupied
+                if (item == null) // item no longer available
                 {
-                    String realName = itemName.substring(1);
+                    // can't fix items without reliably checking if they are ItemBlocks
-                    String blockName = '\u0001' + realName;
+                    FMLLog.warning("Item %s (old id %d) is no longer available and thus can't be fixed.", realName, oldId);
+                    itemsToRemove.add(itemName);
+                    blockThisId = true;
+                }
+                else if (item instanceof ItemBlock)
+                {
+                    if (dataList.containsKey(blockName)) // the item was an ItemBlock before
+                    {
+                        int blockId = dataList.get(blockName);
 
-                    if (!dataList.containsKey(blockName) ||
+                        if (blockId != oldId) // mis-located ItemBlock
-                            (getMain().iItemRegistry.getRaw(realName) != null && // don't assume missing items are no ItemBlock
+                        {
-                            !(getMain().iItemRegistry.getRaw(realName) instanceof ItemBlock))) // the slot is occupied by something else and this item is no ItemBlock
+                            // relocate to the matching block
-                    {
+                            FMLLog.warning("ItemBlock %s (old id %d) doesn't have the same id as its block (%d).", realName, oldId, blockId);
-                        // allocate the item later, after all correct ids have been claimed
+                            itemsToRelocate.put(entry.getKey(), blockId);
-                        itemsToAllocate.add(itemName);
+                            blockThisId = true;
+                        }
+                        else // intact ItemBlock
+                        {
+                            availabilityMap.set(oldId); // occupy id
+                        }
                     }
-                    else if (dataList.get(blockName) != oldId) // occupied, but this is an ItemBlock for a different block than whatever may use its id
+                    else // the item hasn't been an ItemBlock before, but it's now
                     {
-                        // relocate to the matching block
+                        // can't fix these, drop them
-                        int newId = dataList.get(blockName);
+                        FMLLog.warning("Item %s (old id %d) has been migrated to an ItemBlock and can't be fixed.", realName, oldId);
-                        itemsToRelocate.put(entry.getKey(), newId);
+                        itemsToRemove.add(itemName);
+                        blockThisId = true;
                     }
                 }
-                else // unused id, occupy
+                else if (availabilityMap.get(oldId)) // normal item, id is already occupied
                 {
+                    // remove the item mapping
+                    FMLLog.warning("Item %s (old id %d) is conflicting with another block/item and can't be fixed.", realName, oldId);
+                    itemsToRemove.add(itemName);
+                }
+                else // intact Item
+                {
+                    availabilityMap.set(oldId); // occupy id
+                }
+
+                // handle blocking the id from future use if possible (i.e. not used by a conflicting block)
+                // blockThisId requests don't modify availabilityMap, it could only be set by a block (or another item, which isn't being handled)
+                if (blockThisId && !availabilityMap.get(oldId))
+                {
+                    // there's no block occupying this id, thus block the id from future use
+                    // as there may still be ItemStacks in the world referencing it
+                    newBlockedIds.add(oldId);
                     availabilityMap.set(oldId);
                 }
             }
         }
 
-        if (itemsToAllocate.isEmpty() && itemsToRelocate.isEmpty()) return; // nothing to do
+        if (itemsToRemove.isEmpty() && itemsToRelocate.isEmpty()) return; // nothing to do
 
         String text = "Forge Mod Loader detected that this save is damaged.\n\n" +
                 "It's likely that an automatic repair can successfully restore\n" +
                 "most of it, except some items which may get swapped with others.\n\n" +
-                "A world backup will be created as a zip file in your saves\n"+
+                "A world backup will be created as a zip file in your saves\n" +
-                "directory automatically.";
+                "directory automatically.\n\n" +
+                itemsToRemove.size()+" items need to be removed.\n"+
+                itemsToRelocate.size()+" items need to be relocated.";
 
         boolean confirmed = StartupQuery.confirm(text);
         if (!confirmed) StartupQuery.abort();
@@ -303,14 +343,11 @@ public class GameData {
             StartupQuery.abort();
         }
 
-        for (String itemName : itemsToAllocate)
+        for (String itemName : itemsToRemove)
         {
-            int oldId = dataList.get(itemName);
+            int id = dataList.remove(itemName);
-            int newId = availabilityMap.nextClearBit(4096);
 
-            dataList.put(itemName, newId);
+            FMLLog.warning("Removed Item %s, old id %d.", itemName.substring(1), id);
-
-            FMLLog.warning("Fixed Item %s conflicting with another block/item, old id %d, new id %d.", itemName.substring(1), oldId, newId);
         }
 
         for (Map.Entry<String, Integer> entry : itemsToRelocate.entrySet())
@@ -320,16 +357,18 @@ public class GameData {
 
             int oldId = dataList.put(itemName, newId);
 
-            FMLLog.warning("Fixed ItemBlock %s not using the id of its block, old id %d, new id %d.", itemName.substring(1), oldId, newId);
+            FMLLog.warning("Remapped Item %s to id %d, old id %d.", itemName.substring(1), newId, oldId);
         }
+
+        blockedIds.addAll(newBlockedIds);
     }
 
     public static List<String> injectWorldIDMap(Map<String, Integer> dataList, boolean injectFrozenData, boolean isLocalWorld)
     {
-        return injectWorldIDMap(dataList, new int[0], new HashMap<String, String>(), new HashMap<String, String>(), injectFrozenData, isLocalWorld);
+        return injectWorldIDMap(dataList, new HashSet<Integer>(), new HashMap<String, String>(), new HashMap<String, String>(), injectFrozenData, isLocalWorld);
     }
 
-    public static List<String> injectWorldIDMap(Map<String, Integer> dataList, int[] blockedIds, Map<String, String> blockAliases, Map<String, String> itemAliases, boolean injectFrozenData, boolean isLocalWorld)
+    public static List<String> injectWorldIDMap(Map<String, Integer> dataList, Set<Integer> blockedIds, Map<String, String> blockAliases, Map<String, String> itemAliases, boolean injectFrozenData, boolean isLocalWorld)
     {
         FMLLog.info("Injecting existing block and item data into this %s instance", FMLCommonHandler.instance().getEffectiveSide().isServer() ? "server" : "client");
         Map<String, Integer[]> remaps = Maps.newHashMap();
@@ -390,11 +429,13 @@ public class GameData {
 
                 if (currId != newId)
                 {
-                    throw new IllegalStateException(String.format("Can't map %s %s to id %d, already occupied by %s",
+                    throw new IllegalStateException(String.format("Can't map %s %s to id %d, already occupied by %s, blocked %b, ItemBlock %b",
                             isBlock ? "block" : "item",
                                     itemName,
                                     newId,
-                                    isBlock ? newData.iBlockRegistry.getObjectById(newId) : newData.iItemRegistry.getObjectById(newId)));
+                                    isBlock ? newData.iBlockRegistry.getRaw(newId) : newData.iItemRegistry.getRaw(newId),
+                                    newData.blockedIds.contains(newId),
+                                    isBlock ? getMain().iBlockRegistry.getRaw(currId) : getMain().iItemRegistry.getRaw(currId)));
                 }
             }
         }

fml/src/main/java/cpw/mods/fml/server/FMLServerHandler.java

@@ -12,6 +12,7 @@
  */
 package cpw.mods.fml.server;
 
+import java.io.File;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -22,7 +23,7 @@ import net.minecraft.network.NetHandlerPlayServer;
 import net.minecraft.network.NetworkManager;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.dedicated.DedicatedServer;
-import net.minecraft.world.storage.ISaveFormat;
+import net.minecraft.world.storage.SaveFormatOld;
 
 import com.google.common.collect.ImmutableList;
 
@@ -32,7 +33,6 @@ import cpw.mods.fml.common.IFMLSidedHandler;
 import cpw.mods.fml.common.Loader;
 import cpw.mods.fml.common.ModContainer;
 import cpw.mods.fml.common.StartupQuery;
-import cpw.mods.fml.common.event.FMLMissingMappingsEvent;
 import cpw.mods.fml.common.eventhandler.EventBus;
 import cpw.mods.fml.common.network.FMLNetworkEvent;
 import cpw.mods.fml.common.registry.LanguageRegistry;
@@ -102,9 +102,9 @@ public class FMLServerHandler implements IFMLSidedHandler
     }
 
     @Override
-    public ISaveFormat getSaveFormat()
+    public File getSavesDirectory()
     {
-        return server.getActiveAnvilConverter();
+        return ((SaveFormatOld) server.getActiveAnvilConverter()).savesDirectory;
     }
 
     /**

fml/src/main/resources/fml_at.cfg

@@ -78,6 +78,8 @@ public net.minecraft.client.Minecraft field_71446_o # textureManager
 public net.minecraft.item.ItemBlock field_150939_a
 ## DedicatedServer
 public net.minecraft.server.dedicated.DedicatedServer field_71341_l # pendingCommandList
+## SaveFormatOld
+public net.minecraft.world.storage.SaveFormatOld field_75808_a # savesDirectory
 
 protected net.minecraft.util.ObjectIntIdentityMap field_148749_a # internal map
 protected net.minecraft.util.ObjectIntIdentityMap field_148748_b # internal index list