#3396 closed enhancement (fixed)
[PATCH] Lobby - Clear chat command / button
Reported by: | elexis | Owned by: | ruiRanger |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 20 |
Component: | Multiplayer lobby | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
It would be nice if we could blank the chat history by typing /clear
or hitting a button.
This should be especially easy to implement if #3241 is commited, as this provides a way to process local commands.
Attachments (3)
Change History (14)
comment:1 by , 8 years ago
Keywords: | simple added |
---|
by , 8 years ago
Attachment: | clearChat.patch added |
---|
follow-up: 3 comment:2 by , 8 years ago
Keywords: | patch added |
---|---|
Milestone: | Backlog → Alpha 20 |
Summary: | Lobby - Clear chat command / button → [PATCH] Lobby - Clear chat command / button |
comment:3 by , 8 years ago
With regards to the code:
{
in a new line after the function header (see wiki:Coding_Conventions)... = []
and...=""
would be simpler
Replying to stanislas69:
Can you replace "Blank the chat history" by "Clear the chat history" ?
Comments are useful if they add information that is not already contained in the function name.
unify them to use the same code.
As we currently don't have a suitable file that contains common chat functions, we should do this later in #2640
It would be nice to have a hotkey (maybe ctrl+l) to clear the chat without typing.
by , 8 years ago
Attachment: | clearChat_v2.patch added |
---|
comment:4 by , 8 years ago
Keywords: | review added |
---|---|
Owner: | set to |
Status: | new → assigned |
Changed the code as suggested by elexis. Added the ctrl+L hotkey.
follow-up: 6 comment:5 by , 8 years ago
I would argue against a hotkey, as that would be inconsistent with the rest of the lobby commands.
follow-up: 10 comment:6 by , 8 years ago
The patch works and it could be committed (without the hotkey part).
The only thing that should be updated is that you define the exact same function in two places (clearChatMessages in gamesetup.js and lobby.js). Duplicate code must be avoided. you can define the function in functions_global_object.js
or functions_utility.js
(and would be moved later to a new functions_chat.js
).
With regards to hotkeys, we could exclude them for this ticket and maybe create a specific ticket for menu hotkeys (if there isn't one already).
by , 8 years ago
Attachment: | clearChat_v3.patch added |
---|
clear Chat without hotkeys and clearChatMessages function not duplicated
comment:9 by , 8 years ago
Keywords: | simple review removed |
---|
Thanks for the patch.
As r17464 renames chatMessages
to g_ChatMessages
, your command doesn't throw errors when used for the session. The timers needed to be cleared too in that case. I committed this separately as it was not in the scope of this ticket.
comment:10 by , 5 years ago
Description: | modified (diff) |
---|
Replying to elexis:
Duplicate code must be avoided. you can define the function in
functions_global_object.js
It should be avoided, but not at the cost of refering to page-specific globals in gui/common/
.
The issue is related to #2640 which aims at unifying chat parsing, but this never really works out conceptually, since the three chat pages have different features, different variables, different presentation. If deduplicating / unifying the chat, it would probably be cleaner to introduce a prototype about ChatHandling that pleases features of all three pages without becoming odd, without refering to page-specific globals or names (but receiving those as arguments) (#5387).
(Also the patch has turned out to be useful in some situations, thanks again)
Hey thanks for the patch.
Can you replace "Blank the chat history" by "Clear the chat history" ? also document functions in both places, or better,unify them to use the same code.
Next time add the review and patch keywords and edit the title. Please read http://trac.wildfiregames.com/wiki/SubmittingPatches
If you plan to continue working on this, please modify ticket, and click accept so it's assigned to you.