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

Mod transformers are applied inconsistently if loading fails #43

Closed
embeddedt opened this issue Jul 22, 2023 · 2 comments
Closed

Mod transformers are applied inconsistently if loading fails #43

embeddedt opened this issue Jul 22, 2023 · 2 comments
Labels
bug A bug or error

Comments

@embeddedt
Copy link
Member

embeddedt commented Jul 22, 2023

If early mod loading fails (e.g. due to a missing dependency), mod access transformers are not applied (since only the ATs from system mods are loaded) but their mixins are still applied. If the mod mixes into a class that is used within the code path to display the dependency error screen, and that mixin references a field in a manner that requires the mod's access transformer to also have been applied, the game will crash.

To see this in the wild, install Rubidium 0.6.5 and Sophisticated Backpacks 3.18.56.890 on Forge 47.1.0. Be sure not to install Sophisticated Core. Rather than displaying the missing dependency screen, the game will crash with an IllegalAccessError, as Rubidium includes a mixin that depends on its access transformer.

In my opinion this design is confusing and inconsistent. It also creates misleading crash reports for users that show the IllegalAccessError instead of the actual problem (which is that a a dependency is installed). Both access transformers and mixins are modifying core game code, so it seems prudent to either not apply mod mixins in this scenario, or also apply mod ATs. This current design works in the case that the mod uses only ATs and Forge events (as its event handlers are skipped) but does not work in a Mixin world.

It's possible to work around the issue by checking ModLoader.isLoadingStateValid() (after constructors fire) or LoadingModList.get().getErrors().isEmpty() (before constructors fire) in critical mixins, but I don't think modders should have to do this.

@sciwhiz12 sciwhiz12 added the bug A bug or error label Jul 23, 2023
@MehVahdJukaar
Copy link

This would prevent so many headaches to us modders as people don't install dependencies and get hard crashes like this all the times. I did point this out before in MCF and answer was that it didn't matter as mixins/coremods shouldn't be used

@Technici4n
Copy link
Member

Fixed by neoforged/FancyModLoader#31.

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

No branches or pull requests

5 participants
@Technici4n @sciwhiz12 @MehVahdJukaar @embeddedt and others