Split chat lines into pools and allow the player to filter them #18905
Conversation
|
at the end of a long game i tried to read back at what play time another player left the game. when who said what. i noticed the chat lines did not contain a timestamp. perhaps a separate feature. not important as you may be able to see in replay. |
|
Needs a rebase but I'll wait for #18948 |
dragunoff
force-pushed
the
feature/chat-pools-core
branch
from
c65306c
to
151e5ac
Compare
|
Rebased and updated to work with the new settings logic split. |
Yeah, "UI Feedback" and the pool associated with it is a bit problematic. I had difficulty coming up with a name for that pool. Currently it only includes the "Selected across screen/map" messages and they probably don't belong in the chat at all. There was an idea to move them to a dedicated widget for transient messages (it was discussed in #16355). And perhaps that's a good way to handle them. |
|
Another thing that may be a problem with the "repeatable" lines that I introduce in this PR is the number appended to the end. Because "Selected across map (3)" is a somewhat ambiguous - does that mean that 3 units were selected or that the message was repeated 3 times? 🤔 |
dragunoff
force-pushed
the
feature/chat-pools-core
branch
from
151e5ac
to
b717d3b
Compare
|
Update: I'm trying to get back to this but I should wait for #19360 (or rebase on top of it) because it moves the chat handling functions out of |
|
Needs a rebase now. |
|
Update:
|
dragunoff
force-pushed
the
feature/chat-pools-core
branch
from
b717d3b
to
cb0414a
Compare
dragunoff
force-pushed
the
feature/chat-pools-core
branch
from
cb0414a
to
63e6c3a
Compare
|
Update: Addressed some feedback by @teinarss and shuffled the methods in |
| chatLineToDisplay = new TextNotification( | ||
| chatLine.Pool, | ||
| chatLine.Prefix, | ||
| string.Format("{0} ({1})", chatLine.Text, repetitions + 1), |
There was a problem hiding this comment.
Can now be $"{chatLine.Text} ({repetitions + 1})"
| public readonly Color PrefixColor; | ||
| public readonly string Prefix; | ||
| public readonly string Text; | ||
| public readonly Color TextColor; |
There was a problem hiding this comment.
Pedantic nit (feel free to ignore): we list (color, text, text, color) here compared to (text, text, color, color) in the ctor. Could we reorder these fields to either (text, color, text, color) or (text, text, color, color) for consistency?
|
|
||
| static void AddTextNotification(TextNotificationPool pool, string prefix, string text, Color? prefixColor = null, Color? textColor = null) | ||
| { | ||
| var thePrefixColor = prefixColor ?? chatMessageColor; |
There was a problem hiding this comment.
Style nit: we don't use "the" prefixes anywhere else in the code so these stick out as looking a bit odd.
These two lines are quite short and only used once, so how about we just inline them directly below? That would also save us from evaluating them if the pool is not enabled.
- Add a common class for passing around chat lines - Add wrapper methods for adding chat lines - Combine repeated chat lines in the display widget
dragunoff
force-pushed
the
feature/chat-pools-core
branch
from
63e6c3a
to
153220d
Compare
|
Update: addressed the nits from pchote |
This is #17737 stripped down to the core idea:
This is the only UI exposed to the player - a checkbox in "Display settings":
Display settings
Repeated messages are merged into one and a counter is displayed:
In game chat
Closes #16148
Provides the basis for solving #5775, #6400, #6354, #3278 and #12421. These will come in follow-ups.