The bug
In 1.16 a change was made that allows the client to run the code to dismount their current vehicle. This change should be reverted as allowing the client to dismount vehicles makes the server unable to keep a player in a vehicle. If the player keeps the shift key pressed the server is completely unable to put them in any vehicle because the player will immediately dismount.
This used to work in 1.15.2 and before.
This change also likely lead to related issues such as MC-191714 as the client can now decide to stop riding a vehicle without informing the server properly. Related issues likely occur because the information that the player is sneaking is not immediately send so the server may never know the player is sneaking and as such never dismount them whilst the client did dismount them.
As @Bytzo pointed out the change causing this issue did not cause the other mentioned issue.
As @Bytzo later discovered a different change did lead to related issues.
Impact:
The ability to keep players in a vehicle is used by various servers for features like custom camera's, rollercoasters or for various minigames based around vehicles like jousting or Hypixel's Turbo Kart Racers. We use it for our camera paths as well as unrevealed content in MC Championship.
How to reproduce
Add code to the server to prevent a player from exiting their vehicle. For example by removing the contents of the
removePassenger(Entity)method in the Entity class on the server. This is equivalent to cancelling theVehicleExitEventon Bukkit.Join the server with a 1.16 client
Hop in a boat
Press shift, you'l be able to exit the boat on the client, the server and any other players will see you as still sitting in the boat but the client is no longer in the boat on their screen.
If a player is prevented from dismounting their vehicle by the server they are still in the vehicle for other players, however on the client the player has successfully dismounted and they can walk around and interact with the world but in a glitched state as the server still sees them as being in the vehicle.
Expected Results:
If the server stops the client from dismounting an entity they should remain a passenger of the entity.
Code analysis
Specifically, in 1.16 a change was made to the rideTick() method in net.minecraft.world.entity.player.Player where the if statement on the first line no longer starts with the clause !this.level.isClientSide. If this change is reverted the behaviour is identical to how it used to work in 1.15.2. This change is part of the issue but if reverted will not fully address the issue.
Please view the comment by @Bytzo for further detailed code analysis.
Linked issues
relates to 2
Comments 5
After comparing the rideTick() method across several versions, I can confirm this change was made in snapshot 20w09a. This means that versions 20w09a through 20w49a are affected by this bug.
As a temporary solution, I have created a Fabric mod for 1.16 that implements the proposed change by @Daniël Goossens. Using this mod, players are unable to dismount entities through the client, meaning servers that attempt to block entity dismounting will avoid a de-sync issue with the client. This is especially useful on third-party mini-game servers that make use of this behavior, such as Hypixel's Turbo Kart Racers which had to be disabled on 1.16 and above due to this change. The source for the Fabric mod can be found at https://github.com/bytzo/mc-202202, with pre-compiled releases here.
It is however important to note that the proposed solution does not fix other mentioned issues such as MC-191714 and MC-205477. I was able to reproduce both of these issues with the Fabric mod containing the fix.
Hmm, could this have been caused by the fix for this issue?: MC-186085
Since we know this behavior was changed in 20w09a, we can determine that the fix for MC-186085 was not the cause of this bug, as MC-186085 was fixed in 1.16 Pre-release 3 which is after 20w09a. In addition to this, we can check the fixed bugs from 20w09a to see if any of those fixes may have caused this bug. 20w09a has three fixed bugs related to entity mounting, which are MC-111726, MC-148869, and MC-149042. I was not able to reproduce any of these bugs using the Fabric mod containing the fix. This should mean that this bug was not caused by fixes for other issues.
Thank you for looking more into this. That this change lead to the related issues was an assumption I made that I did not confirm and I should have phrased it as such. I've now modified the issue to reflect this.
Recently on various mini-game servers, I noticed that the client is still able to dismount vehicles and experience a de-sync in some rare scenarios even with the modification containing the current proposed fix. While looking into what caused this, I found a second change that allows the client to dismount an entity, causing the de-sync I experienced. This change was made in snapshot 20w22a and is located in the handleMovePlayer() method in net.minecraft.client.multiplayer.ClientPacketListener. In this method, a conditional statement was added that allows the client to dismount an entity if certain conditions are met. In order to completely fix this bug, the change in the handleMovePlayer() method during 20w22a should be reverted in addition to the current proposed fix in the rideTick() method.
Unlike the current proposed change in the rideTick() method, the new proposed change in the handleMovePlayer() method does resolve MC-191714 and MC-205477. Using an updated version of the Fabric mod on 1.16.5 which reverts the change in the handleMovePlayer() method, I was unable to reproduce both of these issues. This means that @Daniël Goossens was initially correct that a change allowing clients to dismount entities lead to both MC-191714 and MC-205477, however the mentioned change in the rideTick() method was not the specific cause.
By implementing the changes above in both the rideTick() and handleMovePlayer() methods, there should no longer be any de-sync issues related to entity dismounting, as it should not be possible to dismount entities through the client. In addition, third-party servers regain the ability to keep players mounted on entities, which was used in many different ways (such as preventing players from dismounting minecarts on a roller coaster) before these changes occurred. Many third-party servers still attempt to make use of this behavior, causing undesirable issues when using the newest version of the game compared to prior versions.
Of course, the changes in both the rideTick() and handleMovePlayer() methods causing this bug might have been done to fix other bugs or intentionally change behavior, so reverting them would not be beneficial to the game. As done before, we can check the changelog and fixed bugs from 20w22a relating to entity mounting in order to see if this is the case. These are MC-182967, MC-184629, and MC-184640. A change was also made in 20w22a that prevents mounting entities while the shift key is held. I was unable to reproduce these previously resolved issues and was still unable to mount entities when the shift key was held using both proposed fixes above, meaning that the proposed fix in this issue should not conflict with other changes or issues from 20w22a.
Finally, the reproduction steps for this issue can be updated since we know more about the bug.
To observe the impact of the change in the rideTick() method (reproduction steps by @Daniël Goossens still work, however require a modified server):
Join a world with cheats enabled using version 20w09a or higher.
Run the command /summon pig ~ ~ ~ {Invulnerable:1b,NoAI:1b,PersistenceRequired:1b,Saddle:1} in-game.
Mount the pig.
Pause the server thread to simulate a lag spike.
Dismount the pig using the shift key.
Resume the server thread.
Notice the de-sync. Walking around and attempting to break blocks far away from the pig will cause anti-cheat to prevent the blocks from being broken, as the server still thinks the player remains mounted on the pig. Sneaking causes the player to teleport back to the pig, as the server thinks the player has just dismounted the pig.
To observe the impact of the change in the handleMovePlayer() method:
Join a world with cheats enabled using version 20w22a or higher.
Run the command /gamerule randomTickSpeed 0 in-game.
Run the command /fill ~-3 ~-1 ~-3 ~3 ~5 ~3 minecraft:glowstone hollow in-game.
Run the command /fill ~-2 ~-1 ~-2 ~2 ~-1 ~2 minecraft:farmland in-game.
Run the command /summon horse ~ ~ ~ {Invulnerable:1b,NoAI:1b,PersistenceRequired:1b,SaddleItem:{id:saddle,Count:1},Tame:1} in-game.
Mount the horse.
Jump at least 1 block in the air with the horse, so the farmland is broken and you are dismounted from the horse.
Notice the de-sync. Walking around and attempting to break blocks far away from the horse will cause anti-cheat to prevent the blocks from being broken, as the server still thinks the player remains mounted on the horse. Sneaking causes the player to teleport back to the horse, as the server thinks the player has just dismounted the horse.
I can confirm the changes in the rideTick() and handleMovePlayer() methods that cause this bug still remain in 20w51a, 1.16.5-rc1, and 1.16.5.
In snapshot 21w03a, it appears only the fix in the rideTick() method was implemented. This was likely due to the fix in the handleMovePlayer() method being discovered right before the release of 21w03a. I have opened a second issue at MC-212291 to address this as recommended by a moderator.
Felix Jones
Hmm, could this have been caused by the fix for this issue?: MC-186085