Merge pull request #568 from tintinhamans/arctic/ngmp-cache-fix

fix(ngmp): correct stats and media upload caching

Author: x64-dev

Core/GameEngine/Source/GameNetwork/GameSpy/Thread/PersistentStorageThread.cpp

@@ -1094,6 +1094,11 @@ PSPlayerStats::PSPlayerStats()
 void PSPlayerStats::reset()
 {
 	id = 0;
+#if defined(GENERALS_ONLINE)
+	elo_rating = 0;
+	monthly_elo_rating = 0;
+	elo_num_matches = 0;
+#endif
 	locale = 0;
 	gamesAsRandom = 0;
 	lastFPS = 0;

GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h

@@ -532,46 +532,39 @@ class NGMP_OnlineServicesManager
         m_vecCachedScreenshotBytes_MatchStart = vecData;
 	}
 
-    void CacheScreenshotBytes_EndMatch(std::vector<uint8_t>& vecData)
-    {
-        std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
-        m_vecCachedScreenshotBytes_MatchEnd = vecData;
-    }
-
-    void CacheReplayBytes(std::vector<uint8_t>& vecData)
-    {
-        std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
-		m_vecCachedReplayBytes = vecData;
-    }
-
+	void CacheScreenshotBytes_EndMatch(uint64_t matchID, std::vector<uint8_t> data);
+	void CacheReplayBytes(uint64_t matchID, std::vector<uint8_t> data);
     void SetScreenshotS3URI_StartMatch(const char* szURI)
     {
         std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
 		m_strCachedScreenshot_MatchStart_S3URI = std::string(szURI);
     }
 
-    void SetScreenshotS3URI_EndMatch(const char* szURI)
-    {
-        std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
-        m_strCachedScreenshot_MatchEnd_S3URI = std::string(szURI);
-    }
-
-    void SetScreenshotS3URI_Replay(const char* szURI)
-    {
-        std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
-		m_strCacheReplay_S3URI = std::string(szURI);
-    }
+	void SetScreenshotS3URI_EndMatch(uint64_t matchID, std::string uri);
+	void SetScreenshotS3URI_Replay(uint64_t matchID, std::string uri);
 
 private:
 	// NOTE: Accessed from multiple threads, dont access directly, use helpers above to lock
     std::string m_strCachedScreenshot_MatchStart_S3URI;
-    std::string m_strCachedScreenshot_MatchEnd_S3URI;
-    std::string m_strCacheReplay_S3URI;
 
     // screenshots / replays that require caching
     std::vector<uint8_t> m_vecCachedScreenshotBytes_MatchStart;
-    std::vector<uint8_t> m_vecCachedScreenshotBytes_MatchEnd;
-    std::vector<uint8_t> m_vecCachedReplayBytes;
+
+	struct CachedMatchUpload
+	{
+		uint64_t dataMatchID = 0;
+		uint64_t uriMatchID = 0;
+		std::vector<uint8_t> bytes;
+		std::string signedURI;
+	};
+
+	void CacheMatchUploadBytes(CachedMatchUpload& upload, uint64_t matchID, std::vector<uint8_t> data);
+	void CacheMatchUploadURI(CachedMatchUpload& upload, uint64_t matchID, std::string uri);
+
+	// Data and URLs may arrive independently. Keeping their shared match ID here
+	// prevents a late response from being paired with media from another match.
+	CachedMatchUpload m_cachedMatchEndUpload;
+	CachedMatchUpload m_cachedReplayUpload;
 
 	// main thread SS Upload
 	static std::mutex m_ScreenshotMutex;

GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_StatsInterface.h

@@ -5,6 +5,10 @@
 #include "GameNetwork/RankPointValue.h"
 #include "GameNetwork/GameSpy/PersistentStorageThread.h"
 
+#include <chrono>
+#include <cstddef>
+#include <cstdint>
+
 class PSPlayerStats;
 
 struct GlobalStats
@@ -468,21 +472,37 @@ class NGMP_OnlineServices_StatsInterface
 
 	void findPlayerStatsByID(int64_t userID, std::function<void(bool, PSPlayerStats)> cb, EStatsRequestPolicy requestPolicy);
 	void findPlayerStatsByBatch(std::vector<int64_t> vecUserIDs, std::function<void(bool)> cb);
+	// Returns the last-known cached values, even when stale. Use
+	// HasFreshPlayerStats() to decide whether a refresh is required.
 	bool getPlayerStatsFromCache(int64_t userID, PSPlayerStats* outStats);
-	bool ArePlayerStatsCached(int64_t userID)
-	{
-		return m_mapCachedStats.contains(userID);
-	}
+	bool HasFreshPlayerStats(int64_t userID);
 
-	void UpdateMyStats(PSPlayerStats stats);
+	void UpdateMyStats(const PSPlayerStats& stats);
 
 	void CommitMyOutcome(ScoreKeeper* pScoreKeeper, bool bWon);
 
 private:
