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

@@ -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();
 }
 

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;

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);

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<GUIMessage> 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

source/lobby/scripting/JSInterface_Lobby.cpp

@@ -52,10 +52,10 @@ void JSI_Lobby::RegisterScriptFunctions(const ScriptInterface& scriptInterface)
 	scriptInterface.RegisterFunction<void, &JSI_Lobby::SendUnregisterGame>("SendUnregisterGame");
 	scriptInterface.RegisterFunction<void, std::wstring, std::wstring, &JSI_Lobby::SendChangeStateGame>("SendChangeStateGame");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::GetPlayerList>("GetPlayerList");
-	scriptInterface.RegisterFunction<void, &JSI_Lobby::LobbyClearPresenceUpdates>("LobbyClearPresenceUpdates");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::GetGameList>("GetGameList");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::GetBoardList>("GetBoardList");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::GetProfile>("GetProfile");
+	scriptInterface.RegisterFunction<bool, &JSI_Lobby::LobbyGuiPollPresenceStatusUpdate>("LobbyGuiPollPresenceStatusUpdate");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::LobbyGuiPollNewMessage>("LobbyGuiPollNewMessage");
 	scriptInterface.RegisterFunction<JS::Value, &JSI_Lobby::LobbyGuiPollHistoricMessages>("LobbyGuiPollHistoricMessages");
 	scriptInterface.RegisterFunction<void, std::wstring, &JSI_Lobby::LobbySendMessage>("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)

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);