fix(online): Make P2P connections and the lobby flow reliable

Core/GameEngine/Include/GameNetwork/GameSpy/LobbyUtils.h

@@ -40,6 +40,10 @@ void RefreshGameInfoListBox( GameWindow *mainWin, GameWindow *win );
 void RefreshGameListBoxes();
 void ToggleGameListType();
 
+// The game listbox's item data is a row index (32-bit safe), not the 64-bit lobby ID itself.
+// Resolve the real ID with this before calling anything that needs it (e.g. GetLobbyFromID).
+int64_t ResolveGameListLobbyID(Int rowItemData);
+
 void playerTemplateComboBoxTooltip(GameWindow *wndComboBox, WinInstanceData *instData, UnsignedInt mouse);
 void playerTemplateListBoxTooltip(GameWindow *wndListBox, WinInstanceData *instData, UnsignedInt mouse);
 

Core/GameEngine/Source/GameNetwork/GameSpy/LobbyUtils.cpp

@@ -273,7 +273,7 @@ static void gameTooltip(GameWindow* window,
 		return;
 	}
 
-	LobbyEntry lobbyEntry = pLobbyInterface->GetLobbyFromID(gameID);
+	LobbyEntry lobbyEntry = pLobbyInterface->GetLobbyFromID(ResolveGameListLobbyID(gameID));
 	if (lobbyEntry.lobbyID == -1)
 	{
 		return;
@@ -614,6 +614,20 @@ static Bool lobbyHasBuddy(GameSpyStagingRoom *room)
 }
 #endif
 
+// Lobby IDs are 64-bit, but GadgetListBox item data is a 32-bit void* on this platform, so we
+// can't stash the ID itself there. Instead we store the row index and keep the real ID here,
+// rebuilt every time the game listbox is repopulated (see RefreshGameListBox/insertGame).
+static std::vector<int64_t> s_lobbyRowIDs;
+
+int64_t ResolveGameListLobbyID(Int rowItemData)
+{
+	if (rowItemData < 0 || (size_t)rowItemData >= s_lobbyRowIDs.size())
+	{
+		return -1;
+	}
+	return s_lobbyRowIDs[rowItemData];
+}
+
 #if defined(GENERALS_ONLINE)
 static void populateBuddyGames(std::vector<LobbyEntry>& vecLobbies)
 #else
@@ -841,7 +855,6 @@ static Int insertGame(GameWindow* win, LobbyEntry& lobbyInfo, Bool showMap)
 	AsciiString lobbyMapName = AsciiString(lobbyInfo.map_name.c_str());
 	AsciiString ladder = AsciiString("TODO_NGMP");
 	USHORT ladderPort = 1;
-	int gameID = lobbyInfo.lobbyID; // TODO_NGMP: Downcast. We should use int64 everywhere, although its unlikely we actually need int64 for lobby since its reset regularly.
 
 	bool bHasPassword = lobbyInfo.passworded;
 
@@ -888,7 +901,12 @@ static Int insertGame(GameWindow* win, LobbyEntry& lobbyInfo, Bool showMap)
 		gameColor = GameMakeColor(191, 198, 201, 255);
 	}
 	Int index = GadgetListBoxAddEntryText(win, gameName, gameColor, -1, COLUMN_NAME);
-	GadgetListBoxSetItemData(win, (void*)gameID, index);
+	if ((size_t)index >= s_lobbyRowIDs.size())
+	{
+		s_lobbyRowIDs.resize(index + 1);
+	}
+	s_lobbyRowIDs[index] = lobbyInfo.lobbyID;
+	GadgetListBoxSetItemData(win, (void*)(intptr_t)index, index);
 
 	UnicodeString s;
 
@@ -1177,11 +1195,11 @@ void RefreshGameListBox(GameWindow* win, Bool showMap)
 
 	// save off selection
 	Int selectedIndex = -1;
-	Int selectedID = 0;
+	int64_t selectedID = -1;
 	GadgetListBoxGetSelected(win, &selectedIndex);
 	if (selectedIndex != -1)
 	{
-		selectedID = (Int)GadgetListBoxGetItemData(win, selectedIndex);
+		selectedID = ResolveGameListLobbyID((Int)GadgetListBoxGetItemData(win, selectedIndex));
 	}
 	int prevPos = GadgetListBoxGetTopVisibleEntry(win);
 
@@ -1199,6 +1217,7 @@ void RefreshGameListBox(GameWindow* win, Bool showMap)
 		{
 			// empty listbox
 			GadgetListBoxReset(win);
+			s_lobbyRowIDs.clear();
 
 			size_t numResults = vecLobbies.size();
 
@@ -1270,7 +1289,7 @@ void RefreshGameListBox(GameWindow* win, Bool showMap)
 				//	if(prevPos > 10)
 				GadgetListBoxSetTopVisibleEntry(win, prevPos);//+ 1
 
-				if (indexToSelect < 0 && selectedID)
+				if (indexToSelect < 0 && selectedID >= 0)
 				{
 					TheWindowManager->winSetLoneWindow(NULL);
 				}

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

@@ -128,6 +128,26 @@ struct QueuedGamePacket
 	int64_t m_userID = -1;
 };
 
