Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#3244 closed enhancement (fixed)

[PATCH] Lobby - keep the game and player selection

Reported by: elexis Owned by: elexis
Priority: Nice to Have Milestone: Alpha 20
Component: Multiplayer lobby Keywords: patch
Cc: Patch:

Description (last modified by elexis)

If you are waiting for a particular multiplayer lobby game to finish, then you select the game and take a look every now and then.

However if the gamelist updates and the number of games (or order of the list) has changed, then a different item will be selected (the list index stays the same), while the details of the previously selected game are still displayed.

If the game goes offline and you don't select another game, then the details of that offline game are still displayed.

This means each time when you want to check if that game is still being played, you have to look through the whole list again in order to see if they are still playing. If the same item will be selected again after every update, then you only have to look at the one element selected.

This can be reproduced by having one client connected to the lobby, two more clients hosting a game and one of those two stops the game in order to change the selected gamelist item. Only reproducible if there is another computer hosting a second match, since you can only host one match per IP. It can be tested with alpha 18 then, since there are almost always multiple games running.

Fix it by reselecting the previously selected game, after updating the game list. If the game has gone offline, then no list item should be selected.


The same is true for the playerlist. If the playerlist is updated, the selection index and the player details will stay the same. This means if a selected player goes offline, his/her rating details are still being displayed, which might leave the impression that the player is still online.

Fix it by reselecting the previously selected player, after updating the player list. If the player has gone offline, then no player should be selected and the rating information should be cleared again.

The fact that player details are displayed after the selected player goes offline might be considered a feature. But you can still click the User Profile Lookup button, enter the playername and see the details. Thats why it would be better to clear the information after the player left.

Attachments (9)

