From 1365e6079cc43b092b1c62a7ab94f1aa29971a84 Mon Sep 17 00:00:00 2001 From: _0Steven <42909981+00-Steven@users.noreply.github.com> Date: Fri, 8 Nov 2024 03:19:47 +0100 Subject: [PATCH] Fix NTNRC duplicate message jank and new message header, makes program headers actually update when there's none. (#87610) ## About The Pull Request As before, more fiddling with NTNRC, more bugs. This time, the same user sending the same message within the same second would cause this message to spawn a new copy each time the channel was opened, until the UI was closed and re-opened. Looking into it, this seemed to be because we would set the `Box`'s `key` value to the message contents: Which isn't actually unique. To fix this, we instead make each channel keep track of an incrementing `id` number to assign to each message, and convert the `messages` list to an associative list using those numerical ids stringified. We don't just use the list index as a key, as we later may want to target specific messages, so a consistent unique key is important. This fixes our primary issue. In the process of making the rest of the code account for this, I noticed that the NTNRC program header that's supposed to show new messages in the active channel when the program is idle didn't actually work. Just at all. So I rewrote the entire thing, and it now tracks the last read message's id rather than the full message, and sets this when you actually background the program. The rest still runs on process tick, where it updates the header if there's a new message with a different id on top. Finally, the header part of the UI wasn't actually updating if there were no headers, so now it forwards a lack of headers change as well. ## Why It's Good For The Game Reduces more NTNRC jank. Good if shit like, actually works. ## Changelog :cl: fix: NTNRC no longer endlessly duplicates messages with duplicate contents upon switching channels. fix: The new message header you get when NTNRC runs in the background actually works. fix: NtOS header actually updates if there are no program headers. /:cl: --------- Co-authored-by: SmArtKar <44720187+SmArtKar@users.noreply.github.com> --- .../computers/item/computer.dm | 4 +- .../programs/chatroom/conversation.dm | 11 +++-- .../programs/chatroom/ntnrc_client.dm | 49 +++++++++++++------ tgui/packages/tgui/interfaces/NtosNetChat.jsx | 2 +- 4 files changed, 44 insertions(+), 22 deletions(-) diff --git a/code/modules/modular_computers/computers/item/computer.dm b/code/modules/modular_computers/computers/item/computer.dm index 74001a88524..4923479d71d 100644 --- a/code/modules/modular_computers/computers/item/computer.dm +++ b/code/modules/modular_computers/computers/item/computer.dm @@ -610,14 +610,14 @@ if(NTNET_ETHERNET_SIGNAL) data["PC_ntneticon"] = "sig_lan.gif" + var/list/program_headers = list() if(length(idle_threads)) - var/list/program_headers = list() for(var/datum/computer_file/program/idle_programs as anything in idle_threads) if(!idle_programs.ui_header) continue program_headers.Add(list(list("icon" = idle_programs.ui_header))) - data["PC_programheaders"] = program_headers + data["PC_programheaders"] = program_headers data["PC_stationtime"] = station_time_timestamp() data["PC_stationdate"] = "[time2text(world.realtime, "DDD, Month DD")], [CURRENT_STATION_YEAR]" diff --git a/code/modules/modular_computers/file_system/programs/chatroom/conversation.dm b/code/modules/modular_computers/file_system/programs/chatroom/conversation.dm index fcfa41e3c90..969e56195ed 100644 --- a/code/modules/modular_computers/file_system/programs/chatroom/conversation.dm +++ b/code/modules/modular_computers/file_system/programs/chatroom/conversation.dm @@ -13,8 +13,10 @@ ///ID using the UID. var/id - ///List of all messages sent in the conversation. + ///Associative list of all messages sent in the conversation. id > message var/list/messages = list() + ///ID used for next message, increments each use. Convert to string before use. + var/next_message_id = 0 ///The "Administrator" of the channel, the creator starts as channel's operator by default. var/datum/computer_file/program/chatclient/channel_operator @@ -45,11 +47,14 @@ /datum/ntnet_conversation/proc/add_message(message, username) message = "[station_time_timestamp(format = "hh:mm")] [username]: [message]" - messages.Add(message) + messages["[next_message_id]"] = message + next_message_id++ trim_message_list() /datum/ntnet_conversation/proc/add_status_message(message) - messages.Add("[station_time_timestamp(format = "hh:mm")] -!- [message]") + message = "[station_time_timestamp(format = "hh:mm")] -!- [message]" + messages["[next_message_id]"] = message + next_message_id++ trim_message_list() /datum/ntnet_conversation/proc/trim_message_list() diff --git a/code/modules/modular_computers/file_system/programs/chatroom/ntnrc_client.dm b/code/modules/modular_computers/file_system/programs/chatroom/ntnrc_client.dm index 9e7385c0b65..8045bcb1db0 100644 --- a/code/modules/modular_computers/file_system/programs/chatroom/ntnrc_client.dm +++ b/code/modules/modular_computers/file_system/programs/chatroom/ntnrc_client.dm @@ -23,8 +23,8 @@ ///The user's screen name. var/username - ///The last message you sent in a channel, used to tell if someone has sent a new message yet. - var/last_message + ///The id of the last message sent in a channel, used to tell if someone has sent a new message yet. + var/last_message_id ///The channel currently active in. var/active_channel ///If the tablet is in Admin mode, you bypass Passwords and aren't announced when entering a channel. @@ -133,8 +133,8 @@ // Now we will generate HTML-compliant file that can actually be viewed/printed. logfile.filename = logname logfile.stored_text = "\[b\]Logfile dump from NTNRC channel [channel.title]\[/b\]\[BR\]" - for(var/logstring in channel.messages) - logfile.stored_text = "[logfile.stored_text][logstring]\[BR\]" + for(var/message_id in channel.messages) + logfile.stored_text = "[logfile.stored_text][channel.messages[message_id]]\[BR\]" logfile.stored_text = "[logfile.stored_text]\[b\]Logfile dump completed.\[/b\]" logfile.calculate_size() if(!computer || !computer.store_file(logfile)) @@ -183,19 +183,23 @@ /datum/computer_file/program/chatclient/process_tick(seconds_per_tick) . = ..() - var/datum/ntnet_conversation/channel = SSmodular_computers.get_chat_channel_by_id(active_channel) - if(src in computer.idle_threads) + + if(!(src in computer.idle_threads)) + return + + var/datum/ntnet_conversation/watched_channel = SSmodular_computers.get_chat_channel_by_id(active_channel) + if(isnull(watched_channel)) // If we're not in a channel, no need for a message notification header. + ui_header = null + return + if(!length(watched_channel.messages)) // But if there's no messages, we do still wait for a message. ui_header = "ntnrc_idle.gif" - if(channel) - // Remember the last message. If there is no message in the channel remember null. - last_message = length(channel.messages) ? channel.messages[length(channel.messages)] : null - else - last_message = null - return TRUE - if(channel?.messages?.len) - ui_header = (last_message == channel.messages[length(channel.messages)] ? "ntnrc_idle.gif" : "ntnrc_new.gif") - else + return + + var/last_message_id_found = watched_channel.messages[length(watched_channel.messages)] + if(last_message_id_found == last_message_id) ui_header = "ntnrc_idle.gif" + return + ui_header = "ntnrc_new.gif" /datum/computer_file/program/chatclient/on_start(mob/living/user) . = ..() @@ -212,6 +216,17 @@ active_channel = null return ..() +/datum/computer_file/program/chatclient/background_program(mob/user) + . = ..() + var/datum/ntnet_conversation/open_channel = SSmodular_computers.get_chat_channel_by_id(active_channel) + if(isnull(open_channel) || !length(open_channel.messages)) + last_message_id = null + ui_header = null + return + + last_message_id = open_channel.messages[length(open_channel.messages)] + ui_header = "ntnrc_idle.gif" + /// Converts active/idle/closed to a numerical status for sorting clients by. /datum/computer_file/program/chatclient/proc/get_numerical_status() if(src == computer.active_program) @@ -265,8 +280,10 @@ data["clients"] = clients var/list/messages = list() for(var/i=channel.messages.len to 1 step -1) + var/message_id = channel.messages[i] messages.Add(list(list( - "msg" = channel.messages[i], + "key" = message_id, + "msg" = channel.messages[message_id], ))) data["messages"] = messages data["is_operator"] = (channel.channel_operator == src) || netadmin_mode diff --git a/tgui/packages/tgui/interfaces/NtosNetChat.jsx b/tgui/packages/tgui/interfaces/NtosNetChat.jsx index 61f4a7998cc..0bfa826847f 100644 --- a/tgui/packages/tgui/interfaces/NtosNetChat.jsx +++ b/tgui/packages/tgui/interfaces/NtosNetChat.jsx @@ -151,7 +151,7 @@ export const NtosNetChat = (props) => { {(in_channel && (authorized ? ( messages.map((message) => ( - {message.msg} + {message.msg} )) ) : (