From 578aea3b09a11e5848707add61b803b6ccc248ab Mon Sep 17 00:00:00 2001 From: phosit Date: Mon, 2 Mar 2026 21:35:28 +0100 Subject: [PATCH] Remove CNetClient::SetupServerData It has to be called before `SetupConnection` is called. Now the server-data is setup during the constructor. When connecting using the lobby, the data isn't known at construction time. Then it is done at the start of `TryToConnectWithSTUN`. --- source/lobby/XmppClient.cpp | 3 +- source/network/NetClient.cpp | 37 +++++++++---------- source/network/NetClient.h | 15 +++----- .../network/scripting/JSInterface_Network.cpp | 7 ++-- source/network/tests/test_Net.h | 16 ++++---- 5 files changed, 34 insertions(+), 44 deletions(-) diff --git a/source/lobby/XmppClient.cpp b/source/lobby/XmppClient.cpp index fb5124cded..9ccfba9220 100644 --- a/source/lobby/XmppClient.cpp +++ b/source/lobby/XmppClient.cpp @@ -862,8 +862,7 @@ bool XmppClient::handleIq(const gloox::IQ& iq) return true; } - g_NetClient->SetupServerData(cd->m_Ip, stoi(cd->m_Port)); - g_NetClient->TryToConnectWithSTUN(iq.from().full(), !cd->m_IsLocalIP.empty()); + g_NetClient->TryToConnectWithSTUN(cd->m_Ip, stoi(cd->m_Port), iq.from().full(), !cd->m_IsLocalIP.empty()); } if (gq) { diff --git a/source/network/NetClient.cpp b/source/network/NetClient.cpp index 5198e6561a..a2cd2ece24 100644 --- a/source/network/NetClient.cpp +++ b/source/network/NetClient.cpp @@ -66,11 +66,14 @@ constexpr u32 NETWORK_BAD_PING = DEFAULT_TURN_LENGTH * COMMAND_DELAY_MP / 2; CNetClient *g_NetClient = NULL; -CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID, - std::string hashedPassword, std::string controllerSecret) : +CNetClient::CNetClient(CGame* game, std::string serverAddressOrHostname, std::uint16_t serverPort, + const CStrW& username, const CStr& hostJID, std::string hashedPassword, + std::string controllerSecret) : m_UserName{username}, m_HostJID{hostJID}, m_Game{game}, + m_ServerAddressOrHostname{std::move(serverAddressOrHostname)}, + m_ServerPort{serverPort}, // Hash on top with the user's name, to make sure not all // hashing data is in control of the host. m_Password{HashCryptographically(std::move(hashedPassword), m_UserName.ToUTF8())}, @@ -140,7 +143,7 @@ CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID, CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID, std::string hashedPassword, IXmppClient& xmppClient) : - CNetClient{game, username, hostJID, std::move(hashedPassword)} + CNetClient{game, {}, 0, username, hostJID, std::move(hashedPassword)} { xmppClient.SendIqGetConnectionData(m_HostJID, m_Password, m_UserName.ToUTF8(), false); } @@ -158,21 +161,13 @@ CNetClient::~CNetClient() bool CNetClient::SetupConnection(ENetHost* enetClient) { CNetClientSession* session = new CNetClientSession(*this); - bool ok = session->Connect(m_ServerAddress, m_ServerPort, enetClient); + bool ok = session->Connect(m_ServerAddressOrHostname, m_ServerPort, enetClient); SetAndOwnSession(session); if (ok) m_PollingThread = std::thread(Threading::HandleExceptions::Wrapper, m_Session); return ok; } -void CNetClient::SetupServerData(CStr address, u16 port) -{ - ENSURE(!m_Session); - - m_ServerAddress = address; - m_ServerPort = port; -} - void CNetClient::HandleGetServerDataFailed(const CStr& error) { if (m_Session) @@ -185,14 +180,18 @@ void CNetClient::HandleGetServerDataFailed(const CStr& error) ); } -bool CNetClient::TryToConnectWithSTUN(const CStr& hostJID, bool localNetwork) +bool CNetClient::TryToConnectWithSTUN(std::string serverAddressOrHostname, std::uint16_t serverPort, + const CStr& hostJID, bool localNetwork) { ENSURE(g_XmppClient); + m_ServerAddressOrHostname = std::move(serverAddressOrHostname); + m_ServerPort = serverPort; + if (m_Session) return false; - if (m_ServerAddress.empty()) + if (m_ServerAddressOrHostname.empty()) { PushGuiMessage( "type", "netstatus", @@ -229,7 +228,7 @@ bool CNetClient::TryToConnectWithSTUN(const CStr& hostJID, bool localNetwork) // If the host is on the same network, we risk failing to connect // on routers that don't support NAT hairpinning/NAT loopback. // To work around that, send again a connection data request, but for internal IP this time. - if (ip == m_ServerAddress) + if (ip == m_ServerAddressOrHostname) { g_XmppClient->SendIqGetConnectionData(m_HostJID, m_Password, m_UserName.ToUTF8(), true); // Return true anyways - we're on a success path here. @@ -246,15 +245,15 @@ bool CNetClient::TryToConnectWithSTUN(const CStr& hostJID, bool localNetwork) // Check if we're hosting on localhost, and if so, explicitly use that // (this circumvents, at least, the 'block all incoming connections' setting // on the MacOS firewall). - if (ip == m_ServerAddress) + if (ip == m_ServerAddressOrHostname) { - m_ServerAddress = "127.0.0.1"; + m_ServerAddressOrHostname = "127.0.0.1"; ip = ""; } port = enetClient->address.port; } - LOGMESSAGE("NetClient: connecting to server at %s:%i", m_ServerAddress, m_ServerPort); + LOGMESSAGE("NetClient: connecting to server at %s:%i", m_ServerAddressOrHostname, m_ServerPort); if (!ip.empty()) { @@ -268,7 +267,7 @@ bool CNetClient::TryToConnectWithSTUN(const CStr& hostJID, bool localNetwork) // Step 2: Send a message ourselves to the server so that the NAT, if any, routes incoming trafic correctly. // TODO: verify if this step is necessary, since we'll try and connect anyways below. - StunClient::SendHolePunchingMessages(*enetClient, m_ServerAddress, m_ServerPort); + StunClient::SendHolePunchingMessages(*enetClient, m_ServerAddressOrHostname, m_ServerPort); } if (!g_NetClient->SetupConnection(enetClient)) diff --git a/source/network/NetClient.h b/source/network/NetClient.h index 66cdb76be8..42e1146ac6 100644 --- a/source/network/NetClient.h +++ b/source/network/NetClient.h @@ -79,7 +79,8 @@ public: * The user's name will be displayed to all players. * The JID of the host is needed for the secure lobby authentication. */ - CNetClient(CGame* game, const CStrW& username = L"anonymous", const CStr& hostJID = {}, + CNetClient(CGame* game, std::string serverAddressOrHostname, std::uint16_t serverPont, + const CStrW& username = L"anonymous", const CStr& hostJID = {}, std::string hashedPassword = {}, std::string controllerSecret = {}); /** @@ -99,15 +100,8 @@ public: */ CStr GetGUID() const { return m_GUID; } - /** - * Set connection data to the remote networked server. - * @param address IP address or host name to connect to - */ - void SetupServerData(CStr address, u16 port); - /** * Set up a connection to the remote networked server. - * Must call SetupServerData first. * @return true on success, false on connection failure */ bool SetupConnection(ENetHost* enetClient); @@ -118,7 +112,8 @@ public: * @param localNetwork - if true, assume we are trying to connect on the local network. * @return true on success, false on connection failure */ - bool TryToConnectWithSTUN(const CStr& hostJID, bool localNetwork); + bool TryToConnectWithSTUN(std::string serverAddressOrHostname, std::uint16_t serverPort, + const CStr& hostJID, bool localNetwork); /** * Destroy the connection to the server. @@ -313,7 +308,7 @@ private: CStrW m_UserName; CStr m_HostJID; - CStr m_ServerAddress; + CStr m_ServerAddressOrHostname; u16 m_ServerPort{0}; /** diff --git a/source/network/scripting/JSInterface_Network.cpp b/source/network/scripting/JSInterface_Network.cpp index f4189c8f4d..3752988ae3 100644 --- a/source/network/scripting/JSInterface_Network.cpp +++ b/source/network/scripting/JSInterface_Network.cpp @@ -125,8 +125,8 @@ void StartNetworkHost(const CStrW& playerName, const u16 serverPort, const CStr& // Generate a secret to identify the host client. g_Game = new CGame(storeReplay); - g_NetClient = new CNetClient(g_Game, playerName, hostJID, hashedPassword, secret); - g_NetClient->SetupServerData("127.0.0.1", serverPort); + g_NetClient = new CNetClient(g_Game, "127.0.0.1", serverPort, playerName, hostJID, hashedPassword, + secret); if (!g_NetClient->SetupConnection(nullptr)) { @@ -143,8 +143,7 @@ void StartNetworkJoin(const CStrW& playerName, const CStr& serverAddress, u16 se ENSURE(!g_Game); g_Game = new CGame(storeReplay); - g_NetClient = new CNetClient(g_Game, playerName); - g_NetClient->SetupServerData(serverAddress, serverPort); + g_NetClient = new CNetClient(g_Game, serverAddress, serverPort, playerName); if (!g_NetClient->SetupConnection(nullptr)) { diff --git a/source/network/tests/test_Net.h b/source/network/tests/test_Net.h index f8359a15d4..0fc8956b41 100644 --- a/source/network/tests/test_Net.h +++ b/source/network/tests/test_Net.h @@ -92,7 +92,6 @@ public: TS_ASSERT(server.SetupConnection(PS_DEFAULT_PORT)); for (CNetClient* client: clients) { - client->SetupServerData("127.0.0.1", PS_DEFAULT_PORT); TS_ASSERT(client->SetupConnection(nullptr)); } @@ -182,9 +181,9 @@ public: server.UpdateInitAttributes(&attrs, scriptInterface); - CNetClient client1(&client1Game); - CNetClient client2(&client2Game); - CNetClient client3(&client3Game); + CNetClient client1(&client1Game, "127.0.0.1", PS_DEFAULT_PORT); + CNetClient client2(&client2Game, "127.0.0.1", PS_DEFAULT_PORT); + CNetClient client3(&client3Game, "127.0.0.1", PS_DEFAULT_PORT); clients.push_back(&client1); clients.push_back(&client2); @@ -261,9 +260,9 @@ public: server.UpdateInitAttributes(&attrs, scriptInterface); - CNetClient client1(&client1Game, L"alice"); - CNetClient client2(&client2Game, L"bob"); - CNetClient client3(&client3Game, L"charlie"); + CNetClient client1(&client1Game, "127.0.0.1", PS_DEFAULT_PORT, L"alice"); + CNetClient client2(&client2Game, "127.0.0.1", PS_DEFAULT_PORT, L"bob"); + CNetClient client3(&client3Game, "127.0.0.1", PS_DEFAULT_PORT, L"charlie"); clients.push_back(&client1); clients.push_back(&client2); @@ -318,10 +317,9 @@ public: debug_printf("==== Connecting client 2B\n"); CGame client2BGame(false); - CNetClient client2B(&client2BGame, L"bob"); + CNetClient client2B(&client2BGame, "127.0.0.1", PS_DEFAULT_PORT, L"bob"); clients.push_back(&client2B); - client2B.SetupServerData("127.0.0.1", PS_DEFAULT_PORT); TS_ASSERT(client2B.SetupConnection(nullptr)); for (size_t i = 0; ; ++i)