MC-193749Nether portals play the trigger sound again when the other dimension is loaded
Notice the presence of the Portal noise intensifies subtitle, indicating that the sound block.portal.trigger has begun playing again even though the player isn't about to go through the portal, and in fact will exit it. This sound playing is erroneous and should be removed.
I tried testing this in 12w17a and 12w19a as I suspected this bug may have been first introduced with the client-server split in singleplayer but it didn't actually affect anything.
Linked Issues
Created Issue:
Nether portals play the trigger sound again when the other dimension is loaded
Notice the presence of the Portal noise intensifies subtitle, indicating that the sound block.portal.trigger has begun playing again even though the player isn't about to go through the portal, and in fact will exit it. This sound playing is erroneous and should be removed.
I tried testing this in 12w17a and 12w19a as I suspected this bug may have been first introduced with the client-server split in singleplayer but it didn't actually affect anything.
Attachments3
![[Mod] violine1101](../../../data/avatars/users/92858.png){kind=small}
relates to
![[Mod] Jarl-Penguin](../../../data/avatars/users/92849.png){kind=small}
Relates to
MC-180
![[Mod] SPGoding](../../../data/avatars/users/44622.png){kind=small}
relates to
![[Mod] Michael Wobst](../../../data/avatars/users/37868.png){kind=small}
![[Helper] Johnibur](../../../data/avatars/users/28381.png){kind=small}
![[Bot] Arisa](../../../data/avatars/users/43007.png){kind=small}
relates to
![[Mod] markderickson](../../../data/avatars/users/18946.svg){kind=small}
This comment is an extensive bug (and code) analysis of the fifth oldest persisting issue, along with a working and fully tested workaround (fix), so this comment will be split in different categories.
What @Connor Steppie Meant with MC-217613
Then, we have MC-217613 which indeed is (and it's not) a duplicate of this, depending on the point of view; Connor Steppie means that MC-217613 is the bug that refers to the lack of a slow-to-stop animation for the portal (it happened for the first time in 12w18a), while MC-180 refers to the visual "re-trigger" of the portal (it happenned for the first time in 12w34a). It's like these two bugs have been "stacked" or that they are "sister bugs", because they represent a very similar issue.
Code Analysis
Here, a bit of history of the code that worked, using Mojang's mapping names for this analysis:
- 1. Prior to 12w18a, it only worked on singleplayer, as the whole world was client. This is because the method we now call LocalPlayer.handleNetherPortalClient() (was part of aiStep() back then) handled the whole teleportation and then used the "distortion fade out" behaviour using field LocalPlayer.portalTime. The code below is a recreation of the sourcecode present in 12w17a, using mojang names and my own ones.
public class LocalPlayer extends Player { ... public void aiStep() { ... this.oPortalTime = this.portalTime; if (this.isInsidePortal) { if (!this.level.isClientSide && this.vehicle != null) { this.startRiding((Entity)null); } if (this.minecraft.screen != null) { this.minecraft.setScreen((Screen)null); } if (this.portalTime == 0.0F) { this.minecraft.soundManager.play("portal.trigger", 1.0F, this.random.nextFloat() * 0.4F + 0.8F); } this.portalTime += 0.0125F; //START OF CODE REMOVED IN 12w18a if (this.portalTime >= 1.0F) { this.portalTime = 1.0F; if (!this.level.isClientSide) { this.portalCooldown = 10; this.minecraft.soundManager.play("portal.travel", 1.0F, this.random.nextFloat() * 0.4F + 0.8F); byte newDimension = 0; if (this.dimension == -1) { newDimension = 0; } else { newDimension = -1; } this.minecraft.changeDimension(newDimension); this.takeAchievement(Achievements.portal); } } //END OF REMOVED CODE this.isInsidePortal = false; } else if (this.hasEffect(MobEffects.CONFUSION) && this.getEffect(MobEffects.CONFUSION).getDuration() > 60) { this.portalTime += 0.006666667F; if (this.portalTime > 1.0F) { this.portalTime = 1.0F; } } else { if (this.portalTime > 0.0F) { this.portalTime -= 0.05F; } if (this.portalTime < 0.0F) { this.portalTime = 0.0F; } } if (this.portalCooldown > 0) {//old equivalent to Entity.processPortalCooldown() this.portalCooldown--; } ... } ... }
- 2. After the changes made in 12w18a, the code described above was completely removed without adding a replacement, causing the unwanted behavior described in
MC-217613. - 3. In 12w34a, the way nether portals were handled by entities was changed internally, causing the actual behavior where the animation plays forever.
Note in the code that method this.minecraft.changeDimension(newDimension) was also removed from Minecraft.java and also it was forgotten to be re-added for the client listener, causing MC-12789.
Workaround and Fix
After the client-server singleplayer split up, the game switched to fully use the network system, and the source code described in history would not be able to just be copied and pasted, however, it's still useful for reference.
A possible way to fix this is to let the client know when a dimension change was caused by a nether portal to set the proper animation.
But first, we are going to head to LocalPlayer.java and add a new method (named justPostalTraveled() in this example), that will prevent the player to enter to the in-portal state, and set the Animation/Portal Effect at 100%. This will make the player think it's just exiting a portal and will decrease the Animation/Portal Effect at every aiStep()->handleNetherPortalClient() (This would also fix MC-193749). Now there's a problem with the animation effect: the animation starts before the Downloading Terrain screen is gone, causing the animation to be gone at the time or shortly after the game is rendering the world. To fix this, we are gonna make handleNetherPortalClient() know when it should progress the animation using a check. The end result is shown here in the code:
public class LocalPlayer extends AbstractClientPlayer { ... private void handleNetherPortalClient() { this.oPortalTime = this.portalTime; boolean shouldStatusProgress = !(this.minecraft.screen instanceof ReceivingLevelScreen); if (this.isInsidePortal) { if (this.minecraft.screen != null && !this.minecraft.screen.isPauseScreen() && !(this.minecraft.screen instanceof DeathScreen) && shouldStatusProgress) { if (this.minecraft.screen instanceof AbstractContainerScreen) { this.closeContainer(); } this.minecraft.setScreen((Screen)null); } if (this.portalTime == 0.0F) { this.minecraft.getSoundManager().play(SimpleSoundInstance.forLocalAmbience(SoundEvents.PORTAL_TRIGGER, this.random.nextFloat() * 0.4F + 0.8F, 0.25F)); } this.portalTime += 0.0125F; this.isInsidePortal = false; } else if (this.hasEffect(MobEffects.CONFUSION) && this.getEffect(MobEffects.CONFUSION).getDuration() > 60) { this.portalTime += 0.006666667F; } else if (shouldStatusProgress) { if (this.portalTime > 0.0F) { this.portalTime -= 0.05F; } } this.portalTime = Mth.clamp(this.portalTime, 0.0F, 1.0F); if (shouldStatusProgress) { this.processPortalCooldown(); } } public void justPortalTraveled() { this.isInsidePortal = false; this.oPortalTime = this.portalTime = 1.0F; this.setPortalCooldown(); } ... }
In other hand, we need to use this new method somewhere, and returning to the second paragraph, we'll make the client know when the player used a portal. I added a condition at the packet ClientboundRespawnPacket.java and I called it boolean fromPortal. It will also be read and written to the packet.
The change is quite obvious so I will not show the end result for that. Instead we are going to make this fix fully efective. At the ClientPacketListener.java at method handleRespawn(), after the player is being initialized and "adjusted", let's check whether if the player came from a portal (using the previously added fromPortal field) to call for justPortalTraveled():
public class ClientPacketListener implements TickablePacketListener, ClientGamePacketListener { ... public void handleRespawn(ClientboundRespawnPacket p_105066_) { ... localplayer1.resetPos(); localplayer1.setServerBrand(s); this.level.addPlayer(i, localplayer1); localplayer1.setYRot(-180.0F); localplayer1.input = new KeyboardInput(this.minecraft.options); this.minecraft.gameMode.adjustPlayer(localplayer1); localplayer1.setReducedDebugInfo(localplayer.isReducedDebugInfo()); localplayer1.setShowDeathScreen(localplayer.shouldShowDeathScreen()); localplayer1.setLastDeathLocation(p_105066_.getLastDeathLocation()); if (packet.isPortalTravel()) { localplayer.justPortalTraveled(); } if (this.minecraft.screen instanceof DeathScreen) { this.minecraft.setScreen((Screen)null); } this.minecraft.gameMode.setLocalMode(p_105066_.getPlayerGameType(), p_105066_.getPreviousPlayerGameType()); } ... }
The fromPortal field should be true only when we are sure that the player is naturally changing from dimensions, like in ServerPlayer.changeDimension(ServerLevel), which is triggered only when the player is joing to change dimensions throught portals.
However, this fix didn't always work; in certain cases that i couldn't determine, the glitched animation played intead. That left me confused for a while until I found an alternative to this fix.
The new fix comes from the same concept of letting the player know whether or not the dimension travel was a because of a portal, but splitting ClientboundRespawnPacket.java's dimension change function into ClientboundDimensionTravelPacket.java. This new packet didn't create a new player instance and will reuse the previous one. I'm sure of something; that the previous fix did not work because the player could have lost some data. This new fix has worked at the 100% of tests. This is the new dimension travel packet handler:
public class ClientPacketListener implements TickablePacketListener, ClientGamePacketListener { ... @Override public void handleDimensionTravel(ClientboundDimensionTravelPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, this.minecraft); Holder<DimensionType> holder = this.registryAccess.compositeAccess().registryOrThrow(Registries.DIMENSION_TYPE).getHolderOrThrow(packet.getDimensionType()); LocalPlayer localplayer = this.minecraft.player; ResourceKey<Level> newDimension = packet.getDimension(); ResourceKey<Level> oldDimension = localplayer.level.dimension(); if (newDimension != oldDimension) { Scoreboard scoreboard = this.level.getScoreboard(); Map<String, MapItemSavedData> map = this.level.getAllMapData(); boolean flag = packet.isDebug(); boolean flag1 = packet.isFlat(); ClientLevel.ClientLevelData clientlevel$clientleveldata = new ClientLevel.ClientLevelData(this.levelData.getDifficulty(), this.levelData.isHardcore(), flag1); this.levelData = clientlevel$clientleveldata; this.level = new ClientLevel(this, clientlevel$clientleveldata, newDimension, holder, this.serverChunkRadius, this.serverSimulationDistance, this.minecraft::getProfiler, this.minecraft.levelRenderer, flag, packet.getSeed()); this.level.setScoreboard(scoreboard); this.level.addMapData(map); Component title; if (newDimension == Level.OVERWORLD) {//this is exta code from my mod, it fixes MC-12789 ResourceLocation resourcelocation = oldDimension.location(); String s = resourcelocation.toLanguageKey("dimension"); title = Component.translatable("menu.leavingDimension", Language.getInstance().has(s) ? Component.translatable(s) : Component.literal(resourcelocation.toString())); } else { ResourceLocation resourcelocation = newDimension.location(); String s = resourcelocation.toLanguageKey("dimension"); title = Component.translatable("menu.enteringDimension", Language.getInstance().has(s) ? Component.translatable(s) : Component.literal(resourcelocation.toString())); } this.minecraft.setLevel(this.level, new ReceivingLevelScreen(title)); localplayer.fishing = null;//This a fix for an issue that happened in one of my tests. Since the player instance is not a new one and because the client doesn't unload entities from previous level (yeah client worlds work like that) and the player doesn't to check if the fishing rod is a valid entity to unset it, I had to put that here. if (packet.isPortalTravel()) { localplayer.justPortalTraveled(); this.minecraft.getSoundManager().play(SimpleSoundInstance.forLocalAmbience(SoundEvents.PORTAL_TRAVEL, this.random.nextFloat() * 0.4F + 0.8F, 0.25F)); //You can see the portal travel sound playing from here and not from levelEvent() because with this fix we no longer really need to use another packet (I removed the level event 1032 as well) } } if (localplayer.hasContainerOpen()) { localplayer.closeContainer(); } localplayer.setLevel(this.level); if (newDimension != oldDimension) { this.minecraft.getMusicManager().stopPlaying(); } this.level.addPlayer(localplayer.getId(), localplayer); } ... }
Well I hope my fix is clear to understand, if not, or if there are any questions, feel free to ask 
It's time to get this fixed
Like any other subtitle, it will continue to show for a moment in the bottom right before disappearing, it doesn't suddenly disappear when changing dimensions.
That doesn't change the fact that the sound starts again.
In 1.16.1
Can confirm in 20w46a.
Can confirm in 1.17.1 Release Candidate 1.
This is fixed in 1.20 pre-release 1.