search

iPortalTeam / ImmersivePortalsMod

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

Add melius-vanish compatibility #1358

Closed DrexHD closed 1 year ago

DrexHD commented 1 year ago

This PR fixes a mod compatibility with melius-vanish. I have recently received an issue for this on my repository. After looking into this, the issue turned out to be that: Immersive Portals @Overwrites this method, which melius-vanish attempts to modify, to hide sound events from vanished players. I have now released a version of my mod, which doesn't load the conflicting mixin if imm_ptl_ore is present. This PR re-implements the missing feature using my mods API.

qouteall commented 1 year ago

Thanks for the contribution. However adding a new dependency that ImmPtl does not directly use could increase furture development burden. I am considering using MixinExtra to make the different injections merge instead of making ImmPtl to call VanishAPI.

qouteall commented 1 year ago

If there is no better way I will merge

qouteall commented 1 year ago

I made the overwrite's priority to be 800 and call connection.send.

https://github.com/iPortalTeam/ImmersivePortalsMod/commit/71ee113d3d6b746a3855422dd698bf935ab46c83#diff-d85943b48463e46f9d92db43a2bf7aa5802fe95abef78b4ace71f4282fe26c8dR35

As I tested this works https://github.com/iPortalTeam/ImmersivePortalsMod/commit/71ee113d3d6b746a3855422dd698bf935ab46c83#diff-5b77bc70dd0ad13335fd75fdca2efd745525eef0fafd003ed27b4a7cd52eaa53R13

So this mixin conflict should be solved in the next version of ImmPtl.

DrexHD commented 1 year ago

Looks like a good solution to the problem, thanks!