CleanroomMC / MixinBooter

Allows any mixins that work on mods to work effortlessly. With a single class and an annotation. On 1.12.2.
GNU Lesser General Public License v2.1
55 stars 17 forks source link

Crash with newest version. #12

Closed JonCris closed 2 years ago

JonCris commented 2 years ago

Everything was fine with the now archived version: .mixinbooter-4.1 but after updating to [mixinbooter]-4.1.jar It crashes on startup, no way of going back to the archived version and all the other versions seem to crash as well.

Crashlog: https://gist.github.com/JonCris/68e7a6d7ca5ba4c7715571b64cc66947

Full log: https://gist.github.com/JonCris/4d16ff9ac00c2ac1f00b8387fcb9393a

embeddedt commented 2 years ago

It seems to be caused by SecurityCraft having a name that orders before MixinBooter. This causes it to load its own, bundled copy of Mixin 0.8.3. That version is too new and crashes with Phosphor.

Renaming the MixinBooter JAR to its old name will probably solve this.

Rongmario commented 2 years ago

SecurityCraft should not have to take priority over any other mods...

embeddedt commented 2 years ago

I am working on modifying some mods (like Phosphor) to use MixinBooter instead of shading Mixin in themselves. What's the recommended approach for instantiating vanilla/Forge mixins that does not crash before getting to the dependencies screen when MixinBooter isn't installed?

Currently it does this:

public class PhosphorFMLSetupHook implements IFMLCallHook {
    private static final Logger logger = LogManager.getLogger("Phosphor Forge Core");

    @Override
    public void injectData(Map<String, Object> data) {
    }

    @Override
    public Void call() {
        logger.debug("Success! Phosphor has been called into from Forge... initializing Mixin environment and configurations");

        Mixins.addConfiguration("mixins.phosphor.json");

        return null;
    }
}

... which crashes like this outside of dev:

[13:07:02] [main/INFO] [STDERR]: [org.multimc.onesix.OneSixLauncher:launchWithMainClass:214]: Caused by: java.lang.NoClassDefFoundError: org/spongepowered/asm/mixin/Mixins
[13:07:02] [main/INFO] [STDERR]: [org.multimc.onesix.OneSixLauncher:launchWithMainClass:214]:   at me.jellysquid.mods.phosphor.core.PhosphorFMLSetupHook.call(PhosphorFMLSetupHook.java:23)
[13:07:02] [main/INFO] [STDERR]: [org.multimc.onesix.OneSixLauncher:launchWithMainClass:214]:   at me.jellysquid.mods.phosphor.core.PhosphorFMLSetupHook.call(PhosphorFMLSetupHook.java:11)
[13:07:02] [main/INFO] [STDERR]: [org.multimc.onesix.OneSixLauncher:launchWithMainClass:214]:   at net.minecraftforge.fml.relauncher.CoreModManager$FMLPluginWrapper.injectIntoClassLoader(CoreModManager.java:165)
Rongmario commented 2 years ago

How do you want it to react? If you don't shade in mixins and do not have MixinBooter installed, I don't think there would be any way (unless some other mod shades in) for that to work?

embeddedt commented 2 years ago

Perhaps MixinBooter can provide another annotation (or interface) that could be used to instantiate vanilla mixins and not just mod mixins? Then mods can use the Optional.Interface feature from Forge to strip that off if MixinBooter isn't installed. As long as the mod has MixinBooter in its dependencies, Forge should display a proper "MixinBooter is missing" message quite early on in startup.

Rongmario commented 2 years ago

That is doable, I can offer an interface instead.

Rongmario commented 2 years ago

@embeddedt can you build commit https://github.com/LoliKingdom/MixinBooter/commit/6abad6422a60baeb27f7bb5fe67558c46d893dac and see if the interface is a better way?

embeddedt commented 2 years ago

I'll try it. Does this work for mixins into vanilla code as well, or just for mods?

embeddedt commented 2 years ago

Unfortunately it doesn't seem to work for some reason. Here's the crash I get (this happens in dev and in a normal environment):

java.lang.ClassNotFoundException: me/jellysquid/mods/phosphor/core/PhosphorMixinLoader
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:264)
    at net.minecraftforge.fml.common.LoadController.handler$zza000$beforeConstructing(LoadController.java:543)
    at net.minecraftforge.fml.common.LoadController.distributeStateMessage(LoadController.java)
    at net.minecraftforge.fml.common.Loader.loadMods(Loader.java:595)
    at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:232)
    at net.minecraft.client.Minecraft.init(Minecraft.java:514)
    at net.minecraft.client.Minecraft.run(Minecraft.java:422)
    at net.minecraft.client.main.Main.main(Main.java:118)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
    at net.minecraft.launchwrapper.Launch.main(Launch.java:28)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97)
    at GradleStart.main(GradleStart.java:25)

