From 0a59aec1ba38a528ffabf90d7dd2ad245d76d581 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Wed, 7 Oct 2026 22:37:40 +0200 Subject: [PATCH 1/9] perf(screenshot): Avoid redundant buffers and copies in screenshot uploads --- .../GeneralsOnline/OnlineServices_Init.h | 4 +- .../GeneralsOnline/HTTP/HTTPManager.cpp | 2 +- .../GeneralsOnline/OnlineServices_Init.cpp | 203 ++++++++---------- 3 files changed, 88 insertions(+), 121 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h index 7d1419d2ee1..27d5fffdfcd 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h @@ -586,10 +586,10 @@ class NGMP_OnlineServicesManager ServiceConfig& GetServiceConfig() { return m_ServiceConfig; } public: - void CacheScreenshotBytes_StartMatch(std::vector& vecData) + void CacheScreenshotBytes_StartMatch(std::vector vecData) { std::scoped_lock ssLock(m_ScreenshotMutex); - m_vecCachedScreenshotBytes_MatchStart = vecData; + m_vecCachedScreenshotBytes_MatchStart = std::move(vecData); } void CacheScreenshotBytes_EndMatch(uint64_t matchID, std::vector data); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp index a420a04cda6..9b6d94eca4c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp @@ -49,7 +49,7 @@ void HTTPManager::SendS3PUTRequest(const char* szURI, EIPProtocolVersion protove HTTPRequest* pRequest = PlatformCreateRequest(EHTTPVerb::HTTP_VERB_PUT, protover, szURI, inHeaders, completionCallback, progressCallback, timeoutMS); pRequest->DisableServiceAuth(); - pRequest->SetPostDataBuffer(vecBuffer); + pRequest->SetPostDataBuffer(std::move(vecBuffer)); m_vecRequestsPendingStart.push_back(pRequest); } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp index cfe0835efd4..951c40f6dd2 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp @@ -127,7 +127,7 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree // certain screenshots require caching for later upload when we have a valid match id, so we store them if (screenshotType == EScreenshotType::SCREENSHOT_TYPE_LOADSCREEN) { - NGMP_OnlineServicesManager::GetInstance()->CacheScreenshotBytes_StartMatch(vecData); + NGMP_OnlineServicesManager::GetInstance()->CacheScreenshotBytes_StartMatch(std::move(vecData)); } else if (screenshotType == EScreenshotType::SCREENSHOT_TYPE_SCORESCREEN) { @@ -143,7 +143,7 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree newEntry.vecBytes = std::move(vecData); newEntry.strSignedURI = strURI; newEntry.screenshotType = screenshotType; - m_vecGuardedSSData.push_back(newEntry); + m_vecGuardedSSData.push_back(std::move(newEntry)); } }); } @@ -579,7 +579,6 @@ void NGMP_OnlineServicesManager::CaptureScreenshot(bool bResizeForTransmit, std: } SurfaceClass* surface = DX8Wrapper::_Get_DX8_Back_Buffer(); - LPDIRECT3DSURFACE8 surf = nullptr; SurfaceClass* surfaceCopy = nullptr; void* pBits = nullptr; IDirect3DSurface8* pDXsurf = nullptr; @@ -599,119 +598,96 @@ void NGMP_OnlineServicesManager::CaptureScreenshot(bool bResizeForTransmit, std: { DX8Wrapper::_Copy_DX8_Rects(surface->Peek_D3D_Surface(), NULL, 0, surfaceCopy->Peek_D3D_Surface(), NULL); - HRESULT hr; + // gather all our data + int pitch = 0; + pBits = surfaceCopy->Lock(&pitch); - D3DDISPLAYMODE mode; - if (SUCCEEDED(hr = DX8Wrapper::_Get_D3D_Device8()->GetDisplayMode(&mode))) + if (pBits != nullptr) { - if (SUCCEEDED(hr = DX8Wrapper::_Get_D3D_Device8()->CreateImageSurface(mode.Width, mode.Height, - D3DFMT_A8R8G8B8, &surf))) - { - if (SUCCEEDED(hr = DX8Wrapper::_Get_D3D_Device8()->GetFrontBuffer(surf))) + int width = surfaceDesc.Width; + int height = surfaceDesc.Height; + + // Copy pixel data into an owned buffer before spawning the thread so + // the surface can be safely unlocked on the main thread immediately after. + const uint8_t* pSurfaceBytes = static_cast(pBits); + std::vector pixelData(pSurfaceBytes, pSurfaceBytes + height * pitch); + + // process on thread - track the thread so we can join it during shutdown + std::thread* pNewThread = new std::thread([cbOnDataAvailable, width, height, pixelData = std::move(pixelData), pitch, bResizeForTransmit]() mutable { - // gather all our data - int pitch = 0; - pBits = surfaceCopy->Lock(&pitch); + CHECK_WORKER_THREAD; - if (pBits != nullptr) - { - int width = surfaceDesc.Width; - int height = surfaceDesc.Height; - - // Copy pixel data into an owned buffer before spawning the thread so - // the surface can be safely unlocked on the main thread immediately after. - std::vector pixelData(height * pitch); - memcpy(pixelData.data(), pBits, height * pitch); - - // process on thread - track the thread so we can join it during shutdown - std::thread* pNewThread = new std::thread([cbOnDataAvailable, width, height, pixelData = std::move(pixelData), pitch, bResizeForTransmit]() - { - CHECK_WORKER_THREAD; - - unsigned char* rgbData = new unsigned char[width * height * 3]; - - std::vector vecData; - - int finalWidth = width; - int finalHeight = height; - - for (int y = 0; y < height; ++y) { - const uint8_t* row = pixelData.data() + y * pitch; - int rowOffset = y * width * 3; - int srcOffset = 0; - for (int x = 0; x < width; ++x, srcOffset += 4) - { - int dstIndex = rowOffset + x * 3; - rgbData[dstIndex + 0] = row[srcOffset + 2]; // R - rgbData[dstIndex + 1] = row[srcOffset + 1]; // G - rgbData[dstIndex + 2] = row[srcOffset + 0]; // B - } - } - - // resize - unsigned char* pBufferToWrite = rgbData; - if (bResizeForTransmit) - { - ServiceConfig& serviceConf = NGMP_OnlineServicesManager::GetInstance()->GetServiceConfig(); - int new_width = serviceConf.screenshot_width; - int new_height = serviceConf.screenshot_height; - int channels = 3; - unsigned char* resized = new unsigned char[new_width * new_height * channels]; - - stbir_resize_uint8(rgbData, width, height, 0, - resized, new_width, new_height, 0, - channels - ); - - // update data - finalWidth = new_width; - finalHeight = new_height; - pBufferToWrite = resized; - } - // end resize - - stbi_write_jpg_to_func([](void* context, void* data, int size) - { - std::vector* buffer = static_cast*>(context); - buffer->insert(buffer->end(), (unsigned char*)data, (unsigned char*)data + size); - }, &vecData, finalWidth, finalHeight, 3, pBufferToWrite, bResizeForTransmit ? 0 : 90); - - // cleanup - if (bResizeForTransmit) - { - delete[] pBufferToWrite; // This is 'resized' - pBufferToWrite = nullptr; - } - - delete[] rgbData; - rgbData = nullptr; - - // invoke cb - if (cbOnDataAvailable != nullptr) - { - cbOnDataAvailable(vecData); - } - } - ); + const int channels = 3; + int finalWidth = width; + int finalHeight = height; - // Store the thread so we can join it during shutdown - // SECURITY FIX: Capture manager pointer before spawning thread to avoid TOCTOU race - NGMP_OnlineServicesManager* pMgr = NGMP_OnlineServicesManager::GetInstance(); - if (pMgr != nullptr) + // Pack the BGRA rows to tight RGB in place. The write position never passes the read position. + uint8_t* pixels = pixelData.data(); + for (int y = 0; y < height; ++y) { - std::scoped_lock lock(pMgr->m_mutexScreenshotThreads); - pMgr->m_vecScreenshotThreads.push_back(pNewThread); + const uint8_t* src = pixels + y * pitch; + uint8_t* dst = pixels + y * width * channels; + for (int x = 0; x < width; ++x, src += 4, dst += channels) + { + const uint8_t blue = src[0]; + dst[0] = src[2]; // R + dst[1] = src[1]; // G + dst[2] = blue; // B + } } - else + + // resize + std::vector resized; + if (bResizeForTransmit) { - // Manager was destroyed, cannot store thread. Thread will leak but won't crash. - NetworkLog(ELogVerbosity::LOG_RELEASE, "[Screenshot] Manager destroyed before thread could be registered"); + ServiceConfig& serviceConf = NGMP_OnlineServicesManager::GetInstance()->GetServiceConfig(); + finalWidth = serviceConf.screenshot_width; + finalHeight = serviceConf.screenshot_height; + resized.resize(finalWidth * finalHeight * channels); + + stbir_resize_uint8(pixels, width, height, 0, + resized.data(), finalWidth, finalHeight, 0, + channels + ); + + // the capture is no longer needed + std::vector().swap(pixelData); } + // end resize + + // room for a typical JPEG, so the write callback rarely has to grow it + std::vector vecData; + vecData.reserve(256 * 1024); + + stbi_write_jpg_to_func([](void* context, void* data, int size) + { + std::vector* buffer = static_cast*>(context); + buffer->insert(buffer->end(), (unsigned char*)data, (unsigned char*)data + size); + }, &vecData, finalWidth, finalHeight, channels, bResizeForTransmit ? resized.data() : pixelData.data(), bResizeForTransmit ? 0 : 90); - bSucceeded = true; + // invoke cb + if (cbOnDataAvailable != nullptr) + { + cbOnDataAvailable(std::move(vecData)); } } + ); + + // Store the thread so we can join it during shutdown + // SECURITY FIX: Capture manager pointer before spawning thread to avoid TOCTOU race + NGMP_OnlineServicesManager* pMgr = NGMP_OnlineServicesManager::GetInstance(); + if (pMgr != nullptr) + { + std::scoped_lock lock(pMgr->m_mutexScreenshotThreads); + pMgr->m_vecScreenshotThreads.push_back(pNewThread); + } + else + { + // Manager was destroyed, cannot store thread. Thread will leak but won't crash. + NetworkLog(ELogVerbosity::LOG_RELEASE, "[Screenshot] Manager destroyed before thread could be registered"); } + + bSucceeded = true; } } } @@ -719,18 +695,9 @@ void NGMP_OnlineServicesManager::CaptureScreenshot(bool bResizeForTransmit, std: // clean everything up, whether we succeeded or not - // release the image surface - if (surf != nullptr) - { - surf->Release(); - //delete surf; - surf = nullptr; - } - // unlock if (surface != nullptr) { - surface->Unlock(); surface->Release_Ref(); surface = nullptr; } @@ -954,7 +921,7 @@ void NGMP_OnlineServicesManager::Tick() // screenshot types that already have a presigned URL - for (S3ScreenshotEntry screenshotEntry : m_vecGuardedSSData) + for (S3ScreenshotEntry& screenshotEntry : m_vecGuardedSSData) { // NOTE: Screenshot types start and end of match are captures and cached in memory until the server tells us where to upload them, so we need to wait for the upload URI @@ -966,7 +933,7 @@ void NGMP_OnlineServicesManager::Tick() { std::map mapHeaders; mapHeaders["Content-Type"] = "image/jpeg"; - NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendS3PUTRequest(screenshotEntry.strSignedURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, screenshotEntry.vecBytes, [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq) + NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendS3PUTRequest(screenshotEntry.strSignedURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, std::move(screenshotEntry.vecBytes), [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq) { #if _DEBUG if (statusCode != 200) @@ -990,9 +957,9 @@ void NGMP_OnlineServicesManager::Tick() // queue it S3ScreenshotEntry newEntry; newEntry.screenshotType = EScreenshotType::SCREENSHOT_TYPE_LOADSCREEN; - newEntry.vecBytes = m_vecCachedScreenshotBytes_MatchStart; - newEntry.strSignedURI = m_strCachedScreenshot_MatchStart_S3URI; - m_vecGuardedSSData.push_back(newEntry); + newEntry.vecBytes = std::move(m_vecCachedScreenshotBytes_MatchStart); + newEntry.strSignedURI = std::move(m_strCachedScreenshot_MatchStart_S3URI); + m_vecGuardedSSData.push_back(std::move(newEntry)); // clear data m_vecCachedScreenshotBytes_MatchStart = std::vector(); @@ -1023,7 +990,7 @@ void NGMP_OnlineServicesManager::Tick() // do the upload std::map mapHeaders; mapHeaders["Content-Type"] = "application/octet-stream"; - 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) + NGMP_OnlineServicesManager::GetInstance()->GetHTTPManager()->SendS3PUTRequest(m_cachedReplayUpload.signedURI.c_str(), EIPProtocolVersion::DONT_CARE, mapHeaders, std::move(m_cachedReplayUpload.bytes), [=](bool bSuccess, int statusCode, std::string strBody, HTTPRequest* pReq) { #if _DEBUG if (statusCode != 200) From 4e21dabadfc6e22df9d19667e2bc7f9d05e22e6d Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:32:41 +0100 Subject: [PATCH 2/9] perf(screenshot): Upload probe screenshots one slot at a time at a capped rate --- .../GeneralsOnline/HTTP/HTTPManager.h | 2 +- .../GeneralsOnline/HTTP/HTTPRequest.h | 7 +++ .../GeneralsOnline/OnlineServices_Init.h | 6 +- .../GeneralsOnline/HTTP/HTTPManager.cpp | 3 +- .../GeneralsOnline/HTTP/HTTPRequest.cpp | 5 ++ .../GeneralsOnline/OnlineServices_Init.cpp | 60 +++++++++++++++++-- 6 files changed, 74 insertions(+), 9 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPManager.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPManager.h index 57a4c498928..d2d5c80f753 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPManager.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPManager.h @@ -54,7 +54,7 @@ class HTTPManager // NOTE: set bDisableServiceAuth for endpoints that must be called with something other than the session token (e.g. the refresh endpoint, which needs the refresh token), otherwise the session token overwrites any Authorization header passed in inHeaders void SendPOSTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, const char* szPostData, std::function completionCallback, std::function progressCallback = nullptr, int timeoutMS = -1, bool bDisableServiceAuth = false); void SendPUTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, const char* szData, std::function completionCallback, std::function progressCallback = nullptr, int timeoutMS = -1); - void SendS3PUTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, std::vector vecBuffer, std::function completionCallback, std::function progressCallback = nullptr, int timeoutMS = -1); + void SendS3PUTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, std::vector vecBuffer, std::function completionCallback, std::function progressCallback = nullptr, int timeoutMS = -1, int maxSendBytesPerSec = 0); void SendDELETERequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, const char* szData, std::function completionCallback, std::function progressCallback = nullptr, int timeoutMS = -1); void Shutdown(); diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.h index f5b901aade5..7dbf977a136 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.h @@ -36,6 +36,11 @@ class HTTPRequest m_bAppendAuthIfPresent = false; } + void SetMaxSendSpeed(int bytesPerSec) + { + m_maxSendBytesPerSec = bytesPerSec; + } + void OnResponsePartialWrite(std::uint8_t* pBuffer, size_t numBytes); bool HasStarted() const { return m_bIsStarted; } @@ -77,6 +82,8 @@ class HTTPRequest bool m_bAppendAuthIfPresent = true; + int m_maxSendBytesPerSec = 0; // 0 = unlimited + EIPProtocolVersion m_protover; int m_timeoutMS = 5000; diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h index 27d5fffdfcd..2096e7b3623 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h @@ -4,6 +4,7 @@ #include #include +#include class HTTPManager; @@ -28,6 +29,7 @@ struct S3ScreenshotEntry std::vector vecBytes; std::string strSignedURI; EScreenshotType screenshotType; + std::chrono::steady_clock::time_point notBefore; // upload is held back until this time bool operator==(const S3ScreenshotEntry& other) const { @@ -341,6 +343,8 @@ struct ServiceConfig int screenshot_width = 557; int screenshot_height = 333; + int screenshot_upload_bytes_per_sec = 131072; // 128 KB/s + int screenshot_slot_offset_ms = 2000; // ICE client used for P2P NAT traversal: 0 = library default, 1 = native, 2 = WebRTC. int ice_implementation = 2; @@ -351,7 +355,7 @@ struct ServiceConfig ra_slack_percent, frame_grouping_frames, enable_host_migration, network_do_immediate_flush_per_frame, network_send_flags, network_latency_logic_model, use_default_config, ra_slack_override_percent_in_default, do_probes, do_replay_upload, network_mesh_histogram_duration, ibra_ra_tweaks, ibra_minslack_default, ibra_maxslack_default, ibra_minslack_greaterthan300ms, ibra_maxslack_greaterthan300ms, ibra_minslack_greaterthan200ms, ibra_maxslack_greaterthan200ms, - screenshot_width, screenshot_height, ice_implementation, use_new_networking) + screenshot_width, screenshot_height, screenshot_upload_bytes_per_sec, screenshot_slot_offset_ms, ice_implementation, use_new_networking) }; class NGMP_OnlineServicesManager diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp index 9b6d94eca4c..e476ced3bb7 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp @@ -43,12 +43,13 @@ void HTTPManager::SendPUTRequest(const char* szURI, EIPProtocolVersion protover, } -void HTTPManager::SendS3PUTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, std::vector vecBuffer, std::function completionCallback, std::function progressCallback /*= nullptr*/, int timeoutMS /*= -1*/) +void HTTPManager::SendS3PUTRequest(const char* szURI, EIPProtocolVersion protover, std::map& inHeaders, std::vector vecBuffer, std::function completionCallback, std::function progressCallback /*= nullptr*/, int timeoutMS /*= -1*/, int maxSendBytesPerSec /*= 0*/) { CHECK_MAIN_THREAD; HTTPRequest* pRequest = PlatformCreateRequest(EHTTPVerb::HTTP_VERB_PUT, protover, szURI, inHeaders, completionCallback, progressCallback, timeoutMS); pRequest->DisableServiceAuth(); + pRequest->SetMaxSendSpeed(maxSendBytesPerSec); pRequest->SetPostDataBuffer(std::move(vecBuffer)); m_vecRequestsPendingStart.push_back(pRequest); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.cpp index 02786d7f3d9..3c77e8f4039 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPRequest.cpp @@ -298,6 +298,11 @@ void HTTPRequest::PlatformStartRequest() } } + if (m_maxSendBytesPerSec > 0) + { + curl_easy_setopt(m_pCURL, CURLOPT_MAX_SEND_SPEED_LARGE, (curl_off_t)m_maxSendBytesPerSec); + } + // needed for PUT etc if (m_httpVerb == EHTTPVerb::HTTP_VERB_PUT) { diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp index 951c40f6dd2..b2fa378fdb1 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp @@ -23,7 +23,10 @@ #include "GameNetwork/GeneralsOnline/Vendor/stb_image/stb_image_write.h" #include "GameNetwork/GeneralsOnline/Vendor/stb_image/stb_image_resize.h" #include "GameClient/GameText.h" +#include "GameLogic/GameLogic.h" #include +#include +#include extern "C" { @@ -99,6 +102,34 @@ void NGMP_OnlineServicesManager::GetAndParseServiceConfig(std::function(); + NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); + if (pAuthInterface != nullptr && pLobbyInterface != nullptr) + { + const int64_t localUserID = pAuthInterface->GetUserID(); + for (const LobbyMemberEntry& lobbyMember : pLobbyInterface->GetMembersListForCurrentRoom()) + { + if (lobbyMember.user_id == localUserID) + { + slotIndex = lobbyMember.m_SlotIndex < MAX_SLOTS ? lobbyMember.m_SlotIndex : 0; + break; + } + } + } + + const int slotOffsetMS = std::clamp(NGMP_OnlineServicesManager::GetInstance()->GetServiceConfig().screenshot_slot_offset_ms, 0, 10000); + + static thread_local std::mt19937 rng{ std::random_device{}() }; + const int jitterMS = std::uniform_int_distribution(0, 500)(rng); + + return std::chrono::steady_clock::now() + std::chrono::milliseconds(slotIndex * slotOffsetMS + jitterMS); +} + void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType screenshotType, std::string strURI) { NGMP_OnlineServicesManager* pOnlineServicesMgr = NGMP_OnlineServicesManager::GetInstance(); @@ -114,7 +145,8 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree if (pLobbyInterface != nullptr) { const uint64_t matchID = pLobbyInterface->GetCurrentMatchID(); - NGMP_OnlineServicesManager::GetInstance()->CaptureScreenshot(true, [strURI = std::move(strURI), screenshotType, matchID](std::vector vecData) + const std::chrono::steady_clock::time_point uploadTime = GetScreenshotUploadTime(); + NGMP_OnlineServicesManager::GetInstance()->CaptureScreenshot(true, [strURI = std::move(strURI), screenshotType, matchID, uploadTime](std::vector vecData) { CHECK_WORKER_THREAD; @@ -143,6 +175,7 @@ void NGMP_OnlineServicesManager::CaptureScreenshotForProbe(EScreenshotType scree newEntry.vecBytes = std::move(vecData); newEntry.strSignedURI = strURI; newEntry.screenshotType = screenshotType; + newEntry.notBefore = uploadTime; m_vecGuardedSSData.push_back(std::move(newEntry)); } }); @@ -920,13 +953,28 @@ void NGMP_OnlineServicesManager::Tick() std::scoped_lock ssLock(m_ScreenshotMutex); + const int uploadBytesPerSec = std::clamp(m_ServiceConfig.screenshot_upload_bytes_per_sec, 16384, 10485760); + const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); + const bool bMatchOver = TheGameLogic == nullptr || !TheGameLogic->isInMultiplayerGame(); // uploads no longer compete with game traffic + // screenshot types that already have a presigned URL - for (S3ScreenshotEntry& screenshotEntry : m_vecGuardedSSData) + for (auto it = m_vecGuardedSSData.begin(); it != m_vecGuardedSSData.end();) { + S3ScreenshotEntry& screenshotEntry = *it; + const bool bUnthrottled = bMatchOver || screenshotEntry.screenshotType == EScreenshotType::SCREENSHOT_TYPE_SCORESCREEN; + + // not this client's turn to upload yet + if (!bUnthrottled && screenshotEntry.notBefore > now) + { + ++it; + continue; + } + // NOTE: Screenshot types start and end of match are captures and cached in memory until the server tells us where to upload them, so we need to wait for the upload URI if ((screenshotEntry.screenshotType == EScreenshotType::SCREENSHOT_TYPE_LOADSCREEN || screenshotEntry.screenshotType == EScreenshotType::SCREENSHOT_TYPE_SCORESCREEN) && screenshotEntry.strSignedURI.empty()) { + it = m_vecGuardedSSData.erase(it); continue; } else // we have the data we need, send away @@ -942,12 +990,11 @@ void NGMP_OnlineServicesManager::Tick() } #endif NetworkLog(ELogVerbosity::LOG_RELEASE, "Screenshot upload, result: %d", statusCode); - }, nullptr, HTTP_UPLOAD_TIMEOUT); + }, nullptr, HTTP_UPLOAD_TIMEOUT, bUnthrottled ? 0 : uploadBytesPerSec); } - - } - m_vecGuardedSSData.clear(); + it = m_vecGuardedSSData.erase(it); + } // screenshots and replays that are cached and awaiting S3 url, and now have said URL if (!m_vecCachedScreenshotBytes_MatchStart.empty()) // we have data waiting @@ -959,6 +1006,7 @@ void NGMP_OnlineServicesManager::Tick() newEntry.screenshotType = EScreenshotType::SCREENSHOT_TYPE_LOADSCREEN; newEntry.vecBytes = std::move(m_vecCachedScreenshotBytes_MatchStart); newEntry.strSignedURI = std::move(m_strCachedScreenshot_MatchStart_S3URI); + newEntry.notBefore = GetScreenshotUploadTime(); m_vecGuardedSSData.push_back(std::move(newEntry)); // clear data From 56aa7643ac9ffa47926ee5768c119f8e930599ec Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:26:08 +0200 Subject: [PATCH 3/9] fix(mesh): Accept signalling only for the lobby we are in and for connections the server asked for --- .../GeneralsOnline/OnlineServices_Init.h | 7 +++++ .../GeneralsOnline/NetworkMesh.cpp | 13 ++------ .../OnlineServices_LobbyInterface.cpp | 7 +++++ .../OnlineServices_RoomsInterface.cpp | 30 ++++++++++++------- 4 files changed, 36 insertions(+), 21 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h index 2096e7b3623..5f602cbbd6c 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h @@ -217,6 +217,13 @@ class WebSocket m_pendingSignals.push(std::move(payload)); } + // signals queued for a lobby we've since left must not reach the next one + void ClearPendingSignals() + { + std::scoped_lock lock(m_pendingSignalsMutex); + m_pendingSignals = std::queue>(); + } + std::queue> DrainPendingSignals() { std::scoped_lock lock(m_pendingSignalsMutex); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp index 5957924e4c2..81e955534db 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp @@ -266,23 +266,16 @@ void OnSteamNetConnectionStatusChanged(SteamNetConnectionStatusChangedCallback_t } } + // the server started signalling with this peer, so accept it even if our roster is stale if (bPlayerIsInLobby) { NetworkLog(ELogVerbosity::LOG_RELEASE, "[STEAM NETWORKING][%s] Accepting - Player is in lobby\n", pInfo->m_info.m_szConnectionDescription); - SteamNetworkingSockets()->AcceptConnection(pInfo->m_hConn); } else { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[STEAM NETWORKING][%s] Rejecting - Player is not in lobby\n", pInfo->m_info.m_szConnectionDescription); - - NetworkLog(ELogVerbosity::LOG_RELEASE, "[DC] Closing connection not in lobby %lld", plrConnection.m_userID); - SteamNetworkingSockets()->CloseConnection(pInfo->m_hConn, 1000, "Player is not in lobby (Rejected)", false); - - if (TheNetwork != nullptr) - { - TheNetwork->GetConnectionManager()->disconnectPlayer(plrConnection.m_userID); - } + NetworkLog(ELogVerbosity::LOG_RELEASE, "[STEAM NETWORKING][%s] Accepting - Server started signalling, roster not refreshed yet\n", pInfo->m_info.m_szConnectionDescription); } + SteamNetworkingSockets()->AcceptConnection(pInfo->m_hConn); } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index 4b5b9da9cbc..d0a044cb280 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -1375,6 +1375,7 @@ void NGMP_OnlineServices_LobbyInterface::LeaveCurrentLobby() if (pWS != nullptr) { pWS->ClearConnectivityCheckCallback(); + pWS->ClearPendingSignals(); } // reset host migration flags @@ -1425,6 +1426,12 @@ void NGMP_OnlineServices_LobbyInterface::ResetForMatchmakingRequeue() ResetHostMigrationFlags(); AnticheatPlugInterface::EndSession(); + std::shared_ptr pWS = NGMP_OnlineServicesManager::GetWebSocket(); + if (pWS != nullptr) + { + pWS->ClearPendingSignals(); + } + if (m_pLobbyMesh != nullptr) { m_pLobbyMesh->Disconnect(); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp index 07e13867223..dc05e987f9c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp @@ -1294,22 +1294,30 @@ void WebSocket::Tick() WebSocketMessage_NetworkStartSignalling startSignallingData; bool bParsed = JSONGetAsObject(jsonObject, &startSignallingData); - // TODO_NGMP: Better location for this - // When we find a new player, get their latest stats. Tooltip and loading screen need it, so we'll grab it now and then use cached data later since it cannot possibly change while in a lobby - NGMP_OnlineServices_StatsInterface* pStatsInterface = NGMP_OnlineServicesManager::GetInterface(); - if (pStatsInterface != nullptr) - { - pStatsInterface->findPlayerStatsByID(startSignallingData.user_id, [=](bool bSuccess, PSPlayerStats stats) - { - - }, EStatsRequestPolicy::BYPASS_CACHE_FORCE_REQUEST); - } - if (bParsed) { NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); if (pLobbyInterface != nullptr) { + // -1 while a join is still in flight, when this can legitimately arrive first + const int64_t currentLobbyID = pLobbyInterface->GetCurrentLobby().lobbyID; + if (currentLobbyID != -1 && currentLobbyID != startSignallingData.lobby_id) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NETWORK_CONNECTION_START_SIGNALLING] Ignoring signalling for lobby %lld, we are in lobby %lld", startSignallingData.lobby_id, currentLobbyID); + break; + } + + // TODO_NGMP: Better location for this + // When we find a new player, get their latest stats. Tooltip and loading screen need it, so we'll grab it now and then use cached data later since it cannot possibly change while in a lobby + NGMP_OnlineServices_StatsInterface* pStatsInterface = NGMP_OnlineServicesManager::GetInterface(); + if (pStatsInterface != nullptr) + { + pStatsInterface->findPlayerStatsByID(startSignallingData.user_id, [=](bool bSuccess, PSPlayerStats stats) + { + + }, EStatsRequestPolicy::BYPASS_CACHE_FORCE_REQUEST); + } + NetworkMesh* pMesh = pLobbyInterface->GetNetworkMeshForLobby(); if (pMesh != nullptr) From ad30d3233e2f89f61cde6b8075a15b960b20f2d2 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:26:09 +0200 Subject: [PATCH 4/9] fix(online): Parse server JSON and log anticheat ids without undefined behaviour --- .../GameNetwork/GeneralsOnline/JsonHelpers.h | 21 +++ .../OnlineServices_LobbyInterface.cpp | 146 +++++++++--------- .../OnlineServices_MatchmakingInterface.cpp | 34 ++-- .../OnlineServices_RoomsInterface.cpp | 4 +- 4 files changed, 121 insertions(+), 84 deletions(-) create mode 100644 GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/JsonHelpers.h diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/JsonHelpers.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/JsonHelpers.h new file mode 100644 index 00000000000..7de223155b2 --- /dev/null +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/JsonHelpers.h @@ -0,0 +1,21 @@ +#pragma once + +#include "GameNetwork/GeneralsOnline/json.hpp" + +// Optional field: dest keeps its default when the key is missing or null +template +inline void JsonGetOptional(const nlohmann::json& j, const char* szKey, T& dest) +{ + auto it = j.find(szKey); + if (it != j.end() && !it->is_null()) + { + it->get_to(dest); + } +} + +// Required field: throws when the key is missing or the wrong type +template +inline void JsonGetRequired(const nlohmann::json& j, const char* szKey, T& dest) +{ + j.at(szKey).get_to(dest); +} diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index d0a044cb280..43149488b2c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -1,5 +1,6 @@ #include "GameNetwork/GeneralsOnline/NGMP_interfaces.h" #include "GameNetwork/GeneralsOnline/json.hpp" +#include "GameNetwork/GeneralsOnline/JsonHelpers.h" #include "GameNetwork/GeneralsOnline/HTTP/HTTPManager.h" #include "GameNetwork/GeneralsOnline/OnlineServices_Init.h" #include "GameClient/MapUtil.h" @@ -552,7 +553,7 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function std::vector vecLatencies; std::map mapPlayerLatencies; - jsonObject["latencies"].get_to(vecLatencies); + JsonGetOptional(jsonObject, "latencies", vecLatencies); // player latencies for (const auto& playerLatencyEntryIter : jsonObject["playerlatencies"]) @@ -560,8 +561,8 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function int64_t user_id = -1; int latency = -1; - playerLatencyEntryIter["user_id"].get_to(user_id); - playerLatencyEntryIter["latency"].get_to(latency); + JsonGetOptional(playerLatencyEntryIter, "user_id", user_id); + JsonGetOptional(playerLatencyEntryIter, "latency", latency); if (user_id != -1 && latency != -1) { @@ -572,41 +573,43 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function int latencyIndex = 0; for (const auto& lobbyEntryIter : jsonObject["lobbies"]) { + // latencies are indexed by the server's lobby order, so count skipped entries too + const size_t entryLatencyIndex = latencyIndex++; + try + { LobbyEntry lobbyEntry; - lobbyEntryIter["LobbyID"].get_to(lobbyEntry.lobbyID); - lobbyEntryIter["Owner"].get_to(lobbyEntry.owner); - lobbyEntryIter["Name"].get_to(lobbyEntry.name); - lobbyEntryIter["MapName"].get_to(lobbyEntry.map_name); - lobbyEntryIter["MapPath"].get_to(lobbyEntry.map_path); + JsonGetRequired(lobbyEntryIter, "LobbyID", lobbyEntry.lobbyID); + JsonGetRequired(lobbyEntryIter, "Owner", lobbyEntry.owner); + JsonGetOptional(lobbyEntryIter, "Name", lobbyEntry.name); + JsonGetOptional(lobbyEntryIter, "MapName", lobbyEntry.map_name); + JsonGetOptional(lobbyEntryIter, "MapPath", lobbyEntry.map_path); lobbyEntry.map_path = utf8_to_local(lobbyEntry.map_path); // local file path from here on - lobbyEntryIter["IsMapOfficial"].get_to(lobbyEntry.map_official); - lobbyEntryIter["NumCurrentPlayers"].get_to(lobbyEntry.current_players); - lobbyEntryIter["MaxPlayers"].get_to(lobbyEntry.max_players); - lobbyEntryIter["IsVanillaTeamsOnly"].get_to(lobbyEntry.vanilla_teams); - lobbyEntryIter["StartingCash"].get_to(lobbyEntry.starting_cash); - lobbyEntryIter["IsLimitSuperweapons"].get_to(lobbyEntry.limit_superweapons); - lobbyEntryIter["IsTrackingStats"].get_to(lobbyEntry.track_stats); - lobbyEntryIter["IsPassworded"].get_to(lobbyEntry.passworded); - lobbyEntryIter["AllowObservers"].get_to(lobbyEntry.allow_observers); - lobbyEntryIter["MaximumCameraHeight"].get_to(lobbyEntry.max_cam_height); - lobbyEntryIter["ExeCRC"].get_to(lobbyEntry.exe_crc); - lobbyEntryIter["IniCRC"].get_to(lobbyEntry.ini_crc); - lobbyEntryIter["MatchID"].get_to(lobbyEntry.match_id); - lobbyEntryIter["LobbyType"].get_to(lobbyEntry.lobby_type); - lobbyEntryIter["Region"].get_to(lobbyEntry.region); + JsonGetOptional(lobbyEntryIter, "IsMapOfficial", lobbyEntry.map_official); + JsonGetOptional(lobbyEntryIter, "NumCurrentPlayers", lobbyEntry.current_players); + JsonGetOptional(lobbyEntryIter, "MaxPlayers", lobbyEntry.max_players); + JsonGetOptional(lobbyEntryIter, "IsVanillaTeamsOnly", lobbyEntry.vanilla_teams); + JsonGetOptional(lobbyEntryIter, "StartingCash", lobbyEntry.starting_cash); + JsonGetOptional(lobbyEntryIter, "IsLimitSuperweapons", lobbyEntry.limit_superweapons); + JsonGetOptional(lobbyEntryIter, "IsTrackingStats", lobbyEntry.track_stats); + JsonGetOptional(lobbyEntryIter, "IsPassworded", lobbyEntry.passworded); + JsonGetOptional(lobbyEntryIter, "AllowObservers", lobbyEntry.allow_observers); + JsonGetOptional(lobbyEntryIter, "MaximumCameraHeight", lobbyEntry.max_cam_height); + JsonGetOptional(lobbyEntryIter, "ExeCRC", lobbyEntry.exe_crc); + JsonGetOptional(lobbyEntryIter, "IniCRC", lobbyEntry.ini_crc); + JsonGetOptional(lobbyEntryIter, "MatchID", lobbyEntry.match_id); + JsonGetOptional(lobbyEntryIter, "LobbyType", lobbyEntry.lobby_type); + JsonGetOptional(lobbyEntryIter, "Region", lobbyEntry.region); // attach latency - if (latencyIndex < vecLatencies.size()) + if (entryLatencyIndex < vecLatencies.size()) { - lobbyEntry.latency = vecLatencies[latencyIndex]; + lobbyEntry.latency = vecLatencies[entryLatencyIndex]; } else { // dummy value lobbyEntry.latency = 9001; } - ++latencyIndex; - // correct map path if (lobbyEntry.map_official) { @@ -620,16 +623,16 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function // NOTE: These fields won't be present becauase they're private properties //memberEntryIter["enc_key"].get_to(strEncKey); - for (const auto& memberEntryIter : lobbyEntryIter["Members"]) + for (const auto& memberEntryIter : lobbyEntryIter.at("Members")) { LobbyMemberEntry memberEntry; - memberEntryIter["UserID"].get_to(memberEntry.user_id); - memberEntryIter["DisplayName"].get_to(memberEntry.display_name); - memberEntryIter["IsReady"].get_to(memberEntry.m_bIsReady); - memberEntryIter["SlotIndex"].get_to(memberEntry.m_SlotIndex); - memberEntryIter["SlotState"].get_to(memberEntry.m_SlotState); - memberEntryIter["Region"].get_to(memberEntry.region); + JsonGetRequired(memberEntryIter, "UserID", memberEntry.user_id); + JsonGetOptional(memberEntryIter, "DisplayName", memberEntry.display_name); + JsonGetOptional(memberEntryIter, "IsReady", memberEntry.m_bIsReady); + JsonGetOptional(memberEntryIter, "SlotIndex", memberEntry.m_SlotIndex); + JsonGetOptional(memberEntryIter, "SlotState", memberEntry.m_SlotState); + JsonGetOptional(memberEntryIter, "Region", memberEntry.region); // store latency if (mapPlayerLatencies.contains(memberEntry.user_id)) @@ -645,6 +648,11 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function } parsedLobbies.push_back(std::move(lobbyEntry)); + } + catch (...) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Skipping malformed lobby list entry"); + } } m_vecLobbies = std::move(parsedLobbies); bSearchSucceeded = true; @@ -910,28 +918,28 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::function vecPlaylists; for (const auto& playlistEntryIter : jsonObject["playlists"]) { + try + { PlaylistEntry playlistEntry; - playlistEntryIter["PlaylistID"].get_to(playlistEntry.PlaylistID); - playlistEntryIter["Name"].get_to(playlistEntry.Name); - playlistEntryIter["MinPlayers"].get_to(playlistEntry.MinPlayers); - playlistEntryIter["DesiredPlayers"].get_to(playlistEntry.DesiredPlayers); - playlistEntryIter["MinSelectedMaps"].get_to(playlistEntry.MinSelectedMaps); - playlistEntryIter["AllowTeams"].get_to(playlistEntry.AllowTeams); - playlistEntryIter["TeamSize"].get_to(playlistEntry.TeamSize); - playlistEntryIter["AllowArmySelection"].get_to(playlistEntry.AllowArmySelection); - playlistEntryIter["GracePeriodAtMinPlayersMSec"].get_to(playlistEntry.GracePeriodAtMinPlayersMSec); + JsonGetRequired(playlistEntryIter, "PlaylistID", playlistEntry.PlaylistID); + JsonGetOptional(playlistEntryIter, "Name", playlistEntry.Name); + JsonGetOptional(playlistEntryIter, "MinPlayers", playlistEntry.MinPlayers); + JsonGetOptional(playlistEntryIter, "DesiredPlayers", playlistEntry.DesiredPlayers); + JsonGetOptional(playlistEntryIter, "MinSelectedMaps", playlistEntry.MinSelectedMaps); + JsonGetOptional(playlistEntryIter, "AllowTeams", playlistEntry.AllowTeams); + JsonGetOptional(playlistEntryIter, "TeamSize", playlistEntry.TeamSize); + JsonGetOptional(playlistEntryIter, "AllowArmySelection", playlistEntry.AllowArmySelection); + JsonGetOptional(playlistEntryIter, "GracePeriodAtMinPlayersMSec", playlistEntry.GracePeriodAtMinPlayersMSec); // maps - for (const auto& mapEntryIter : playlistEntryIter["Maps"]) + for (const auto& mapEntryIter : playlistEntryIter.at("Maps")) { PlaylistMapEntry mapEntry; - mapEntryIter["Name"].get_to(mapEntry.Name); - mapEntryIter["Path"].get_to(mapEntry.Path); + JsonGetOptional(mapEntryIter, "Name", mapEntry.Name); + JsonGetRequired(mapEntryIter, "Path", mapEntry.Path); mapEntry.Path = utf8_to_local(mapEntry.Path); // used as a local file path - mapEntryIter["Custom"].get_to(mapEntry.Custom); + JsonGetOptional(mapEntryIter, "Custom", mapEntry.Custom); playlistEntry.Maps.push_back(mapEntry); } vecPlaylists.push_back(playlistEntry); + } + catch (...) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Skipping malformed playlist entry"); + } } // cache diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp index dc05e987f9c..e622a13b659 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp @@ -1347,7 +1347,7 @@ void WebSocket::Tick() if (bParsed) { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] Websocket AC_REGISTER_PLAYER for %lld and %s", acData.user_id, acData.mwid); + NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] Websocket AC_REGISTER_PLAYER for %lld and %s", acData.user_id, acData.mwid.c_str()); if (!AnticheatPlugInterface::RegisterPlayer(acData.mwid, acData.user_id)) { NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] AnticheatPlugInterface::RegisterPlayer failed"); @@ -1363,7 +1363,7 @@ void WebSocket::Tick() if (bParsed) { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] Websocket AC_DEREGISTER_PLAYER for %lld and %s", acData.user_id, acData.mwid); + NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] Websocket AC_DEREGISTER_PLAYER for %lld and %s", acData.user_id, acData.mwid.c_str()); if (!AnticheatPlugInterface::DeregisterPlayer(acData.mwid, acData.user_id)) { NetworkLog(ELogVerbosity::LOG_RELEASE, "[AC] AnticheatPlugInterface::DeregisterPlayer failed"); From 709449b04b94372deb65ddcd151afbb6493bb332 Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:26:10 +0200 Subject: [PATCH 5/9] fix(lobby): Keep the lobby flow consistent across stale responses, exits and the start check --- .../GeneralsOnline/OnlineServices_Init.h | 4 ++ .../OnlineServices_LobbyInterface.h | 4 ++ .../GUICallbacks/Menus/WOLGameSetupMenu.cpp | 5 +- .../GeneralsOnline/OnlineServices_Init.cpp | 1 + .../OnlineServices_LobbyInterface.cpp | 66 +++++++++++++++++-- .../OnlineServices_RoomsInterface.cpp | 10 ++- 6 files changed, 83 insertions(+), 7 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h index 5f602cbbd6c..d755a004041 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h @@ -187,12 +187,16 @@ class WebSocket // lobby ID the check was started for; a stale reply (different lobby) is ignored int64_t m_connectivityCheckLobbyID = -1; + // when the check was sent; Tick() gives up on it after a client-side timeout + int64_t m_connectivityCheckStartMs = -1; + void SendData_StartFullMeshConnectivityCheck(std::function>, std::string)> cbOnConnectivityCheckComplete); void ClearConnectivityCheckCallback() { m_cbOnConnectivityCheckComplete = nullptr; m_connectivityCheckLobbyID = -1; + m_connectivityCheckStartMs = -1; } void Tick(); diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h index 5e89ed8d313..d852d5bf315 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h @@ -551,6 +551,10 @@ class NGMP_OnlineServices_LobbyInterface #endif bool m_bAttemptingToJoinLobby = false; + + // GET Lobby/{id} responses can arrive out of order; only apply ones newer than the last applied + std::atomic m_LobbyUpdateRequestSeq = 0; + std::atomic m_LobbyUpdateAppliedSeq = 0; // Invalidates asynchronous callbacks left behind by an abandoned lobby join. std::atomic m_LobbyJoinGeneration = 0; LobbyEntry m_LobbyTryingToJoin; diff --git a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp index d8c0b2c2eba..4faf834c161 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp @@ -1223,7 +1223,10 @@ static void StartPressed() // who is missing who? //std::list> missingConnections - GadgetListBoxAddEntryText(listboxGameSetupChat, UnicodeString(L"Connections: Missing links:"), GameMakeColor(255, 194, 15, 255), -1, -1); + if (!missingConnections.empty()) + { + GadgetListBoxAddEntryText(listboxGameSetupChat, UnicodeString(L"Connections: Missing links:"), GameMakeColor(255, 194, 15, 255), -1, -1); + } for (auto& missingPair : missingConnections) { bool bFoundPlayer = false; diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp index b2fa378fdb1..7e8df7393d3 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp @@ -1361,6 +1361,7 @@ void WebSocket::SendData_StartFullMeshConnectivityCheck(std::function(); m_connectivityCheckLobbyID = pLobbyInterface != nullptr ? pLobbyInterface->GetCurrentLobby().lobbyID : -1; + m_connectivityCheckStartMs = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); nlohmann::json j; j["msg_id"] = EWebSocketMessageID::FULL_MESH_CONNECTIVITY_CHECK_HOST_REQUESTS_BEGIN; diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index 43149488b2c..b809954492a 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -780,13 +780,13 @@ void NGMP_OnlineServices_LobbyInterface::Tick() m_pLobbyMesh->Tick(); } + // stays pending until a menu has registered to handle it if (m_bCannotConnectToLobbyPending) { - m_bCannotConnectToLobbyPending = false; - auto callbackCopy = m_OnCannotConnectToLobbyCallback; if (callbackCopy != nullptr) { + m_bCannotConnectToLobbyPending = false; callbackCopy(); } } @@ -877,6 +877,7 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::function mapHeaders; @@ -888,11 +889,20 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::functionStopCountdown(); } + // a player who joined after the connectivity check passed hasn't been checked, so the host must re-run it + if (IsHost() + && TheNGMPGame != nullptr + && TheNGMPGame->IsCountdownStarted() + && lobbyEntry.lobby_type != ELobbyType::QuickMatch + && !TheNGMPGame->isQMGame()) + { + bool bPlayerJoined = false; + for (const LobbyMemberEntry& newMember : lobbyEntry.members) + { + if (!newMember.IsHuman()) + { + continue; + } + + bool bWasInLobby = false; + for (const LobbyMemberEntry& oldMember : m_CurrentLobby.members) + { + if (oldMember.IsHuman() && oldMember.user_id == newMember.user_id) + { + bWasInLobby = true; + break; + } + } + + if (!bWasInLobby) + { + bPlayerJoined = true; + break; + } + } + + if (bPlayerJoined) + { + TheNGMPGame->StopCountdown(); + + UnicodeString strInform = UnicodeString(L"Countdown stopped: a player joined, start again to re-check connections."); + SendAnnouncementMessageToCurrentLobby(strInform, true); + } + } + // store m_CurrentLobby = lobbyEntry; RecordJoinOrder(lobbyEntry.members); @@ -1156,6 +1210,7 @@ void NGMP_OnlineServices_LobbyInterface::JoinLobby(LobbyEntry lobbyInfo, std::st m_bAttemptingToJoinLobby = true; m_CurrentLobby = LobbyEntry(); ResetJoinOrder(); + ResetHostMigrationFlags(); NGMP_OnlineServicesManager::GetInstance()->GetAndParseServiceConfig([=]() { @@ -1506,6 +1561,7 @@ void NGMP_OnlineServices_LobbyInterface::CreateLobby(UnicodeString strLobbyName, { m_CurrentLobby = LobbyEntry(); ResetJoinOrder(); + ResetHostMigrationFlags(); m_bJoinOrderKnown = true; std::string strURI = NGMP_OnlineServicesManager::GetAPIEndpoint("Lobbies"); std::map mapHeaders; diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp index e622a13b659..1d31b7a474f 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_RoomsInterface.cpp @@ -751,7 +751,15 @@ void WebSocket::Tick() // Main thread only; m_pCurlWS/m_vecWSPartialBuffer/m_bConnected are unlocked here. UpdateReconnect(); - + // longer than the server's 20s backstop; a late reply then finds no callback and is ignored + static constexpr int64_t CONNECTIVITY_CHECK_TIMEOUT_MS = 30000; + if (m_cbOnConnectivityCheckComplete != nullptr && m_connectivityCheckStartMs != -1 && (NowMs() - m_connectivityCheckStartMs) > CONNECTIVITY_CHECK_TIMEOUT_MS) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[FULL_MESH_CONNECTIVITY_CHECK] No reply from the server, timing out"); + auto cbTimedOut = std::move(m_cbOnConnectivityCheckComplete); + ClearConnectivityCheckCallback(); + cbTimedOut(false, {}, "timeout"); + } /* if (strSignal.length() == 6) From 2f90d211ce910a2f440d9a94ebf9207c7cfed12e Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:26:10 +0200 Subject: [PATCH 6/9] fix(quickmatch): Don't cancel the server's requeue or hide a failed match join --- .../GeneralsOnline/OnlineServices_LobbyInterface.h | 3 ++- .../GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp | 8 ++++++++ .../OnlineServices_LobbyInterface.cpp | 14 ++++++++++++-- .../OnlineServices_RoomsInterface.cpp | 11 ++++++++--- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h index d852d5bf315..f8e1810d3d2 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h @@ -472,7 +472,8 @@ class NGMP_OnlineServices_LobbyInterface NetworkMesh* GetNetworkMeshForLobby() { return m_pLobbyMesh; } - void JoinLobby(LobbyEntry lobby, std::string strPassword); + // false when a join is already in progress and this request was dropped + bool JoinLobby(LobbyEntry lobby, std::string strPassword); void LeaveCurrentLobby(); void ResetForMatchmakingRequeue(); diff --git a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp index ad271978319..474f1a16921 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp @@ -1345,6 +1345,14 @@ void WOLQuickMatchMenuInit( WindowLayout *layout, void *userData ) if (!pLobbyInterface->IsInLobby()) { + Int index = GadgetListBoxAddEntryText(quickmatchTextWindow, UnicodeString(L"Could not join the match lobby, waiting for the matchmaker..."), GameSpyColor[GSCOLOR_DEFAULT], -1, -1); + GadgetListBoxSetItemData(quickmatchTextWindow, (void*)-1, index); + + matchFoundTimeoutStart = 0; + matchStartCountdownLastSecond = 0; + + buttonBack->winEnable(TRUE); + buttonStop->winEnable(TRUE); return; } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index b809954492a..d039a262a4f 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -916,6 +916,14 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::functionisQMGame() || m_CurrentLobby.lobby_type == ELobbyType::QuickMatch) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Quick match lobby lookup returned 404, tearing down locally only"); + ResetForMatchmakingRequeue(); + return; + } + LeaveCurrentLobby(); // after leaving, which clears it, so the setup menu backs out @@ -1195,12 +1203,12 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::functionJoinLobby(lobbyEntry, std::string()); - - pLobbyInterface->InvokeMatchmakingMessageCallback("Joining QuickMatch Lobby"); + if (pLobbyInterface->JoinLobby(lobbyEntry, std::string())) + { + pLobbyInterface->InvokeMatchmakingMessageCallback("Joining QuickMatch Lobby"); + } + else + { + pLobbyInterface->InvokeMatchmakingMessageCallback("Could not join the QuickMatch lobby, another join is still in progress"); + } } else { From 1b0f4e155e0a898b7ebf69b094aac2e548e417df Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:21:32 +0200 Subject: [PATCH 7/9] fix(lobby): Store last lobby name as UTF-8 to preserve Unicode characters --- .../Include/Common/UserPreferences.h | 2 ++ .../Source/Common/UserPreferences.cpp | 34 +++++++++++++------ .../Include/Common/CustomMatchPreferences.h | 4 +-- .../GUI/GUICallbacks/Menus/PopupHostGame.cpp | 21 ++---------- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Core/GameEngine/Include/Common/UserPreferences.h b/Core/GameEngine/Include/Common/UserPreferences.h index f1b9f3ea786..dd7ce6b100d 100644 --- a/Core/GameEngine/Include/Common/UserPreferences.h +++ b/Core/GameEngine/Include/Common/UserPreferences.h @@ -60,11 +60,13 @@ class UserPreferences : public PreferenceMap Real getReal(AsciiString key, Real defaultValue) const; Int getInt(AsciiString key, Int defaultValue) const; AsciiString getAsciiString(AsciiString key, AsciiString defaultValue) const; + UnicodeString getUnicodeString(AsciiString key, UnicodeString defaultValue) const; void setBool(AsciiString key, Bool val); void setReal(AsciiString key, Real val); void setInt(AsciiString key, Int val); void setAsciiString(AsciiString key, AsciiString val); + void setUnicodeString(AsciiString key, UnicodeString val); protected: AsciiString m_filename; diff --git a/Core/GameEngine/Source/Common/UserPreferences.cpp b/Core/GameEngine/Source/Common/UserPreferences.cpp index ae9f7636b7b..c90c795ae02 100644 --- a/Core/GameEngine/Source/Common/UserPreferences.cpp +++ b/Core/GameEngine/Source/Common/UserPreferences.cpp @@ -260,6 +260,19 @@ AsciiString UserPreferences::getAsciiString(AsciiString key, AsciiString default return it->second; } +UnicodeString UserPreferences::getUnicodeString(AsciiString key, UnicodeString defaultValue) const +{ + UserPreferences::const_iterator it = find(key); + if (it == end()) + { + return defaultValue; + } + + UnicodeString ret; + ret.translate(it->second); + return ret; +} + void UserPreferences::setBool(AsciiString key, Bool val) { (*this)[key] = boolAsStr(val); @@ -280,6 +293,13 @@ void UserPreferences::setAsciiString(AsciiString key, AsciiString val) (*this)[key] = val; } +void UserPreferences::setUnicodeString(AsciiString key, UnicodeString val) +{ + AsciiString str; + str.translate(val); + (*this)[key] = str; +} + //----------------------------------------------------------------------------- // QuickMatchPreferences base class //----------------------------------------------------------------------------- @@ -515,22 +535,16 @@ void CustomMatchPreferences::setLastLadder(const AsciiString& addr, UnsignedShor (*this)["LastLadderPort"] = strVal; } -AsciiString CustomMatchPreferences::getLastLobbyName() const +UnicodeString CustomMatchPreferences::getLastLobbyName() const { - CustomMatchPreferences::const_iterator it = find("LastLobbyName"); - if (it == end()) - { - return AsciiString::TheEmptyString; - } - - AsciiString ret = it->second; + UnicodeString ret = getUnicodeString("LastLobbyName", UnicodeString::TheEmptyString); ret.trim(); return ret; } -void CustomMatchPreferences::setLastLobbyName(const AsciiString& name) +void CustomMatchPreferences::setLastLobbyName(const UnicodeString& name) { - (*this)["LastLobbyName"] = name; + setUnicodeString("LastLobbyName", name); } AsciiString CustomMatchPreferences::getLastLadderAddr( void ) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/CustomMatchPreferences.h b/GeneralsMD/Code/GameEngine/Include/Common/CustomMatchPreferences.h index 228a7e46b4d..5aea5cee8a4 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/CustomMatchPreferences.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/CustomMatchPreferences.h @@ -44,8 +44,8 @@ class CustomMatchPreferences : public UserPreferences CustomMatchPreferences(); virtual ~CustomMatchPreferences() override; - AsciiString getLastLobbyName() const; - void setLastLobbyName(const AsciiString& name); + UnicodeString getLastLobbyName() const; + void setLastLobbyName(const UnicodeString& name); void setLastLadder(const AsciiString& addr, UnsignedShort port); AsciiString getLastLadderAddr(); diff --git a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp index 074b472fa6d..b5f98aed513 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp @@ -336,12 +336,8 @@ void PopupHostGameInit( WindowLayout *layout, void *userData ) #if defined(GENERALS_ONLINE) { CustomMatchPreferences pref; - AsciiString lastLobbyName = pref.getLastLobbyName(); - if (!lastLobbyName.isEmpty()) - { - name.translate(lastLobbyName.str()); - } - else + name = pref.getLastLobbyName(); + if (name.isEmpty()) { name.translate("Generals Online Lobby"); } @@ -605,19 +601,8 @@ WindowMsgHandledType PopupHostGameSystem( GameWindow *window, UnsignedInt msg, W #if defined(GENERALS_ONLINE) // save last used lobby name to CustomPref.ini { - char buffer[256]; - const WideChar* w = name.str(); - int i = 0; - for (; w[i] != 0 && i < 255; ++i) - { - buffer[i] = (char)(w[i] & 0xFF); - } - buffer[i] = 0; - - AsciiString lobbyNameAscii = buffer; - CustomMatchPreferences pref; - pref.setLastLobbyName(lobbyNameAscii); + pref.setLastLobbyName(name); pref.write(); } #endif From 288ba60c0e1a769aa627e1b1654ffe8a1fbdf03f Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:35:30 +0200 Subject: [PATCH 8/9] fix(lobby): Place lobby members by their own slot and hold the match start until the roster is complete --- .../OnlineServices_LobbyInterface.h | 13 +- .../GUICallbacks/Menus/WOLGameSetupMenu.cpp | 153 +++++++++++++++--- .../GameNetwork/GeneralsOnline/NGMPGame.cpp | 72 +++++---- .../OnlineServices_LobbyInterface.cpp | 35 +++- 4 files changed, 222 insertions(+), 51 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h index f8e1810d3d2..5f8d1c39d64 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.h @@ -25,13 +25,20 @@ struct LobbyMemberEntry : public NetworkMemberBase int startpos = -1; bool has_map = false; - uint16_t m_SlotIndex = 999999; + // marks a member the service sent without a usable slot + static constexpr uint16_t NO_SLOT_INDEX = UINT16_MAX; + uint16_t m_SlotIndex = NO_SLOT_INDEX; uint16_t m_SlotState = SlotState::SLOT_OPEN; std::string region; std::string middlewareUserID; int latency = 0; + bool HasValidSlot() const + { + return m_SlotIndex < MAX_SLOTS; + } + bool IsHuman() const { return user_id != -1 && m_SlotState == SlotState::SLOT_PLAYER; @@ -384,6 +391,10 @@ class NGMP_OnlineServices_LobbyInterface void UpdateRoomDataCache(std::function fnCallback = nullptr); + // re-fetches the lobby when the roster could not be trusted, throttled + void RequestRosterRefresh(); + int64_t m_lastRosterRefreshRequestMs = 0; + std::function m_cbPlayerDoesntHaveMap = nullptr; void RegisterForPlayerDoesntHaveMapCallback(std::function cb) { diff --git a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp index 4faf834c161..7cbb947a095 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp @@ -977,6 +977,129 @@ static void WOLLockSettings() } } +// lobby humans (the service's view) the game has no human slot for +static Int CountLobbyHumansMissingFromGame(NGMPGame* game) +{ + NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); + if (pLobbyInterface == nullptr || game == nullptr) + { + return 0; + } + + Int missing = 0; + for (const LobbyMemberEntry& member : pLobbyInterface->GetCurrentLobby().members) + { + if (!member.IsHuman()) + { + continue; + } + + bool bInGame = false; + for (Int i = 0; i < MAX_SLOTS; ++i) + { + NGMPGameSlot* slot = static_cast(game->getSlot(i)); + if (slot != nullptr && slot->isHuman() && slot->m_userID == member.user_id) + { + bInGame = true; + break; + } + } + + if (!bInGame) + { + ++missing; + } + } + + return missing; +} + +// other lobby humans we have no connected link to, a mesh check can't be complete with these +static Int CountLobbyHumansWithoutMeshLink() +{ + NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); + NGMP_OnlineServices_AuthInterface* pAuthInterface = NGMP_OnlineServicesManager::GetInterface(); + NetworkMesh* pMesh = NGMP_OnlineServicesManager::GetNetworkMesh(); + if (pLobbyInterface == nullptr || pAuthInterface == nullptr || pMesh == nullptr) + { + return 0; + } + + const int64_t myUserID = pAuthInterface->GetUserID(); + Int unlinked = 0; + for (const LobbyMemberEntry& member : pLobbyInterface->GetCurrentLobby().members) + { + if (!member.IsHuman() || member.user_id == myUserID) + { + continue; + } + + PlayerConnection* pConnection = pMesh->GetConnectionForUser(member.user_id); + if (pConnection == nullptr || pConnection->GetState() != EConnectionState::CONNECTED_DIRECT) + { + ++unlinked; + } + } + + return unlinked; +} + +static const Int MAX_ROSTER_REFRESHES_BEFORE_START = 3; +static bool s_startWaitingForRoster = false; + +// the game must have a slot for every human the lobby lists, otherwise the match would start without them +static void StartMatchWhenRosterMatches(Int refreshesDone) +{ + NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); + NGMPGame* myGame = pLobbyInterface == nullptr ? nullptr : pLobbyInterface->GetCurrentGame(); + + if (pLobbyInterface == nullptr || !myGame || !myGame->isInGame()) + { + s_startWaitingForRoster = false; + return; + } + + if (!TheNGMPGame) + { + s_startWaitingForRoster = false; + return; + } + + const Int missingHumans = CountLobbyHumansMissingFromGame(myGame); + if (missingHumans > 0) + { + if (refreshesDone < MAX_ROSTER_REFRESHES_BEFORE_START) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Start: the game is missing %d lobby player(s), refreshing the lobby (%d/%d)", missingHumans, refreshesDone + 1, MAX_ROSTER_REFRESHES_BEFORE_START); + pLobbyInterface->UpdateRoomDataCache([refreshesDone](bool) + { + StartMatchWhenRosterMatches(refreshesDone + 1); + }); + return; + } + + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Start: the game is still missing %d lobby player(s), not starting", missingHumans); + s_startWaitingForRoster = false; + GSMessageBoxOk(TheGameText->fetch("GUI:Error"), UnicodeString(L"The player list was incomplete, so the match was not started. Please rejoin the lobby.")); + PopBackToLobby(); + return; + } + + s_startWaitingForRoster = false; + + // TODO_NGMP + //SendStatsToOtherPlayers(TheNGMPGame); + + GameWindow* buttonBuddy = TheWindowManager->winGetWindowFromId(NULL, NAMEKEY("GameSpyGameOptionsMenu.wnd:ButtonCommunicator")); + if (buttonBuddy) + buttonBuddy->winEnable(FALSE); + GameSpyCloseOverlay(GSOVERLAY_BUDDY); + GameSpyCloseOverlay(GSOVERLAY_PLAYERINFO); + + *TheNGMPGame = *myGame; + TheNGMPGame->startGame(0); +} + static void StartPressed() { Bool isReady = TRUE; @@ -1164,6 +1287,13 @@ static void StartPressed() pWS->SendData_StartFullMeshConnectivityCheck([=](bool bMeshFullyConnected, std::list> missingConnections, std::string strFailureReason) { + // the service can report a complete mesh while we hold no link to players the lobby lists + if (bMeshFullyConnected && CountLobbyHumansWithoutMeshLink() > 0) + { + bMeshFullyConnected = false; + strFailureReason = "no_link_to_lobby_players"; + } + if (bMeshFullyConnected) { UnicodeString strInform = UnicodeString(L"Connections: All players are connected."); @@ -2012,26 +2142,13 @@ void WOLGameSetupMenuInit( WindowLayout *layout, void *userData ) pLobbyInterface->RegisterForGameStartPacket([]() { - NGMP_OnlineServices_LobbyInterface* pLobbyInterface = NGMP_OnlineServicesManager::GetInterface(); - NGMPGame* myGame = pLobbyInterface == nullptr ? nullptr : pLobbyInterface->GetCurrentGame(); - - if (pLobbyInterface == nullptr || !myGame || !myGame->isInGame()) - return; - - if (!TheNGMPGame) + if (s_startWaitingForRoster) + { return; + } - // TODO_NGMP - //SendStatsToOtherPlayers(TheNGMPGame); - - GameWindow* buttonBuddy = TheWindowManager->winGetWindowFromId(NULL, NAMEKEY("GameSpyGameOptionsMenu.wnd:ButtonCommunicator")); - if (buttonBuddy) - buttonBuddy->winEnable(FALSE); - GameSpyCloseOverlay(GSOVERLAY_BUDDY); - GameSpyCloseOverlay(GSOVERLAY_PLAYERINFO); - - *TheNGMPGame = *myGame; - TheNGMPGame->startGame(0); + s_startWaitingForRoster = true; + StartMatchWhenRosterMatches(0); }); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NGMPGame.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NGMPGame.cpp index 62f7b703cba..4afd7f10852 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NGMPGame.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NGMPGame.cpp @@ -149,9 +149,38 @@ void NGMPGame::UpdateSlotsFromCurrentLobby() return; } - for (Int i = 0; i < MAX_SLOTS; ++i) + const std::vector& members = pLobbyInterface->GetCurrentLobby().members; + + // a real member without a usable slot means the roster is incomplete, keep the previous slots and ask for a fresh one + for (const LobbyMemberEntry& lobbyMember : members) { - LobbyMemberEntry pLobbyMember = pLobbyInterface->GetRoomMemberFromIndex(i); + if (lobbyMember.user_id != -1 && !lobbyMember.HasValidSlot()) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: user %lld has no valid slot index (%u), keeping the previous roster", lobbyMember.user_id, lobbyMember.m_SlotIndex); + pLobbyInterface->RequestRosterRefresh(); + return; + } + } + + bool claimedSlots[MAX_SLOTS] = {}; + + for (const LobbyMemberEntry& pLobbyMember : members) + { + // empty placeholder without a slot, nothing to place + if (!pLobbyMember.HasValidSlot()) + { + continue; + } + + NGMPGameSlot* slot = (NGMPGameSlot*)getSlot(pLobbyMember.m_SlotIndex); + + if (slot == nullptr) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: getSlot(%u) returned null, skipping", pLobbyMember.m_SlotIndex); + continue; + } + + claimedSlots[pLobbyMember.m_SlotIndex] = true; // TODO_NGMP: Support spectators int playerTemplate = -1; @@ -169,20 +198,6 @@ void NGMPGame::UpdateSlotsFromCurrentLobby() { bool bIsAI = (pLobbyMember.m_SlotState == SlotState::SLOT_EASY_AI || pLobbyMember.m_SlotState == SlotState::SLOT_MED_AI|| pLobbyMember.m_SlotState == SlotState::SLOT_BRUTAL_AI); - if (pLobbyMember.m_SlotIndex >= MAX_SLOTS) - { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: bad slot index %u for user %lld, skipping", pLobbyMember.m_SlotIndex, pLobbyMember.user_id); - continue; - } - - NGMPGameSlot* slot = (NGMPGameSlot*)getSlot(pLobbyMember.m_SlotIndex); - - if (slot == nullptr) - { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: getSlot(%u) returned null, skipping", pLobbyMember.m_SlotIndex); - continue; - } - // NOTE: Internally generals uses 'local ip' to detect which user is local... we dont have an IP, so just use player index for ip slot->setState((SlotState)pLobbyMember.m_SlotState, UnicodeString(from_utf8(pLobbyMember.display_name).c_str()), pLobbyMember.m_SlotIndex); @@ -219,24 +234,21 @@ void NGMPGame::UpdateSlotsFromCurrentLobby() else { // handle open/closed - if (pLobbyMember.m_SlotIndex >= MAX_SLOTS) - { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: bad slot index %u for user %lld, skipping", pLobbyMember.m_SlotIndex, pLobbyMember.user_id); - continue; - } - - NGMPGameSlot* slot = (NGMPGameSlot*)getSlot(pLobbyMember.m_SlotIndex); + slot->setState((SlotState)pLobbyMember.m_SlotState); + } + } - if (slot == nullptr) + // slots no member claims are open + for (Int i = 0; i < MAX_SLOTS; ++i) + { + if (!claimedSlots[i]) + { + GameSlot* slot = getSlot(i); + if (slot != nullptr) { - NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] UpdateSlotsFromCurrentLobby: getSlot(%u) returned null, skipping", pLobbyMember.m_SlotIndex); - continue; + slot->setState(SLOT_OPEN); } - - slot->setState((SlotState)pLobbyMember.m_SlotState); } - - // dont need to handle else here, we set it up upon lobby creation } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp index d039a262a4f..5f853062be5 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_LobbyInterface.cpp @@ -518,6 +518,21 @@ NGMP_OnlineServices_LobbyInterface::NGMP_OnlineServices_LobbyInterface() } +// a missing or out of range slot index leaves the member at NO_SLOT_INDEX +static void ParseMemberSlotIndex(const nlohmann::json& memberJson, LobbyMemberEntry& memberEntry) +{ + if (!memberJson.contains("SlotIndex") || !memberJson["SlotIndex"].is_number_integer()) + { + return; + } + + const int64_t slotIndex = memberJson["SlotIndex"].get(); + if (slotIndex >= 0 && slotIndex < MAX_SLOTS) + { + memberEntry.m_SlotIndex = static_cast(slotIndex); + } +} + void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function onStartCallback, std::function)> onCompleteCallback) { const bool lobbyListWasDirty = m_bLobbyListDirty.exchange(false); @@ -630,7 +645,7 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function JsonGetRequired(memberEntryIter, "UserID", memberEntry.user_id); JsonGetOptional(memberEntryIter, "DisplayName", memberEntry.display_name); JsonGetOptional(memberEntryIter, "IsReady", memberEntry.m_bIsReady); - JsonGetOptional(memberEntryIter, "SlotIndex", memberEntry.m_SlotIndex); + ParseMemberSlotIndex(memberEntryIter, memberEntry); JsonGetOptional(memberEntryIter, "SlotState", memberEntry.m_SlotState); JsonGetOptional(memberEntryIter, "Region", memberEntry.region); @@ -678,6 +693,18 @@ void NGMP_OnlineServices_LobbyInterface::SearchForLobbies(std::function }); } +void NGMP_OnlineServices_LobbyInterface::RequestRosterRefresh() +{ + const int64_t nowMs = std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()).count(); + if (nowMs - m_lastRosterRefreshRequestMs < 2000) + { + return; + } + + m_lastRosterRefreshRequestMs = nowMs; + UpdateRoomDataCache(); +} + void NGMP_OnlineServices_LobbyInterface::ResetJoinOrder() { m_setMembersBeforeUs.clear(); @@ -1038,7 +1065,7 @@ void NGMP_OnlineServices_LobbyInterface::UpdateRoomDataCache(std::functionGetDisplayName(); me.m_bIsReady = true; // host is always ready + // the host always takes the first slot + me.m_SlotIndex = 0; + me.m_SlotState = SlotState::SLOT_PLAYER; + // TODO_NGMP: Remove this and just hardcode it or provide from service me.preferredPort = 0; From f666a92515b08ed0b0e174884acf37522565476b Mon Sep 17 00:00:00 2001 From: tintinhamans <5984296+tintinhamans@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:35:31 +0200 Subject: [PATCH 9/9] fix(mesh): Remove a peer that left cleanly so it is no longer polled --- .../GameNetwork/GeneralsOnline/NetworkMesh.h | 6 +++++ .../GeneralsOnline/NetworkMesh.cpp | 23 +++++++++++++++++++ .../GeneralsOnline/NextGenTransport.cpp | 5 ++++ 3 files changed, 34 insertions(+) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/NetworkMesh.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/NetworkMesh.h index fe3c7ac9c6c..789e9d00559 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/NetworkMesh.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/NetworkMesh.h @@ -66,6 +66,9 @@ class PlayerConnection m_State != EConnectionState::CONNECTION_DISCONNECTED; } + // a closed connection has nothing to receive + bool CanRecv() const { return m_hSteamConnection != k_HSteamNetConnection_Invalid; } + int Recv(SteamNetworkingMessage_t** pMsg); int GetHighestHistoricalLatency() @@ -245,6 +248,9 @@ class NetworkMesh m_vecPendingStateUpdates.emplace_back(userID, newState); } + // drops a peer that left for good so nothing keeps polling it + void RemoveConnection(int64_t userID); + PlayerConnection* GetConnectionForUser(int64_t user_id) { if (m_mapConnections.contains(user_id)) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp index 81e955534db..ca1529a191c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp @@ -191,6 +191,12 @@ void OnSteamNetConnectionStatusChanged(SteamNetConnectionStatusChangedCallback_t } + // a clean close is a leave, the peer is gone for good; the game's own disconnect already ran above + if (!bWasError) + { + pMesh->RemoveConnection(userID); + } + // In this example, we will bail the test whenever this happens. // Was this a normal termination? NetworkLog(ELogVerbosity::LOG_RELEASE, "[STEAM NETWORKING]DISCONNECTED OR PROBLEM DETECTED %d\n", pInfo->m_info.m_eEndReason); @@ -1051,6 +1057,18 @@ void NetworkMesh::SetTurnCredentials(const std::string& strUsername, const std:: } +void NetworkMesh::RemoveConnection(int64_t userID) +{ + std::lock_guard lock(m_mapConnectionsMutex); + + auto it = m_mapConnections.find(userID); + if (it != m_mapConnections.end()) + { + NetworkLog(ELogVerbosity::LOG_RELEASE, "[ERASE 4] Removing user %lld", it->second.m_userID); + m_mapConnections.erase(it); + } +} + void NetworkMesh::DisconnectUser(int64_t remoteUserID) { std::lock_guard lock(m_mapConnectionsMutex); @@ -1189,6 +1207,11 @@ void PlayerConnection::LiteUpdateForAC() } else { + if (!CanRecv()) + { + return; + } + SteamNetworkingMessage_t* pMsg[255] = { nullptr }; int numPackets = Recv(pMsg); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NextGenTransport.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NextGenTransport.cpp index d6032342218..1a6f634ebbd 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NextGenTransport.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NextGenTransport.cpp @@ -332,6 +332,11 @@ Bool NextGenTransport::doRecv(void) std::map& connections = pMesh->GetAllConnections(); for (auto& kvPair : connections) { + if (!kvPair.second.CanRecv()) + { + continue; + } + SteamNetworkingMessage_t* pMsg[255] = { nullptr }; int numPackets = kvPair.second.Recv(pMsg);