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

Description

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

Details

Committed

elexis

Sep 6 2019, 4:53 AM

Differential Revision

D2265: Delete Lobby Presence Update messages altogether

Parents

rP22854: Rewrite Main Menu.

Branches

Unknown

Tags

Unknown

Build Status

Buildable 9240
Build 15225: Trigger Windows Autobuild
Build 15224: Post-Commit Build	Jenkins

Event Timeline

elexis committed rP22855: Drop lobby presence GUI messages altogether for better performance and less….Sep 6 2019, 4:53 AM

elexis added an edge: D2265: Delete Lobby Presence Update messages altogether.

Harbormaster completed building B9240: rP22855: Drop lobby presence GUI messages altogether for better performance and less….Sep 6 2019, 5:03 AM

elexis mentioned this in rP22859: Fix wrong "player is not a moderator anymore" lobby chat message following….Sep 6 2019, 6:56 PM

elexis mentioned this in P170 Conditional jump or move depends on uninitialised value IGUIObject::SendEvent IXmppClient::create JS_CallFunctionValue.Sep 9 2019, 9:31 PM

elexis mentioned this in D2278: Fix uninitialized XmppClient members.Sep 9 2019, 9:40 PM

elexis mentioned this in rP22879: Fix missing XmppClient m_certStatus and m_PresenceUpdate member initialization….Sep 9 2019, 10:50 PM

elexis mentioned this in D2283: Fix lobby playerlist rebuild following presence update message removal in rP22855/D2265.Sep 11 2019, 1:15 PM

elexis mentioned this in rP22888: Fix lobby playerlist rebuild following presence update message removal in….Sep 11 2019, 1:51 PM

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

/ps/trunk/source/lobby/XmppClient.cpp
664	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

/ps/trunk/source/lobby/XmppClient.cpp
664	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.