Immersive Portals

Immersive Portals

5M Downloads

Incompatible with Quilt

Nolij opened this issue ยท 11 comments

commented

Version 1.3.7 on Minecraft 1.18.2 with Quilt Loader 0.16.0 and QSL 1.0.0-beta.14+0.51.1-1.18.2 fails to spawn in nether or end portals. They do not render, and you cannot see through them, even if the world is reloaded. All that remains is the blocks. Strangely enough, this does not extend to dimension stacking, which works perfectly (or as perfectly as it does on Fabric that is).

I've traced the cause back to this error on launch, where this mod incorrectly uses children of the impl class, which is not supported by Fabric and is therefore not implemented by Quilt:

[12:52:08] [main/WARN]: Error loading class: net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon (java.lang.ClassNotFoundException: net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon)
[12:52:08] [main/WARN]: @Mixin target net.fabricmc.fabric.impl.networking.client.ClientPlayNetworkAddon was not found imm_ptl_fabric.mixins.json:client.MixinFabricClientPlayNetworkAddon
[12:52:08] [main/WARN]: Force-disabling mixin 'world.player_chunk_tick.ThreadedAnvilChunkStorageMixin' as rule 'mixin.world.player_chunk_tick' (added by mods [krypton, immersive_portals]) disables it and children

Additionally, when lighting a nether portal (or reloading a world with a lit portal in loaded chunks) this error occurs (likely due to the mixin failing):

[12:53:51] [Render thread/WARN]: Unknown custom packed identifier: imm_ptl:spawn_entity
[12:53:51] [Render thread/ERROR]: missing entity for data tracking ClientWorld minecraft:overworld1512
[12:53:51] [Render thread/WARN]: Unknown custom packed identifier: imm_ptl:spawn_entity
[12:53:51] [Render thread/ERROR]: missing entity for data tracking ClientWorld minecraft:overworld1513
[12:53:53] [Render thread/INFO]: WorldRenderer reloaded minecraft:the_nether
[12:53:53] [Render thread/INFO]: Started 10 worker threads
[12:53:53] [Render thread/INFO]: Started 32 worker threads
[12:53:53] [Render thread/INFO]: Client World Created minecraft:the_nether
[12:53:53] [Render thread/WARN]: Unknown custom packed identifier: imm_ptl:spawn_entity
[12:53:53] [Render thread/ERROR]: missing entity for data tracking ClientWorld minecraft:the_nether2195
[12:53:53] [Render thread/WARN]: Unknown custom packed identifier: imm_ptl:spawn_entity
[12:53:53] [Render thread/ERROR]: missing entity for data tracking ClientWorld minecraft:the_nether2196

Hacky solution (please don't do this):
Also mixin this class in the Quilted Fabric API impl package. There will always be a failed mixin error, but it should theoretically work fine on both loaders until one's version of this class diverges from the other.

Proper solution:
Implement this logic without requiring modifying or even interacting with Fabric/Quilt internals, as this is not supported, and may break in the future.

Link to QSL/Quilted Fabric API: https://github.com/QuiltMC/quilt-standard-libraries

NOTE: I have not opened an issue on Quilt, as it is not their issue, and they're just going to tell me what I've already been told in their discord: they do not support interacting in any way with Fabric's or even their own impl package.

commented

Upon further reflection it appears that the incompatibility is technically in QSL/Fabric API, not the loaders. I have updated the issue to reflect this. My previous (deleted) comment was incorrect. The hacky solution should theoretically work, but I once again encourage against this (at least in the long term).

commented

You mean that if I use any Impl thing of Fabric API then quilt cannot support it?

commented

You mean that if I use any Impl thing of Fabric API then quilt cannot support it?

Maybe they could, but I'm not sure that they should, and regardless they've made it clear that they won't. Additionally, from what I can tell, Fabric doesn't necessarily support this either, it just happens to work. The impl package is supposed to be considered internal and volatile, not something mods should depend on.

commented

Immptl hacks into Fabric API https://github.com/qouteall/ImmersivePortalsMod/blob/25668608f40249380c23d5832b26ff92dd7e323f/imm_ptl_core/src/main/java/qouteall/imm_ptl/core/platform_specific/mixin/client/MixinFabricClientPlayNetworkAddon.java

Fabric API ignores mod packets on the client thread. Immptl disables this behavior because dimension redirection packets are handled in the client thread and mod packets can be redirected.

commented

I can do similar hack to quilt standard library

commented

I heard that quilt use Mojang mapping but in
https://github.com/QuiltMC/quilt-standard-libraries/blob/1.18/library/core/networking/src/main/java/org/quiltmc/qsl/networking/impl/client/ClientPlayNetworkAddon.java#L85
I didn't find isOnThread method in Mojang mapping. Is QSL using yarn mapping?

commented

QSL uses Quilt Mappings - a modified version of yarn

commented

I can do similar hack to quilt standard library

This is the hacky solution I mentioned in my issue report. I'd really like to stress that this is not supported by Quilt or Fabric, it just happens to work.

commented

@Nolij I know, but Fabric API code breaks immptl's packet redirection so this hack is needed.

commented

@Nolij I know, but Fabric API code breaks immptl's packet redirection so this hack is needed.

Has an attempt been made to just remove this check from upstream Fabric API? Clearly it doesn't break things to disable it (unless there are mitigations this mod implements which I've missed). My understanding is they intend to be more open-minded than Fabric, so they may consider it even if Fabric rejected it. I think if it could be upstreamed that would be a much better solution.

commented

I will try that