From 160427b12c9706e73c2fed41e8498bf565dfad73 Mon Sep 17 00:00:00 2001 From: LexManos Date: Mon, 5 Dec 2016 17:19:15 -0800 Subject: [PATCH] Properly filter out annotations that are for multiple mods in the same mod source. This prevents disabled mods from getting their proxies injected. Also prevents mods from getting their proxies injected multiple times. Effects the @EventBusSubscriber, @Instance, @Metadata, @SidedProxy annotations. If the target modid is not in the annotation data, and there are multiple mods in the source, it will attempt to find it by matching the annotations's class names. This is a potentially breaking change. Review your logs for "skipping" messages. --- .../fml/common/AutomaticEventSubscriber.java | 19 +++++++++++++- .../fml/common/FMLModContainer.java | 18 ++++++++++++- .../net/minecraftforge/fml/common/Mod.java | 19 ++++++++++++++ .../fml/common/ProxyInjector.java | 26 ++++++++++++++----- .../minecraftforge/fml/common/SidedProxy.java | 5 ++-- .../fml/common/discovery/ASMDataTable.java | 15 +++++++++++ .../debug/ObjectHolderTest.java | 2 +- 7 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/minecraftforge/fml/common/AutomaticEventSubscriber.java b/src/main/java/net/minecraftforge/fml/common/AutomaticEventSubscriber.java index d1fe34258..e622d9f88 100644 --- a/src/main/java/net/minecraftforge/fml/common/AutomaticEventSubscriber.java +++ b/src/main/java/net/minecraftforge/fml/common/AutomaticEventSubscriber.java @@ -20,6 +20,8 @@ package net.minecraftforge.fml.common; import com.google.common.base.Strings; +import com.google.common.collect.SetMultimap; + import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.fml.common.discovery.ASMDataTable; import net.minecraftforge.fml.common.discovery.ASMDataTable.ASMData; @@ -44,7 +46,9 @@ public class AutomaticEventSubscriber public static void inject(ModContainer mod, ASMDataTable data, Side side) { FMLLog.fine("Attempting to inject @EventBusSubscriber classes into the eventbus for %s", mod.getModId()); - Set targets = data.getAnnotationsFor(mod).get(Mod.EventBusSubscriber.class.getName()); + SetMultimap modData = data.getAnnotationsFor(mod); + Set mods = modData.get(Mod.class.getName()); + Set targets = modData.get(Mod.EventBusSubscriber.class.getName()); ClassLoader mcl = Loader.instance().getModClassLoader(); for (ASMDataTable.ASMData targ : targets) @@ -62,6 +66,19 @@ public class AutomaticEventSubscriber } if (sides == DEFAULT || sides.contains(side)) { FMLLog.fine("Found @EventBusSubscriber class %s", targ.getClassName()); + String amodid = (String)targ.getAnnotationInfo().get("modid"); + if (Strings.isNullOrEmpty(amodid)) { + amodid = ASMDataTable.getOwnerModID(mods, targ); + if (Strings.isNullOrEmpty(amodid)) { + FMLLog.bigWarning("Could not determine owning mod for @EventBusSubscriber on %s for mod %s", targ.getClassName(), mod.getModId()); + continue; + } + } + if (!mod.getModId().equals(amodid)) + { + FMLLog.fine("Skipping @EventBusSubscriber injection for %s since it is not for mod %s", targ.getClassName(), mod.getModId()); + continue; //We're not injecting this guy + } Class subscriptionTarget = Class.forName(targ.getClassName(), true, mcl); MinecraftForge.EVENT_BUS.register(subscriptionTarget); FMLLog.fine("Injected @EventBusSubscriber class %s", targ.getClassName()); diff --git a/src/main/java/net/minecraftforge/fml/common/FMLModContainer.java b/src/main/java/net/minecraftforge/fml/common/FMLModContainer.java index a6a67ba94..33d64dd9a 100644 --- a/src/main/java/net/minecraftforge/fml/common/FMLModContainer.java +++ b/src/main/java/net/minecraftforge/fml/common/FMLModContainer.java @@ -435,11 +435,27 @@ public class FMLModContainer implements ModContainer private void parseSimpleFieldAnnotation(SetMultimap annotations, String annotationClassName, Function retriever) throws IllegalAccessException { + Set mods = annotations.get(Mod.class.getName()); String[] annName = annotationClassName.split("\\."); String annotationName = annName[annName.length - 1]; for (ASMData targets : annotations.get(annotationClassName)) { String targetMod = (String)targets.getAnnotationInfo().get("value"); + String owner = (String)targets.getAnnotationInfo().get("owner"); + if (Strings.isNullOrEmpty(owner)) + { + owner = ASMDataTable.getOwnerModID(mods, targets); + if (Strings.isNullOrEmpty(owner)) + { + FMLLog.bigWarning("Could not determine owning mod for @%s on %s for mod %s", annotationClassName, targets.getClassName(), this.getModId()); + continue; + } + } + if (!this.getModId().equals(owner)) + { + FMLLog.fine("Skipping @%s injection for %s.%s since it is not for mod %s", annotationClassName, targets.getClassName(), targets.getObjectName(), this.getModId()); + continue; + } Field f = null; Object injectedMod = null; ModContainer mc = this; @@ -751,4 +767,4 @@ public class FMLModContainer implements ModContainer { return this.classVersion; } -} \ No newline at end of file +} diff --git a/src/main/java/net/minecraftforge/fml/common/Mod.java b/src/main/java/net/minecraftforge/fml/common/Mod.java index 24afe9d33..830bddcfe 100644 --- a/src/main/java/net/minecraftforge/fml/common/Mod.java +++ b/src/main/java/net/minecraftforge/fml/common/Mod.java @@ -282,6 +282,12 @@ public @interface Mod * The mod object to inject into this field */ String value() default ""; + + /** + * Optional owner modid, required if this annotation is on something that is not inside the main class of a mod container. + * This is required to prevent mods from classloading other, potentially disabled mods. + */ + String owner() default ""; } /** * Populate the annotated field with the mod's metadata. @@ -295,6 +301,12 @@ public @interface Mod * The mod id specifying the metadata to load here */ String value() default ""; + + /** + * Optional owner modid, required if this annotation is on something that is not inside the main class of a mod container. + * This is required to prevent mods from classloading other, potentially disabled mods. + */ + String owner() default ""; } /** @@ -314,5 +326,12 @@ public @interface Mod @Target(ElementType.TYPE) public @interface EventBusSubscriber { Side[] value() default { Side.CLIENT, Side.SERVER }; + + /** + * Optional value, only nessasary if tis annotation is not on the same class that has a @Mod annotation. + * Needed to prevent early classloading of classes not owned by your mod. + * @return + */ + String modid() default ""; } } diff --git a/src/main/java/net/minecraftforge/fml/common/ProxyInjector.java b/src/main/java/net/minecraftforge/fml/common/ProxyInjector.java index 9c6669467..8f468939d 100644 --- a/src/main/java/net/minecraftforge/fml/common/ProxyInjector.java +++ b/src/main/java/net/minecraftforge/fml/common/ProxyInjector.java @@ -30,6 +30,7 @@ import net.minecraftforge.fml.relauncher.Side; import org.apache.logging.log4j.Level; import com.google.common.base.Strings; +import com.google.common.collect.SetMultimap; /** * @author cpw @@ -40,13 +41,31 @@ public class ProxyInjector public static void inject(ModContainer mod, ASMDataTable data, Side side, ILanguageAdapter languageAdapter) { FMLLog.fine("Attempting to inject @SidedProxy classes into %s", mod.getModId()); - Set targets = data.getAnnotationsFor(mod).get(SidedProxy.class.getName()); + SetMultimap modData = data.getAnnotationsFor(mod); + Set mods = modData.get(Mod.class.getName()); + Set targets = modData.get(SidedProxy.class.getName()); ClassLoader mcl = Loader.instance().getModClassLoader(); for (ASMData targ : targets) { try { + String amodid = (String)targ.getAnnotationInfo().get("modid"); + if (Strings.isNullOrEmpty(amodid)) + { + amodid = ASMDataTable.getOwnerModID(mods, targ); + if (Strings.isNullOrEmpty(amodid)) + { + FMLLog.bigWarning("Could not determine owning mod for @SidedProxy on %s for mod %s", targ.getClassName(), mod.getModId()); + continue; + } + } + if (!mod.getModId().equals(amodid)) + { + FMLLog.fine("Skipping proxy injection for %s.%s since it is not for mod %s", targ.getClassName(), targ.getObjectName(), mod.getModId()); + continue; + } + Class proxyTarget = Class.forName(targ.getClassName(), true, mcl); Field target = proxyTarget.getDeclaredField(targ.getObjectName()); if (target == null) @@ -58,11 +77,6 @@ public class ProxyInjector target.setAccessible(true); SidedProxy annotation = target.getAnnotation(SidedProxy.class); - if (!Strings.isNullOrEmpty(annotation.modId()) && !annotation.modId().equals(mod.getModId())) - { - FMLLog.fine("Skipping proxy injection for %s.%s since it is not for mod %s", targ.getClassName(), targ.getObjectName(), mod.getModId()); - continue; - } String targetType = side.isClient() ? annotation.clientSide() : annotation.serverSide(); if(targetType.equals("")) { diff --git a/src/main/java/net/minecraftforge/fml/common/SidedProxy.java b/src/main/java/net/minecraftforge/fml/common/SidedProxy.java index fc79499ea..011946d1b 100644 --- a/src/main/java/net/minecraftforge/fml/common/SidedProxy.java +++ b/src/main/java/net/minecraftforge/fml/common/SidedProxy.java @@ -69,9 +69,8 @@ public @interface SidedProxy String serverSide() default ""; /** - * The (optional) name of a mod to load this proxy for. This will help ensure correct behaviour when loading a combined - * scala/java mod package. It is almost never going to be required, unless you ship both Scala and Java {@link Mod} content - * in a single jar. + * The name of a mod to load this proxy for. This is required if this annotation is not in the class with @Mod annotation. + * Or there is no other way to determine the mod this annotation belongs to. When in doubt, add this value. */ String modId() default ""; } diff --git a/src/main/java/net/minecraftforge/fml/common/discovery/ASMDataTable.java b/src/main/java/net/minecraftforge/fml/common/discovery/ASMDataTable.java index 3ded1d194..37106e67a 100644 --- a/src/main/java/net/minecraftforge/fml/common/discovery/ASMDataTable.java +++ b/src/main/java/net/minecraftforge/fml/common/discovery/ASMDataTable.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.Set; import net.minecraftforge.fml.common.ModContainer; +import net.minecraftforge.fml.common.discovery.ASMDataTable.ASMData; import com.google.common.base.Predicate; import com.google.common.collect.HashMultimap; @@ -146,4 +147,18 @@ public class ASMDataTable { return this.packageMap.get(pkg); } + + public static String getOwnerModID(Set mods, ASMData targ) + { + if (mods.size() == 1) { + return (String)mods.iterator().next().getAnnotationInfo().get("modid"); + } else { + for (ASMData m : mods) { + if (targ.getClassName().startsWith(m.getClassName())) { + return (String)m.getAnnotationInfo().get("modid"); + } + } + } + return null; + } } diff --git a/src/test/java/net/minecraftforge/debug/ObjectHolderTest.java b/src/test/java/net/minecraftforge/debug/ObjectHolderTest.java index 7d807246d..8d7b5db1b 100644 --- a/src/test/java/net/minecraftforge/debug/ObjectHolderTest.java +++ b/src/test/java/net/minecraftforge/debug/ObjectHolderTest.java @@ -44,7 +44,7 @@ public class ObjectHolderTest } } - @Mod.EventBusSubscriber + @Mod.EventBusSubscriber(modid = MODID) public static class Registration { @SubscribeEvent public static void newRegistry(RegistryEvent.NewRegistry event) {