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

Incompatibility with Music Triggers Mod #116

Open Darkosto opened 1 year ago

Darkosto commented 1 year ago

According to your testing, does the issue occur with ONLY Immersive Portals mod installed?

Yes

Forge Version

40.2.4

ImmersivePortals Version

1.4.17

Latest Log

N/A

Crash Report (if applicable)

N/A

Steps to Reproduce

  1. Setup Music Triggers to play audio based on player dimension
  2. Cross Dimensional Portal Boundary
  3. Music Triggers does not update the player current dimension and will not play the correct audio based on location

What You Expected

Music triggers is meant to track player location and audio events can be assigned accordingly. When dimensions are stacked using Immersive Portals, the player location according to the Music Triggers debug menu will update when the player crosses the portal boundaries.

What Happened Instead

The player current dimension. according to Music Triggers, remains in the first dimension they spawned in.

Additional Details

No response

Please Read and Confirm The Following

Darkosto commented 1 year ago

Here's an example multi mc export to showcase the bug. Loading up a new world and stacking the Overworld on top of the Nether dimensions will cause the music track to restart constantly and/or bounce back and forth between the 2 tracks.

It seems as though Music Triggers is not aware of what dimension the player is in and constantly swaps back and forth: music_triggers_imm_portal_test.zip

Nick1st commented 1 year ago

Music Triggers is reading the LocalPlayer.level field, that is swapped between all visible dimensions in order to allow ticking of the (client)world to occur correctly. As it runs it own thread, from time to time Music Triggers ends up with the "wrong" dimension, thus changing the played music. To resolve this, I currently see the following potential fixes:

  1. I'm simply going to mixin into Music Triggers in order to patch each mention of LocalPlayer.level with our own helpers (ClientWorldLoader.getWorldAsync(RenderStates.originalPlayerDimension)). However, the triggers are actually Bi-Functions, Mixin into them is not fun, as it randomly likes to explode /hj. Also I'd like to avoid using any more mod-specific Mixins, as that blows compile times if I want to actually test my fixes.
  2. The Entity class (LocalPlayer extends Entity) contains a getter for this field. (getLevel()). If Music Triggers would switch from direct field access to using the getter, I'd be able to mix into the getter, thus enhancing it with the ability to give the expected value to Music Triggers thread. This would however require checking the thread, as such adding quite some overhead to an often called method.
  3. An option to easily overwrite the triggers in music triggers. This would allow IP to load a module simply replacing all Bi-Functions with patched ones. This would be my preferred fix, but would requires some work on Music Triggers side. (I'll be in touch with them shortly)
  4. Simply adding multiworld aware triggers. But that wouldn't fix the issue, just circumvent it.