t3244_keep_player_selection_r16665.patch (2.9 KB ) - added by elexis 9 years ago.
Fixes the second part of the bug. This patch can be tested with svn and two lobby accounts. If a player goes online or offline, the previously selected player will be selected again. If a player goes offline, then no player will be selected and the rating information will be hidden automatically. If the player comes online again, and no different player was selected meanwhile, then that previously selected player will be selected again (feature). Test the patch by selecting players with the first connected lobby account and change the playerlist by rejoining the lobby with the second account. To make sure that it works correctly, try selecting the first, the last, the rejoined player and another player from the list. If you change the lobby.js file, make sure to reconnect to the lobby. Either the hotloading doesn't work properly, or it is #3171.
t3244_keep_gamelist_selection_r16665.patch (2.5 KB ) - added by elexis 9 years ago.
Fixes the first part of this ticket. You can test it briefly in svn by hosting a match with only 1 player slot, another client in the lobby and then cliking the show-full-games trigger.
t3244_keep_gamelist_selection_a18.patch (2.3 KB ) - added by elexis 9 years ago.
Same patch, but for a18.
t3244_keep_player_and_gamelist_selection_a18.patch (4.7 KB ) - added by elexis 9 years ago.
Both patches combined to a single file and made compatible to a18.
t3244_keep_player_and_gamelist_selection_v2_a18.patch (5.2 KB ) - added by elexis 9 years ago.
Patch to test with a18 (a lobby with more than 0-1 games online).
t3244_keep_player_and_gamelist_selection_v2_r16728.patch (5.2 KB ) - added by elexis 9 years ago.
Same patch for svn. Diff the patches to see that they are equal.
t3244_keep_player_and_gamelist_selection_v3_16789.patch (5.1 KB ) - added by elexis 9 years ago.
Optimized a bit (Doesn't filter all games again to create g_filteredGameList). Applicable to most recent svn version.
t3244_keep_player_and_gamelist_selection_v4_16839.patch (4.7 KB ) - added by elexis 9 years ago.
Changed g_filteredGameList so that it only stores IP addresses (to save some memory). Couldn't find any trailing whitespace. Only one local variable var selectedIndex = -1; was added. Used var instead of let because it is used everywhere else in that code and changing every occurance without thorough testing might be a bad idea.
t3244_keep_player_and_gamelist_selection_v5_16839.patch (12.3 KB ) - added by elexis 9 years ago.
Removes trailing whitespace. Changes some let to var.

Download all attachments as: .zip

Change History (18)

comment:1 by elexis, 9 years ago

There is another side effect of this problem.

If you want to join a server, but the server selection changes before you click the button, then you won't join the server you intended to join.

comment:2 by elexis, 9 years ago

Description: modified (diff)

comment:3 by elexis, 9 years ago

Keywords: patch review added; simple removed
Milestone: BacklogAlpha 19
Summary: Lobby - keep the game and player selection[PATCH] Lobby - keep the game and player selection

by elexis, 9 years ago

Fixes the second part of the bug. This patch can be tested with svn and two lobby accounts. If a player goes online or offline, the previously selected player will be selected again. If a player goes offline, then no player will be selected and the rating information will be hidden automatically. If the player comes online again, and no different player was selected meanwhile, then that previously selected player will be selected again (feature). Test the patch by selecting players with the first connected lobby account and change the playerlist by rejoining the lobby with the second account. To make sure that it works correctly, try selecting the first, the last, the rejoined player and another player from the list. If you change the lobby.js file, make sure to reconnect to the lobby. Either the hotloading doesn't work properly, or it is #3171.

by elexis, 9 years ago

Fixes the first part of this ticket. You can test it briefly in svn by hosting a match with only 1 player slot, another client in the lobby and then cliking the show-full-games trigger.

by elexis, 9 years ago

Same patch, but for a18.

by elexis, 9 years ago

Both patches combined to a single file and made compatible to a18.

comment:4 by elexis, 9 years ago

While testing with a18 I encountered the following error:

ERROR: JavaScript error: gui/lobby/lobby.js line 401
TypeError: g_GameList[gamesBox.selected] is undefined
  updateGameList@gui/lobby/lobby.js:401:3
  onTick@gui/lobby/lobby.js:748:6
  __eventhandler264 (tick)@lobbyWind

Some relevant code:

 	var gamesBox = Engine.GetGUIObjectByName("gamesBox");
 	var gameList = Engine.GetGameList();
...

if (gamesBox.selected > -1)
	g_selectedGameIP = g_GameList[gamesBox.selected].ip;

So we know that g_GameList[gamesBox.selected] is undefined and gamesBox.selected > -1. This means that gamesBox has more elements than g_GameList. This must mean that a lobby game has gone offline.

The mechanism can stay the same, but in order to get the correct list, we need to save the index before refreshing the list, i.e. before those lines in updateGameList():

	var gameList = Engine.GetGameList();

	// Store the game whole game list data so that we can access it later
	// to update the game info panel.
	g_GameList = gameList;

i.e. in the beginning of the function. D'oh!

comment:5 by elexis, 9 years ago

Hosting a game with a second account in a18, then selecting the last element in the gamelist and going offline with the game allowed me to reproduce the error.

There was another error in the code above. g_GameList was used for saving the previously selected game:

g_selectedGameIP = g_GameList[gamesBox.selected].ip;

However gamesBox contains the filtered list, while g_GameList contains all games. This resulted in a different game being selected after selecting a game with a high index and then clicking the show-full-games trigger twice.

The playerindex is saved correctly. First of all it uses the index of the same list object: playersBox.list[playersBox.selected] and secondly it saves the index before the playerlist data changes (in the beginning of the function updatePlayerList).

The following patch solves all those issues.

by elexis, 9 years ago

Patch to test with a18 (a lobby with more than 0-1 games online).

by elexis, 9 years ago

Same patch for svn. Diff the patches to see that they are equal.

by elexis, 9 years ago

Optimized a bit (Doesn't filter all games again to create g_filteredGameList). Applicable to most recent svn version.

comment:6 by Itms, 9 years ago

  • lots of trailing spaces
  • var -> let at multiple places
  • you're redefining g_filteredGameList on line 473, I don't know if that affects the behavior but just remove that var

Apart from that I don't see any problem :)

Last edited 9 years ago by Itms (previous) (diff)

by elexis, 9 years ago

Changed g_filteredGameList so that it only stores IP addresses (to save some memory). Couldn't find any trailing whitespace. Only one local variable var selectedIndex = -1; was added. Used var instead of let because it is used everywhere else in that code and changing every occurance without thorough testing might be a bad idea.

by elexis, 9 years ago

Removes trailing whitespace. Changes some let to var.

comment:7 by elexis, 9 years ago

Milestone: Alpha 19Alpha 20

comment:8 by elexis, 8 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 17383:

Restore player- and game-selection when updating the lists. Fixes #3244.
The previous selection is saved globally so that we don't lose it in case of no element being selected.
Also remove an unused variable.

comment:9 by elexis, 8 years ago

Keywords: review removed
Type: defectenhancement
Note: See TracTickets for help on using tickets.