#3956 closed defect (fixed)
[PATCH] Chat addressee dropdown selects "Everyone" if someone joins/leaves
Reported by: | elexis | Owned by: | Pilzschaf |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 21 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description
If the chat window is opened and someone rejoins or leaves, the dropdown selects "Everyone" as addressee (even if team-chat was selected before).
This is because the dropdown is updated and a new element being added or removed in that case. To solve it, the previously selected index must be remembered and restored.
Bug reported by Hannibal Barca and moe
Attachments (5)
Change History (15)
comment:1 by , 8 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
by , 8 years ago
Attachment: | patch_3956.patch added |
---|
comment:2 by , 8 years ago
comment:3 by , 8 years ago
Keywords: | review patch added |
---|---|
Summary: | Chat addressee dropdown selects "Everyone" if someone joins/leaves → [PATCH] Chat addressee dropdown selects "Everyone" if someone joins/leaves |
The selected addressee now stays the same when a player leaves or reconnects. If the player who is leaving is selected, the addressee is reset to Everybody.
follow-up: 7 comment:4 by , 8 years ago
- You can also merge the last two code-lines.
- if the selected player has left
I'm not sure if it's a good choice to select everyone if it was supposed to be a private-message, but probably the lesser evil in comparison to closing the dialog or selecting someone else.
We could also only update the dropdowns if the dialog is opened again to avoid that issue. This way the invalid addresse will stay selected and the message won't arrive. As that might also be a bit misleading, we might want want to just use your patch, unless someone has a strong opinion against it or a better proposal.
by , 8 years ago
Attachment: | patch_3956.3.patch added |
---|
comment:5 by , 8 years ago
I see you are not included in the credits yet, so add yourself to programming.json
(with or wihtout full name)
comment:6 by , 8 years ago
Is that second variable really needed ? It's easier to read but that makes one variable more.
comment:7 by , 8 years ago
Replying to stanislas69:
Is that second variable really needed ? It's easier to read but that makes one variable more.
There is only one variable introduced. It is needed as chatAddressee.list_data is overwritten between those two lines.
Replying to elexis:
unless someone has a strong opinion against it or a better proposal
Got a probably better idea:
If the previously selected addressee is a player (string starts with /msg) and there is no playername in g_PlayerAssignments
with that name, you could add a new element with label
"Username (OFFLINE)" and cmd "invalid" and select that instead of "Everyone".
Then when submitChatInput
is called and "invalid" is selected, abort.
Another way to have dropdown elements disapear is if a player is being defeated or wins. You can see the element changing to "Everyone" quickly, but then a message box pops up hiding the chat input, so that case can be ignored for the time being.
A third way to have a disappearing element is using the developer overlay and switching the perspective to someone else. That case can be ignored too (as long as it's reset to Everybody), as developers should know what theyre doing.
(Those are all cases, which can be found by searching for the calls to updateChatAddressees
.)
For the Offline hint, you can use the existing translation:
playerName = sprintf(translate("\\[OFFLINE] %(player)s"), { "player": playerName });
in selection_details.js
.
comment:8 by , 8 years ago
Keywords: | review removed |
---|
Adding the placeholder dropdown item should be done as it is simple enough and fixes the issue for the edgecase where a selected player goes offline. The item can be added to the addressees
array right in line 545. Notice chatAddressee.list_data[chatAddressee.selected]
contains /msg playername
if a player was selected previously. If g_PlayerAssignments
doesn't contain such a name, the player is offline and the item has to be added with that name + offline hint.
by , 8 years ago
Attachment: | t3956_chataddressee_rough_sketch_v1.patch added |
---|
The code should look something like that, it's just a broken rough sketch. Can be finished by anyone.
comment:10 by , 8 years ago
Keywords: | simple removed |
---|
Use .selected = Math.max(0, foo) to simplify