Reimplement @ObjectHolder scanning, and expose system for others to add handlers.

Fix forgedev and userdev run configs.
Fix issue in log functions assuming String arguments.

build.gradle

@@ -161,7 +161,7 @@ project(':forge') {
         exc = file("$rootDir/src/main/resources/forge.exc")
         srgPatches = true
         runs {
-            client = {
+            forge_client = {
                 main 'net.minecraftforge.fml.LaunchTesting'
                 environment = [
                     target: 'fmldevclient',
@@ -180,7 +180,7 @@ project(':forge') {
                     'fmllauncher.version': SPEC_VERSION
                 ]
             }
-            server = {
+            forge_server = {
                 main 'net.minecraftforge.fml.LaunchTesting'
                 environment = [
                     target: 'fmldevserver'
@@ -190,7 +190,8 @@ project(':forge') {
                     'mcp.version': MCP_VERSION,
                     'forge.version': "${project.version.substring(MC_VERSION.length() + 1)}".toString(),
                     'forge.spec': SPEC_VERSION,
-                    'forge.group': project.group
+                    'forge.group': project.group, 
+                    'fmllauncher.version': SPEC_VERSION
                 ]
             }
         }
@@ -285,9 +286,9 @@ project(':forge') {
         }
         //jvmArgs = ['-verbose:class']
         classpath sourceSets.main.runtimeClasspath
-        main patcher.runs.client.main
+        main patcher.runs.forge_client.main
-        systemProperties = patcher.runs.client.properties
+        systemProperties = patcher.runs.forge_client.properties
-        environment += patcher.runs.client.environment
+        environment += patcher.runs.forge_client.environment
         workingDir 'runclient'
     }
 
@@ -302,10 +303,10 @@ project(':forge') {
             }
         }
         classpath sourceSets.main.runtimeClasspath
-        main patcher.runs.server.main
+        main patcher.runs.forge_server.main
         args 'nogui'
-        systemProperties = patcher.runs.server.properties
+        systemProperties = patcher.runs.forge_server.properties
-        environment += patcher.runs.server.environment
+        environment += patcher.runs.forge_server.environment
         workingDir 'runserver'
         standardInput = System.in
     }
@@ -808,7 +809,7 @@ project(':forge') {
             }
             server = {
                 main 'net.minecraftforge.userdev.UserdevLauncher'
-                environment 'target', 'fmldevserver'
+                environment 'target', 'fmluserdevserver'
                 environment 'FORGE_VERSION', project.version.substring(MC_VERSION.length() + 1)
                 environment 'FORGE_GROUP', project.group
                 environment 'MCP_VERSION', MCP_VERSION

patches/minecraft/net/minecraft/init/Fluids.java.patch

@@ -1,10 +0,0 @@
---- a/net/minecraft/init/Fluids.java
-+++ b/net/minecraft/init/Fluids.java
-@@ -6,6 +6,7 @@
- import net.minecraft.fluid.Fluid;
- import net.minecraft.util.ResourceLocation;
- 
-+@net.minecraftforge.registries.ObjectHolder("minecraft")
- public class Fluids {
-    private static final Set<Fluid> field_207214_f;
-    public static final Fluid field_204541_a;

src/fmllauncher/java/net/minecraftforge/fml/language/ModFileScanData.java

@@ -22,6 +22,7 @@ package net.minecraftforge.fml.language;
 
 import org.objectweb.asm.Type;
 
+import java.lang.annotation.ElementType;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -71,13 +72,15 @@ public class ModFileScanData
     }
     public static class AnnotationData {
         private final Type annotationType;
+        private final ElementType targetType;
         private final Type clazz;
         private final String memberName;
         private final Map<String,Object> annotationData;
 
 
-        public AnnotationData(final Type annotationType, final Type clazz, final String memberName, final Map<String, Object> annotationData) {
+        public AnnotationData(final Type annotationType, final ElementType targetType, final Type clazz, final String memberName, final Map<String, Object> annotationData) {
             this.annotationType = annotationType;
+            this.targetType = targetType;
             this.clazz = clazz;
             this.memberName = memberName;
             this.annotationData = annotationData;
@@ -87,6 +90,10 @@ public class ModFileScanData
             return annotationType;
         }
 
+        public ElementType getTargetType() {
+            return targetType;
+        }
+
         public Type getClassType() {
             return clazz;
         }

src/fmllauncher/java/net/minecraftforge/fml/loading/StringUtils.java

@@ -26,9 +26,6 @@ import java.net.URL;
 import java.util.Locale;
 import java.util.Map;
 
-/**
- * Created by cpw on 05/06/17.
- */
 public class StringUtils
 {
     public static String toLowerCase(final String str) {

src/fmllauncher/java/net/minecraftforge/fml/loading/moddiscovery/ModAnnotation.java

@@ -33,7 +33,7 @@ import com.google.common.collect.Maps;
 public class ModAnnotation
 {
     public static ModFileScanData.AnnotationData fromModAnnotation(final Type clazz, final ModAnnotation annotation) {
-        return new ModFileScanData.AnnotationData(annotation.asmType, clazz, annotation.member, annotation.values);
+        return new ModFileScanData.AnnotationData(annotation.asmType, annotation.type, clazz, annotation.member, annotation.values);
     }
 
     public static class EnumHolder

src/main/java/net/minecraftforge/fml/ForgeI18n.java

@@ -44,9 +44,9 @@ public class ForgeI18n {
         // {0,modinfo,id} -> modid from ModInfo object; {0,modinfo,name} -> displayname from ModInfo object
         customFactories.put("modinfo", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> parseModInfo(formatString, stringBuffer, objectToParse)));
         // {0,lower} -> lowercase supplied string
-        customFactories.put("lower", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> stringBuffer.append(StringUtils.toLowerCase((String)objectToParse))));
+        customFactories.put("lower", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> stringBuffer.append(StringUtils.toLowerCase(String.valueOf(objectToParse)))));
         // {0,upper> -> uppercase supplied string
-        customFactories.put("upper", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> stringBuffer.append(StringUtils.toUpperCase((String)objectToParse))));
+        customFactories.put("upper", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> stringBuffer.append(StringUtils.toUpperCase(String.valueOf(objectToParse)))));
         // {0,exc,class} -> class of exception; {0,exc,msg} -> message from exception
         customFactories.put("exc", (name, formatString, locale) -> new CustomReadOnlyFormat((stringBuffer, objectToParse) -> parseException(formatString, stringBuffer, objectToParse)));
         // {0,vr} -> transform VersionRange into cleartext string using fml.messages.version.restriction.* strings
@@ -112,4 +112,4 @@ public class ForgeI18n {
             throw new UnsupportedOperationException("Parsing is not supported");
         }
     }
-}
+}

src/main/java/net/minecraftforge/fml/ModLoader.java

@@ -30,6 +30,8 @@ import net.minecraftforge.fml.loading.LoadingModList;
 import net.minecraftforge.fml.loading.moddiscovery.ModFile;
 import net.minecraftforge.fml.loading.moddiscovery.ModFileInfo;
 import net.minecraftforge.registries.GameData;
+import net.minecraftforge.registries.ObjectHolderRegistry;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -86,6 +88,7 @@ public class ModLoader
         modList.setLoadedMods(modContainerStream.collect(Collectors.toList()));
         dispatchAndHandleError(LifecycleEventProvider.CONSTRUCT);
         GameData.fireCreateRegistryEvents();
+        ObjectHolderRegistry.findObjectHolders();
         CapabilityManager.INSTANCE.injectCapabilities(modList.getAllScanData());
         GameData.fireRegistryEvents();
         dispatchAndHandleError(LifecycleEventProvider.PREINIT);

src/main/java/net/minecraftforge/registries/GameData.java

@@ -695,7 +695,7 @@ public class GameData
         //Loader.instance().fireRemapEvent(remaps, false);
 
         // The id map changed, ensure we apply object holders
-        ObjectHolderRegistry.INSTANCE.applyObjectHolders();
+        ObjectHolderRegistry.applyObjectHolders();
 
         // Return an empty list, because we're good
         return ArrayListMultimap.create();

src/main/java/net/minecraftforge/registries/ObjectHolderRef.java

@@ -26,17 +26,15 @@ import java.util.LinkedList;
 import java.util.Queue;
 
 import net.minecraft.util.ResourceLocation;
+import net.minecraft.util.ResourceLocationException;
 
 import javax.annotation.Nullable;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-/**
- * Internal class used in tracking {@link ObjectHolder} references
- */
 @SuppressWarnings("rawtypes")
-class ObjectHolderRef
+public class ObjectHolderRef implements Runnable
 {
     private static final Logger LOGGER  = LogManager.getLogger();
     private Field field;
@@ -44,19 +42,24 @@ class ObjectHolderRef
     private boolean isValid;
     private ForgeRegistry<?> registry;
 
-    ObjectHolderRef(Field field, ResourceLocation injectedObject, boolean extractFromExistingValues)
+    public ObjectHolderRef(Field field, ResourceLocation injectedObject)
     {
-        registry = getRegistryForType(field);
+        this(field, injectedObject.toString(), false);
+    }
 
+    ObjectHolderRef(Field field, String injectedObject, boolean extractFromExistingValues)
+    {
+        this.registry = getRegistryForType(field);
         this.field = field;
         this.isValid = registry != null;
+
         if (extractFromExistingValues)
         {
             try
             {
                 Object existing = field.get(null);
                 // nothing is ever allowed to replace AIR
-                if (existing == null || existing == registry.getDefault())
+                if (!isValid || (existing == null || existing == registry.getDefault()))
                 {
                     this.injectedObject = null;
                     this.field = null;
@@ -75,7 +78,14 @@ class ObjectHolderRef
         }
         else
         {
-            this.injectedObject = injectedObject;
+            try
+            {
+                this.injectedObject = new ResourceLocation(injectedObject);
+            }
+            catch (ResourceLocationException e)
+            {
+                throw new IllegalArgumentException("Invalid @ObjectHolder annotation on \"" + field.toString() + "\"", e);
+            }
         }
 
         if (this.injectedObject == null || !isValid())
@@ -103,9 +113,9 @@ class ObjectHolderRef
         {
             Class<?> type = typesToExamine.remove();
             Collections.addAll(typesToExamine, type.getInterfaces());
-            if (ForgeRegistryEntry.class.isAssignableFrom(type))
+            if (IForgeRegistryEntry.class.isAssignableFrom(type))
             {
-                registry = (ForgeRegistry<?>)RegistryManager.ACTIVE.getRegistry((Class<ForgeRegistryEntry>)type);
+                registry = (ForgeRegistry<?>)RegistryManager.ACTIVE.getRegistry((Class<IForgeRegistryEntry>)type);
                 final Class<?> parentType = type.getSuperclass();
                 if (parentType != null)
                 {
@@ -121,7 +131,8 @@ class ObjectHolderRef
         return isValid;
     }
 
-    public void apply()
+    @Override
+    public void run()
     {
         Object thing;
         if (isValid && registry.containsKey(injectedObject) && !registry.isDummied(injectedObject))
@@ -147,4 +158,19 @@ class ObjectHolderRef
             LOGGER.warn("Unable to set {} with value {} ({})", this.field, thing, this.injectedObject, e);
         }
     }
+
+    @Override
+    public int hashCode()
+    {
+        return field.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object other)
+    {
+        if (!(other instanceof ObjectHolderRef))
+            return false;
+        ObjectHolderRef o = (ObjectHolderRef)other;
+        return this.field.equals(o.field);
+    }
 }

src/main/java/net/minecraftforge/registries/ObjectHolderRegistry.java

@@ -19,17 +19,20 @@
 
 package net.minecraftforge.registries;
 
+import java.lang.annotation.ElementType;
 import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
-import net.minecraft.util.ResourceLocation;
+import net.minecraftforge.fml.ModList;
 import net.minecraftforge.fml.common.Mod;
-import net.minecraftforge.fml.common.registry.GameRegistry;
+import net.minecraftforge.fml.language.ModFileScanData;
 
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
 import javax.annotation.Nullable;
@@ -37,52 +40,85 @@ import javax.annotation.Nullable;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
 
 /**
  * Internal registry for tracking {@link ObjectHolder} references
  */
-public enum ObjectHolderRegistry
+public class ObjectHolderRegistry
 {
-    INSTANCE;
+    /**
+     * Exposed to allow modders to register their own notification handlers.
+     * This runnable will be called after a registry snapshot has been injected and finalized.
+     * The internal list is backed by a HashSet so it is HIGHLY recommended you implement a proper equals
+     * and hashCode function to de-duplicate callers here.
+     * The default @ObjectHolder implementation uses the hashCode/equals for the field the annotation is on.
+     */
+    public static void addHandler(Runnable ref)
+    {
+        objectHolders.add(ref);
+    }
+
+    /**
+     * Removed the specified handler from the notification list.
+     *
+     * The internal list is backed by a hash set, and so proper hashCode and equals operations are required for success.
+     *
+     * The default @ObjectHolder implementation uses the hashCode/equals for the field the annotation is on.
+     *
+     * @return true if handler was matched and removed.
+     */
+    public static boolean removeHandler(Runnable ref)
+    {
+        return objectHolders.remove(ref);
+    }
+
+    //==============================================================
+    // Everything below is internal, do not use.
+    //==============================================================
+
     private static final Logger LOGGER = LogManager.getLogger();
-    private List<ObjectHolderRef> objectHolders = Lists.newArrayList();
+    private static final Set<Runnable> objectHolders = new HashSet<>();
-/* TODO annotation data
+    private static final Type OBJECT_HOLDER = Type.getType(ObjectHolder.class);
-    public void findObjectHolders(ASMDataTable table)
+    private static final Type MOD = Type.getType(Mod.class);
+
+    public static void findObjectHolders()
     {
         LOGGER.info("Processing ObjectHolder annotations");
-        Set<ASMData> allObjectHolders = table.getAll(ObjectHolder.class.getName());
+        final List<ModFileScanData.AnnotationData> annotations = ModList.get().getAllScanData().stream()
-        Map<String, String> classModIds = Maps.newHashMap();
+            .map(ModFileScanData::getAnnotations)
-        Map<String, Class<?>> classCache = Maps.newHashMap();
+            .flatMap(Collection::stream)
+            .filter(a -> OBJECT_HOLDER.equals(a.getAnnotationType()) || MOD.equals(a.getAnnotationType()))
+            .collect(Collectors.toList());
 
-        table.getAll(Mod.class.getName()).forEach(data -> classModIds.put(data.getClassName(), (String)data.getAnnotationInfo().get("value")));
+        Map<Type, String> classModIds = Maps.newHashMap();
+        Map<Type, Class<?>> classCache = Maps.newHashMap();
+
+        // Gather all @Mod classes, so that @ObjectHolder's in those classes don't need to specify the mod id, Modder convince
+        annotations.stream().filter(a -> MOD.equals(a.getAnnotationType())).forEach(data -> classModIds.put(data.getClassType(), (String)data.getAnnotationData().get("value")));
 
         // double pass - get all the class level annotations first, then the field level annotations
-        allObjectHolders.stream().filter(data -> data.getObjectName().equals(data.getClassName())).forEach(data ->
+        annotations.stream().filter(a -> OBJECT_HOLDER.equals(a.getAnnotationType())).filter(a -> a.getTargetType() == ElementType.TYPE)
-        {
+        .forEach(data -> scanTarget(classModIds, classCache, data.getClassType(), null,                 (String)data.getAnnotationData().get("value"), true, data.getClassType().getClassName().startsWith("net.minecraft.init")));
-            String value = (String)data.getAnnotationInfo().get("value");
+
-            scanTarget(classModIds, classCache, data.getClassName(), data.getObjectName(), value, true, data.getClassName().startsWith("net.minecraft.init"));
+        annotations.stream().filter(a -> OBJECT_HOLDER.equals(a.getAnnotationType())).filter(a -> a.getTargetType() == ElementType.FIELD)
-        });
+        .forEach(data -> scanTarget(classModIds, classCache, data.getClassType(), data.getMemberName(), (String)data.getAnnotationData().get("value"), false, false));
-        allObjectHolders.stream().filter(data -> !data.getObjectName().equals(data.getClassName())).forEach(data ->
-        {
-            String value = (String)data.getAnnotationInfo().get("value");
-            scanTarget(classModIds, classCache, data.getClassName(), data.getObjectName(), value, false, false);
-        });
         LOGGER.info("Found {} ObjectHolder annotations", objectHolders.size());
     }
-*/
+
-    private void scanTarget(Map<String, String> classModIds, Map<String, Class<?>> classCache, String className, @Nullable String annotationTarget, String value, boolean isClass, boolean extractFromValue)
+    private static void scanTarget(Map<Type, String> classModIds, Map<Type, Class<?>> classCache, Type type, @Nullable String annotationTarget, String value, boolean isClass, boolean extractFromValue)
     {
         Class<?> clazz;
-        if (classCache.containsKey(className))
+        if (classCache.containsKey(type))
         {
-            clazz = classCache.get(className);
+            clazz = classCache.get(type);
         }
         else
         {
             try
             {
-                clazz = Class.forName(className, extractFromValue, getClass().getClassLoader());
+                clazz = Class.forName(type.getClassName(), extractFromValue, ObjectHolderRegistry.class.getClassLoader());
-                classCache.put(className, clazz);
+                classCache.put(type, clazz);
             }
             catch (ClassNotFoundException ex)
             {
@@ -92,24 +128,26 @@ public enum ObjectHolderRegistry
         }
         if (isClass)
         {
-            scanClassForFields(classModIds, className, value, clazz, extractFromValue);
+            scanClassForFields(classModIds, type, value, clazz, extractFromValue);
         }
         else
         {
             if (value.indexOf(':') == -1)
             {
-                String prefix = classModIds.get(className);
+                String prefix = classModIds.get(type);
                 if (prefix == null)
                 {
-                    LOGGER.warn("Found an unqualified ObjectHolder annotation ({}) without a modid context at {}.{}, ignoring", value, className, annotationTarget);
+                    LOGGER.warn("Found an unqualified ObjectHolder annotation ({}) without a modid context at {}.{}, ignoring", value, type, annotationTarget);
                     throw new IllegalStateException("Unqualified reference to ObjectHolder");
                 }
-                value = prefix + ":" + value;
+                value = prefix + ':' + value;
             }
             try
             {
                 Field f = clazz.getDeclaredField(annotationTarget);
-                addHolderReference(new ObjectHolderRef(f, new ResourceLocation(value), extractFromValue));
+                ObjectHolderRef ref = new ObjectHolderRef(f, value, extractFromValue);
+                if (ref.isValid())
+                    addHandler(ref);
             }
             catch (NoSuchFieldException ex)
             {
@@ -119,32 +157,24 @@ public enum ObjectHolderRegistry
         }
     }
 
-    private void scanClassForFields(Map<String, String> classModIds, String className, String value, Class<?> clazz, boolean extractFromExistingValues)
+    private static void scanClassForFields(Map<Type, String> classModIds, Type targetClass, String value, Class<?> clazz, boolean extractFromExistingValues)
     {
-        classModIds.put(className, value);
+        classModIds.put(targetClass, value);
         final int flags = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC;
         for (Field f : clazz.getFields())
         {
             if (((f.getModifiers() & flags) != flags) || f.isAnnotationPresent(ObjectHolder.class))
-            {
                 continue;
-            }
+            ObjectHolderRef ref = new ObjectHolderRef(f, value + ':' + f.getName().toLowerCase(Locale.ENGLISH), extractFromExistingValues);
-            addHolderReference(new ObjectHolderRef(f, new ResourceLocation(value, f.getName()), extractFromExistingValues));
+            if (ref.isValid())
+                addHandler(ref);
         }
     }
 
-    private void addHolderReference(ObjectHolderRef ref)
+    public static void applyObjectHolders()
-    {
-        if (ref.isValid())
-        {
-            objectHolders.add(ref);
-        }
-    }
-
-    public void applyObjectHolders()
     {
         LOGGER.info("Applying holder lookups");
-        objectHolders.forEach(ObjectHolderRef::apply);
+        objectHolders.forEach(Runnable::run);
         LOGGER.info("Holder lookups applied");
     }