[PATCH] "Show Full Games" option hides games that are not full

[elexis]

The "Show Full Games" button in the game lobby hides all games where the number of clients (not players) is greater or equal to the number of player slots. It is enabled by default.

This means if one player spectates (or is not assigned because of other reasons), the server will be hidden although there are open player slots (if observer-count + playercount = slot count).

As a result the server will usually never be filled.

The only workaround for a18 is then that the host adds one more player slot temporarily, so that the game will not be listed as full.

Quick Fix: Disable the option (the way it was on a17) - then full games are listed too by default.

Better Fix: List games only as full if the number of assigned players is equal to the number of player slots.

[leper]

No to the quick fix, users joining full games is annoying.

[Stan]

Looking at the lobby.js code, line 152 is responsible for it :

   if (!showFullFilter.checked && game.tnbp <= game.nbp)
       return true;

Just removing the number of observers should solve the problem.

[Imarok]

Should game.nbp just be number of players without observers or should there be some new data containing the number of observers transmitted?

[elexis]

Since we want more information for lobby matches (#3476), better send two lists. One for the players and one for the observers. Instead of sending the number. If needed i can lookup my old broken WIP patch for that ticket.

[Imarok]

Show only full games as full. Display the connected players arranged according their team in the lobby window. Display observers in the lobby window. refs #3476

[elexis]

See comments in irc (​http://irclogs.wildfiregames.com/2016-06-14-QuakeNet-%230ad-dev.log)

[Imarok]

Fixed comments from irc

[Imarok]

updated year in the source files

[Imarok]

removed the commented out code (thx leper)

[elexis]

Broken due to using CSV and assuming players not having commas in their names. That might be true for lobby registered nicknames, but not people who join by IP. The code doesn't throw errors, but it still displays broken stuff in the lobby status.

At least use JSON.stringify and escapeText / unescapeText if you don't have time to check how to add actual XML tags in the XmppClient.cpp and glooxwrapper.cpp. (It should still be considered an hack as there should be no technical need to mix JSON and XML.)

[Imarok]

Use JSON.stringify and encodeURI/decodeURI (escape is deprecated) to pass the player and observer list

[Imarok]

use escapeText instead of encodeURI.

[Imarok]

Move duplicating formatting into common. Translate ,

[elexis]

Should use the strings from formatPlayerInfo introduced in r18440, or even better that function itself, especially for #3476.

[Imarok]

preparation patch defining g_GameAttributes for session.js

[elexis]

In 18452:

Cache gameAttributes in the session. Patch by Imarok, refs #3143, #3263.

[Imarok]

The patch should be finished. But when testing my game wasn't even displayed in the lobbys game list...

[Imarok]

Refreshes the "players" too when executing changeGameState

[Imarok]

Fixes #3143 and #3476 besides the gamestart time

[Imarok]

only remove the lengt limitation of escapeText for some parts

[elexis]

• Some things mentioned in IRC
• formatClients is ambiguous, it should be clear that this is used for the lobby report
• The JSON data sent via network would become smaller if it would use the format { "observers": ["name1", "name2", ...], "1": ["player1", ...] , ...}. The code might become ugly when still reusing that format function so prepare mentally ;)
• //if client is no observer not needed as that fact emerges from the else part
• JSON.parse(JSON.stringify really? Use clone or deepcopy if you have to, but it would be likely cleaner to create a new object / array, perhaps with map if it doesn't become too entangled
• sendLobbyReportUpdate sounds like that game report for ratings that sends half the simulation state to the server, while the functions you change have something with "gamestate" or "stanza" in it. Perhaps sendChangeStateGame?

[Imarok]

Made the changes proposed by elexis

[Imarok]

Move the strinigfication and escapement into the new format changing functions

[Imarok]

use whitelist in sendLobbyStanzaUpdate and changed the comment of the format function

[elexis]

In 18510:

Fix autostart replays following r18452 by grabbing g_GameAttributes from the simulation instead of the GUI. Patch by Imarok, refs #3143.
Don't remove attribs from the init attributes as that is still used by the loading screen to show the mapname.

[elexis]

Incomplete review:

Issues:

• Typo: "AIDiff": pData.AI should be "AIDiff": pData.AIDiff
• Lots of unneeded packet load. network data should be as small as possible, as great as needed.
• nbp doesn't need to be sended anymore at all, since you already distinguish observers from players in the list maxnbp is only needed in the gamesetup now, but we likely won't get away with only optionally sending it

Code style fun:

• Duplicate check if (g_IsController && Engine.HasXmppClient()) before every sendLobbyStanzaUpdate
• The defeat/won case of g_NotificationsTypes should be used, rather than the formatDefeatMessage
• The patch adds a call to the defeat case, but not the won case. So if the "won" message is sent last, it wouldn't show up in the lobby.
• clone isn't needed at all since the calls to that function already deliver a copy
• Adding Object.freeze() to ensure it can't be changed by stupid mistakes (also briefly discussed with sanderd17 on irc yesterday)
• renaming player and playerData to pData, since they are an item of g_GameAttributes.settings.PlayerData
• renaming originalplayerDataArray to playerData since it was copied from g_GameAttributes.settings.PlayerData
• JS peculiarity: the for...in returns strings as keys, not numbers, so that one function doesn't invert the other (thereby also introduces 2 bytes per player of useless traffic ), therefore pData.Team = +team;
• removing unneeded comment // Update lobby gamestatus

session/utility_functions.js:

• one wrong space before function sendLobbyStanzaUpdate()
• renaming sendLobbyStanzaUpdate to sendLobbyPlayerlistUpdate
• numberOfPlayers and playerStates are declared long before its first usage
• Adding a comment why that map is done in the first place
• Why are there 3 loops over the player index? they can all be merged
• Caching team in a variable as its reused often there
• playerStates[player.name] = { "State": player.state -> unneeded package load
• Offline -> unneeded package load if its false
• State and Offline is looped twice`, the reader doesn't have time to read that code twice and find the diff
• First you copy the Team, then you remove it in some circumstance. So why not just copy it if you need it only.

gamesetup.js

• Renaming numberOfPlayers and nbp to connectedPlayers

Note for the commit message:

• unescapeText by sanderd17.

[elexis]

Fixes what was encountered and can be fixed without rewriting or extending a lot of c++ lobby / XML code. Didn't test yet and didn't review some parts of the patch yet.

[elexis]

Closing as dupe of #3476 as there has to be a distinction between players and observers in the clientlist to fix this. Having two tickets in the review queue for one patch, nor uploading one patch here and some other patch there doesn't really help.

[elexis]

In 18534:

Submit and display more information about matches in the lobby. Patch by Imarok, refs #3476.

Includes team numbers, online/offline- and won/defeated state, AI type and difficulty for running games and
only the playernames with observer-player distinction in the gamesetup.
Use JSON format inside the XML stanza and minimize traffic by packing teams.

Use the observer distinction to correctly apply the "full games" trigger in the lobby, fixes #3143.

XPartaMupp patch applied by scythetwirler.
unescapeText function by sanderd17, refs #3409.