Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force injection of FML coremods/mods does not work on MC 1.7.10 and below #17

Closed
yuuka-miya opened this issue Feb 22, 2015 · 8 comments
Closed
Assignees
Labels

Comments

@yuuka-miya
Copy link

MixinTweaker only supports the post-MC 1.8 FML CoreModManager (net.minecraftforge.fml namespace), whereas other parts of the mixin codebase support the cpw.mods.fml namespace used by FML before MC 1.8.

This "bug" has prevented me from being able to test FE, which employs mixins in a 1.7.10 environment, within a production workspace.

Will this be fixed?

yuuka-miya pushed a commit to ForgeEssentials/ForgeEssentials that referenced this issue Feb 22, 2015
DO NOT ATTEMPT TO BUILD THIS BRANCH, it will not work due to SpongePowered/Mixin#17
@Mumfrey Mumfrey added the bug label Feb 22, 2015
@Mumfrey Mumfrey self-assigned this Feb 22, 2015
@Mumfrey
Copy link
Member

Mumfrey commented Feb 22, 2015

That should fix it, let me know if it doesn't.

@yuuka-miya
Copy link
Author

It doesn't. Here's a build of FE including mixins and this bug: http://1drv.ms/1EHWnl6

EDIT: Seems as though FML still thinks it's looked through the file: http://puu.sh/g9eC8/074a31570b.png

It seems as though I may be forced to write my own tweaker which implements both Mixin's and my own coremod, which then injects my forge mod with voodoo magic (since coremods are treated as tweakers).

Another EDIT: On further investigation, it seems that the Mixin tweaker is removing the class from the list of loaded coremods BEFORE FML even adds it. This is either inconsistencies in FML behaviour that needs to be changed (is it even possible), or the need for Mixin to be able to initialize via host coremod/tweaker instead of specifying its own which then loads the host coremod/tweaker (once again, if possible).

@Mumfrey
Copy link
Member

Mumfrey commented Feb 23, 2015

What I don't quite understand is how this is working for @Zidane (who is also using it in 1.7.10) and not for you. The logic for the tweaker goes like this:

  • Mixins must be loaded by a tweaker in production to end up in the correct classloader
  • If FML parses a file and it has a tweaker, it does no further processing of that file and doesn't look for coremods or anything, this means the tweaker has to
  • First, try to forcibly load a coremod specified in the manifest, this process should return a wrapper which is itself an ITweaker and is a handle to the coremod instance
  • If this fails and the user specifies the ForceLoadAsMod key in the manifest, the tweaker attempts to remove its container from the loaded list in order to give FML a shot at loading the coremod normally on a second pass.

However, all of this is based on 1.8 behaviour because Mixins is a 1.8 project and that's all I care about, 1.7.10 compatibility is added as a courtesy but I cannot really devote the time to troubleshooting 1.7.10 issues.

Things I know:

  • The current approach works fine in 1.8
  • The current approach is working fine for @Zidane in 1.7.10
  • It's entirely possible that some or all of the above assumptions that the tweaker makes are not valid for all historic 1.7.10 releases (they must be true for at least one release because it's working in at least one case)

This means you should:

  • First, I suggest, chat to @Zidane on IRC and see if your approaches differ, and find out if he's doing something you're not
  • Second, compare notes on which FML version you're both using. If you're using an outdated version of FML that may be the issue, or it could be some other combination of circumstances the current tweaker has not accounted for

This is the limit of support I'm prepared to provide for 1.7.10 though, developing for outdated versions of minecraft is something we should all collectively discourage because all it does is pour more fuel on the fire of modders not bothering to update their stuff because "everyone's still on x version". The absolute limit of previous versions I'm prepared to entertain support for is the very latest release on 1.7.10, but no other previous versions under any circumstances.

@yuuka-miya
Copy link
Author

Alright, thanks for the help anyway.

EDIT: Is it possible for me to use my own tweaker to do mixin initialization?

@Zidane
Copy link
Member

Zidane commented Feb 23, 2015

@luacs1998

https://github.com/AlmuraDev/AlmuraSDK

Take a look at how I use Mixins here. I can absolutely confirm that this does work for 1.7.10.

@Mumfrey
Copy link
Member

Mumfrey commented Feb 23, 2015

Alright, thanks for the help anyway.

Don't get me wrong, if you come back and say to me "I've figured out you need to make x, y, z change and then this'll work in 1.7.10" then I'll add that no problem, but the focus of development has to be 1.8 and I can't really devote the time for troubleshooting on an outdated platform.

EDIT: Is it possible for me to use my own tweaker to do mixin initialization?

Yes absolutely, the tweaker in the package is provided for convenience but ultimately it doesn't matter which tweaker actually gets the ball rolling.

@yuuka-miya
Copy link
Author

Hmm, I'm doing the exact same things as Zidane (from what I see), but why is mixin complaining that it isn't in the right classloader when I try to getEnvironment() in my coremod? Must I reflectively call MixinBootstrap.init()?

Just thinking out loud here.

@Mumfrey
Copy link
Member

Mumfrey commented Feb 23, 2015

Because you can't initialise the mixin environment from a coremod in production, as I said, the mixin environment must be initialised from a tweaker in a production environment. In dev, it's safe to dispense with the tweaker and use the coremod, that's why both bits of code exist: one handles dev, the other handles production.

Once it's successfully initalised in the right classloader, it's safe to call getEnvironment from elsewhere, but the MixinBootstrap.init() must be called in the system classloader. The bootstrap handles excluding the mixin package from the LaunchClassLoader in order that this state of affairs obtains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants