Up-Mods / OkZoomer

Adds a highly-configurable zoom key for Fabric and Quilt. The zoom is yours!
131 stars 24 forks source link

[1.21] OkZoomer allows mods to load when they shouldn't due to ResourceLocation changes #104

Closed Stereo528 closed 3 months ago

Stereo528 commented 3 months ago

This doesn't affect users as much as it does devs (kinda?), and isn't super duper important.

So I'm the dev for MainMenuChanger, and I threw the 1.20.4 version of my mod into my 1.21 instance to see if it would work. At first I didn't disable any of the other mods I had downloaded, and the mod loaded fine. I decided to disable all other mods except mine to see if it would load still, and found out it doesn't!

I've linked this to the changes of ResourceLocation in 1.21 via my crashlog without other mods enabled, and found that my mod only loads when OkZoomer is also loaded. I have a feeling it could somehow be related to either the ModUtils class or ZoomUtils class, both adding ResourceLocation(String path).

I feel this could have minor ramifications for mod devs who might have OkZoomer in dev envs or otherwise testing their mod to see if its suitable for a 1.21 release with minor/no changes when without OkZoomer the mod would otherwise crash... This is more of an issue for the dev of whatever mod for not testing the mod on its own/well enough, but a fix to this could at least prevent minor confusion.

TropheusJ commented 3 months ago

ok zoomer access widens Identifier so the old method still works, making your mod not crash

EnnuiL commented 3 months ago

I'm confused; the access widener on Ok Zoomer and LibZoomer are indeed there so that I could pull a Minecraft and not execute the validation of the namespace while keeping the validation of the path. However, these access wideners are not transitive, so it should not be affecting other mods in the first place!

I'll probably convert these AWs to accessor mixins in order to fix this next update; I just did the AW due to a file size microoptimization :p

EnnuiL commented 3 months ago

Update: I've just been let known of how severe this issue is; expect Ok Zoomer 9.0.1 to be worked on and released later this morning

EnnuiL commented 3 months ago

Fixed on Ok Zoomer 9.0.1 (which was quickly followed by 9.0.2 because I love shrinking file sizes (#103))