-	std::string JSONSerialize(PSPlayerStats stats);
+	struct PlayerStatsCacheEntry
+	{
+		PSPlayerStats stats;
+		std::chrono::steady_clock::time_point lastRefreshAt{};
+		std::chrono::steady_clock::time_point lastAccessAt{};
+		uint64_t cacheRevision = 0;
+		uint64_t updateRevision = 0;
+		bool hasStats = false;
+	};
+
+	std::string JSONSerialize(const PSPlayerStats& stats) const;
+	PlayerStatsCacheEntry& GetOrCreatePlayerStatsCacheEntry(int64_t userID);
+	uint64_t AdvancePlayerStatsCacheRevision(int64_t userID);
+	uint64_t AdvancePlayerStatsUpdateRevision(int64_t userID);
+	bool TryCachePlayerStats(const PSPlayerStats& stats, uint64_t expectedRevision);
+	void MarkPlayerStatsCacheStale(int64_t userID);
 
 private:
-	std::unordered_map<int64_t, int64_t> m_mapStatsLastRefresh;
-	std::unordered_map<int64_t, PSPlayerStats> m_mapCachedStats;
-	const static int64_t m_cacheTTL = 600000; // 10 minutes
+	std::unordered_map<int64_t, PlayerStatsCacheEntry> m_playerStatsCache;
+	uint64_t m_nextStatsCacheRevision = 0;
+	uint64_t m_nextStatsUpdateRevision = 0;
+	static constexpr std::chrono::minutes STATS_CACHE_TTL{ 10 };
+	static constexpr std::size_t MAX_PLAYER_STATS_CACHE_ENTRIES = 128;
 };

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLLobbyMenu.cpp