+// Owns the process-wide GameNetworkingSockets library lifetime.
+class NetworkMeshLibrary
+{
+public:
+	// Inits once with userID's identity; stays up until Shutdown (a new login always follows a full teardown).
+	// Returns false on failure; caller must not create a listen socket/connection in that case.
+	static bool EnsureInitialized(int64_t userID);
+
+	// Tears the library down. Safe to call when not initialized.
+	static void Shutdown();
+
+	static bool IsInitialized() { return s_bInitialized; }
+
+	// Drives SteamNetworkingSockets()->RunCallbacks(), independent of NetworkMesh lifetime.
+	static void Tick();
+
+private:
+	static bool s_bInitialized;
+};
+
 class NetworkMesh
 {
 public:
@@ -203,23 +223,21 @@ class NetworkMesh
 
 	void Tick();
 
+	// false if construction failed; callers must not use a mesh that failed to initialize.
+	bool IsInitialized() const { return m_bInitialized; }
+
 	HSteamListenSocket GetListenSocketHandle() const { return m_hListenSock; }
 
 	std::map<int64_t, PlayerConnection>& GetAllConnections()
 	{
 		return m_mapConnections;
 	}
 
-	// Thread-safe: may be invoked from anticheat plugin threads
+	// Thread-safe: may be invoked from anticheat plugin threads; applied on the main thread in Tick
 	void UpdateConnectionStateForUser(int64_t userID, EConnectionState newState)
 	{
-		std::lock_guard<std::recursive_mutex> lock(m_mapConnectionsMutex);
-
-		auto it = m_mapConnections.find(userID);
-		if (it != m_mapConnections.end())
-		{
-			it->second.UpdateState(newState, this);
-		}
+		std::lock_guard<std::mutex> lock(m_pendingStateUpdatesMutex);
+		m_vecPendingStateUpdates.emplace_back(userID, newState);
 	}
 
 	PlayerConnection* GetConnectionForUser(int64_t user_id)
@@ -234,9 +252,14 @@ class NetworkMesh
 
 
 private:
+	bool m_bInitialized = false;
+
 	std::map<int64_t, PlayerConnection> m_mapConnections;
 	mutable std::recursive_mutex m_mapConnectionsMutex;  // Synchronizes access to m_mapConnections
 
+	std::mutex m_pendingStateUpdatesMutex;
+	std::vector<std::pair<int64_t, EConnectionState>> m_vecPendingStateUpdates;
+
 	ISignalingClient* m_pSignaling = nullptr;
 
 	HSteamListenSocket m_hListenSock = k_HSteamListenSocket_Invalid;
@@ -247,4 +270,9 @@ class NetworkMesh
 	std::string m_strTurnToken;
 	std::string m_strTurnUsernameString;
 	std::string m_strTurnTokenString;
+	std::string m_strTurnServerList;
+
+	// k_nSteamNetworkingConfig_P2P_Transport_ICE_Enable_* for this lobby's connections.
+	int m_iceEnable = 0;
+	int m_iceImplementation = 2;
 };

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

@@ -151,8 +151,6 @@ class WebSocket
 
 	std::vector<char> m_vecWSPartialBuffer;
 
-	std::vector<std::string> m_vecQueuedOutboungMsgs;
-
 	std::function<void(void)> m_fnWebsocketConnectedCallback = nullptr;
 
 	void Shutdown();
@@ -180,26 +178,49 @@ class WebSocket
 
 	void SendData_CountdownStarted();
 
-	std::function<void(bool, std::list<std::pair<int64_t, int64_t>>)> m_cbOnConnectivityCheckComplete = nullptr;
-	void SendData_StartFullMeshConnectivityCheck(std::function<void(bool, std::list<std::pair<int64_t, int64_t>>)> cbOnConnectivityCheckComplete);
+	// params: fully connected, missing links, reason ("" on success, else e.g. "missing_connections",
+	// "timeout", "member_left", "check_superseded")
+	std::function<void(bool, std::list<std::pair<int64_t, int64_t>>, std::string)> m_cbOnConnectivityCheckComplete = nullptr;
+
+	// lobby ID the check was started for; a stale reply (different lobby) is ignored
+	int64_t m_connectivityCheckLobbyID = -1;
+
+	void SendData_StartFullMeshConnectivityCheck(std::function<void(bool, std::list<std::pair<int64_t, int64_t>>, std::string)> cbOnConnectivityCheckComplete);
+
+	void ClearConnectivityCheckCallback()
+	{
+		m_cbOnConnectivityCheckComplete = nullptr;
+		m_connectivityCheckLobbyID = -1;
+	}
 
 	void Tick();
 
 	int Ping();
 
