#4069 closed enhancement (fixed)
[PATCH] Ingame chat history
Reported by: | elexis | Owned by: | elexis |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 21 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | andy011973@… | Patch: |
Description (last modified by )
Since chat is only shown for 30 seconds, sometimes chat messages are not read while they might have contained some important message. Thus it should be possible to browse through prior chat messages.
It might be nice to query the chat history for only relevant messages, for example only team chat or hiding observer spam, refs #3380.
Notice if the "extended" checkbox is unchecked, it will keep the old style.
Attachments (9)
Change History (27)
by , 8 years ago
Attachment: | screenshot1.jpg added |
---|
by , 8 years ago
Attachment: | screenshot2.jpg added |
---|
by , 8 years ago
Attachment: | chathistory_wip1.patch added |
---|
comment:1 by , 8 years ago
Description: | modified (diff) |
---|
comment:2 by , 8 years ago
Keywords: | rfc added |
---|
A call to rfc!
Does it make sense to show the bottom panel (addressee, text, send button) also when having that other tab "Filter" opened? To me it seems more natural to have that always show up.
comment:3 by , 8 years ago
I think so. Now what you could do is switch back to the other tab after sending a message.
Also, can you make the tabs look more like the summary screen ? https://wildfiregames.com/forum/index.php?/topic/20963-epic-battles/
comment:4 by , 8 years ago
Cc: | added |
---|
by , 8 years ago
Attachment: | 4069_chat_history_v1.patch added |
---|
reused some parts from elexis patch. Final patch
by , 8 years ago
Attachment: | 4069_chat_history_v1.1.patch added |
---|
Disable the filtering for Observers, as it makes no sense there
comment:6 by , 8 years ago
About your patch:
- What's wrong about using
lobby.chattimestamp
insession/
? - When changing the dropdown filter, only new messages are affected. In my version, one could change the filter ingame and see all messages sent at any time with the currently specified filters.
- Not sure about the window size, windows ought to be vertically centered I think, or slightly biased towards the top, but likely not that much
- The filter dropdown will only be confusing above the other two. It should rather go above the chat window, thus also hint that changing the filter dropdown will change the text below it.
- It's probably best to allow both hosts to mute clients globally (#3380) and in this ticket only implement the local muting.
About my patch: Indeed it was silly to add one checkbox for every client connected. An individual muting system could be a two step process:
- first a dropdown to select a target to mute ("everything", "connect/disconnect notifications", "game notifications", "observers", "allies", "enemies", "players", "one player", ...)
- then if "one player" was selected, a dropdown box to pick some client.
Thus avoiding the pointless checkboxes.
It's perhaps not needed to have a client specific filtering at all. There were thoughts about adding a bunch of specific optional game notifications (for example if an ally reached city phase, built a civic center or a market #3512).
So if the latter would be implemented, the prior one could be done too. Other than that the client-based filtering isn't needed now.
by , 8 years ago
Attachment: | 4069_chat_history_v1.2.patch added |
---|
always center(with offset) the window. move the filterdropdown to the top. rename lobby.chattimestamp to chat.timestamp
follow-up: 8 comment:7 by , 8 years ago
Looks good, only some minor optimizations:
- L718 could use a comment stating that these keys are used for filtering the chat history
styles.xml
straight copy of the gamesetupChatPanel
? Move it tocommon/styles.xml
until someone decides to use two different formats IMO.translate("All")
-> `translateWithContext("chat history filter", "Notifications and Chat")translate("Chat")
-> `translateWithContext("chat history filter", "Chat messages")initChatHistoryFilter
could useprepareForDropdown
to specify the options in a more grouped manner:let options = prepareForDropdown([{"key": foo, "text": translate(...)}, {...}, ...];
options.js
entry could mention that it's used in the lobby and ingame chat history- Being attacked by Gaia throws:
WARNING: JavaScript warning: gui/session/messages.js line 713 reference to undefined property msg.guid
- The window could be a bit wider IMO, just so that the proportions are more close to the golden angle.
- Since the extended checkbox is now only about the history, we can show translateWithContext("chat", "History") there. (That might change in the future when more stuff is added. So perhaps keep the internal "extended" variable naming)
- Would rename
noObserver
toplayerChat
for consistency - It seems weird to hide that dropdown to observers. There is a particular use case where some player is sending resources to an ally, thus spamming the chatlog which could be filtered with that dropdown. Since the chat history recalls who was an ally at that time the message was received, it should work without issues in a post defeat state too.
by , 8 years ago
Attachment: | 4069_chat_history_v1.3.patch added |
---|
fixed everything besides the style.xml thing
comment:8 by , 8 years ago
Replying to elexis:
Looks good, only some minor optimizations:
styles.xml
straight copy of the gamesetupChatPanel
? Move it tocommon/styles.xml
until someone decides to use two different formats IMO.
Sorry, I don't understand what you mean. I just copied the styles.xml changes from your patch
by , 8 years ago
Attachment: | diplomacy_screen_chatPanel_fix.patch added |
---|
Note to oneself: commit after #3934 is merged.
comment:9 by , 8 years ago
Added functionality:
- The filters didn't work they are supposed to. For example the ally filter should display only messages that were sent to allies, i.e. exclude public messages (even if they were sent by an ally).
- Adding support to filter enemy, observer and private messages
- Added timestamp functionality to the gamesetup
- Saving the checkbox state to the user config file, so people don't have to click their preference every gamestart.
Style fixes:
let options
can become a globalg_ChatHistoryFilters
right near those other chat objects inmessages.js
. The logic behind theisChat
,isAllyChat
keys can be moved to functions of that global, thus becoming more grouped. Renaming "data" to "key".- Adding newlines to
chat_window.xml
where needed to stay under the 80 character limit. Unfortunately we can't have linebreaks fortranslatableAttribute
, sinceupdateTemplates.py
will extract the surrounding whitespace:public-gui-ingame.pot:"\t\t\t\tSelect chatmessage addressee.\n"
- Reordering XML tags type, name, size, sprite, style, ...
- Commented out code, really?
- Removing unused name tags
chatHistoryFilterLabel
. - Don't copy my wip mistakes (1): Removed the new chatpanel style that is a duplicate of the gamesetup one.
- Don't copy my wip mistakes (2): The
ChatPanel
theme shouldn't be used for labels. UseModernText
,ModernLeftLabelText
or similar instead. - The entire diplomacy manager was using the ChatPanel theme (while clearly not being one).
- Removed the lobby chatpanel style which is yet another duplicate.
- Renamed the session
chatPanel
style tochatPanelOverlay
to be more precise and differentiate it from the otherChatPanel
style. - Removed trailing whitespace
- Reordering execution: first updating the chat overlay, then updating the chat history
- Always call
updateChatHistory()
, even if the window is hidden. It's not really needed. new Date()
doesn't require the "now" argument. Unneeded variable time.- Adding the user preference loading to
initChatHistoryFilter
, thus renaming it toinitChatWindow
. - Moving
resizeChatWindow
belowinitChatWindow
- Redistributing
updateChatHistory()
andresizeChatWindow()
so that stuff is called as rarely as possible, as often as needed without introducing checks.
follow-up: 13 comment:10 by , 8 years ago
Mostly things said in IRC yesterday (leaving out things I've changed my mind on):
menu.js
L242-3 Can't that be merged to Engine.GetGUIObjectByName("extendedChat").checked = Engine.ConfigDB_GetValue("user", "chat.session.extended") == "true";
?
L250 // Hide/show the panel
The players messages to enemy's and observers are not shown on filter. On this I don't believe a player is the enemy of itself: messages.js L209-13.
Not all AI ally message are showed due to being "Team" messages cleanup patch in #3102 solves that.
Extended chat just borders the bottom panel at min res: http://imgur.com/a/c09aw
The ceasefire messages (maybe wonder too) are on top of the extended dialog making it look odd: http://imgur.com/a/a57J6
Lobby chat broken because missed file inclusion.
For long player names (AI names) the diplo screen looks broken (it was actually broken already but this patch make it show up): http://imgur.com/a/HWX1C
comment:11 by , 8 years ago
Owner: | changed from | to
---|
comment:13 by , 8 years ago
Keywords: | rfc removed |
---|
Replying to bb:
menu.js L242-3 Can't that be merged to
Engine.GetGUIObjectByName("extendedChat").checked = Engine.ConfigDB_GetValue("user", "chat.session.extended") == "true";
?
Indeed, Press
is only called when the user clicks it. The Press event isn't triggered when the .checked property changes.
The players messages to enemy's and observers are not shown on filter. On this I don't believe a player is the enemy of itself: messages.js L209-13.
Agree, g_Players[Engine.GetPlayerID()].isEnemy[senderID]
is false
for senderID = Engine.GetPlayerID()
afaik (Player
component). However since messages that are not addressed to the player are filtered out beforehand, these logic checks are not needed at all.
The /enemies
bug was a missing msg.guid != "local"
check.
That check was missing in another place (that offered the player a dropdown element of himself).
(The chat system is a bit complex:
- Chat messages sent by the player in singleplayer come with
guid
==local
- Chat messages sent by the player in multiplayer come with
guid
== 16 character hex - We need to check for GUIDs preferably since the playerID test fails for observers (every observer has playerID -1)
- Chat messages sent by the AI or simulation otherwise comes with
player
ID
)
Not all AI ally message are showed due to being "Team" messages cleanup patch in #3102 solves that.
Fixed that part. I can rebase your patch on request, since I didn't commit it entirely.
Extended chat just borders the bottom panel at min res: http://imgur.com/a/c09aw
Fixed by resizing and chaning topOffset
.
The ceasefire messages (maybe wonder too) are on top of the extended dialog making it look odd: http://imgur.com/a/a57J6
Fixed by adding z="90"
.
Lobby chat broken because missed file inclusion.
Fixed by including the forgotton style.
For long player names (AI names) the diplo screen looks broken (it was actually broken already but this patch make it show up): http://imgur.com/a/HWX1C
Yes, not enough space for long botnames has been there before. The vertical centering is the wrong part.
Thanks for your review(s)!
comment:16 by , 7 years ago
Description: | modified (diff) |
---|
comment:18 by , 7 years ago
Description: | modified (diff) |
---|
This feature must be avaible in the GUI as icon.
Filtering logic not implemented yet. Gamesetup chat should be preserved too.