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

Mixins can not modify non-coremods in a production environment. #57

Closed
thomas15v opened this issue Jul 6, 2015 · 5 comments
Closed

Comments

@thomas15v
Copy link

I realise that this is not the intended purpose of this library, but it can come in handy. What I tried to do is write a mixins for the mod ComputerCraft. As mixins are much easier than manually ASM everything around. I wanted to add additional behaviour to the turtle block, so that it will give a break event with the last clicked user as breaking user.

MixinClass
(As far I understood I need to use the obfuscated names, as mixins doesn't check inheritance on compiletime)
ErrorLog

As far I understood, this is caused because mixins thinks the loading is done after coremods are loaded.

@thomas15v
Copy link
Author

I just realise that the problem is this:

@Mixin(TileTurtle.class)

When the classloader loads the above. It will try to look for TileTurtle.class The problem is that the class does not exist yet. A solution would be:

@Mixin(targets = "dan200/computercraft/shared/turtle/blocks/TileTurtle", remap = false)

Only problem is that the mixin compiler doesn't allow this:

MixinTileTurtle.java:14: error: Mixin target dan200/computercraft/shared/turtle/blocks/TileTurtle is public and must be specified in value

Edit: just tried that with removing the errors. And the result is the same 😟.

Edit2: I temporary fixed it by converting normal mods to coremods. Sadly enough it requires to crash the server on initial setup. https://gist.github.com/thomas15v/1c0fd9919198f1eb7775

EDIT3: I think I found the issue. Mixins tries to get the bytecode of all the classes in his config on load. Problem is that the class I need to modify isn't in the classpath on initiation. And also will never be. As mod classes only get loaded if they got trough all the IClassTransformers.

@thomas15v thomas15v changed the title Mixins can not modify non-mods in a production environment. Mixins can not modify non-coremods in a production environment. Jul 6, 2015
@Mumfrey
Copy link
Member

Mumfrey commented Jul 7, 2015

Please stop guessing what the issue is, it's not helpful. Just state the problem and leave it at that.

It's quite simple: mixins cannot transform mods because they are not on the classpath at the transition to the DEFAULT Environment, when those configs are loaded and the metadata generation pass occurs. Allowing this to happen would require another Environment Phase to be added. Since this cannot be done in a platform-agnostic way, it would require the additon of arbitrary Environments (or at least some pre-defined ones), which is not impossible but is not going to be a priority to implement because in order to implement it neatly it obviously needs some extra infrastructure.

I'm going to officially state for the moment that transforming mods is currently not supported, but will be supported at some time in the future.

@thomas15v
Copy link
Author

Oke 😟. I will currently will just convert normal mods to coremods in order to patch them. Somehow forge doesn't really seems to care if the mods are already in the classpath or not. But I have to say. Mod modification support would definitely help out a lot of modded server communities. As ASM and reflection are mostly their only hope, unless they want to ban the specific modded items.

But I understand, it doesn't really add much to the sponge project and it is probably a lot of work to make it work to. Not to mention that it might break compatibility with SpongeVanilla... .

@Mumfrey
Copy link
Member

Mumfrey commented Jul 9, 2015

But I understand, it doesn't really add much to the sponge project and it is probably a lot of work to make it work to. Not to mention that it might break compatibility with SpongeVanilla... .

It's not that really, I mean there are other use-cases for additional environment phases. It's just that they're not particularly high priority and as I said it needs careful planning in order to not become messy.

@thomas15v
Copy link
Author

thomas15v commented Sep 25, 2016

I just leave this for people who also have this problem and want to mix non-coremods. To make normal mods available in the DEFAULT Environment all you have to do is this: https://github.com/densitycraft/Glue/blob/master/src/main/java/com/densitycraft/glue/loader/PatchLoader.java#L55-L58.

Please note that the CoreModManager.getReparseableCoremods().add(jar.getName()); is important to make sure that forge rechecks the jar for modded content and doesn't reload it in the classpath.

phase pushed a commit to LunarClient/Mixin that referenced this issue Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants