Draconic Evolution

Draconic Evolution

77M Downloads

Incompatibility with BetterPortals

FreezePhoenix opened this issue ยท 11 comments

commented

Base information

  • Minecraft version: 1.12.2
  • Mod version: Draconic-Evolution-1.12.2-2.3.27.353-universal
  • Minecraft Forge version: Forge 14.23.5.2854
  • Mod Pack: N/A

Crash report

java.lang.IllegalStateException: Player changed world without us noticing. Did some third-party mod do something weird?
	at de.johni0702.minecraft.view.impl.server.ServerWorldsManagerImpl.updateActiveViews(ServerWorldsManagerImpl.kt:67)
	at net.minecraft.server.management.PlayerChunkMap.handler$zci000$setPlayerViewRadiusWithViews(PlayerChunkMap.java:817)
	at net.minecraft.server.management.PlayerChunkMap.func_152622_a(PlayerChunkMap.java)
	at net.minecraft.server.management.PlayerList.func_152611_a(PlayerList.java:1155)
	at net.minecraft.server.integrated.IntegratedServer.func_71217_p(IntegratedServer.java:284)
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:526)
	at java.lang.Thread.run(Thread.java:745)

Description

Long version: Johni0702/BetterPortals#609 (comment)
TL;DR: BetterPortals is not compatible with the dislocators or the portals using them.

commented

BetterPortals does some very hacky stuff to get things to work correctly. This is probably very low on the priority list of being fixed, if we can even fix it on this side at all.

commented

The BetterPortals dev seems to disagree:

No, this needs to be fixed in DE.

BP needs to unload all but the main world whenever you use a third-party teleporter, or the be more precise, shortly before you use a third-party teleporter. It needs to do this because the third-party teleporter is usually not aware that there are multiple worlds loaded on the client and will only unload the primary one before loading its destination. As a result, if its destination was already loaded before the teleport (e.g. because of a portal), then it would now be loaded twice, hence why BP must unload all but the main world before the third-party teleporter does its thing.
This should explain why "disabling BP for DE" or a blacklist doesn't work: It's not that BP does additional stuff which breaks. It breaks because it does not do a specific thing (the unloading).

Now, why does it not do that thing? Well, normally, BP dynamically (i.e. each time the game starts) modifies a specific part of MC's code and adds its unloading code there. Most mods then as part of their teleporter, call that part of MC's code, which gives BP a chance to do its thing.

The issue with DE is that they just made a verbatim copy of a good chunk of MC's code, including the part which BP hooks into. So even though BP hooks into the specific part of MC's code as usual, DE just never calls that and switches dimension anyway. BP eventually detects that (but after DE's code has ran, so it's too late) and decides it's better to fail fast than to continue at the risk of world corruption or who-knows-what.

While I could change BP to hook into DE's copy as well (and I have done so for Sponge), I don't think that's a good idea because it's a huge maintenance burden (especially once I start working around bugs other mods as well), there's really no good reason for DE to copy the code instead of calling the original (whereas Sponge had a very valid reason to do so), and it should be fairly easy to fix in DE (easier even than to add the workaround to BP).

commented

Again, this is low priority as Brandon is working on porting to 1.16.5 currently. This likely won't be fixed any time soon unless someone steps up and fixes it themselves.

commented

I believe a fix would be relatively easy:
Given the injection point here: https://github.com/Johni0702/BetterPortals/blob/master/src/transition/java/de/johni0702/minecraft/betterportals/impl/transition/mixin/MixinPlayerList.java

And the method named very similarly to the injection point here: https://github.com/brandon3055/BrandonsCore/blob/b3e0bd872b5b47dcd2e14a2b71facaede791d3e4/src/main/java/com/brandon3055/brandonscore/lib/TeleportUtils.java#L148

That you should be able to call their method and return early based on the result.

I might download the source of Draconic in order to test the feasibility of this fix, if it does work, I will post back here, and likely make a PR.

commented

If you've got it figured out, fix it up and submit a pull request! Brandon appreciates help if the code is well thought out.

commented

I've had no luck even setting up a development environment for BetterPortals >.< It references maven's that don't even exist anymore, and other horrors
.

commented

Now you know why I didn't even attempt it! All I can say is good luck.

commented

It may be worth my while to simply not set up BetterPortals, and instead use some launcher stuff to determine if it is installed, and call their API through Reflection...

commented

Note to all reading this: This is actually a compatibility issue between BradonsCore and BetterPortals, this issue was mistakenly made on this repository due to lack of knowledge about how DE handles teleports. As such, it is duplicate of this issue:
Draconic-Inc/BrandonsCore#69

commented

Created a PR that fixes this issue on BrandonsCore: Draconic-Inc/BrandonsCore#81

commented

This should be fixed by the latest BCore version.