chat message manager: don't apply color changes to message nodes

Apply the color changes at chat build time directly to the message being
set on the component, instead of prepending the color change onto the
message itself.

This is a bit more robust since it doesn't break things which depend on
the chat message not starting with a color, which they would not
otherwise, such as the chat filter.

runelite-client/src/main/java/net/runelite/client/chat/ChatMessageManager.java

@@ -24,6 +24,7 @@
  */
 package net.runelite.client.chat;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.HashMultimap;
@@ -44,7 +45,6 @@ import net.runelite.api.Client;
 import net.runelite.api.MessageNode;
 import net.runelite.api.Player;
 import net.runelite.api.Varbits;
-import net.runelite.api.events.ChatMessage;
 import net.runelite.api.events.ResizeableChanged;
 import net.runelite.api.events.ScriptCallbackEvent;
 import net.runelite.api.events.VarbitChanged;
@@ -111,15 +111,23 @@ public class ChatMessageManager
 		}
 	}
 
-	@Subscribe(priority = -1) // run after all plugins
+	@VisibleForTesting
-	public void onChatMessage(ChatMessage chatMessage)
+	void colorChatMessage()
 	{
-		MessageNode messageNode = chatMessage.getMessageNode();
+		final String[] stringStack = client.getStringStack();
-		ChatMessageType chatMessageType = chatMessage.getType();
+		final int size = client.getStringStackSize();
+		final int uid = client.getIntStack()[client.getIntStackSize() - 1];
 
-		boolean isChatboxTransparent = client.isResized() && client.getVar(Varbits.TRANSPARENT_CHATBOX) == 1;
+		final MessageNode messageNode = client.getMessages().get(uid);
+		assert messageNode != null : "chat message build for unknown message";
+
+		final String username = stringStack[size - 3];
+		final String channel = stringStack[size - 4];
+		final ChatMessageType chatMessageType = messageNode.getType();
+
+		final boolean isChatboxTransparent = client.isResized() && client.getVar(Varbits.TRANSPARENT_CHATBOX) == 1;
 		Color usernameColor = null;
-		Color senderColor = null;
+		Color channelColor = null;
 
 		switch (chatMessageType)
 		{
@@ -130,7 +138,7 @@ public class ChatMessageManager
 			case PUBLICCHAT:
 			case MODCHAT:
 			{
-				String sanitizedUsername = Text.removeTags(chatMessage.getName()).replace('\u00A0', ' ');
+				String sanitizedUsername = Text.removeTags(username).replace('\u00A0', ' ');
 
 				if (client.getLocalPlayer().getName().equals(sanitizedUsername))
 				{
@@ -149,31 +157,30 @@ public class ChatMessageManager
 			case FRIENDSCHAT:
 			case FRIENDSCHATNOTIFICATION:
 				usernameColor = isChatboxTransparent ? chatColorConfig.transparentFriendsChatUsernames() : chatColorConfig.opaqueFriendsChatUsernames();
-				senderColor = isChatboxTransparent ? chatColorConfig.transparentFriendsChatChannelName() : chatColorConfig.opaqueFriendsChatChannelName();
+				channelColor = isChatboxTransparent ? chatColorConfig.transparentFriendsChatChannelName() : chatColorConfig.opaqueFriendsChatChannelName();
 				break;
 			case CLAN_CHAT:
 			case CLAN_MESSAGE:
 			case CLAN_GIM_CHAT:
 			case CLAN_GIM_MESSAGE:
 				usernameColor = isChatboxTransparent ? chatColorConfig.transparentClanChatUsernames() : chatColorConfig.opaqueClanChatUsernames();
-				senderColor = isChatboxTransparent ? chatColorConfig.transparentClanChannelName() : chatColorConfig.opaqueClanChannelName();
+				channelColor = isChatboxTransparent ? chatColorConfig.transparentClanChannelName() : chatColorConfig.opaqueClanChannelName();
 				break;
 			case CLAN_GUEST_CHAT:
 			case CLAN_GUEST_MESSAGE:
 				usernameColor = isChatboxTransparent ? chatColorConfig.transparentClanChatGuestUsernames() : chatColorConfig.opaqueClanChatGuestUsernames();
-				senderColor = isChatboxTransparent ? chatColorConfig.transparentClanChannelGuestName() : chatColorConfig.opaqueClanGuestChatChannelName();
+				channelColor = isChatboxTransparent ? chatColorConfig.transparentClanChannelGuestName() : chatColorConfig.opaqueClanGuestChatChannelName();
 				break;
 		}
 
 		if (usernameColor != null)
 		{
-			messageNode.setName(ColorUtil.wrapWithColorTag(messageNode.getName(), usernameColor));
+			stringStack[size - 3] = ColorUtil.wrapWithColorTag(username, usernameColor);
 		}
 
-		String sender = messageNode.getSender();
+		if (channelColor != null && !Strings.isNullOrEmpty(channel))
-		if (senderColor != null && !Strings.isNullOrEmpty(sender))
 		{
-			messageNode.setSender(ColorUtil.wrapWithColorTag(sender, senderColor));
+			stringStack[size - 4] = ColorUtil.wrapWithColorTag(channel, channelColor);
 		}
 
 		final Collection<ChatColor> chatColors = colorCache.get(chatMessageType);
@@ -186,9 +193,9 @@ public class ChatMessageManager
 
 			// Replace </col> tags in the message with the new color so embedded </col> won't reset the color
 			final Color color = chatColor.getColor();
-			messageNode.setValue(ColorUtil.wrapWithColorTag(
+			stringStack[size - 2] = ColorUtil.wrapWithColorTag(
-				messageNode.getValue().replace(ColorUtil.CLOSING_COLOR_TAG, ColorUtil.colorTag(color)),
+				stringStack[size - 2].replace(ColorUtil.CLOSING_COLOR_TAG, ColorUtil.colorTag(color)),
-				color));
+				color);
 			break;
 		}
 	}
@@ -207,6 +214,9 @@ public class ChatMessageManager
 			case "privChatUsername":
 				wrap = true;
 				break;
+			case "chatMessageBuilding":
+				colorChatMessage();
+				return;
 			default:
 				return;
 		}

