Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SystemChatPacket: read boolean instead of varint #1461

Merged

Conversation

shoker537
Copy link
Contributor

Since Velocity does not use SystemChatPacket's decode, it's missing proper attention and has a mistake in decoding.
But plugins managing protocol changes on proxy rely on it, so it's better to keep it working (working code is better than non-working code, right?).

The packet provides a boolean which determines if it is an actionbar message or a chat message, but for some reason SystemChatPacket reads varint which breaks ChatType determination during decode.

@electronicboy
Copy link
Member

electronicboy commented Nov 19, 2024

it's read as a varint because that's what it used to be, this needs to be properly updated to handle the two different versions of this packet

@shoker537
Copy link
Contributor Author

shoker537 commented Nov 19, 2024

it's read as a varint because that's what it used to be, this needs to be properly updated to handle the two different versions of this packet

That's true, I was looking for SystemChatPacket only, but now I see there was ChatMessage packet before 1.19 that actually had 3 states which varint read.

So, it appears the code should look like this if I'm not missing something else:

    if (version.noLessThan(ProtocolVersion.MINECRAFT_1_19_1)){
      type = buf.readBoolean() ? ChatType.GAME_INFO : ChatType.SYSTEM;
    } else {
      type = ChatType.values()[ProtocolUtils.readVarInt(buf)];
    }

@electronicboy
Copy link
Member

Yea, that would look about right to me

@electronicboy electronicboy merged commit 9cfcfcf into PaperMC:dev/3.0.0 Nov 19, 2024
1 check passed
pull bot pushed a commit to WiIIiam278/Velocity that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants