From 90f200be531b8a899a605521f3e9840de6f9794d Mon Sep 17 00:00:00 2001 From: Adam Date: Fri, 26 Nov 2021 20:19:13 -0500 Subject: [PATCH] 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. --- .../client/chat/ChatMessageManager.java | 46 ++++++++----- .../src/main/scripts/ChatBuilder.rs2asm | 6 ++ .../src/main/scripts/ChatSplitBuilder.rs2asm | 10 ++- .../client/chat/ChatMessageManagerTest.java | 69 +++++++++++-------- 4 files changed, 81 insertions(+), 50 deletions(-) diff --git a/runelite-client/src/main/java/net/runelite/client/chat/ChatMessageManager.java b/runelite-client/src/main/java/net/runelite/client/chat/ChatMessageManager.java index d5ac3525af..f673ea0e60 100644 --- a/runelite-client/src/main/java/net/runelite/client/chat/ChatMessageManager.java +++ b/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 - public void onChatMessage(ChatMessage chatMessage) + @VisibleForTesting + void colorChatMessage() { - MessageNode messageNode = chatMessage.getMessageNode(); - ChatMessageType chatMessageType = chatMessage.getType(); + final String[] stringStack = client.getStringStack(); + 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 (senderColor != null && !Strings.isNullOrEmpty(sender)) + if (channelColor != null && !Strings.isNullOrEmpty(channel)) { - messageNode.setSender(ColorUtil.wrapWithColorTag(sender, senderColor)); + stringStack[size - 4] = ColorUtil.wrapWithColorTag(channel, channelColor); } final Collection chatColors = colorCache.get(chatMessageType); @@ -186,9 +193,9 @@ public class ChatMessageManager // Replace tags in the message with the new color so embedded won't reset the color final Color color = chatColor.getColor(); - messageNode.setValue(ColorUtil.wrapWithColorTag( - messageNode.getValue().replace(ColorUtil.CLOSING_COLOR_TAG, ColorUtil.colorTag(color)), - color)); + stringStack[size - 2] = ColorUtil.wrapWithColorTag( + stringStack[size - 2].replace(ColorUtil.CLOSING_COLOR_TAG, ColorUtil.colorTag(color)), + color); break; } } @@ -207,6 +214,9 @@ public class ChatMessageManager case "privChatUsername": wrap = true; break; + case "chatMessageBuilding": + colorChatMessage(); + return; default: return; } diff --git a/runelite-client/src/main/scripts/ChatBuilder.rs2asm b/runelite-client/src/main/scripts/ChatBuilder.rs2asm index 89e852e785..59c9b36675 100644 --- a/runelite-client/src/main/scripts/ChatBuilder.rs2asm +++ b/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 diff --git a/runelite-client/src/main/scripts/ChatSplitBuilder.rs2asm b/runelite-client/src/main/scripts/ChatSplitBuilder.rs2asm index 53a08ad34f..80cdf54e09 100644 --- a/runelite-client/src/main/scripts/ChatSplitBuilder.rs2asm +++ b/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 diff --git a/runelite-client/src/test/java/net/runelite/client/chat/ChatMessageManagerTest.java b/runelite-client/src/test/java/net/runelite/client/chat/ChatMessageManagerTest.java index 97b2caaef0..f389702dce 100644 --- a/runelite-client/src/test/java/net/runelite/client/chat/ChatMessageManagerTest.java +++ b/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 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(); - chatMessage.setType(ChatMessageType.GAMEMESSAGE); + setupVm(ChatMessageType.GAMEMESSAGE, "", "Your dodgy necklace protects you. It has 1 charge left."); + chatMessageManager.colorChatMessage(); - MessageNode messageNode = mock(MessageNode.class); - chatMessage.setMessageNode(messageNode); - - when(messageNode.getValue()).thenReturn("Your dodgy necklace protects you. It has 1 charge left."); - chatMessageManager.onChatMessage(chatMessage); - - verify(messageNode).setValue("Your dodgy necklace protects you. It has 1 charge left."); + assertEquals("Your dodgy necklace protects you. It has 1 charge left.", sstack[2]); } @Test @@ -95,14 +118,7 @@ public class ChatMessageManagerTest when(chatColorConfig.opaquePublicFriendUsernames()).thenReturn(Color.decode("#b20000")); - // Setup message - ChatMessage chatMessage = new ChatMessage(); - chatMessage.setType(ChatMessageType.PUBLICCHAT); - chatMessage.setName(friendName); - - MessageNode messageNode = mock(MessageNode.class); - chatMessage.setMessageNode(messageNode); - when(messageNode.getName()).thenReturn(friendName); + setupVm(ChatMessageType.PUBLICCHAT, 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("" + friendName + ""); + assertEquals("" + friendName + "", sstack[1]); } @Test @@ -125,14 +141,7 @@ public class ChatMessageManagerTest when(chatColorConfig.opaquePublicFriendUsernames()).thenReturn(Color.decode("#b20000")); - // Setup message - ChatMessage chatMessage = new ChatMessage(); - chatMessage.setType(ChatMessageType.PUBLICCHAT); - chatMessage.setName(friendName); - - MessageNode messageNode = mock(MessageNode.class); - chatMessage.setMessageNode(messageNode); - when(messageNode.getName()).thenReturn(friendName); + setupVm(ChatMessageType.PUBLICCHAT, 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("" + friendName + ""); + assertEquals("" + friendName + "", sstack[1]); } @Test