HomeWildfire Games

Drop lobby presence GUI messages altogether for better performance and less…


Drop lobby presence GUI messages altogether for better performance and less code complexity.

They were only used to determine whether the playerlist should be rebuilt, which can be achieved by a boolean member.
So this patch is removing unnecessary indirection from the original solution in rP16997 and the reduction in rP20040, refs #3386.
In particular the LobbyClearPresenceUpdates call was ugly and one doesnt need hundreds of messages with data on it each if one only wants to know if there was one.
Makes past (rP20070) and future (D2264) patches less complex, refs #4877.

Differential Revision: https://code.wildfiregames.com/D2265
Tested on: gcc 9.1.0, clang 8.0.1, Jenkins

Event Timeline

elexis added inline comments.Nov 19 2019, 10:37 AM

This was not only relevant to filter out presence ones but also to filter out the non-chat ones.

For example it is important that XmppClient doesn't copy every gamelist update ever.
The only ones that we are mostly intersted in are the chat messages.
Its wrong to process connect and disconnect events of the local player when opening the lobby dialog from a running match again in case there was a disconnect without recreating the XmppClient instance. The disconnect event should only be triggered when the current client disconnected, but we don't intend to process the historic replay of that message.

Historic replay of nick changes, subject changes, clients joining/leaving/kicked from the muc room is also somewhat dangerous. For example it might hypothetically make sense to replay keep a replay the "has been kicked" message of the local client for chat, but the client should not assume that it happens again. That distinction is possible by the historic message property.

For the redundant "gamelist" updates, it would be better to only keep at most one message in the state and not have it be a queue.
If one plays for one hour and gets hundreds of playerlist and gamelist updates, theyre all be cached until theyre pullled, even if not "historic" messaes yet.
For presence changes, they can consume a lot of memory since there are many of them and their replay is not used, so I must not only add this check back but make it more restrictive and eventually also remove playerlist/gamelist/ratinglist updates from the queue if we want to avoid having them stacked up during long games.

elexis added inline comments.Nov 20 2019, 11:19 AM

It already was broken before this commit, the commit just worsened it.

For example "Announcements" are stored as historic messages as well in a23b and the user receives them everytime the lobby dialog is opened again. So it seems this should become much more narrow.