Change mod type to LIBRARY, as opposed to GAMELIBRARY, on neoforge#136
Change mod type to LIBRARY, as opposed to GAMELIBRARY, on neoforge#136lukebemish wants to merge 3 commits into
Conversation
I'm unclear on how this worked to begin with?
|
The only adverse effect I can think of is it would break this workaround not sure if ML-era neo has any other way to get the transforming CL? |
|
do bear in mind in your testing that this will need to work on all neo versions |
|
What is that workaround currently used for? As in, what resources are you getting as streams that have to come from the TCL? |
|
Mixin configs |
|
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. |
|
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. |
|
Yes well, mixin doing it that way feels mildly unreliable and cursed as well :P. I'll take a poke. |
|
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. |
|
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. |
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 toGAMELIBRARY, is a good idea since otherwise the contract ofClassProcessors 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/tohttps://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.