The relevant class is right here. Unless I missed something this looked like it should work.

For running in a normal environment, I did look within the JAR and the class file is present within it, so it seems like there's a classloader issue?

JonCris commented 2 years ago

It seems to be caused by SecurityCraft having a name that orders before MixinBooter. This causes it to load its own, bundled copy of Mixin 0.8.3. That version is too new and crashes with Phosphor.

Renaming the MixinBooter JAR to its old name will probably solve this.

Renaming worked, but not ideal since this will be a public modpack, also only gives me the option to either remove securitycraft or remove everything using MixinBooter :P

Rongmario commented 2 years ago

@embeddedt try again with https://github.com/LoliKingdom/MixinBooter/commit/970ee10e1f619411cf3e74e1e4fdede6eecee6db. By the way, I saw that you were interested in Fluidlogging API as well, do you use Discord?

@JonCris unfortunately I might have to try another renaming scheme, something like [.mixinbooter].jar to be on top of the directory ahead of SecurityCraft, it sucks.

embeddedt commented 2 years ago

That has fixed the ClassNotFoundException. Thanks! However, I now have the usual problem of classes being loaded too late.

[21:27:55] [Client thread/FATAL] [mixin]: Mixin prepare failed preparing client.MixinMinecraft in mixins.phosphor.json: org.spongepowered.asm.mixin.transformer.throwables.MixinTargetAlreadyLoadedException Critical problem: mixins.phosphor.json:client.MixinMinecraft target net.minecraft.client.Minecraft was loaded too early.
org.spongepowered.asm.mixin.transformer.throwables.MixinTargetAlreadyLoadedException: Critical problem: mixins.phosphor.json:client.MixinMinecraft target net.minecraft.client.Minecraft was loaded too early.
        at org.spongepowered.asm.mixin.transformer.MixinInfo.readDeclaredTargets(MixinInfo.java:937) ~[mixinbooter-4.2.jar:?]
        at org.spongepowered.asm.mixin.transformer.MixinInfo.<init>(MixinInfo.java:871) ~[mixinbooter-4.2.jar:?]
        at org.spongepowered.asm.mixin.transformer.MixinConfig.prepareMixins(MixinConfig.java:713) ~[mixinbooter-4.2.jar:?]
        at org.spongepowered.asm.mixin.transformer.MixinConfig.prepare(MixinConfig.java:646) ~[mixinbooter-4.2.jar:?]
        at org.spongepowered.asm.mixin.transformer.MixinProcessor.prepareConfigs(MixinProcessor.java:514) [mixinbooter-4.2.jar:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_332]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_332]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_332]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_332]
        at net.minecraftforge.fml.common.LoadController.handler$zza000$beforeConstructing(LoadController.java:573) [LoadController.class:?]
        at net.minecraftforge.fml.common.LoadController.distributeStateMessage(LoadController.java) [LoadController.class:?]
        at net.minecraftforge.fml.common.Loader.loadMods(Loader.java:595) [Loader.class:?]
        at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:232) [FMLClientHandler.class:?]
        at net.minecraft.client.Minecraft.init(Minecraft.java:514) [Minecraft.class:?]
        at net.minecraft.client.Minecraft.run(Minecraft.java:422) [Minecraft.class:?]
        at net.minecraft.client.main.Main.main(Main.java:118) [Main.class:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_332]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_332]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_332]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_332]
        at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) [launchwrapper-1.12.jar:?]
        at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_332]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_332]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_332]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_332]
        at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97) [start/:?]

Maybe a second interface is needed that loads before mods do (still within the coremod)?

By the way, I saw that you were interested in Fluidlogging API as well, do you use Discord?

I have a Discord account now but I don't really use it much, as I'm used to development on GitHub.

Rongmario commented 2 years ago

It'd be very annoying to gather mixins that mixin into vanilla/forge classes, I'm already doing it at the earliest time possible with Forge's current structure. You'll have to bootstrap and add the configs yourself...

We do a lot of 1.12 modding and discussion is always on discord, if you're interested just add me, I left my name in the Fluidlogging API issue thread.

Rongmario commented 2 years ago

With latest version, this should be fixed.