From 6bf74902a7ca8e2bf364a854068b1164a75e5534 Mon Sep 17 00:00:00 2001 From: elexis Date: Fri, 6 Sep 2019 02:53:22 +0000 Subject: [PATCH] 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 8b437a0b1c and the reduction in 16b976fc35, 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 (e8dfde9ba6) 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 This was SVN commit r22855. --- binaries/data/mods/public/gui/lobby/lobby.js | 35 +++----------------- source/lobby/IXmppClient.h | 2 +- source/lobby/XmppClient.cpp | 32 +++++++----------- source/lobby/XmppClient.h | 4 ++- source/lobby/scripting/JSInterface_Lobby.cpp | 15 ++++----- source/lobby/scripting/JSInterface_Lobby.h | 4 +-- 6 files changed, 29 insertions(+), 63 deletions(-) diff --git a/binaries/data/mods/public/gui/lobby/lobby.js b/binaries/data/mods/public/gui/lobby/lobby.js index ccec2fcdbe..1c4a486b1c 100644 --- a/binaries/data/mods/public/gui/lobby/lobby.js +++ b/binaries/data/mods/public/gui/lobby/lobby.js @@ -135,12 +135,12 @@ var g_AskedReconnect = false; var g_NetMessageTypes = { "system": { // Three cases are handled in prelobby.js - "registered": msg => false, + "registered": msg => { + }, "connected": msg => { g_AskedReconnect = false; updateConnectedState(); - return false; }, "disconnected": msg => { @@ -157,8 +157,6 @@ var g_NetMessageTypes = { }); reconnectMessageBox(); } - - return true; }, "error": msg => { addChatMessage({ @@ -166,7 +164,6 @@ var g_NetMessageTypes = { "time": msg.time, "text": msg.text }); - return false; } }, "chat": { @@ -182,7 +179,6 @@ var g_NetMessageTypes = { "time": msg.time, "isSpecial": true }); - return false; }, "join": msg => { addChatMessage({ @@ -192,7 +188,6 @@ var g_NetMessageTypes = { "time": msg.time, "isSpecial": true }); - return true; }, "leave": msg => { addChatMessage({ @@ -205,10 +200,7 @@ var g_NetMessageTypes = { if (msg.nick == g_Username) Engine.DisconnectXmppClient(); - - return true; }, - "presence": msg => true, "role": msg => { Engine.GetGUIObjectByName("chatInput").hidden = Engine.LobbyGetPlayerRole(g_Username) == "visitor"; @@ -239,8 +231,6 @@ var g_NetMessageTypes = { if (g_SelectedPlayer == msg.nick) updateUserRoleText(g_SelectedPlayer); - - return false; }, "nick": msg => { addChatMessage({ @@ -251,15 +241,12 @@ var g_NetMessageTypes = { "time": msg.time, "isSpecial": true }); - return true; }, "kicked": msg => { handleKick(false, msg.nick, msg.reason, msg.time, msg.historic); - return true; }, "banned": msg => { handleKick(true, msg.nick, msg.reason, msg.time, msg.historic); - return true; }, "room-message": msg => { addChatMessage({ @@ -268,7 +255,6 @@ var g_NetMessageTypes = { "time": msg.time, "historic": msg.historic }); - return false; }, "private-message": msg => { // Announcements and the Message of the Day are sent by the server directly @@ -276,8 +262,7 @@ var g_NetMessageTypes = { messageBox( 400, 250, msg.text.trim(), - translate("Notice") - ); + translate("Notice")); // We intend to not support private messages between users if (!msg.from || Engine.LobbyGetPlayerRole(msg.from) == "moderator") @@ -289,24 +274,19 @@ var g_NetMessageTypes = { "historic": msg.historic, "private": true }); - return false; } }, "game": { "gamelist": msg => { updateGameList(); - return false; }, "profile": msg => { updateProfile(); - return false; }, "leaderboard": msg => { updateLeaderboard(); - return false; }, "ratinglist": msg => { - return true; } } }; @@ -414,8 +394,6 @@ function init(attribs) Engine.LobbySetPlayerPresence("available"); - // When rejoining the lobby after a game, we don't need to process presence changes - Engine.LobbyClearPresenceUpdates(); updatePlayerList(); updateSubject(Engine.LobbyGetRoomSubject()); updateLobbyColumns(); @@ -1309,13 +1287,10 @@ function onTick() continue; } - if (g_NetMessageTypes[msg.type][msg.level](msg)) - updateList = true; + g_NetMessageTypes[msg.type][msg.level](msg); } - // To improve performance, only update the playerlist GUI when - // the last update in the current stack is processed - if (updateList) + if (Engine.LobbyGuiPollPresenceStatusUpdate()) updatePlayerList(); } diff --git a/source/lobby/IXmppClient.h b/source/lobby/IXmppClient.h index a069043311..9759899004 100644 --- a/source/lobby/IXmppClient.h +++ b/source/lobby/IXmppClient.h @@ -51,10 +51,10 @@ public: virtual void GetRole(const std::string& nickname, std::string& role) = 0; virtual void GetSubject(std::string& subject) = 0; virtual void GUIGetPlayerList(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret) = 0; - virtual void ClearPresenceUpdates() = 0; virtual void GUIGetGameList(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret) = 0; virtual void GUIGetBoardList(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret) = 0; virtual void GUIGetProfile(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret) = 0; + virtual bool GuiPollPresenceStatusUpdate() = 0; virtual JS::Value GuiPollNewMessage(const ScriptInterface& scriptInterface) = 0; virtual JS::Value GuiPollHistoricMessages(const ScriptInterface& scriptInterface) = 0; virtual void SendMUCMessage(const std::string& message) = 0; diff --git a/source/lobby/XmppClient.cpp b/source/lobby/XmppClient.cpp index b47cd4a454..1cc2781628 100644 --- a/source/lobby/XmppClient.cpp +++ b/source/lobby/XmppClient.cpp @@ -652,6 +652,13 @@ JS::Value XmppClient::GuiMessageToJSVal(const ScriptInterface& scriptInterface, return ret; } +bool XmppClient::GuiPollPresenceStatusUpdate() +{ + bool hasUpdate = m_PresenceUpdate; + m_PresenceUpdate = false; + return hasUpdate; +} + JS::Value XmppClient::GuiPollNewMessage(const ScriptInterface& scriptInterface) { if (m_GuiMessageQueue.empty()) @@ -659,10 +666,7 @@ JS::Value XmppClient::GuiPollNewMessage(const ScriptInterface& scriptInterface) GUIMessage message = m_GuiMessageQueue.front(); m_GuiMessageQueue.pop_front(); - - // Since there can be hundreds of presence changes while playing a game, ignore these for performance - if (message.type == "chat" && message.level != "presence") - m_HistoricGuiMessages.push_back(message); + m_HistoricGuiMessages.push_back(message); return GuiMessageToJSVal(scriptInterface, message, false); } @@ -692,21 +696,6 @@ void XmppClient::SendMUCMessage(const std::string& message) m_mucRoom->send(message); } -/** - * Clears all presence updates from the message queue. - * Used when rejoining the lobby, since we don't need to handle past presence changes. - */ -void XmppClient::ClearPresenceUpdates() -{ - m_GuiMessageQueue.erase( - std::remove_if(m_GuiMessageQueue.begin(), m_GuiMessageQueue.end(), - [](XmppClient::GUIMessage& message) - { - return message.type == "chat" && message.level == "presence"; - } - ), m_GuiMessageQueue.end()); -} - /** * Handle a room message. */ @@ -885,7 +874,10 @@ void XmppClient::handleMUCParticipantPresence(glooxwrapper::MUCRoom*, const gloo else if (m_PlayerMap[nick][2] != roleString) CreateGUIMessage("chat", "role", "nick", nick, "oldrole", m_PlayerMap[nick][2]); else - CreateGUIMessage("chat", "presence", "nick", nick); + // Don't create a GUI message for regular presence changes, because + // several hundreds of them accumulate during a match, impacting performance terribly and + // the only way they are used is to determine whether to update the playerlist. + m_PresenceUpdate = true; DbgXMPP(nick << " is in the room, presence : " << (int)presenceType); m_PlayerMap[nick].resize(3); diff --git a/source/lobby/XmppClient.h b/source/lobby/XmppClient.h index 5f4702663d..07a459e259 100644 --- a/source/lobby/XmppClient.h +++ b/source/lobby/XmppClient.h @@ -152,10 +152,10 @@ public: std::time_t time; }; JS::Value GuiMessageToJSVal(const ScriptInterface& scriptInterface, const GUIMessage& message, const bool historic); + bool GuiPollPresenceStatusUpdate(); JS::Value GuiPollNewMessage(const ScriptInterface& scriptInterface); JS::Value GuiPollHistoricMessages(const ScriptInterface& scriptInterface); void SendMUCMessage(const std::string& message); - void ClearPresenceUpdates(); protected: void CreateGUIMessage( const std::string& type, @@ -181,6 +181,8 @@ private: std::vector m_HistoricGuiMessages; /// Current room subject/topic. std::string m_Subject; + /// Whether or not a player has changed the presence status since the last time the GUI checked. + bool m_PresenceUpdate; }; #endif // XMPPCLIENT_H diff --git a/source/lobby/scripting/JSInterface_Lobby.cpp b/source/lobby/scripting/JSInterface_Lobby.cpp index 94a3c27f77..a15df5ce5a 100644 --- a/source/lobby/scripting/JSInterface_Lobby.cpp +++ b/source/lobby/scripting/JSInterface_Lobby.cpp @@ -52,10 +52,10 @@ void JSI_Lobby::RegisterScriptFunctions(const ScriptInterface& scriptInterface) scriptInterface.RegisterFunction("SendUnregisterGame"); scriptInterface.RegisterFunction("SendChangeStateGame"); scriptInterface.RegisterFunction("GetPlayerList"); - scriptInterface.RegisterFunction("LobbyClearPresenceUpdates"); scriptInterface.RegisterFunction("GetGameList"); scriptInterface.RegisterFunction("GetBoardList"); scriptInterface.RegisterFunction("GetProfile"); + scriptInterface.RegisterFunction("LobbyGuiPollPresenceStatusUpdate"); scriptInterface.RegisterFunction("LobbyGuiPollNewMessage"); scriptInterface.RegisterFunction("LobbyGuiPollHistoricMessages"); scriptInterface.RegisterFunction("LobbySendMessage"); @@ -195,14 +195,6 @@ JS::Value JSI_Lobby::GetPlayerList(ScriptInterface::CxPrivate* pCxPrivate) return playerList; } -void JSI_Lobby::LobbyClearPresenceUpdates(ScriptInterface::CxPrivate* UNUSED(pCxPrivate)) -{ - if (!g_XmppClient) - return; - - g_XmppClient->ClearPresenceUpdates(); -} - JS::Value JSI_Lobby::GetGameList(ScriptInterface::CxPrivate* pCxPrivate) { if (!g_XmppClient) @@ -245,6 +237,11 @@ JS::Value JSI_Lobby::GetProfile(ScriptInterface::CxPrivate* pCxPrivate) return profileFetch; } +bool JSI_Lobby::LobbyGuiPollPresenceStatusUpdate(ScriptInterface::CxPrivate* UNUSED(pCxPrivate)) +{ + return g_XmppClient && g_XmppClient->GuiPollPresenceStatusUpdate(); +} + JS::Value JSI_Lobby::LobbyGuiPollNewMessage(ScriptInterface::CxPrivate* pCxPrivate) { if (!g_XmppClient) diff --git a/source/lobby/scripting/JSInterface_Lobby.h b/source/lobby/scripting/JSInterface_Lobby.h index 9e5c4faf22..da0bc7ff03 100644 --- a/source/lobby/scripting/JSInterface_Lobby.h +++ b/source/lobby/scripting/JSInterface_Lobby.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2018 Wildfire Games. +/* Copyright (C) 2019 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -45,10 +45,10 @@ namespace JSI_Lobby void SendUnregisterGame(ScriptInterface::CxPrivate* pCxPrivate); void SendChangeStateGame(ScriptInterface::CxPrivate* pCxPrivate, const std::wstring& nbp, const std::wstring& players); JS::Value GetPlayerList(ScriptInterface::CxPrivate* pCxPrivate); - void LobbyClearPresenceUpdates(ScriptInterface::CxPrivate* pCxPrivate); JS::Value GetGameList(ScriptInterface::CxPrivate* pCxPrivate); JS::Value GetBoardList(ScriptInterface::CxPrivate* pCxPrivate); JS::Value GetProfile(ScriptInterface::CxPrivate* pCxPrivate); + bool LobbyGuiPollPresenceStatusUpdate(ScriptInterface::CxPrivate* pCxPrivate); JS::Value LobbyGuiPollNewMessage(ScriptInterface::CxPrivate* pCxPrivate); JS::Value LobbyGuiPollHistoricMessages(ScriptInterface::CxPrivate* pCxPrivate); void LobbySendMessage(ScriptInterface::CxPrivate* pCxPrivate, const std::wstring& message);