Lobby rejoin independent of rating
ClosedPublic
Actions

Authored by elexis on Jun 22 2017, 2:33 PM.

Details

Reviewers

Imarok

Commits

rP19828: Allow lobby players to rejoin games, even if the rating changed meanwhile or if…

Trac Tickets

#3686

Summary

As reported in #3686, the locally loaded rating of the current lobby user is not always in sync with the rating seen by other players.

This can occur if a player (for an unknown reason) didn't receive the rating list (yet?) from the lobby bot when joining the lobby.
It can also occur when having played a rated game, returned to the lobby and then having the rating adapted.
There might be more cases.

Hence the rejoin won't work for these users anymore and the player might rejoin as a late observer or not being able to rejoin at all if late observers are disallowed.
Both cases are very bad as they can end the game unintentionally (and players might not believe each other that this was a bug).

Not saving the rating in the nickname is an idea, but it will come with complications (changing every place that displays the playername in the gamesetup, session, summary and replaymenu GUI).

Test Plan

You can mock a wrong rating by pressing F9 and typing g_UserRating = -99;.
Start a game in one window, join the game in a second window, start the game.
Quit the second window, open it again, set a wrong rating and try to rejoin.

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.Jun 22 2017, 2:33 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1608/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  41|  41|  * The playerlist will be assembled using these values.
|  42|  42|  */
|  43|  43| const g_PlayerStatuses = {
|  44|    |-	"available": { "color": "0 219 0",     "status": translate("Online") },
|    |  44|+	"available": { "color": "0 219 0", "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  42|  42|  */
|  43|  43| const g_PlayerStatuses = {
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  45|+	"away":      { "color": "229 76 13", "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  48|  48| 	"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/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  43|  43| const g_PlayerStatuses = {
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  46|+	"playing":   { "color": "200 0 0", "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  48|  48| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  49|  49| };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  47|+	"offline":   { "color": "0 0 0", "status": translate("Offline") },
|  48|  48| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  49|  49| };
|  50|  50| 

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

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

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

binaries/data/mods/public/gui/lobby/lobby.js
| 456| »   if·(mapSizeFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/lobby/lobby.js
| 460| »   if·(playersNumberFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/lobby/lobby.js
| 464| »   if·(mapTypeFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

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

binaries/data/mods/public/gui/lobby/lobby.js
|1160| »   »   if·(msg.type·==·"chat"·&&·Engine.LobbyGetMucMessageCount()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/226/ for more details.

Harbormaster completed remote builds in B2376: Diff 2660.Jun 22 2017, 3:22 PM

elexis retitled this revision from Lobby rejoin workaround to Lobby rejoin independent of rating.Jun 23 2017, 1:05 AM

Imarok added a subscriber: Imarok.Jun 24 2017, 5:10 PM

Imarok added inline comments.

binaries/data/mods/public/gui/lobby/lobby.js
1046 ↗	(On Diff #2660)	shouldn't this be `getRejoinRating(game) ? g_Username + " (" + getRejoinRating(game) + ")" : g_Username;`? So that a user that joined a game without rating and is now trying to rejoin with rating, still rejoins with no rating.

Rephrase comment. Fix ternary.

Harbormaster completed remote builds in B2399: Diff 2689.Jun 25 2017, 6:28 PM

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  41|  41|  * The playerlist will be assembled using these values.
|  42|  42|  */
|  43|  43| const g_PlayerStatuses = {
|  44|    |-	"available": { "color": "0 219 0",     "status": translate("Online") },
|    |  44|+	"available": { "color": "0 219 0", "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  42|  42|  */
|  43|  43| const g_PlayerStatuses = {
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  45|+	"away":      { "color": "229 76 13", "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  48|  48| 	"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/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  43|  43| const g_PlayerStatuses = {
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  46|+	"playing":   { "color": "200 0 0", "status": translate("Busy") },
|  47|  47| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  48|  48| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  49|  49| };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/lobby/lobby.js
|  44|  44| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  45|  45| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  46|  46| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  47|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  47|+	"offline":   { "color": "0 0 0", "status": translate("Offline") },
|  48|  48| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  49|  49| };
|  50|  50| 

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

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

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

binaries/data/mods/public/gui/lobby/lobby.js
| 456| »   if·(mapSizeFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/lobby/lobby.js
| 460| »   if·(playersNumberFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/lobby/lobby.js
| 464| »   if·(mapTypeFilter.selected·!=·0·&&
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

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

binaries/data/mods/public/gui/lobby/lobby.js
|1159| »   »   if·(msg.type·==·"chat"·&&·Engine.LobbyGetMucMessageCount()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/236/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1623/ for more details.

Harbormaster completed remote builds in B2399: Diff 2689.Jun 25 2017, 9:46 PM

Works

This revision is now accepted and ready to land.Jun 25 2017, 10:49 PM

Closed by commit rP19828: Allow lobby players to rejoin games, even if the rating changed meanwhile or if… (authored by elexis). · Explain WhyJun 25 2017, 10:56 PM

This revision was automatically updated to reflect the committed changes.