#3143 closed defect (fixed)
[PATCH] "Show Full Games" option hides games that are not full
Reported by: | elexis | Owned by: | elexis |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 21 |
Component: | Multiplayer lobby | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
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.
Attachments (17)
Change History (35)
comment:1 by , 9 years ago
Description: | modified (diff) |
---|
comment:2 by , 9 years ago
Milestone: | Alpha 19 → Backlog |
---|
comment:3 by , 9 years ago
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.
comment:4 by , 8 years ago
Should game.nbp just be number of players without observers or should there be some new data containing the number of observers transmitted?
comment:5 by , 8 years ago
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.
by , 8 years ago
Attachment: | 3143_lobby_data.patch added |
---|
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
comment:6 by , 8 years ago
Keywords: | review patch added |
---|---|
Milestone: | Backlog → Alpha 21 |
Summary: | "Show Full Games" option hides games that are not full → [PATCH] "Show Full Games" option hides games that are not full |
comment:7 by , 8 years ago
Keywords: | review removed |
---|
See comments in irc (http://irclogs.wildfiregames.com/2016-06-14-QuakeNet-%230ad-dev.log)
comment:8 by , 8 years ago
Keywords: | review added |
---|
by , 8 years ago
Attachment: | 3143_lobby_data_v2.2.patch added |
---|
removed the commented out code (thx leper)
comment:9 by , 8 years ago
Keywords: | review removed |
---|
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.)
by , 8 years ago
Attachment: | 3143_lobby_data_v2.3.patch added |
---|
Use JSON.stringify
and encodeURI
/decodeURI
(escape
is deprecated) to pass the player and observer list
comment:10 by , 8 years ago
Keywords: | review added |
---|
by , 8 years ago
Attachment: | 3143_lobby_data_v2.5.patch added |
---|
Move duplicating formatting into common. Translate ,
comment:11 by , 8 years ago
by , 8 years ago
Attachment: | 3143_lobby_data_gameattribs.patch added |
---|
preparation patch defining g_GameAttributes for session.js
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.03.patch added |
---|
The patch should be finished. But when testing my game wasn't even displayed in the lobbys game list...
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6_lobby_bot.patch added |
---|
Refreshes the "players" too when executing changeGameState
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6_lobby_bot_v2.patch added |
---|
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.2.patch added |
---|
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.3.patch added |
---|
only remove the lengt limitation of escapeText for some parts
comment:13 by , 8 years ago
- 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 partJSON.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 withmap
if it doesn't become too entangledsendLobbyReportUpdate
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. PerhapssendChangeStateGame
?
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.4.patch added |
---|
Made the changes proposed by elexis
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.5.patch added |
---|
Move the strinigfication and escapement into the new format changing functions
by , 8 years ago
Attachment: | 3143_lobby_data_v2.6.6.patch added |
---|
use whitelist in sendLobbyStanzaUpdate and changed the comment of the format function
comment:15 by , 8 years ago
Component: | Core engine → Multiplayer lobby |
---|
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 listmaxnbp
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 everysendLobbyStanzaUpdate
- The defeat/won case of
g_NotificationsTypes
should be used, rather than theformatDefeatMessage
- 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
andplayerData
topData
, since they are an item ofg_GameAttributes.settings.PlayerData
- renaming
originalplayerDataArray
toplayerData
since it was copied fromg_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
tosendLobbyPlayerlistUpdate
numberOfPlayers
andplayerStates
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 loadOffline
-> unneeded package load if its falseState
andOffline
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
andnbp
toconnectedPlayers
Note for the commit message:
unescapeText
by sanderd17.
by , 8 years ago
Attachment: | 3143_lobby_data_v3.patch added |
---|
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.
comment:16 by , 8 years ago
Keywords: | review removed |
---|---|
Milestone: | Alpha 21 |
Resolution: | → duplicate |
Status: | new → closed |
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.
comment:18 by , 8 years ago
Milestone: | → Alpha 21 |
---|
No to the quick fix, users joining full games is annoying.