Page MenuHomeWildfire Games
Differential D2266

Fix wrong "player is not a moderator anymore" message
ClosedPublic
Actions

Authored by elexis on Sep 6 2019, 7:57 AM.

Details

Reviewers
None
Commits
rP22859: Fix wrong "player is not a moderator anymore" lobby chat message following…
Trac Tickets
#4877
Summary

Since the initial implementation for xmpp muc room role changes, there was a bug where some players would see the "player is not a moderator" chat message when in fact the player was only muted or unmuted.

From #4877:

The reason must be that the XmppClient compares the old user role against the the current user role, not against the new user role at that time of the presence change.

Test Plan

Make sure to reproduce the bug, since I wrote the patch on theory.
This patch was implemented using rP22856 / D2264.

Reproduce:

1. join lobby with player and mod account
2. player is muted
3. player hosts a game
4. player becomes unmuted during the game
5. player returns to the lobby

result:
	1. You are not a moderator anymore.
	2. You have been unmuted.

expected result:
	1. You have been muted.
	2. You have been unmuted.


Step 2 and 3 are interchangeable.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Sep 6 2019, 7:57 AM
elexis updated the Trac tickets for this revision.Sep 6 2019, 7:58 AM
Vulcan added a comment.Sep 6 2019, 7:59 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/85/display/redirect

Vulcan added a comment.Sep 6 2019, 8:02 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  47|  47|  * The playerlist will be assembled using these values.
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|    |-	"available": { "color": "0 219 0",     "status": translate("Online") },
|    |  50|+	"available": { "color": "0 219 0", "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'away'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  51|+	"away": { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  51|+	"away":      { "color": "229 76 13", "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'playing'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  52|+	"playing": { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  52|+	"playing":   { "color": "200 0 0", "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'offline'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  53|+	"offline": { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  53|+	"offline":   { "color": "0 0 0", "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'unknown'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|    |-	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    |  54|+	"unknown": { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|  57|  57| var g_RoleNames = {
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 210| 210| 					me ?
| 211| 211| 						translate("You have been muted.") :
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213|    |-				msg.newrole == "moderator" ?
|    | 213|+					msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 211| 211| 						translate("You have been muted.") :
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213| 213| 				msg.newrole == "moderator" ?
| 214|    |-					me ?
|    | 214|+						me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213| 213| 				msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215|    |-						translate("You are now a moderator.") :
|    | 215|+							translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 213| 213| 				msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216|    |-						translate("%(nick)s is now a moderator.") :
|    | 216|+							translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217|    |-				msg.oldrole == "visitor" ?
|    | 217|+						msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218|    |-					me ?
|    | 218|+							me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219|    |-						translate("You have been unmuted.") :
|    | 219|+								translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220|    |-						translate("%(nick)s has been unmuted.") :
|    | 220|+								translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221|    |-					me ?
|    | 221|+							me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222|    |-						translate("You are not a moderator anymore.") :
|    | 222|+								translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
| 225| 225| 			addChatMessage({
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223|    |-						translate("%(nick)s is not a moderator anymore.");
|    | 223|+								translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
| 225| 225| 			addChatMessage({
| 226| 226| 				"text": "/special " + sprintf(txt, { "nick": msg.nick }),

binaries/data/mods/public/gui/lobby/lobby.js
|1012| »   »   switch·(sortBy)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/594/display/redirect

Harbormaster completed remote builds in B9243: Diff 9646.Sep 6 2019, 8:02 AM
elexis edited the test plan for this revision. (Show Details)Sep 6 2019, 6:45 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 6 2019, 6:56 PM
Closed by commit rP22859: Fix wrong "player is not a moderator anymore" lobby chat message following… (authored by elexis). · Explain Why
This revision was automatically updated to reflect the committed changes.
elexis added a commit: rP22859: Fix wrong "player is not a moderator anymore" lobby chat message following….
Owners added a subscriber: Restricted Owners Package.Sep 6 2019, 6:56 PM

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
gui/
lobby/
5 lines
source/
lobby/
2 lines

Diff 9648

View Options

ps/trunk/binaries/data/mods/public/gui/lobby/lobby.js

Loading...
View Options

ps/trunk/source/lobby/XmppClient.cpp

Loading...
Wildfire Games · Phabricator