Player#chat method doesn't accept text containing ChatColors

[Mrcomputer1]

Expected behavior

I expected a chat message with colour sent as player, as this is what happens on servers running Spigot.

Observed/Actual behavior

On Paper, the player is kicked for Illegal characters in chat.

Steps/models to reproduce

player.chat(ChatColor.GREEN + "Test");

Test plugin (/chattest): ColourChatTestPlugin-src.zip.

Plugin and Datapack List

Plugins (1): ColourChatTest There are 2 data packs enabled: [vanilla (built-in)], [file/bukkit (world)]

Paper version

[19:12:37 INFO]: Checking version, please wait...
[19:12:38 INFO]: This server is running Paper version git-Paper-166 (MC: 1.19.2) (Implementing API version 1.19.2-R0.1-SNAPSHOT) (Git: f528f53)
You are running the latest version

Other

Version of Spigot I used to test: This server is running CraftBukkit version 3590-Spigot-4c157bb-08cdd26 (MC: 1.19.2) (Implementing API version 1.19.2-R0.1-SNAPSHOT).

[lynxplay]

This is an active change on the side of paper. https://github.com/PaperMC/Paper/blob/f528f53e81fe12887629d9a2c12893a20b8c0ce3/patches/server/0055-Ensure-commands-are-not-ran-async.patch#L117-L130

Players cannot include the legacy colour prefix in their chat message, it makes perfect sense to prevent it from being used in what is essentially emulated player chat.

I won't close this issue directly because that is simply my opinion, so other peoples input here is needed, but I believe this is a "works-as-intended".

[MiniDigger]

I am inclined to agree with lynxplay, but am happy to hear your usecase for sending such messages @Mrcomputer1

[Mrcomputer1]

The way I was using it is this: when a player purchases an item from another player with my plugin, it sends a thanks chat message as the player the item was purchased from. The chat message my plugin sends is in green, which I use as a way to differentiate it from a real message the player sent and my plugin's one.

Although, I do agree that it makes sense to prevent legacy colour prefixes being used with Player#chat. The reason I reported this here is because the change breaks compatibility with Spigot and older Paper versions so I wasn't sure if it was intentional.

Also, with that change, since the chat message is being sent by a plugin via Player#chat and not by the player themself, wouldn't it make more sense to throw an IllegalArgumentException instead of kicking the player?

[Machine-Maker]

The Player#chat method is supposed to, as close as possible, emulate a player typing the string into their chat box and sending the message. So as much of the chat logic as possible is replicated for that method. Including illegal char checks, no colors, everything.

[willkroboth]

I agree with Mrcomputer1 that the Paper patch in question is a compatibility issue with Spigot. It seems completely reasonable to me that plugins written for Spigot might use Player#chat to send messages with the ChatColor characters. If this plugin was moved over to Paper this patch means that the plugin wouldn't work the same. Since Paper wants to be compatible with Spigot plugins, it makes the most sense to me that this patch should be removed to give Paper the same behavior as Spigot here.

Alternative 1:

So as much of the chat logic as possible is replicated for that method. Including illegal char checks, no colors, everything.

Are players ever kicked after sending a message into chat containing the illegal ChatColor characters? Where did this behavior come from? If players are never kicked for sending ChatColors on Spigot or Paper, there is no basis for kicking players when ChatColor characters are found and this patch should be removed. If Paper added this, I think that would be another compatibility issue with Spigot. On Spigot players can send ChatColors while on Paper they cannot. If this is the case, I think both this patch and the patch that kicks players for typing ChatColors should be removed. However, if players are kicked for typing ChatColors on Spigot, I could see this as a behavioral inconsistency that Paper has fixed. On Spigot there is an inconsistency that players cannot send ChatColors but Player#chat can which is supposed to emulate a player typing into chat. If this is the case Paper should probably stay the same and this behavioral inconsistency should be presented to Spigot.

I can't currently test which of those 3 cases we are in, but whichever one it is, that is what I think.

Alternative 2:

If this behavior is going to be kept where Spigot Player#chat can contain ChatColors while Paper Player#chat cannot, is there some alternative Mrcomputer can use? Is there some alternative line of code that sends a colored message as a player on both Spigot and Paper? Do plugin developers just need to detect if their plugin is on Spigot or Paper and manually change their behavior (I don't think that's a very nice solution)? Is there just no hope for plugins to be able to easily send colored messages while being compatible with Spigot and Paper?

[Machine-Maker]

the Paper patch in question is a compatibility issue with Spigot

just because behavior is different, doesn't mean its a compatibility. It's a bug in spigot that Paper has fixed. If the behavior is different because of that, that's just how it is.

Are players ever kicked after sending a message into chat containing the illegal ChatColor characters? Where did this behavior come from?

Players on vanilla, spigot, and paper servers are all kicked if the string in the chat packet contains an illegal character (§ being one of them). This doesn't happen often because on a vanilla client, you can't even input the § character into the chat box, but modified client can include them in the server-bound chat packet.

Is there some alternative line of code that sends a colored message as a player on both Spigot and Paper?

You can just iterate over all the players on the server, and use the regular sendMessage(String) method (or preferably if using Paper, use the sendMessage(Component) method). This is just sending a message to each player. You might have to do the formatting of it yourself, like add the sending name as a prefix, and what not.

