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.
This commit is contained in:
LexManos 2016-12-05 17:19:15 -08:00
parent aee9d8c9b4
commit 160427b12c
7 changed files with 92 additions and 12 deletions

View file

@ -20,6 +20,8 @@
package net.minecraftforge.fml.common; package net.minecraftforge.fml.common;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.SetMultimap;
import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.fml.common.discovery.ASMDataTable; import net.minecraftforge.fml.common.discovery.ASMDataTable;
import net.minecraftforge.fml.common.discovery.ASMDataTable.ASMData; 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) public static void inject(ModContainer mod, ASMDataTable data, Side side)
{ {
FMLLog.fine("Attempting to inject @EventBusSubscriber classes into the eventbus for %s", mod.getModId()); FMLLog.fine("Attempting to inject @EventBusSubscriber classes into the eventbus for %s", mod.getModId());
Set<ASMDataTable.ASMData> targets = data.getAnnotationsFor(mod).get(Mod.EventBusSubscriber.class.getName()); SetMultimap<String, ASMData> modData = data.getAnnotationsFor(mod);
Set<ASMDataTable.ASMData> mods = modData.get(Mod.class.getName());
Set<ASMDataTable.ASMData> targets = modData.get(Mod.EventBusSubscriber.class.getName());
ClassLoader mcl = Loader.instance().getModClassLoader(); ClassLoader mcl = Loader.instance().getModClassLoader();
for (ASMDataTable.ASMData targ : targets) for (ASMDataTable.ASMData targ : targets)
@ -62,6 +66,19 @@ public class AutomaticEventSubscriber
} }
if (sides == DEFAULT || sides.contains(side)) { if (sides == DEFAULT || sides.contains(side)) {
FMLLog.fine("Found @EventBusSubscriber class %s", targ.getClassName()); 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); Class<?> subscriptionTarget = Class.forName(targ.getClassName(), true, mcl);
MinecraftForge.EVENT_BUS.register(subscriptionTarget); MinecraftForge.EVENT_BUS.register(subscriptionTarget);
FMLLog.fine("Injected @EventBusSubscriber class %s", targ.getClassName()); FMLLog.fine("Injected @EventBusSubscriber class %s", targ.getClassName());

View file

@ -435,11 +435,27 @@ public class FMLModContainer implements ModContainer
private void parseSimpleFieldAnnotation(SetMultimap<String, ASMData> annotations, String annotationClassName, Function<ModContainer, Object> retriever) throws IllegalAccessException private void parseSimpleFieldAnnotation(SetMultimap<String, ASMData> annotations, String annotationClassName, Function<ModContainer, Object> retriever) throws IllegalAccessException
{ {
Set<ASMDataTable.ASMData> mods = annotations.get(Mod.class.getName());
String[] annName = annotationClassName.split("\\."); String[] annName = annotationClassName.split("\\.");
String annotationName = annName[annName.length - 1]; String annotationName = annName[annName.length - 1];
for (ASMData targets : annotations.get(annotationClassName)) for (ASMData targets : annotations.get(annotationClassName))
{ {
String targetMod = (String)targets.getAnnotationInfo().get("value"); 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; Field f = null;
Object injectedMod = null; Object injectedMod = null;
ModContainer mc = this; ModContainer mc = this;
@ -751,4 +767,4 @@ public class FMLModContainer implements ModContainer
{ {
return this.classVersion; return this.classVersion;
} }
} }

View file

@ -282,6 +282,12 @@ public @interface Mod
* The mod object to inject into this field * The mod object to inject into this field
*/ */
String value() default ""; 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. * 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 * The mod id specifying the metadata to load here
*/ */
String value() default ""; 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) @Target(ElementType.TYPE)
public @interface EventBusSubscriber { public @interface EventBusSubscriber {
Side[] value() default { Side.CLIENT, Side.SERVER }; 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 "";
} }
} }

View file

@ -30,6 +30,7 @@ import net.minecraftforge.fml.relauncher.Side;
import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Level;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.SetMultimap;
/** /**
* @author cpw * @author cpw
@ -40,13 +41,31 @@ public class ProxyInjector
public static void inject(ModContainer mod, ASMDataTable data, Side side, ILanguageAdapter languageAdapter) public static void inject(ModContainer mod, ASMDataTable data, Side side, ILanguageAdapter languageAdapter)
{ {
FMLLog.fine("Attempting to inject @SidedProxy classes into %s", mod.getModId()); FMLLog.fine("Attempting to inject @SidedProxy classes into %s", mod.getModId());
Set<ASMData> targets = data.getAnnotationsFor(mod).get(SidedProxy.class.getName()); SetMultimap<String, ASMData> modData = data.getAnnotationsFor(mod);
Set<ASMData> mods = modData.get(Mod.class.getName());
Set<ASMData> targets = modData.get(SidedProxy.class.getName());
ClassLoader mcl = Loader.instance().getModClassLoader(); ClassLoader mcl = Loader.instance().getModClassLoader();
for (ASMData targ : targets) for (ASMData targ : targets)
{ {
try 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); Class<?> proxyTarget = Class.forName(targ.getClassName(), true, mcl);
Field target = proxyTarget.getDeclaredField(targ.getObjectName()); Field target = proxyTarget.getDeclaredField(targ.getObjectName());
if (target == null) if (target == null)
@ -58,11 +77,6 @@ public class ProxyInjector
target.setAccessible(true); target.setAccessible(true);
SidedProxy annotation = target.getAnnotation(SidedProxy.class); 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(); String targetType = side.isClient() ? annotation.clientSide() : annotation.serverSide();
if(targetType.equals("")) if(targetType.equals(""))
{ {

View file

@ -69,9 +69,8 @@ public @interface SidedProxy
String serverSide() default ""; String serverSide() default "";
/** /**
* The (optional) name of a mod to load this proxy for. This will help ensure correct behaviour when loading a combined * The name of a mod to load this proxy for. This is required if this annotation is not in the class with @Mod annotation.
* scala/java mod package. It is almost never going to be required, unless you ship both Scala and Java {@link Mod} content * Or there is no other way to determine the mod this annotation belongs to. When in doubt, add this value.
* in a single jar.
*/ */
String modId() default ""; String modId() default "";
} }

View file

@ -24,6 +24,7 @@ import java.util.Map;
import java.util.Set; import java.util.Set;
import net.minecraftforge.fml.common.ModContainer; import net.minecraftforge.fml.common.ModContainer;
import net.minecraftforge.fml.common.discovery.ASMDataTable.ASMData;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
@ -146,4 +147,18 @@ public class ASMDataTable
{ {
return this.packageMap.get(pkg); return this.packageMap.get(pkg);
} }
public static String getOwnerModID(Set<ASMData> 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;
}
} }

View file

@ -44,7 +44,7 @@ public class ObjectHolderTest
} }
} }
@Mod.EventBusSubscriber @Mod.EventBusSubscriber(modid = MODID)
public static class Registration { public static class Registration {
@SubscribeEvent @SubscribeEvent
public static void newRegistry(RegistryEvent.NewRegistry event) { public static void newRegistry(RegistryEvent.NewRegistry event) {