+	// Queues the message; Tick() flushes it on the main thread. Thread-safe.
 	void Send(const char* message);
 
-	// TODO_STEAM: clear this on connect
-	std::queue<std::vector<uint8_t>> m_pendingSignals;
-
-	bool AcquireLock()
+	// Thread-safe queue of inbound P2P signal payloads; drained by CSignalingClient::Poll().
+	void PushPendingSignal(std::vector<uint8_t> payload)
 	{
-		return m_mutex.try_lock_for(std::chrono::milliseconds(1));
+		std::scoped_lock<std::mutex> lock(m_pendingSignalsMutex);
+
+		static constexpr size_t kMaxPendingSignals = 256;
+		if (m_pendingSignals.size() >= kMaxPendingSignals)
+		{
+			NetworkLog(ELogVerbosity::LOG_RELEASE, "[WebSocket] Pending signal queue full (%zu), discarding oldest signal", m_pendingSignals.size());
+			m_pendingSignals.pop();
+		}
+
+		m_pendingSignals.push(std::move(payload));
 	}
 
-	void ReleaseLock()
+	std::queue<std::vector<uint8_t>> DrainPendingSignals()
 	{
-		m_mutex.unlock();
+		std::scoped_lock<std::mutex> lock(m_pendingSignalsMutex);
+		std::queue<std::vector<uint8_t>> drained;
+		drained.swap(m_pendingSignals);
+		return drained;
 	}
 
 private:
@@ -236,7 +257,12 @@ class WebSocket
 
 	std::atomic<bool> m_bShuttingDown = false;
 
-	std::recursive_timed_mutex m_mutex;
+	// Outbound messages queued for Tick() to flush.
+	std::mutex m_outboundQueueMutex;
+	std::vector<std::string> m_vecQueuedOutboungMsgs;
+
+	std::mutex m_pendingSignalsMutex;
+	std::queue<std::vector<uint8_t>> m_pendingSignals;
 };
 
 enum class ERoomFlags : int
@@ -316,12 +342,14 @@ struct ServiceConfig
 	int screenshot_width = 557;
 	int screenshot_height = 333;
 
-
-	NLOHMANN_DEFINE_TYPE_INTRUSIVE(ServiceConfig, retry_signalling, use_mapped_port, min_run_ahead_frames, ra_update_frequency_frames, relay_all_traffic,
+	// ICE client used for P2P NAT traversal: 0 = library default, 1 = native, 2 = WebRTC.
+	int ice_implementation = 2;
+
+	NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(ServiceConfig, retry_signalling, use_mapped_port, min_run_ahead_frames, ra_update_frequency_frames, relay_all_traffic,
 		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)
+		screenshot_width, screenshot_height, ice_implementation)
 };
 
 class NGMP_OnlineServicesManager

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

@@ -9,6 +9,7 @@
 #include "GameClient/InGameUI.h"
 #include "GameLogic/VictoryConditions.h"
 #include <atomic>
+#include <set>
 
 extern NGMPGame* TheNGMPGame;
 
@@ -373,6 +374,14 @@ class NGMP_OnlineServices_LobbyInterface
 
 	bool IsHost();
 
+private:
+	std::set<int64_t> m_setMembersBeforeUs;
+	bool m_bJoinOrderKnown = false;
+	void ResetJoinOrder();
+	void RecordJoinOrder(const std::vector<LobbyMemberEntry>& members);
+
+public:
+
 	void UpdateRoomDataCache(std::function<void(bool)> fnCallback = nullptr);
 
 	std::function<void(LobbyMemberEntry)> m_cbPlayerDoesntHaveMap = nullptr;
@@ -397,6 +406,36 @@ class NGMP_OnlineServices_LobbyInterface
 		m_OnCannotConnectToLobbyCallback = nullptr;
 	}
 
+	// true if userID was already in the lobby when we joined, i.e. we are the later joiner of that pair
+	bool JoinedAfter(int64_t userID) const
+	{
+		return m_bJoinOrderKnown && m_setMembersBeforeUs.contains(userID);
+	}
+
+	bool IsJoinOrderKnown() const
+	{
+		return m_bJoinOrderKnown;
+	}
+
+	bool IsLobbyMember(int64_t userID) const
+	{
+		for (const LobbyMemberEntry& member : m_CurrentLobby.members)
+		{
+			if (member.user_id == userID)
+			{
+				return true;
+			}
+		}
+		return false;
+	}
+
+	// raised from inside the mesh's GNS callbacks, dispatched by Tick once the mesh is done with them
+	bool m_bCannotConnectToLobbyPending = false;
+	void QueueCannotConnectToLobby()
+	{
+		m_bCannotConnectToLobbyPending = true;
+	}
+
 	std::function<void(UnicodeString strMessage, Color color)> m_OnChatCallback = nullptr;
 	void RegisterForChatCallback(std::function<void(UnicodeString strMessage, Color color)> cb)
 	{