Keep in mind, that since message signatures have been introduced, there is no way for the server to send a message as another player and have it be signed by that player.

[willkroboth]

just because behavior is different, doesn't mean its a compatibility.

Ok, I wasn't sure what exactly the behavior was at the time. It seemed most likely to me that it would have been a compatibility issue, so I put that first, but I also put those alternatives in case it was something else (maybe I could have made that clearer \_ o~o _/ ?).

Players on vanilla, spigot, and paper servers are all kicked if the string in the chat packet contains an illegal character (§ being one of them)

Got it, that places my opinion on my 3rd point. Player#chat is supposed to emulate a player typing in chat, which is supposed to kick them when they try to type a chat color, but it doesn't on Spigot, so Paper fixed that. Going off from that, it seems that this is instead a Spigot issue. Do you think it's worth telling Spigot about this then so they can fix it too?

The loop and sendMessage(String) sound good to me (I don't think sendMessage(Component) applies here since it's supposed to work on Spigot and Paper, but whatever). That does support ChatColors as far as I know, but I suppose it's really up to Mrcomputer now to determine if that resolves the issue.

[electronicboy]

Part of the headache of chat is that it's one of the mostly forgotten parts of the API, most of it's abusage is for people dispatching commands for players, rather than actually using it to, well, chat.

Ofc, this create a headache as the linked patch was intended to fix some inconsistencies in behavior for Player#chat vs real chat; Part of the issue here is the long term ramifications; i.e. the ONLY way to allow colors to work in Player#chat, is to allow improperly formed Components, make the server parse those into full properly defined components, or just ask that plugins generally do what plugins have done for years and handled color inside of that event system itself

An argument can be made for turning that thing into an illegal arg, rather than a kick, as plugins doing bad stuff in the API generally shouldn't induce a kick where viable, but, long term, you should look to avoid using ChatColor in general, mojang deprecated that thing 7 years ago

[Owen1212055]

Closing as won't fix.

Although yes this is a rather large behavior change, it's meant to be an exact simulation of a player chatting. Going through this kicking logic makes sense, as in general this is the exact behavior that will happen if a player were to chat this.

As stated above, avoid using chatcolor like this (or at all) in the future.

Especially since now chat is signed, it's very easy to figure out if a message is truly from a player or was fabricated by the server.

[willkroboth]

So, should Spigot be informed? It seems they have a bug where their implementation of Player#chat does not quite emulate a Player chatting in this instance. Fixing bugs is good, so they will probably like it if someone tells them about this bug they can fix.

[Owen1212055]

@willkroboth Feel free to make an issue on their JIRA, sure. As if this is behavior you went to see fixed on spigot you’ll need to report it there.

[willkroboth]

Okay, I'm trying to make sure I have all the information straight so I can make a competent report on Spigot.

First, I was wondering where exactly the idea that Player#chat should emulate Player chat exactly came from. I checked the Spigot javadocs first, but they weren't much help. They just say Player#chat "Says a message (or runs a command)." Strangely nothing about the message even being from the Player, so obviously not the whole story. Next, I tried to figure out when this bug was first fixed by Paper to use the same reason when proposing to Spigot to fix it. I found this patch from this PR (8131), which references this issue (8197). Strangely, that issue seems to be only concerned with the improper handling of commands with Player#chat, not the improper handling of the ChatColor kicking with chat. Somehow, between the issue and the PR, the check for illegal chat got added in as well (Illegal chat could have been ignored if just this part of the original patch was used:

+            if (msg.startsWith("/")) {
+                this.getHandle().connection.handleCommand(msg);
+            } else {
+                // TODO text filtering
+                // TODO chat decorating
+                this.getHandle().connection.chat(msg, PlayerChatMessage.unsigned(net.minecraft.network.chat.MessageSigner.create(this.getUniqueId()), new net.minecraft.network.chat.ChatMessageContent(msg)), false);
+            }

) Conclusion: I don't know where the idea that Player#chat should be perfectly consistent with a Player typing in chat came from. It sounds like that could just be obvious, but I don't think Spigot would accept that, since when Spigot fixed the same issue as #8197, (SPIGOT-7123), they didn't make it so ChatColor in Player#chat kicked players. If @Machine-Maker (or someone else) could explain what the thoughts and reasoning behind adding the kick on ChatColor in Player#chat into that patch, that would greatly help me write the Spigot issue.

Second, I'm kinda confused about the thing where ChatColor was deprecated by Mojang 7 years ago. I know about the org.bukkit.ChatColor class, and that doesn't seem to be deprecated. The closest Mojang class I've found to ChatColor, net.minecraft.ChatFormatting (which also has an enum of colors that are probably put into chat), also doesn't seem to be deprecated. I just need to be sure what you're talking about so I can support the idea that developers shouldn't be putting ChatColor in Player#chat anyways, due to it being deprecated and replaced with something else.

Thanks for any help!

[Doc94]

About the chat color thing... Mojang use json for all the format things... ChatColor of Bukkit/CraftBukkit its the simple implementation of the old system... The bungeecord-chat (or how md5 name this) has a ChatColor with a little more of support (same name not same package).

Send from a phone then i can make a few of mistakes...