Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#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)

patch_3956.patch (879 bytes ) - added by Pilzschaf 8 years ago.
patch_3956.2.patch (865 bytes ) - added by Pilzschaf 8 years ago.
Modified based on elexis suggestion
patch_3956.3.patch (812 bytes ) - added by Pilzschaf 8 years ago.
patch_3956.4.patch (1.5 KB ) - added by Pilzschaf 8 years ago.
Added myself to the credits
t3956_chataddressee_rough_sketch_v1.patch (1.6 KB ) - added by elexis 8 years ago.
The code should look something like that, it's just a broken rough sketch. Can be finished by anyone.

Download all attachments as: .zip

Change History (15)

comment:1 by Pilzschaf, 8 years ago

Owner: set to Pilzschaf
Status: newassigned

by Pilzschaf, 8 years ago

Attachment: patch_3956.patch added

comment:2 by elexis, 8 years ago

Use .selected = Math.max(0, foo) to simplify

comment:3 by Pilzschaf, 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.

by Pilzschaf, 8 years ago

Attachment: patch_3956.2.patch added

Modified based on elexis suggestion

comment:4 by elexis, 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 Pilzschaf, 8 years ago

Attachment: patch_3956.3.patch added

comment:5 by elexis, 8 years ago

I see you are not included in the credits yet, so add yourself to programming.json (with or wihtout full name)

by Pilzschaf, 8 years ago

Attachment: patch_3956.4.patch added

Added myself to the credits

comment:6 by Stan, 8 years ago

Is that second variable really needed ? It's easier to read but that makes one variable more.

in reply to:  4 comment:7 by elexis, 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 elexis, 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 elexis, 8 years ago

The code should look something like that, it's just a broken rough sketch. Can be finished by anyone.

comment:9 by elexis, 8 years ago

Resolution: fixed
Status: assignedclosed

In 18369:

Don't select "everyone" but the previously selected chat addressee when someone connected or disconnected. Fixes #3956.

comment:10 by elexis, 8 years ago

Keywords: simple removed
Note: See TracTickets for help on using tickets.