runelite-client/src/main/scripts/ChatBuilder.rs2asm

@@ -440,11 +440,17 @@ LABEL391:
    jump                   LABEL1721
 LABEL405:
    iload                  10 ; message uid
+   sload                  17 ; message channel
+   sload                  16 ; message name
+   sload                  18 ; message
    sload                  21 ; message timestamp
    sconst                 "chatMessageBuilding"
    runelite_callback     
    pop_int                   ; pop uid
    sstore                 21 ; message timestamp
+   sstore                 18 ; message
+   sstore                 16 ; message name
+   sstore                 17 ; message channel
    iload                  11
    switch                
       1: LABEL408

runelite-client/src/main/scripts/ChatSplitBuilder.rs2asm

@@ -413,11 +413,17 @@ CHAT_FILTER:
    jump                   LABEL562
 LABEL368:
    iload                  12 ; message uid
-   sload                  2 ; message timestamp
+   sconst                 "" ; message channel
+   sload                   1 ; message name
+   sload                   0 ; message
+   sload                   2 ; message timestamp
    sconst                 "chatMessageBuilding"
    runelite_callback     
-   pop_int               
+   pop_int                  ; uid
    sstore                 2 ; message timestamp
+   sstore                 0 ; message
+   sstore                 1 ; message name
+   pop_string               ; message channel
    iload                  18
    switch                
       3: LABEL371

runelite-client/src/test/java/net/runelite/client/chat/ChatMessageManagerTest.java

@@ -31,14 +31,16 @@ import com.google.inject.testing.fieldbinder.BoundFieldModule;
 import java.awt.Color;
 import net.runelite.api.ChatMessageType;
 import net.runelite.api.Client;
+import net.runelite.api.IterableHashTable;
 import net.runelite.api.MessageNode;
 import net.runelite.api.Player;
-import net.runelite.api.events.ChatMessage;
 import net.runelite.client.config.ChatColorConfig;
 import net.runelite.client.events.ConfigChanged;
+import static org.junit.Assert.assertEquals;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import static org.mockito.ArgumentMatchers.anyLong;
 import org.mockito.Mock;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -59,12 +61,39 @@ public class ChatMessageManagerTest
 	@Inject
 	private ChatMessageManager chatMessageManager;
 
+	private String[] sstack;
+	private int[] istack;
+
 	@Before
 	public void before()
 	{
 		Guice.createInjector(BoundFieldModule.of(this)).injectMembers(this);
 	}
 
