Skip to content

Change mod type to LIBRARY, as opposed to GAMELIBRARY, on neoforge#136

Open
lukebemish wants to merge 3 commits into
LlamaLad7:masterfrom
lukebemish:library-not-gamelibrary
Open

Change mod type to LIBRARY, as opposed to GAMELIBRARY, on neoforge#136
lukebemish wants to merge 3 commits into
LlamaLad7:masterfrom
lukebemish:library-not-gamelibrary

Conversation

@lukebemish
Copy link
Copy Markdown

Tested on neo 26.1.0.7-beta with a couple mods that use Mixin Extras with no adverse affects; neo seems to pick up the mixin jsons locating the plugin just fine, which was the most likely thing to break anyways.

Some context for those who did not see the discussion on discord: Having things that mixin loads at runtime (mixin plugins and things they reference) be LIBRARY, as opposed to GAMELIBRARY, is a good idea since otherwise the contract of ClassProcessors running after mixin is broken -- they have to worry about changes they make potentially changing what mixin does, and (more importantly) cannot request from their bytecode provider and get mixin-transformed classes if they do so during certain class-loading-attempts (of mixin plugin classes), even if the class they request from the provider is entirely unrelated to the class being loaded.

Note: this PR additionally changes the neo repo from https://maven.neoforged.net/ to https://maven.neoforged.net/releases/. I am not sure how it ever built with the former, seeing as that is not the neo repo's location? If I am missing something let me know and I'll revert that.

@LlamaLad7
Copy link
Copy Markdown
Owner

The only adverse effect I can think of is it would break this workaround

Thread.currentThread().setContextClassLoader(ResourceUtils.class.getClassLoader());

not sure if ML-era neo has any other way to get the transforming CL?

@LlamaLad7
Copy link
Copy Markdown
Owner

do bear in mind in your testing that this will need to work on all neo versions

@lukebemish
Copy link
Copy Markdown
Author

What is that workaround currently used for? As in, what resources are you getting as streams that have to come from the TCL?

@LlamaLad7
Copy link
Copy Markdown
Owner

Mixin configs

@lukebemish
Copy link
Copy Markdown
Author

Ahh it looks like you're using it to read a mixing config directly. That feels... Mildly unreliable but let me think if there's a different way to do this on the neo end that's less cursed.

@LlamaLad7
Copy link
Copy Markdown
Owner

It's the same mechanism mixin itself uses to read the configs, it's just unreliable when done at an arbitrary point in time on ModLauncher in particular, hence the workaround.

@lukebemish
Copy link
Copy Markdown
Author

lukebemish commented Mar 28, 2026

Yes well, mixin doing it that way feels mildly unreliable and cursed as well :P. I'll take a poke.

@lukebemish
Copy link
Copy Markdown
Author

See https://github.com/neoforged/FancyModLoader/blob/ebdbfe7e4c33bef423ff2a870d10f4878feca60f/loader/src/main/java/net/neoforged/fml/loading/mixin/FMLMixinService.java#L195-L209. On modern neo this is why it's not an issue. I'm shocked it's an issue on modlauncher though -- the context classloader would be the TCL when mixin itself loads configs, I would think. Are you loading stuff exceptionally before/after that or something? I'll take a poke.

@lukebemish
Copy link
Copy Markdown
Author

lukebemish commented Mar 30, 2026

I've pushed my initial attempt to get the ResourceUtils stuff fixed up on old FML -- lemme know if it looks structurally/stylistically in line with what you'd want or if there's stuff you'd like me to do another way; I'll try to test it a bit when I have the chance and make sure that the impl actually works how I expect in practice.

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

Successfully merging this pull request may close these issues.

2 participants