Page MenuHomeWildfire Games

Display lobby bots differently in the lobby
AcceptedPublic

Authored by elexis on Nov 2 2018, 2:21 AM.

Details

Reviewers
bb
Summary

Since it was proposed that bots could keep administrative access because it makes them appear differently in the UI, I'm uploading a patch that accomplishes that without the bots having administrative access, refs D1659.
The game requires the bots to be known, the names of the bots is in the config file, so we can detect them directly and show any kind of UI style we want.

Test Plan

Not much to check really. We can observe some code being messy.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6424
Build 10642: Vulcan BuildJenkins
Build 10641: arc lint + arc unit

Event Timeline

elexis created this revision.Nov 2 2018, 2:21 AM
elexis edited the summary of this revision. (Show Details)Nov 2 2018, 2:22 AM
elexis edited the summary of this revision. (Show Details)
Vulcan added a subscriber: Vulcan.Nov 2 2018, 2:30 AM

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

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  42|  42|  * The playerlist will be assembled using these values.
|  43|  43|  */
|  44|  44| var g_PlayerStatuses = {
|  45|    |-	"available": { "color": "0 219 0",     "status": translate("Online") },
|    |  45|+	"available": { "color": "0 219 0", "status": translate("Online") },
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'away'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  43|  43|  */
|  44|  44| var g_PlayerStatuses = {
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  46|+	"away": { "color": "229 76 13",   "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  43|  43|  */
|  44|  44| var g_PlayerStatuses = {
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  46|+	"away":      { "color": "229 76 13", "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'playing'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  44|  44| var g_PlayerStatuses = {
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  47|+	"playing": { "color": "200 0 0",     "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  50|  50| };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  44|  44| var g_PlayerStatuses = {
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  47|+	"playing":   { "color": "200 0 0", "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  50|  50| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'offline'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  48|+	"offline": { "color": "0 0 0",       "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  50|  50| };
|  51|  51| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  45|  45| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  48|+	"offline":   { "color": "0 0 0", "status": translate("Offline") },
|  49|  49| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  50|  50| };
|  51|  51| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'unknown'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|  46|  46| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  47|  47| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  48|  48| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  49|    |-	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    |  49|+	"unknown": { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  50|  50| };
|  51|  51| 
|  52|  52| var g_LobbyRoles = {
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 236| 236| 					me ?
| 237| 237| 						translate("You have been muted.") :
| 238| 238| 						translate("%(nick)s has been muted.") :
| 239|    |-				newrole == "moderator" ?
|    | 239|+					newrole == "moderator" ?
| 240| 240| 					me ?
| 241| 241| 						translate("You are now a moderator.") :
| 242| 242| 						translate("%(nick)s is now a moderator.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 237| 237| 						translate("You have been muted.") :
| 238| 238| 						translate("%(nick)s has been muted.") :
| 239| 239| 				newrole == "moderator" ?
| 240|    |-					me ?
|    | 240|+						me ?
| 241| 241| 						translate("You are now a moderator.") :
| 242| 242| 						translate("%(nick)s is now a moderator.") :
| 243| 243| 				msg.oldrole == "visitor" ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 238| 238| 						translate("%(nick)s has been muted.") :
| 239| 239| 				newrole == "moderator" ?
| 240| 240| 					me ?
| 241|    |-						translate("You are now a moderator.") :
|    | 241|+							translate("You are now a moderator.") :
| 242| 242| 						translate("%(nick)s is now a moderator.") :
| 243| 243| 				msg.oldrole == "visitor" ?
| 244| 244| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 239| 239| 				newrole == "moderator" ?
| 240| 240| 					me ?
| 241| 241| 						translate("You are now a moderator.") :
| 242|    |-						translate("%(nick)s is now a moderator.") :
|    | 242|+							translate("%(nick)s is now a moderator.") :
| 243| 243| 				msg.oldrole == "visitor" ?
| 244| 244| 					me ?
| 245| 245| 						translate("You have been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 240| 240| 					me ?
| 241| 241| 						translate("You are now a moderator.") :
| 242| 242| 						translate("%(nick)s is now a moderator.") :
| 243|    |-				msg.oldrole == "visitor" ?
|    | 243|+						msg.oldrole == "visitor" ?
| 244| 244| 					me ?
| 245| 245| 						translate("You have been unmuted.") :
| 246| 246| 						translate("%(nick)s has been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 241| 241| 						translate("You are now a moderator.") :
| 242| 242| 						translate("%(nick)s is now a moderator.") :
| 243| 243| 				msg.oldrole == "visitor" ?
| 244|    |-					me ?
|    | 244|+							me ?
| 245| 245| 						translate("You have been unmuted.") :
| 246| 246| 						translate("%(nick)s has been unmuted.") :
| 247| 247| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 242| 242| 						translate("%(nick)s is now a moderator.") :
| 243| 243| 				msg.oldrole == "visitor" ?
| 244| 244| 					me ?
| 245|    |-						translate("You have been unmuted.") :
|    | 245|+								translate("You have been unmuted.") :
| 246| 246| 						translate("%(nick)s has been unmuted.") :
| 247| 247| 					me ?
| 248| 248| 						translate("You are not a moderator anymore.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 243| 243| 				msg.oldrole == "visitor" ?
| 244| 244| 					me ?
| 245| 245| 						translate("You have been unmuted.") :
| 246|    |-						translate("%(nick)s has been unmuted.") :
|    | 246|+								translate("%(nick)s has been unmuted.") :
| 247| 247| 					me ?
| 248| 248| 						translate("You are not a moderator anymore.") :
| 249| 249| 						translate("%(nick)s is not a moderator anymore.");
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 244| 244| 					me ?
| 245| 245| 						translate("You have been unmuted.") :
| 246| 246| 						translate("%(nick)s has been unmuted.") :
| 247|    |-					me ?
|    | 247|+							me ?
| 248| 248| 						translate("You are not a moderator anymore.") :
| 249| 249| 						translate("%(nick)s is not a moderator anymore.");
| 250| 250| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 245| 245| 						translate("You have been unmuted.") :
| 246| 246| 						translate("%(nick)s has been unmuted.") :
| 247| 247| 					me ?
| 248|    |-						translate("You are not a moderator anymore.") :
|    | 248|+								translate("You are not a moderator anymore.") :
| 249| 249| 						translate("%(nick)s is not a moderator anymore.");
| 250| 250| 
| 251| 251| 			addChatMessage({
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/lobby/lobby.js
| 246| 246| 						translate("%(nick)s has been unmuted.") :
| 247| 247| 					me ?
| 248| 248| 						translate("You are not a moderator anymore.") :
| 249|    |-						translate("%(nick)s is not a moderator anymore.");
|    | 249|+								translate("%(nick)s is not a moderator anymore.");
| 250| 250| 
| 251| 251| 			addChatMessage({
| 252| 252| 				"text": "/special " + sprintf(txt, { "nick": msg.nick }),

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

binaries/data/mods/public/gui/lobby/lobby.js
|1325| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

binaries/data/mods/public/gui/lobby/lobby.js
| 746| »   »   case·'name':
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'default'.

Link to build: https://jenkins.wildfiregames.com/job/differential/762/display/redirect

lyv added a subscriber: lyv.Nov 2 2018, 5:29 AM

Maybe the list should be split into parts like a normal client. Moderators, bots, players, muted players.

elexis added inline comments.Nov 3 2018, 10:21 AM
source/lobby/XmppClient.cpp
873

The pointer is unset in anonymous rooms, so there safeguards are missing.
https://camaya.net/api/gloox/structgloox_1_1MUCRoomParticipant.html#a4129e754d2f385ceeecae2857282c999

I suppose to support non-anonymous rooms for this feature, the bot could be queried somehow what his /nick is.
If there is an XMPP way supported by gloox to get the nick from a realJID in an anonymous room.
If the bot would need to be patched for that, that may be meh.

bb accepted this revision.Apr 21 2019, 9:39 PM
bb added a subscriber: bb.

Patch still applies correctly, reads correct, front doesn't fall => accept

source/lobby/XmppClient.h
171

if we want it to look like an object, one could add the spaces

This revision is now accepted and ready to land.Apr 21 2019, 9:39 PM

@lyv @Dunedan @user1 @defc0n this pretty nice what do you think?

In D1663#204741, @Stan wrote:

@lyv @Dunedan @user1 @defc0n this pretty nice what do you think?

As long as this doesn't need any changes to the bots and keeps working with anonymous and non-anonymous MUC rooms, I have no objections. The latter one doesn't seem to be the case though, so I'm not sure what's the best way to proceed here.