Fix a bunch of server list issues, including with sort and server-info button

Author: Dutchman101

Client/core/ServerBrowser/CServerBrowser.RemoteMasterServer.cpp

@@ -465,6 +465,9 @@ bool CRemoteMasterServer::ParseListVer2(CServerListItemList& itemList)
         {
             pItem->SetDataQuality(uiDataQuality);
 
+            // Remember previous no-response
+            bool bWasPreviouslyMarkedNoResponse = pItem->bMasterServerSaysNoResponse;
+
             if (bHasPlayerCount)
                 stream.Read(pItem->nPlayers);
             if (bHasMaxPlayerCount)
@@ -503,6 +506,11 @@ bool CRemoteMasterServer::ParseListVer2(CServerListItemList& itemList)
             {
                 stream.Read(pItem->bMasterServerSaysNoResponse);
             }
+            else if (bWasPreviouslyMarkedNoResponse)
+            {
+                // Reset no-response flag when server reappears in master list without responding flag
+                pItem->bMasterServerSaysNoResponse = false;
+            }
 
             if (bHasRestrictionFlags)
             {

Client/core/ServerBrowser/CServerBrowser.cpp

@@ -91,8 +91,8 @@ class CServerBrowser : public CSingleton<CServerBrowser>
     std::string GetServerPassword(const std::string& strHost);
     void        ClearServerPasswords();
 
-    void SetStatusText(std::string strStatus);
-    void SetAddressBarText(std::string strText);
+    void SetStatusText(std::string_view strStatus);
+    void SetAddressBarText(std::string_view strText);
 
     CServerListItem* FindSelectedServer(ServerBrowserType Type);
     CServerListItem* FindServerFromRow(ServerBrowserType Type, int iRow);
@@ -202,6 +202,7 @@ class CServerBrowser : public CSingleton<CServerBrowser>
 private:
     struct SFilterState;
     struct SListRefreshState;
+    struct SSortState;
 
     enum
     {
@@ -216,6 +217,7 @@ class CServerBrowser : public CSingleton<CServerBrowser>
     [[nodiscard]] CServerList* GetServerList(ServerBrowserType Type);
     void         AddServerToList(CServerListItem* pServer, ServerBrowserType Type, const SFilterState& filterState);
     bool         RemoveSelectedServerFromRecentlyPlayedList();
+    void         UpdateFilteredPlayerCountStatus(ServerBrowserType Type, CServerList* pDataList = nullptr);
 
     bool OnClick(CGUIElement* pElement);
     bool OnDoubleClick(CGUIElement* pElement);
@@ -242,6 +244,7 @@ class CServerBrowser : public CSingleton<CServerBrowser>
 
     bool OnServerListChangeRow(CGUIKeyEventArgs Args);
     bool OnServerListKeyDown(CGUIKeyEventArgs Args);
+    bool OnServerListSortChanged(CGUIElement* pElement);
 
     ServerBrowserType GetCurrentServerBrowserType();
     ServerBrowserType GetCurrentServerBrowserTypeForSave();
@@ -256,6 +259,8 @@ class CServerBrowser : public CSingleton<CServerBrowser>
     bool ProcessServerListRefreshBatch(ServerBrowserType type, std::size_t uiMaxSteps);
     void ProcessPendingListRefreshes();
     void SuspendServerLists();
+    void UpdateSortState(ServerBrowserType type);
+    void ApplyListSort(ServerBrowserType type, unsigned int uiColumn, SortDirection direction, bool bUpdateStoredState = false);
     [[nodiscard]] bool        IsListRefreshInProgress(ServerBrowserType type) const;
     [[nodiscard]] std::size_t GetListRefreshBatchSize() const;
     [[nodiscard]] std::size_t GetHiddenListRefreshBatchSize() const;
@@ -289,6 +294,12 @@ class CServerBrowser : public CSingleton<CServerBrowser>
         std::optional<SFilterState> filterSnapshot;
     };
 
+    struct SSortState
+    {
+        unsigned int  uiColumn = 0;
+        SortDirection direction = SortDirections::None;
+    };
+
     CServerListInternet m_ServersInternet;
     CServerListLAN      m_ServersLAN;
     CServerList         m_ServersFavourites;
@@ -318,10 +329,16 @@ class CServerBrowser : public CSingleton<CServerBrowser>
     bool m_bPendingFilterUpdate[SERVER_BROWSER_TYPE_COUNT];
     unsigned long m_ulNextFilterUpdateTime[SERVER_BROWSER_TYPE_COUNT];
     SFilterState  m_FilterState[SERVER_BROWSER_TYPE_COUNT];
+    SSortState    m_SortState[SERVER_BROWSER_TYPE_COUNT];
 
     bool          m_bPendingConfigSave;
     unsigned long m_ulNextConfigSaveTime;
     SListRefreshState m_ListRefreshState[SERVER_BROWSER_TYPE_COUNT];
     unsigned long m_ulNextListLayoutTime[SERVER_BROWSER_TYPE_COUNT] = {};
     unsigned long m_ulHiddenRefreshBoostEnableTime = 0;
+    int           m_iIgnoreSortCallbacks = 0;
+
+    // Player list retry tracking - retry queries for servers that haven't responded with player data
+    CServerListItem* m_pLastSelectedServerForPlayerList[SERVER_BROWSER_TYPE_COUNT] = {};
+    std::chrono::milliseconds m_msLastPlayerListQueryRetryTime[SERVER_BROWSER_TYPE_COUNT] = {};
 };

Client/core/ServerBrowser/CServerBrowser.h

@@ -59,7 +59,8 @@ class CServerCache : public CServerCacheInterface
     virtual void GetServerCachedInfo(CServerListItem* pItem);
     virtual void SetServerCachedInfo(const CServerListItem* pItem);
     virtual void GetServerListCachedInfo(CServerList* pList);
-    virtual bool GenerateServerList(CServerList* pList);
+    virtual bool GenerateServerList(CServerList* pList, bool bAllowNonResponding = false);
+    virtual void SetServerListCachedInfo(CServerList* pList);
 
     CServerCache();
     ~CServerCache();
@@ -93,7 +94,9 @@ CServerCacheInterface* GetServerCache()
 //
 // CServerCache::CServerCache
 //
-//
+// Initialize server cache by loading from persistent disk storage.
+// The cache file (servercache.xml) is never deleted and persists across sessions.
+// Cache entries are synced (updated) on each session, not regenerated.
 //
 ///////////////////////////////////////////////////////////////
 CServerCache::CServerCache()
@@ -117,6 +120,8 @@ CServerCache::~CServerCache()
 // CServerCache::LoadServerCache
 //
 // Load cache data from config
+// The in-memory cache is cleared and reloaded from disk.
+// The disk file is never deleted; it persists across game sessions.
 //
 ///////////////////////////////////////////////////////////////
 bool CServerCache::LoadServerCache()
@@ -197,7 +202,8 @@ bool CServerCache::LoadServerCache()
 //
 // CServerCache::SaveServerCache
 //
-// Save cache data to config
+// Persist cache changes to disk when data has been modified.
+// Always syncs complete state; never clears the disk file.
 //
 ///////////////////////////////////////////////////////////////
 void CServerCache::SaveServerCache(bool bWaitUntilFinished)
@@ -256,7 +262,9 @@ DWORD WINAPI CServerCache::StaticThreadProc(LPVOID lpdwThreadParam)
 //
 // CServerCache::StaticSaveServerCache
 //
-//
+// Synchronize cache to disk by writing all current in-memory entries.
+// The XML tree is rebuilt on each save (not appended to) for consistency,
+// but the file itself is never deleted. Servers are synced, not purged.
 //
 ///////////////////////////////////////////////////////////////
 void CServerCache::StaticSaveServerCache()
@@ -273,18 +281,17 @@ void CServerCache::StaticSaveServerCache()
     if (!pNode)
         return;
 
-    // Start by clearing out all previous nodes
+    // Clear in-memory XML structure and rebuild from current cache state.
+    // This does NOT delete the disk file; it only rewrites its contents with current data.
     pNode->DeleteAllSubNodes();
 
     // Transfer each item from m_ServerCachedMap into dataSet
     CDataInfoSet dataSet;
-    for (std::map<CCachedKey, CCachedInfo>::iterator it = ms_ServerCachedMap.begin(); it != ms_ServerCachedMap.end(); ++it)
+    for (const auto& [key, info] : ms_ServerCachedMap)
     {
-        const CCachedKey&  key = it->first;
-        const CCachedInfo& info = it->second;
 
-        // Don't save cache of non responding servers
-        if (info.nMaxPlayers == 0 || info.uiCacheNoReplyCount > 3)
+        // Only exclude servers that have failed multiple consecutive query attempts
+        if (info.uiCacheNoReplyCount > 3)
             continue;
 
         SDataInfoItem item;
@@ -327,6 +334,9 @@ void CServerCache::StaticSaveServerCache()
 ///////////////////////////////////////////////////////////////
 void CServerCache::GetServerCachedInfo(CServerListItem* pItem)
 {
+    if (!pItem)
+        return;
+
     CCachedKey key;
     key.ulIp = pItem->Address.s_addr;
     key.usGamePort = pItem->usGamePort;
@@ -378,6 +388,9 @@ void CServerCache::GetServerCachedInfo(CServerListItem* pItem)
 ///////////////////////////////////////////////////////////////
 void CServerCache::SetServerCachedInfo(const CServerListItem* pItem)
 {
+    if (!pItem)
+        return;
+
     CCachedKey key;
     key.ulIp = pItem->Address.s_addr;
     key.usGamePort = pItem->usGamePort;
@@ -424,15 +437,21 @@ void CServerCache::SetServerCachedInfo(const CServerListItem* pItem)
 ///////////////////////////////////////////////////////////////
 void CServerCache::GetServerListCachedInfo(CServerList* pList)
 {
-    // Get cached info for each server
-    for (CServerListIterator it = pList->IteratorBegin(); it != pList->IteratorEnd(); it++)
-        GetServerCachedInfo(*it);
+    if (!pList)
+        return;
+
+    for (auto it = pList->IteratorBegin(); it != pList->IteratorEnd(); ++it)
+    {
+        if (*it)
+            GetServerCachedInfo(*it);
+    }
 
-    // Remove servers not in serverlist from cache
     std::map<CCachedKey, CCachedInfo> nextServerCachedMap;
-    for (CServerListIterator it = pList->IteratorBegin(); it != pList->IteratorEnd(); it++)
+    for (auto it = pList->IteratorBegin(); it != pList->IteratorEnd(); ++it)
     {
         CServerListItem* pItem = *it;
+        if (!pItem)
+            continue;
         CCachedKey       key;
         key.ulIp = pItem->Address.s_addr;
         key.usGamePort = pItem->usGamePort;
@@ -442,22 +461,44 @@ void CServerCache::GetServerListCachedInfo(CServerList* pList)
     m_ServerCachedMap = nextServerCachedMap;
 }
 
+///////////////////////////////////////////////////////////////
+//
+// CServerCache::SetServerListCachedInfo
+//
+// Cache all servers in list
+//
+///////////////////////////////////////////////////////////////
+void CServerCache::SetServerListCachedInfo(CServerList* pList)
+{
+    if (!pList)
+        return;
+
+    for (auto it = pList->IteratorBegin(); it != pList->IteratorEnd(); ++it)
+    {
+        if (*it)
+            SetServerCachedInfo(*it);
+    }
+}
+
 ///////////////////////////////////////////////////////////////
 //
 // CServerCache::GenerateServerList
 //
 // Create serverlist content from the cache
 //
 ///////////////////////////////////////////////////////////////
-bool CServerCache::GenerateServerList(CServerList* pList)
+bool CServerCache::GenerateServerList(CServerList* pList, bool bAllowNonResponding)
 {
-    for (std::map<CCachedKey, CCachedInfo>::iterator it = m_ServerCachedMap.begin(); it != m_ServerCachedMap.end(); ++it)
+    if (!pList)
+        return false;
+
+    for (const auto& [key, info] : m_ServerCachedMap)
     {
-        const CCachedKey&  key = it->first;
-        const CCachedInfo& info = it->second;
 
-        // Don't add non responding servers
-        if (info.nMaxPlayers == 0 || info.uiCacheNoReplyCount > 3)
+        // When master server is offline, include all cached servers. Otherwise exclude servers that
+        // have consistently failed to respond (uiCacheNoReplyCount > 3). New servers without response data
+        // should still be included since they may not have been queried yet.
+        if (!bAllowNonResponding && (info.uiCacheNoReplyCount > 3))
             continue;
 
         ushort usGamePort = key.usGamePort;

Client/core/ServerBrowser/CServerCache.cpp

@@ -10,6 +10,9 @@
  *
  *****************************************************************************/
 
+class CServerList;
+class CServerListItem;
+
 class CServerCacheInterface
 {
 public:
@@ -18,7 +21,8 @@ class CServerCacheInterface
     virtual void GetServerCachedInfo(CServerListItem* pItem) = 0;
     virtual void SetServerCachedInfo(const CServerListItem* pItem) = 0;
     virtual void GetServerListCachedInfo(CServerList* pList) = 0;
-    virtual bool GenerateServerList(CServerList* pList) = 0;
+    virtual bool GenerateServerList(CServerList* pList, bool bAllowNonResponding = false) = 0;
+    virtual void SetServerListCachedInfo(CServerList* pList) = 0;  // Cache all servers in the list
 };
 
 CServerCacheInterface* GetServerCache();