SpongePowered / Mixin

Mixin is a trait/mixin and bytecode weaving framework for Java using ASM
MIT License
1.42k stars 193 forks source link

MixinPlatformManager should call scanClasspath immediately #153

Closed Aaron1011 closed 7 years ago

Aaron1011 commented 7 years ago

Currently, MixinPlatformManager first calls its scanClasspath when triggered by MixinTweaker's injectIntoClassLoader method (from ITweaker). This is normally fine, but creates a problem when two different coremods are using the Mixin tweaker.

When MixinTweaker is specified in the manifest of two different coremods, the first of the two loaded by FML will 'win', and MixinPlatformManager will be initialized with the appropriate MixinContainers for that coremod. LaunchWrapper then attempts to load MixinTweaker for second coremod, detects that MixinTweaker has already been loaded, and skips running it again.

Normally, the second coremod will still have its mixins loaded properly, as scanClasspath will detect any other coremods using mixin. Unfortunately, the fact that scanClasspath is called from injectIntoClassLoader means that it is called too late for mixins for certain classes to be registered. Any mixins configs for the PREINIT phase (and probably INIT as well) will not be loaded before their target classes are classloaded.

This issue is creating problems with the MovingWorld mod (here's a log which more clearly shows the issue), which also uses Mixin. This particular crash is caused by Sponge being unable to mixin to Forge's Event crash, which gets classloaded by EventSubscriptionTransformer during PREINIT, before injectIntoClassLoader is called.

I believe the simplest solution to this problem would be to call scanClasspath immediately in the constructor of MixinPlatformManager. At this point, all Mixin-using coremods should be on the classpath, as they all added to the classpath by CoreModManager#discoverCoreMods. This would allow all Mixin-using coremods to be discovered immediately, allowing them to register their mixins before any Minecraft classes are loaded.

Mumfrey commented 7 years ago

I have no idea how I missed this issue. I will look into it.