search

LXGaming / Sledgehammer

Smashes the stupid out of the client & server.
Apache License 2.0
16 stars 5 forks source link

Incompatibility with Aqua Acrobatics #27

Closed mxnmnm closed 2 years ago

mxnmnm commented 2 years ago

https://github.com/Fuzss/aquaacrobatics/issues/51

LXGaming commented 2 years ago

The ConcurrentModificationException in LaunchWrapper is typically caused by an ITweaker adding another ITweaker

embeddedt commented 2 years ago

Thanks for the pointer; I appreciate it.

The change I made in 1.5.2 was to call MixinBootstrap.init directly from the coremod constructor. I had to do this and another hack to make ForgeGradle properly load the main mod when running in userdev, otherwise it just loaded the coremod.

I must admit that while I'm quite familiar with mixins themselves, this initialization logic is a bit out of my league. Is this not the right way of going about it?

LXGaming commented 2 years ago

Interesting, Mixin is constructing SledgehammerLoadingPlugin which is adding SledgehammerTweaker which is what is causing the ConcurrentModificationException, I didn't realize Mixin constructed IFMLLoadingPlugin.

I'll have to add a check as this isn't the expected loading behavior, I'm assuming that calling MixinBootstrap.init inside the IFMLLoadingPlugin constructor is causing Mixin to load IFMLLoadingPlugin before Forge has a chance to process them.

If you're only calling MixinBootstrap.init for userdev purposes you could try checking (Boolean) Launch.blackboard.get("fml.deobfuscatedEnvironment"); and only calling init if it's a userdev environment.

embeddedt commented 2 years ago

Does MixinBootstrap (your mod) call MixinBootstrap.init? I've always assumed I have to call it myself regardless of whether I'm in userdev or not.

LXGaming commented 2 years ago

MixinBootstrap just uses the TweakClass manifest attribute for LaunchWrapper support.

Sledgehammer calls MixinBootstrap.init in the injectIntoClassLoader but only after doing some checks to ensure it's safe to do so.

The TweakerClass attribute should do all of the work unless you need it to load earlier

embeddedt commented 2 years ago

Thanks for all your help, @LXGaming. I will look into doing it that way. I had previously removed the TweakClass attribute because IIRC it was also causing problems in userdev. In general getting mixins to work on 1.12 definitely seems more fragile than with newer versions.

embeddedt commented 2 years ago

I added a TweakClass to the manifest and removed the MixinBootstrap.init call. The mixins/coremod still work, but the main mod is no longer loaded at all by Forge (it doesn't appear in the mod list). As soon as I remove the TweakClass the mod is once again registered. However, as expected, it all works just fine once the mod is compiled. So the issue is clearly within ForgeGradle.

I also removed the other hack with reparseable coremods I was using, in case it was interfering, but it didn't help.

I guess I can fix it by only applying the manifest attribute to JARs, and only calling MixinBootstrap.init in dev, but it'd be nice not to have to!

I'd try to use ForgeGradle 4 or 5 like you do, but every time I try to use one of the newer versions, I either get random compilation failures on a fresh build until the project is opened in an IDE (strange, I know), or the mod resources don't get loaded properly.

LXGaming commented 2 years ago

Have you tried editing the program arguments for the run configuration in your IDE? You can pass Mixin configs and tweak classes using the following: --mixin.config mixins.sledgehammer.platform.json --tweakClass org.spongepowered.asm.launch.MixinTweaker

Random issues with ForgeGradle are usually fixed by setting org.gradle.daemon=false in your gradle.properties

embeddedt commented 2 years ago

Very interesting. If I add --tweakClass org.spongepowered.asm.launch.MixinTweaker to the run arguments, and remove it from the manifest, then dev mode works properly (both the coremod and main mod load). It, of course, crashes when the compiled version is used, presumably because there is nothing in the manifest.

On the other hand, using the manifest works correctly when compiled, but the main mod stops being loaded in dev.

I've also disabled the daemon as you suggested.

Any further pointers? I appreciate your help. It's very strange that ForgeGradle treats the argument and the manifest differently.

embeddedt commented 2 years ago

At the moment I released a hotfix which does the Mixin setup in the constructor when running in dev, and in a setup hook when running outside dev. That seems to fix the crash with your mod, but there's probably other issues I'm not aware of.

LXGaming commented 2 years ago

Warning adding in v1.12.2-2.0.16