+	private void setupVm(ChatMessageType type, String name, String message)
+	{
+		MessageNode messageNode = mock(MessageNode.class);
+		when(messageNode.getType()).thenReturn(type);
+
+		IterableHashTable<MessageNode> tbl = mock(IterableHashTable.class);
+		when(tbl.get(anyLong())).thenReturn(messageNode);
+		when(client.getMessages()).thenReturn(tbl);
+
+		sstack = new String[]{
+			"",
+			name,
+			message,
+			""
+		};
+		istack = new int[]{
+			1
+		};
+		when(client.getStringStack()).thenReturn(sstack);
+		when(client.getStringStackSize()).thenReturn(sstack.length);
+		when(client.getIntStack()).thenReturn(istack);
+		when(client.getIntStackSize()).thenReturn(istack.length);
+	}
+
 	@Test
 	public void testMessageRecoloring()
 	{
@@ -75,16 +104,10 @@ public class ChatMessageManagerTest
 		configChanged.setGroup("textrecolor");
 		chatMessageManager.onConfigChanged(configChanged);
 
-		ChatMessage chatMessage = new ChatMessage();
+		setupVm(ChatMessageType.GAMEMESSAGE, "", "Your dodgy necklace protects you. It has <col=ff0000>1</col> charge left.");
-		chatMessage.setType(ChatMessageType.GAMEMESSAGE);
+		chatMessageManager.colorChatMessage();
 
-		MessageNode messageNode = mock(MessageNode.class);
+		assertEquals("<col=b20000>Your dodgy necklace protects you. It has <col=ff0000>1<col=b20000> charge left.</col>", sstack[2]);
-		chatMessage.setMessageNode(messageNode);
-
-		when(messageNode.getValue()).thenReturn("Your dodgy necklace protects you. It has <col=ff0000>1</col> charge left.");
-		chatMessageManager.onChatMessage(chatMessage);
-
-		verify(messageNode).setValue("<col=b20000>Your dodgy necklace protects you. It has <col=ff0000>1<col=b20000> charge left.</col>");
 	}
 
 	@Test
@@ -95,14 +118,7 @@ public class ChatMessageManagerTest
 
 		when(chatColorConfig.opaquePublicFriendUsernames()).thenReturn(Color.decode("#b20000"));
 
-		// Setup message
+		setupVm(ChatMessageType.PUBLICCHAT, friendName, "");
-		ChatMessage chatMessage = new ChatMessage();
-		chatMessage.setType(ChatMessageType.PUBLICCHAT);
-		chatMessage.setName(friendName);
-
-		MessageNode messageNode = mock(MessageNode.class);
-		chatMessage.setMessageNode(messageNode);
-		when(messageNode.getName()).thenReturn(friendName);
 
 		// Setup friend checking
 		Player localPlayer = mock(Player.class);
@@ -111,9 +127,9 @@ public class ChatMessageManagerTest
 		when(client.getLocalPlayer()).thenReturn(localPlayer);
 		when(localPlayer.getName()).thenReturn(localPlayerName);
 
-		chatMessageManager.onChatMessage(chatMessage);
+		chatMessageManager.colorChatMessage();
 
-		verify(messageNode).setName("<col=b20000>" + friendName + "</col>");
+		assertEquals("<col=b20000>" + friendName + "</col>", sstack[1]);
 	}
 
 	@Test
@@ -125,14 +141,7 @@ public class ChatMessageManagerTest
 
 		when(chatColorConfig.opaquePublicFriendUsernames()).thenReturn(Color.decode("#b20000"));
 
-		// Setup message
+		setupVm(ChatMessageType.PUBLICCHAT, friendName, "");
-		ChatMessage chatMessage = new ChatMessage();
-		chatMessage.setType(ChatMessageType.PUBLICCHAT);
-		chatMessage.setName(friendName);
-
-		MessageNode messageNode = mock(MessageNode.class);
-		chatMessage.setMessageNode(messageNode);
-		when(messageNode.getName()).thenReturn(friendName);
 
 		// Setup friend checking
 		Player localPlayer = mock(Player.class);
@@ -141,9 +150,9 @@ public class ChatMessageManagerTest
 		when(client.getLocalPlayer()).thenReturn(localPlayer);
 		when(localPlayer.getName()).thenReturn(localPlayerName);
 
-		chatMessageManager.onChatMessage(chatMessage);
+		chatMessageManager.colorChatMessage();
 
-		verify(messageNode).setName("<col=b20000>" + friendName + "</col>");
+		assertEquals("<col=b20000>" + friendName + "</col>", sstack[1]);
 	}
 
 	@Test