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

Fixes crashes caused by reflection loading. #602

Closed
wants to merge 3 commits into from
Closed

Fixes crashes caused by reflection loading. #602

wants to merge 3 commits into from

Conversation

ByThePowerOfScience
Copy link

The most popular method to load Mixins in a way that they can modify other mods is to use Dimensional Development's strategy of invoking a few key methods in MixinProcessor via reflection. This is no longer possible since these elements have been removed. This PR adds them back in a deprecated form as a mainstay against an outright crash.

Normally I'd be all for removing these things entirely, but the problem is that this is as far as I can tell the only way to modify other mods in 1.12, meaning that if any mod decides to include 0.8, every other mod that uses this method to load its mixins will cause crashes immediately.

Fixes crash with loaders invoking the class mid-init to modify other mods. As far as I know it's the only way to modify other mods with Mixin, so it really shouldn't crash on newer Mixin versions.
Fixes crash caused by mods accessing this via reflection.
@LlamaLad7
Copy link
Contributor

Can and should be fixed on your end, see CleanroomMC/MixinBooter@095a6e0

@ByThePowerOfScience
Copy link
Author

ByThePowerOfScience commented Sep 9, 2022

Can and should be fixed on your end, see LoliKingdom/MixinBooter@095a6e0

This isn't the only mod that uses this loader. Every mod that implements this itself has this issue, including VanillaFix, every other DimDev mod, and ErebusFix, and most of these mods cannot be changed at this point.

@SizableShrimp
Copy link
Contributor

I don't really think old mods being old is a reason to add deprecated compat into Mixin.

@gabizou
Copy link
Member

gabizou commented Sep 11, 2022

I’m not going to speak for Mumfrey, but the real thing that other mods explicitly used reflection on internal non-api classes was already a poor decision to be made on their end. It’s not our responsibility to maintain other people’s workarounds when we already made what API is available and then implementing workarounds that weren’t supported to begin with.

Adding a deprecated annotation doesn’t do anything to explain to a mod developer why it’s not supported.

@ByThePowerOfScience
Copy link
Author

ByThePowerOfScience commented Sep 15, 2022 via email

@ByThePowerOfScience ByThePowerOfScience closed this by deleting the head repository Oct 30, 2023
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.

None yet

4 participants