@@ -754,7 +754,7 @@ void PopulateLobbyPlayerListbox()
         {
             NetworkRoomMember& netRoomMember = kvPair.second;
 
-			if (!pStatsInterface->ArePlayerStatsCached(netRoomMember.user_id))
+			if (!pStatsInterface->HasFreshPlayerStats(netRoomMember.user_id))
 			{
 				vecUserStatsToRequest.push_back(netRoomMember.user_id);
 			}

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp

@@ -15,6 +15,7 @@
 #include "WW3D2/surfaceclass.h"
 #include "WW3D2/dx8wrapper.h"
 #include <mutex>
+#include <utility>
 
 #define STB_IMAGE_WRITE_IMPLEMENTATION
 #define STB_IMAGE_RESIZE_IMPLEMENTATION
@@ -118,7 +119,8 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree
 			NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface<NGMP_OnlineServices_LobbyInterface>();
 			if (pLobbyInterface != nullptr)
 			{
-				NGMP_OnlineServicesManager::GetInstance()->CaptureScreenshot(true, [strURI, screenshotType](std::vector<uint8_t> vecData)
+				const uint64_t matchID = pLobbyInterface->GetCurrentMatchID();
+				NGMP_OnlineServicesManager::GetInstance()->CaptureScreenshot(true, [strURI = std::move(strURI), screenshotType, matchID](std::vector<uint8_t> vecData)
 					{
 						CHECK_WORKER_THREAD;
 
@@ -135,7 +137,7 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree
 						}
                         else if (screenshotType == EScreenshotType::SCREENSHOT_TYPE_SCORESCREEN)
                         {
-                            NGMP_OnlineServicesManager::GetInstance()->CacheScreenshotBytes_EndMatch(vecData);
+							NGMP_OnlineServicesManager::GetInstance()->CacheScreenshotBytes_EndMatch(matchID, std::move(vecData));
                         }
 						else
 						{
@@ -248,6 +250,13 @@ void NGMP_OnlineServicesManager::CommitReplay(AsciiString absoluteReplayPath)
 
 		if (serviceConf.do_replay_upload)
 		{
+			NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface<NGMP_OnlineServices_LobbyInterface>();
+			const uint64_t matchID = pLobbyInterface == nullptr ? 0 : pLobbyInterface->GetCurrentMatchID();
+			if (matchID == 0)
+			{
+				NetworkLog(ELogVerbosity::LOG_RELEASE, "[MediaUpload] Cannot cache replay: match ID is unavailable");
+				return;
+			}
 			FILE* pFile = fopen(absoluteReplayPath.str(), "rb");
 
 			std::vector<unsigned char> replayData;
@@ -265,11 +274,55 @@ void NGMP_OnlineServicesManager::CommitReplay(AsciiString absoluteReplayPath)
 			}
 
 			// cache the data until we get an S3 URL from server
-			NGMP_OnlineServicesManager::GetInstance()->CacheReplayBytes(replayData);
+			NGMP_OnlineServicesManager::GetInstance()->CacheReplayBytes(matchID, std::move(replayData));
 		}
 	}
 }
 
+void NGMP_OnlineServicesManager::CacheMatchUploadBytes(CachedMatchUpload& upload, uint64_t matchID, std::vector<uint8_t> data)
+{
+	if (matchID == 0)
+	{
+		return;
+	}
+
+	std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
+	upload.dataMatchID = matchID;
+	upload.bytes = std::move(data);
+}
+
+void NGMP_OnlineServicesManager::CacheMatchUploadURI(CachedMatchUpload& upload, uint64_t matchID, std::string uri)
+{
+	if (matchID == 0)
+	{
+		return;
+	}
+
+	std::scoped_lock<std::mutex> ssLock(m_ScreenshotMutex);
+	upload.uriMatchID = matchID;
+	upload.signedURI = std::move(uri);
+}
+
+void NGMP_OnlineServicesManager::CacheScreenshotBytes_EndMatch(uint64_t matchID, std::vector<uint8_t> data)
+{
+	CacheMatchUploadBytes(m_cachedMatchEndUpload, matchID, std::move(data));
+}
+
+void NGMP_OnlineServicesManager::CacheReplayBytes(uint64_t matchID, std::vector<uint8_t> data)
+{
+	CacheMatchUploadBytes(m_cachedReplayUpload, matchID, std::move(data));
+}
+
+void NGMP_OnlineServicesManager::SetScreenshotS3URI_EndMatch(uint64_t matchID, std::string uri)
+{
+	CacheMatchUploadURI(m_cachedMatchEndUpload, matchID, std::move(uri));
+}
+
+void NGMP_OnlineServicesManager::SetScreenshotS3URI_Replay(uint64_t matchID, std::string uri)
+{
+	CacheMatchUploadURI(m_cachedReplayUpload, matchID, std::move(uri));
+}
+
 void NGMP_OnlineServicesManager::WaitForScreenshotThreads()
 {
 	std::scoped_lock<std::mutex> lock(m_mutexScreenshotThreads);
@@ -910,31 +963,30 @@ void NGMP_OnlineServicesManager::Tick()
 			}
 		}
 
-        if (!m_vecCachedScreenshotBytes_MatchEnd.empty()) // we have data waiting
+        if (!m_cachedMatchEndUpload.bytes.empty()) // we have data waiting
         {
-            if (!m_strCachedScreenshot_MatchEnd_S3URI.empty()) // and we have a URL
+            if (m_cachedMatchEndUpload.dataMatchID == m_cachedMatchEndUpload.uriMatchID && !m_cachedMatchEndUpload.signedURI.empty()) // and we have a matching URL
             {
                 // queue it
                 S3ScreenshotEntry newEntry;
                 newEntry.screenshotType = EScreenshotType::SCREENSHOT_TYPE_SCORESCREEN;
-                newEntry.vecBytes = m_vecCachedScreenshotBytes_MatchEnd;
-                newEntry.strSignedURI = m_strCachedScreenshot_MatchEnd_S3URI;
-				m_vecGuardedSSData.push_back(newEntry);
+                newEntry.vecBytes = std::move(m_cachedMatchEndUpload.bytes);
+                newEntry.strSignedURI = std::move(m_cachedMatchEndUpload.signedURI);
+                m_vecGuardedSSData.push_back(std::move(newEntry));
 
                 // clear data
-                m_vecCachedScreenshotBytes_MatchEnd = std::vector<uint8_t>();
-                m_strCachedScreenshot_MatchEnd_S3URI = std::string();
+                m_cachedMatchEndUpload = {};
             }
         }
 
-        if (!m_vecCachedReplayBytes.empty()) // we have data waiting
+        if (!m_cachedReplayUpload.bytes.empty()) // we have data waiting
         {
-            if (!m_strCacheReplay_S3URI.empty()) // and we have a URL
+            if (m_cachedReplayUpload.dataMatchID == m_cachedReplayUpload.uriMatchID && !m_cachedReplayUpload.signedURI.empty()) // and we have a matching URL
             {
 				// do the upload
                 std::map<std::string, std::string> mapHeaders;
                 mapHeaders["Content-Type"] = "application/octet-stream";
-                NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendS3PUTRequest(m_strCacheReplay_S3URI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, m_vecCachedReplayBytes, [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq)
+                NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendS3PUTRequest(m_cachedReplayUpload.signedURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, m_cachedReplayUpload.bytes, [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq)
                     {
 #if _DEBUG
                         if (statusCode != 200)
@@ -947,8 +999,7 @@ void NGMP_OnlineServicesManager::Tick()
                     }, nullptr, HTTP_UPLOAD_TIMEOUT);
 
                 // clear data
-                NGMP_OnlineServicesManager::GetInstance()->m_vecCachedReplayBytes.clear();
-                NGMP_OnlineServicesManager::GetInstance()->m_strCacheReplay_S3URI = std::string();
+                m_cachedReplayUpload = {};
             }
         }
 	}