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

PP: Chat Message #3

Open
JStalnac opened this issue Apr 10, 2022 · 7 comments
Open

PP: Chat Message #3

JStalnac opened this issue Apr 10, 2022 · 7 comments

Comments

@JStalnac
Copy link

Proposal for the Chat Message packet. Includes clientbound (S->C) and serverbound (C->S) versions.

The packet is used for sending messages in chat and displaying messages on the client, like in v0.75.
Notice that the message Content is encoded as UTF-8, not CP437. UTF-8 is easier to work with and it supports more characters.

Chat Message

Info ID
Packet ID: 17

(The ID can be changed, I just picked the one that is in v0.75)

Clientbound

Info Size
Total Size: 1 or 2 + Length of Content
Field Name Field Type Example Notes
Message type UByte 0 See table below
(Sender) UByte 0 The ID of the player who sent the message. Not included if type is not CHAT_ALL or CHAT_TEAM
Content String (UTF-8) Hello, client!

Serverbound

Info Size
Total Size: 1 + Length of Content
Field Name Field Type Example Notes
Message type UByte 0 Only CHAT_ALL and CHAT_TEAM message types are allowed to be sent by client. See table below
Content String (UTF-8) Hello, server!

Message types supported by the protocol (see here for explanation):

Value Type
0 CHAT_ALL
1 CHAT_TEAM
2 CHAT_SYSTEM
3 CHAT_BIG
4 CHAT_INFO
5 CHAT_WARNING
6 CHAT_ERROR
@Haxk20
Copy link

Haxk20 commented Apr 10, 2022

I do not really see a reason why split it between server and client versions. The only difference is that clientbound contains a sender ID. Server either way has to know who sent the message. And while we have a way of detecting that. Its good to have that info included in the ID as well. For verification mostly.

I truly dont see a reason in splitting them.

The UTF-8 is a great idea.

Also sender ID should be ALWAYS on top. So the first byte after packet ID.

So basically the end result is 0.75 chat packet with more message types and UTF-8

@JStalnac
Copy link
Author

The split between client and server was to make it clear that client doesn't send its ID in the packet, there's no other reason for it.

Server either way has to know who sent the message. And while we have a way of detecting that. Its good to have that info included in the ID as well. For verification mostly.

To verify that the ID is correct the server must already know what client the packet came from so the verification doesn't matter. ENet sends an internal peer ID in each packet, that's how it knows where a packet came from.

Also sender ID should be ALWAYS on top. So the first byte after packet ID.

The Sender field is second here because its presence is determined by the Message type field before it. It may not always be there, so it makes reading the packet easier.

Sorry that my reply took a bit, I was busy earlier today.

@Haxk20
Copy link

Haxk20 commented Apr 11, 2022

The split between client and server was to make it clear that client doesn't send its ID in the packet, there's no other reason for it.

Server either way has to know who sent the message. And while we have a way of detecting that. Its good to have that info included in the ID as well. For verification mostly.

To verify that the ID is correct the server must already know what client the packet came from so the verification doesn't matter. ENet sends an internal peer ID in each packet, that's how it knows where a packet came from.

Also sender ID should be ALWAYS on top. So the first byte after packet ID.

The Sender field is second here because its presence is determined by the Message type field before it. It may not always be there, so it makes reading the packet easier.

Sorry that my reply took a bit, I was busy earlier today.

Yes but changing the packet for server and client versions is not great. It was done in the case of physics update cause it was needed to heavily limit bandwidth.

Yes as i said we already know the ID of the player when we receive a packet. The thing is tho the verification is good. Mainly to check if player doesnt change its ID mid game. Unlikely but can happen in rare cases.

I meant that when we always send the ID it has to be on top.

@JStalnac
Copy link
Author

I agree that working with two different versions of a packet can get difficult, but I think it makes sense with the player's own ID because it's data that is not needed by the server as it already knows the client's ID. If a player's ID were to change mid game on a client, it wouldn't even effect other players because it would only be a client side bug.

I meant that when we always send the ID it has to be on top.

I think the order would still make sense if we decide to ditch the serverbound packet because then the sender ID could be left out if type is not ALL or TEAM.

@JStalnac
Copy link
Author

JStalnac commented Oct 10, 2022

Revision 2 with colored messages like mentioned in #5

Chat Message

Packet ID
17

Clientbound

Sent from server to client. The message may be colored and have formatting applied.
If the Content in the packet is invalid, the client should reject the packet or display
partial message.

Size
2 + Length of Content
Field Name Field Type Example Notes
Message type UByte
(Sender) UByte 0 The ID of the player who sent the message. This field is not included if type is not CHAT_ALL or CHAT_TEAM, i.e. there is no sender
Content Message Part[] Array of Message Part (see table below)

Message Part
Size: 2 (+ 12 if Color present) + Length of Content

Field name Field Type Example Notes
Length UByte 13 The length of Message in characters*
Flags UByte 0 Bit flags for text properties (see table below)
(Color) Color3 1f 2f 3f The color of the part, if included (see table below). Three floats of color data, BGR byte order
Message String (UTF-8) Hello, World!

*: Unicode is a multibytes encoding, so there needs to be a way to determine the length.
The bytes of a character must not be split between message parts.

Bit Meaning Notes
1 Color data Whether the part is colored. If set, Color field is followed. If this bit is not set, the client should use its default chat color
2 Bold The client should display the message part in bold
3 Italic The client should display the message part in italics
4-8 Reserved Reserved

Serverbound

Sent from client to server. The server must reject the packet if Message type is
not CHAT_ALL or CHAT_TEAM.

Size
1 + Length of Content
Field Name Field Type Example Notes
Message type UByte
Content String (UTF-8)

I think that client should not be allowed to send colored messages, because it can lead to abuse and it is hard to handle on the server. Chat colors should be done by the server. Because of this, I think it makes sense to have a split between client and serverbound packets.

I used an 8-bit integer for part length instead of the 16-bit integer here because most messages are shorter than 255 characters and it saves bandwidth when there is much formatting.

TODOs:

  • Reject 0 length messages
  • What if UTF-8 in one of the Message Parts in clientbound packet is invalid?
  • Specify how to encode strings

@Haxk20
Copy link

Haxk20 commented Oct 10, 2022

Length is actually not needed as enet already provides that. So its arbitrary limitation. Which we should avoid cause messages can be longer then 255 chars.

As for the flags. Bits start at 0. So only 0,1,2 would be used and 3-7 would be reserved for further use.

@JStalnac
Copy link
Author

Length is actually not needed as enet already provides that. So its arbitrary limitation. Which we should avoid cause messages can be longer then 255 chars.

I'm sorry if the table was unclear, the Content is an array of Message Part, each part has its own length. I'll edit the comment to make this clear.

As for the flags. Bits start at 0. So only 0,1,2 would be used and 3-7 would be reserved for further use.

This isn't code where arrays start at 0, it's the first bit in the byte and that's why it is one. The range in last row is wrong, I'll change that too.

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

No branches or pull requests

2 participants