iPortalTeam / ImmersivePortalsModForForge

Non-Euclidean in Minecraft. See through portals and teleport seamlessly.
https://qouteall.fun/immptl/
Apache License 2.0
43 stars 18 forks source link

Fix mixin for compat #96

Closed Mysticpasta1 closed 9 months ago

Mysticpasta1 commented 1 year ago

Fix a possible compat crash if ChunkHolder#broadcast becomes public from another mod

Nick1st commented 1 year ago

What mod incompatibilities does this fix? I'm asking because I think this currently acts as a fail-fast.

Mysticpasta1 commented 1 year ago

crash-2023-04-04_08.39.10-server.txt here is the crash report that it fixes

it is a mixin error

Caused by: org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException: PRIVATE @Overwrite method m_140063_ in imm_ptl.mixins.json:common.chunk_sync.MixinChunkHolder cannot reduce visibiliy of PUBLIC target method
    at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.conformVisibility(MixinPreProcessorStandard.java:485) ~[mixin-0.8.5.jar:0.8.5+Jenkins-b310.git-155314e6e91465dad727e621a569906a410cd6f4] {}
Mysticpasta1 commented 1 year ago

basically if another mod makes ChunkHolder#broadcast public through an AT/AW it causes this mixin to fail due to you can't reduce visibility (going from public to private) in an Overwrite method, this fixes it by making the Overwrite public and AT/AW the method before other mods do the same thing.

Mysticpasta1 commented 1 year ago

btw the fix is meant for 1.19.2 forge

Nick1st commented 1 year ago

Again, what mod incompatibility does this fix?

Mysticpasta1 commented 1 year ago

not sure which mod causes the method to become public so this is a blanket solution for the crash report I sent

Mysticpasta1 commented 1 year ago

the method becoming public will cause this message in a crash report

Caused by: org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException: PRIVATE @Overwrite method m_140063_ in imm_ptl.mixins.json:common.chunk_sync.MixinChunkHolder cannot reduce visibiliy of PUBLIC target method
    at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.conformVisibility(MixinPreProcessorStandard.java:485) ~[mixin-0.8.5.jar:0.8.5+Jenkins-b310.git-155314e6e91465dad727e621a569906a410cd6f4] {}
Nick1st commented 1 year ago

I don't like solutions where I can't tell what effects it has. Especially as this is in the chunk sync. Will have to think about this.

Mysticpasta1 commented 1 year ago

I have been telling you it fixes the crash report I sent earlier, all it does is make the Overwrite public instead of private, it should be completely fine

Mysticpasta1 commented 1 year ago

crash-2023-04-04_08.39.10-server.txt

Nick1st commented 1 year ago

it should be completely fine

That's my problem. It may be, but it may also cause hard to debug issues with other mods. The current modifier acts like a fail-fast if another mod is calling this method. There are very few mods that'll actually need to call this and most of them will have some sort of incompatibility with ImmersivePortals.

Mysticpasta1 commented 1 year ago

modlist.txt change this to a html, these are the mods that where in the pack that caused the crash report with IP until the change I made fixed it

btw I have a jar with this change :D