From 36d2e67b07a7927219246d4868bd4099fed77bb5 Mon Sep 17 00:00:00 2001 From: LexManos Date: Wed, 9 Jan 2019 20:57:01 -0800 Subject: [PATCH] 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 | 21 +-- .../net/minecraft/init/Fluids.java.patch | 10 -- .../fml/language/ModFileScanData.java | 9 +- .../fml/loading/StringUtils.java | 3 - .../loading/moddiscovery/ModAnnotation.java | 2 +- .../net/minecraftforge/fml/ForgeI18n.java | 6 +- .../net/minecraftforge/fml/ModLoader.java | 3 + .../minecraftforge/registries/GameData.java | 2 +- .../registries/ObjectHolderRef.java | 48 +++++-- .../registries/ObjectHolderRegistry.java | 128 +++++++++++------- 10 files changed, 143 insertions(+), 89 deletions(-) delete mode 100644 patches/minecraft/net/minecraft/init/Fluids.java.patch diff --git a/build.gradle b/build.gradle index 1f725cda0..1cca2fd59 100644 --- a/build.gradle +++ b/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 - systemProperties = patcher.runs.client.properties - environment += patcher.runs.client.environment + main patcher.runs.forge_client.main + systemProperties = patcher.runs.forge_client.properties + 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 - environment += patcher.runs.server.environment + systemProperties = patcher.runs.forge_server.properties + 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 diff --git a/patches/minecraft/net/minecraft/init/Fluids.java.patch b/patches/minecraft/net/minecraft/init/Fluids.java.patch deleted file mode 100644 index 12f17baac..000000000 --- a/patches/minecraft/net/minecraft/init/Fluids.java.patch +++ /dev/null @@ -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 field_207214_f; - public static final Fluid field_204541_a; diff --git a/src/fmllauncher/java/net/minecraftforge/fml/language/ModFileScanData.java b/src/fmllauncher/java/net/minecraftforge/fml/language/ModFileScanData.java index fbe77d5d2..3a2accc63 100644 --- a/src/fmllauncher/java/net/minecraftforge/fml/language/ModFileScanData.java +++ b/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 annotationData; - public AnnotationData(final Type annotationType, final Type clazz, final String memberName, final Map annotationData) { + public AnnotationData(final Type annotationType, final ElementType targetType, final Type clazz, final String memberName, final Map 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; } diff --git a/src/fmllauncher/java/net/minecraftforge/fml/loading/StringUtils.java b/src/fmllauncher/java/net/minecraftforge/fml/loading/StringUtils.java index 3f6a78ae1..bd5be4467 100644 --- a/src/fmllauncher/java/net/minecraftforge/fml/loading/StringUtils.java +++ b/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) { diff --git a/src/fmllauncher/java/net/minecraftforge/fml/loading/moddiscovery/ModAnnotation.java b/src/fmllauncher/java/net/minecraftforge/fml/loading/moddiscovery/ModAnnotation.java index bdd54efb0..e0781b2b5 100644 --- a/src/fmllauncher/java/net/minecraftforge/fml/loading/moddiscovery/ModAnnotation.java +++ b/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 diff --git a/src/main/java/net/minecraftforge/fml/ForgeI18n.java b/src/main/java/net/minecraftforge/fml/ForgeI18n.java index f2a35c430..30729a49c 100644 --- a/src/main/java/net/minecraftforge/fml/ForgeI18n.java +++ b/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"); } } -} \ No newline at end of file +} diff --git a/src/main/java/net/minecraftforge/fml/ModLoader.java b/src/main/java/net/minecraftforge/fml/ModLoader.java index c4f6f6258..12c8f90fc 100644 --- a/src/main/java/net/minecraftforge/fml/ModLoader.java +++ b/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); diff --git a/src/main/java/net/minecraftforge/registries/GameData.java b/src/main/java/net/minecraftforge/registries/GameData.java index 6864f8806..4907918ee 100644 --- a/src/main/java/net/minecraftforge/registries/GameData.java +++ b/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(); diff --git a/src/main/java/net/minecraftforge/registries/ObjectHolderRef.java b/src/main/java/net/minecraftforge/registries/ObjectHolderRef.java index 328bcae72..5ded8f2fe 100644 --- a/src/main/java/net/minecraftforge/registries/ObjectHolderRef.java +++ b/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)type); + registry = (ForgeRegistry)RegistryManager.ACTIVE.getRegistry((Class)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); + } } diff --git a/src/main/java/net/minecraftforge/registries/ObjectHolderRegistry.java b/src/main/java/net/minecraftforge/registries/ObjectHolderRegistry.java index 21115f83e..66ec9f9e2 100644 --- a/src/main/java/net/minecraftforge/registries/ObjectHolderRegistry.java +++ b/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 objectHolders = Lists.newArrayList(); -/* TODO annotation data - public void findObjectHolders(ASMDataTable table) + private static final Set objectHolders = new HashSet<>(); + private static final Type OBJECT_HOLDER = Type.getType(ObjectHolder.class); + private static final Type MOD = Type.getType(Mod.class); + + public static void findObjectHolders() { LOGGER.info("Processing ObjectHolder annotations"); - Set allObjectHolders = table.getAll(ObjectHolder.class.getName()); - Map classModIds = Maps.newHashMap(); - Map> classCache = Maps.newHashMap(); + final List annotations = ModList.get().getAllScanData().stream() + .map(ModFileScanData::getAnnotations) + .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 classModIds = Maps.newHashMap(); + Map> 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 -> - { - String value = (String)data.getAnnotationInfo().get("value"); - scanTarget(classModIds, classCache, data.getClassName(), data.getObjectName(), value, true, data.getClassName().startsWith("net.minecraft.init")); - }); - 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); - }); + 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"))); + + 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)); LOGGER.info("Found {} ObjectHolder annotations", objectHolders.size()); } -*/ - private void scanTarget(Map classModIds, Map> classCache, String className, @Nullable String annotationTarget, String value, boolean isClass, boolean extractFromValue) + + private static void scanTarget(Map classModIds, Map> 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()); - classCache.put(className, clazz); + clazz = Class.forName(type.getClassName(), extractFromValue, ObjectHolderRegistry.class.getClassLoader()); + 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 classModIds, String className, String value, Class clazz, boolean extractFromExistingValues) + private static void scanClassForFields(Map 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; - } - addHolderReference(new ObjectHolderRef(f, new ResourceLocation(value, f.getName()), extractFromExistingValues)); + ObjectHolderRef ref = new ObjectHolderRef(f, value + ':' + f.getName().toLowerCase(Locale.ENGLISH), extractFromExistingValues); + if (ref.isValid()) + addHandler(ref); } } - private void addHolderReference(ObjectHolderRef ref) - { - if (ref.isValid()) - { - objectHolders.add(ref); - } - } - - public void applyObjectHolders() + public static void applyObjectHolders() { LOGGER.info("Applying holder lookups"); - objectHolders.forEach(ObjectHolderRef::apply); + objectHolders.forEach(Runnable::run); LOGGER.info("Holder lookups applied"); }