mojira.dev
MC-189885

New byte previousGamemode is read incorrectly

In 1.16 Prerelease 6 there was a field added to both the Login/Join Game packet and the Respawn Clientbound packets
These packets now contain the previous gamemode ID of the player.
Alsongside this; Gamemode ID -1, (Not defined gamemode) was added.

There is an issue with this however.

Both the current and past gamemodes are written to the netty network buffer by their integer ID.
That's -1 for Not defined.
They are written as *signed bytes*

The client then gets those and reads them as *unsigned bytes*
This does NOT create an issue were all gamemode IDs always positive (prior to this they were)
But since this snapshot there is a negative gamemode ID which can NEVER be read correctly by this

Sending the client -1 as ID will make it read 255 from the buffer.
Solution: Read both gamemode IDs as signed bytes, like they should be read

Attachments

Comments 6

It should be pointed out that this bug report is quite misleading as to what the actual issue is here. writeByte is the method which should be used to write both signed and unsigned data, so the issue isn't a mismatch with serialization.

The actual effect of this issue is that in multiplayer, previousGameMode is deserialized as survival mode when the server sends not_set, whereas in singleplayer (since packets aren't serialized) it is decoded as not_set. This would cause the client to send /gamemode survival when pressing f3+n for the first time if the player starts off in spectator mode, and /gamemode with no argument in singleplayer (although I'm not sure it's possible to first spawn in spectator mode in singleplayer). It also causes different defaults of the game mode GUI depending on whether it's singleplayer or multiplayer, and I think the defaults for singleplayer are the intended behaviour.

@Earthcomputer It’s true I did not research how this negatively impacts the gameplay, and frankly this wasn’t my goal.

I simply reported the obvious inconsistency that occurs when a signed primitive type is read as unsigned type.

It is clear to me however that this creates uncontrolled behavior.

Reading it as an unsigned type would only make sense if you desperately needed the extra positive bit to convey even larger numbers, in which case, it would still be better not to use byte at all for this.

 

The actual bug here is not that it is being read as an unsigned byte, it's that not_set is being sent in the first place. As I explained in the previous comment, this causes issues whether it's read correctly or not. The real fix is to do the logic that the game mode switcher screen currently does for the initial game mode, on the server, rather than on the client, so that not_set doesn't have to be sent in the first place. Or else the client can send "/gamemode " with no arguments.

Confirmed in 1.16 Pre-release 7.

Still in 1.16 Pre-release 8.

LordExecuter

boq

Confirmed

Low

Networking

networking, packet

1.16 Pre-release 6, 1.16 Pre-release 7, 1.16 Pre-release 8, 1.16 Release Candidate 1, 1.16

20w48a

Retrieved