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

Compatibility Issue: NeoForge parses both Manifest.MF and mods.toml [[mixins]], causing unwanted behaviour #264

Closed
ricksouth opened this issue Nov 9, 2023 · 5 comments
Labels
triage Needs triaging and confirmation

Comments

@ricksouth
Copy link

Minecraft Version: 1.20.2

NeoForge Version: 20.2.43-beta

Description of issue:
I'm not sure this would be an issue or a feature/enhancement. It's a compatibility issue but in a specific case between mod loaders within the same jar.

For the past year I've been able to put my Fabric and Forge mod code in the same jar. This is possible by using the Multiloader template, and on export splitting the Common source to common_fabric and common_forge. I have (almost) successfully been able to implement the same principe with Fabric/Forge/NeoForge. And I thought it all worked, but have come across one thing that I'm unable to fix myself.

Because of neoforged/FancyModLoader#31, I was able to specify a mixin file for for NeoForge in the mods.toml and one for Forge in the Manifest.MF file . Forge seems to ignore the [[mixins]] entry, so that's great.

Unfortunately after export, it seems that NeoForge still reads the Manifest.MF file as well. Causing it to load the Forge mixin, instead of what has been referenced in mods.toml.

While this is a very specific issue, and combining mod loaders in the same jar isn't for everyone, I think it might be a viable solution for some. Especially with multiple mod loaders available. Having this fixed would allow some developers to support multiple mod loaders in the same jar. From what I've seen, the mixin loading seems to be the only problem that stands in the way of being able to do this.

A solution would be to first check mods.toml for a [[mixins]] entry. If that entry is found for a specific mod, it should ignore any entry found in Manifest.MF from that same jar. I think this would be preferred behaviour, if someone uses the new mixin system via mods.toml, I don't think they want to implement mixins via the Manifest.MF file anymore.

I hope this can be considered, and that it's a relatively easy thing to implement. I'm looking forward to seeing NeoForge's new features in the upcoming years.

@ricksouth ricksouth added the triage Needs triaging and confirmation label Nov 9, 2023
@Technici4n
Copy link
Member

The manifest is read directly by Mixin, and is fully outside of our control. (Which is the reason why we recommend using the mods.toml option instead).

However unless I am misunderstanding your issue it seems that you can just continue to use the manifest entry on NeoForge too?

@ricksouth
Copy link
Author

Ah that's unfortunate. I was hoping NeoForge had control over that.

Because Forge and NeoForge are in the same jar, some files unfortunately overlap. Forge can only reference a mixin file via the Manifest file. And NeoForge reads that same file, parsing Forge's mixins instead of itself. Due to the different runtime mappings, it crashes.

@ricksouth
Copy link
Author

A solution would be to ignore the Manifest file completely, as Fabric does, but I do understand that it's a very big change for a minority of use cases

@Technici4n
Copy link
Member

You can probably use a Mixin plugin to overcome this difficulty, and disable some mixins depending on the platform.

Disclaimer: We might be making more changes in the future that will make this solution impractical. This is not really a supported use case in the first place, so if it works, good for you, but if it doesn't then that's also not really our problem.

A solution would be to ignore the Manifest file completely, as Fabric does, but I do understand that it's a very big change for a minority of use cases

Unfortunately this is not possible without making changes to Mixin itself, which we do not want to do for the time being.

@ricksouth
Copy link
Author

Got it, I totally understand. I'll take a look at other options. Appreciate the quick response, feel free to close the issue

@Technici4n Technici4n closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triaging and confirmation
Projects
None yet
Development

No branches or pull requests

2 participants