BetterPortals

BetterPortals

1M Downloads

Draconic Evolution Dislocator Crash

FreezePhoenix opened this issue ยท 9 comments

commented

Describe the bug

Using a dislocator from Draconic Evolution that results in cross-dimension travel (In multiplayer) causes the game to effectively force the player into an endless connecting state. They're unable to move, or see any blocks, and cannot be moved by other players (but are able to be moved by other means, such as pistons). Any attempt to reload their view (Lowering the render distance, exiting the server) results in the server crashing and the client being disconnected.

Disabling third party transitions did not resolve the issue.

To Reproduce

Start a LAN world, connect to it with a second client, create a dislocator pair, give each player one of the dislocators from the pair, move the client (or the host) to another dimension, and attempt to teleport from either one to the other.

Server Logs

When the dislocator is used:

java.util.concurrent.ExecutionException: java.lang.NullPointerException
	at java.util.concurrent.FutureTask.report(FutureTask.java:122) ~[?:1.8.0_51]
	at java.util.concurrent.FutureTask.get(FutureTask.java:192) ~[?:1.8.0_51]
	at net.minecraft.util.Util.func_181617_a(Util.java:48) [h.class:?]
	at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:723) [MinecraftServer.class:?]
	at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:668) [MinecraftServer.class:?]
	at net.minecraft.server.integrated.IntegratedServer.func_71217_p(IntegratedServer.java:279) [chd.class:?]
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:526) [MinecraftServer.class:?]
	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_51]
Caused by: java.lang.NullPointerException
	at net.minecraft.server.management.PlayerChunkMap.handler$zci000$updateMovingPlayerWithViews(PlayerChunkMap.java:798) ~[ou.class:?]
	at net.minecraft.server.management.PlayerChunkMap.func_72685_d(PlayerChunkMap.java) ~[ou.class:?]
	at net.minecraft.server.management.PlayerList.func_72358_d(PlayerList.java:383) ~[pl.class:?]
	at net.minecraft.network.NetHandlerPlayServer.func_147347_a(NetHandlerPlayServer.java:584) ~[pa.class:?]
	at net.minecraft.network.play.client.CPacketPlayer.func_148833_a(SourceFile:126) ~[lk.class:?]
	at net.minecraft.network.play.client.CPacketPlayer$PositionRotation.func_148833_a(SourceFile:18) ~[lk$b.class:?]
	at net.minecraft.network.PacketThreadUtil$1.run(PacketThreadUtil.java:22) ~[hv$1.class:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) ~[?:1.8.0_51]
	at java.util.concurrent.FutureTask.run(FutureTask.java:266) ~[?:1.8.0_51]
	at net.minecraft.util.Util.func_181617_a(Util.java:47) ~[h.class:?]
	... 5 more

Further attempts to use the dislocator will result in an identical error (But not crash the server).

Attempting to leave the server or lower render distance results in the following error, crashing the server:

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) ~[ServerWorldsManagerImpl.class:?]
	at net.minecraft.server.management.PlayerChunkMap.handler$zci000$setPlayerViewRadiusWithViews(PlayerChunkMap.java:817) ~[ou.class:?]
	at net.minecraft.server.management.PlayerChunkMap.func_152622_a(PlayerChunkMap.java) ~[ou.class:?]
	at net.minecraft.server.management.PlayerList.func_152611_a(PlayerList.java:1155) ~[pl.class:?]
	at net.minecraft.server.integrated.IntegratedServer.func_71217_p(IntegratedServer.java:284) ~[chd.class:?]
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:526) [MinecraftServer.class:?]
	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_51]

Additional context
Plugin Version: 0.3.7.7
Game Version: Forge 14.23.5.2854

commented

What exactly is the code that is duplicated between Vanilla and Draconic Evolution? I'm having trouble finding it.
In other words, what would be a normal stack trace if that error were thrown in vanilla code?

commented

Draconic Evolution doesn't have a verbatim copy of vanilla code, what the Draconic Evolution code does is preserve the momentum and direction of the player. It also recursively teleports any mobs that are riding the player - as well as any mobs the player is riding, recursively.

Yes they do, though it probably started in an older version so by now it's no longer exactly identical (and that's not a good thing). Neither momentum/direction preservation nor recursively teleporting require the method mentioned in my issue.
In fact, if you look at their 1.16 branch, they already replaced it with the vanilla teleport method.

If it's as easy as inserting the following lines into the top of their teleport code [...]

Then you should let devs know that.

It's easier, and I already told them how to fix it one and a half years ago: just use the vanilla method.
Throwing BP's code into your code just makes your code even worse. Now it not only duplicates vanilla code, but also random third-party code, and if any of it needs to change, your copy won't. So while you've got the original problem "fixed", you just created a larger mess in the process.

I won't reply to further messages, I've already said everything that needs to be said and I'm no longer interested in 1.12.2 anyway.

commented

Draconic Evolution doesn't have a verbatim copy of vanilla code, what the Draconic Evolution code does is preserve the momentum and direction of the player. It also recursively teleports any mobs that are riding the player - as well as any mobs the player is riding, recursively.

commented

If it's as easy as inserting the following lines into the top of their teleport code (with some adaptations to account for the fact that they are trying to preserve momentum, etc.):

        if (DimensionTransitionHandler.INSTANCE.transferPlayerToDimension(player, dimensionIn, teleporter)) { // teleporter would be replaced with a custom implementation.
            ci.cancel(); // Would probably be replaced with return
        }

Taken from https://github.com/Johni0702/BetterPortals/blob/v0.3/src/transition/java/de/johni0702/minecraft/betterportals/impl/transition/mixin/MixinPlayerList.java

Then you should let devs know that.

commented

I feel as though it would be easier if you provided an example on how other mod developers could account for your mod in the case of coding their own teleport mechanism. Obviously other mod developers want their content to not crash in conjunction with your mod, so providing an example of how they would do that would be easier on both you (You don't have to ASM/Mixin into every major mod) and on other mod developers (they can just check the documentation or the README of this mod).

commented

What exactly is the code that is duplicated between Vanilla and Draconic Evolution? I'm having trouble finding it.
In other words, what would be a normal stack trace if that error were thrown in vanilla code?

See Draconic-Inc/BrandonsCore#69.

I feel as though it would be easier if you provided an example on how other mod developers could account for your mod in the case of coding their own teleport mechanism. Obviously other mod developers want their content to not crash in conjunction with your mod, so providing an example of how they would do that would be easier on both you (You don't have to ASM/Mixin into every major mod) and on other mod developers (they can just check the documentation or the README of this mod).

There is nothing special they must do to be compatible. All they need to do is refrain from creating verbatim copies of already existing vanilla methods. And that's just a good rule of thumb for modding in general, cause when you do not give third-party mods a chance to run their code, then it's not surprising when they don't work.

commented

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

commented

Duplicate of #491

commented

I just pushed a new build of BrandonsCore that should fix this issue.