From e99dc4f3650d64631ba68808d41f78289e2db623 Mon Sep 17 00:00:00 2001 From: jmlee337 Date: Wed, 5 Jul 2023 16:20:38 +0900 Subject: [PATCH 1/4] try to connect to remote players by local and external endpoints where applicable tested singles singles via LAN singles vs symmetric NAT doubles vs double symmetric NAT --- Source/Core/Core/Slippi/SlippiMatchmaking.cpp | 88 +-- Source/Core/Core/Slippi/SlippiMatchmaking.h | 9 +- Source/Core/Core/Slippi/SlippiNetplay.cpp | 575 ++++++++++-------- Source/Core/Core/Slippi/SlippiNetplay.h | 49 +- 4 files changed, 414 insertions(+), 307 deletions(-) diff --git a/Source/Core/Core/Slippi/SlippiMatchmaking.cpp b/Source/Core/Core/Slippi/SlippiMatchmaking.cpp index 4cbc9a0ef0..92c8817dae 100644 --- a/Source/Core/Core/Slippi/SlippiMatchmaking.cpp +++ b/Source/Core/Core/Slippi/SlippiMatchmaking.cpp @@ -454,12 +454,11 @@ void SlippiMatchmaking::handleMatchmaking() return; } - m_isSwapAttempt = false; m_netplayClient = nullptr; // Clear old users - m_remoteIps.clear(); m_playerInfo.clear(); + m_remotePlayers.clear(); std::string matchId = getResp.value("matchId", ""); WARN_LOG(SLIPPI_ONLINE, "Match ID: %s", matchId.c_str()); @@ -493,40 +492,40 @@ void SlippiMatchmaking::handleMatchmaking() if (isLocal) { - std::vector localIpParts; - SplitString(el.value("ipAddress", "1.1.1.1:123"), ':', localIpParts); - localExternalIp = localIpParts[0]; m_localPlayerIndex = playerInfo.port - 1; - } - }; - - // Loop a second time to get the correct remote IPs - for (json::iterator it = queue.begin(); it != queue.end(); ++it) - { - json el = *it; - - if (el.value("port", 0) - 1 == m_localPlayerIndex) - continue; - - auto extIp = el.value("ipAddress", "1.1.1.1:123"); - std::vector exIpParts; - SplitString(extIp, ':', exIpParts); - - auto lanIp = el.value("ipAddressLan", "1.1.1.1:123"); - - WARN_LOG(SLIPPI_ONLINE, "LAN IP: %s", lanIp.c_str()); + std::string ownExIp = el.value("ipAddress", ""); + if (!ownExIp.empty()) + { + std::vector ownExIpParts; + SplitString(ownExIp, ':', ownExIpParts); + ENetAddress addr; + enet_address_set_host(&addr, ownExIpParts[0].c_str()); + m_ownExternalAddress = addr.host; + } - if (exIpParts[0] != localExternalIp || lanIp.empty()) + } + else { - // If external IPs are different, just use that address - m_remoteIps.push_back(extIp); - continue; + struct RemotePlayer remotePlayer; + remotePlayer.index = playerInfo.port - 1; + std::string extIp = el.value("ipAddress", ""); + if (!extIp.empty()) + { + std::vector exIpParts; + SplitString(extIp, ':', exIpParts); + enet_address_set_host(&remotePlayer.externalAddress, exIpParts[0].c_str()); + remotePlayer.externalAddress.port = std::stoi(exIpParts[1]); + } + std::string lanIp = el.value("ipAddressLan", ""); + if (!lanIp.empty()) + { + std::vector lanIpParts; + SplitString(lanIp, ':', lanIpParts); + enet_address_set_host(&remotePlayer.localAddress, lanIpParts[0].c_str()); + remotePlayer.localAddress.port = std::stoi(lanIpParts[1]); + } + m_remotePlayers.push_back(remotePlayer); } - - // TODO: Instead of using one or the other, it might be better to try both - - // If external IPs are the same, try using LAN IPs - m_remoteIps.push_back(lanIp); } } m_isHost = getResp.value("isHost", false); @@ -610,33 +609,10 @@ u8 SlippiMatchmaking::RemotePlayerCount() void SlippiMatchmaking::handleConnecting() { - auto userInfo = m_user->GetUserInfo(); - - m_isSwapAttempt = false; m_netplayClient = nullptr; - u8 remotePlayerCount = (u8)m_remoteIps.size(); - std::vector remoteParts; - std::vector addrs; - std::vector ports; - for (int i = 0; i < m_remoteIps.size(); i++) - { - remoteParts.clear(); - SplitString(m_remoteIps[i], ':', remoteParts); - addrs.push_back(remoteParts[0]); - ports.push_back(std::stoi(remoteParts[1])); - } - - std::stringstream ipLog; - ipLog << "Remote player IPs: "; - for (int i = 0; i < m_remoteIps.size(); i++) - { - ipLog << m_remoteIps[i] << ", "; - } - // INFO_LOG(SLIPPI_ONLINE, "[Matchmaking] My port: %d || %s", m_hostPort, ipLog.str()); - // Is host is now used to specify who the decider is - auto client = std::make_unique(addrs, ports, remotePlayerCount, m_hostPort, m_isHost, + auto client = std::make_unique(m_remotePlayers, m_ownExternalAddress, m_hostPort, m_isHost, m_localPlayerIndex); while (!m_netplayClient) diff --git a/Source/Core/Core/Slippi/SlippiMatchmaking.h b/Source/Core/Core/Slippi/SlippiMatchmaking.h index 957f43445f..a9427f08f4 100644 --- a/Source/Core/Core/Slippi/SlippiMatchmaking.h +++ b/Source/Core/Core/Slippi/SlippiMatchmaking.h @@ -91,16 +91,15 @@ class SlippiMatchmaking SlippiUser *m_user; - int m_isSwapAttempt = false; - int m_hostPort; - int m_localPlayerIndex; - std::vector m_remoteIps; + u8 m_localPlayerIndex; + std::vector m_remotePlayers; MatchmakeResult m_mmResult; std::vector m_playerInfo; std::vector m_allowedStages; bool m_joinedLobby; bool m_isHost; + enet_uint32 m_ownExternalAddress; std::unique_ptr m_netplayClient; @@ -115,8 +114,6 @@ class SlippiMatchmaking void sendMessage(json msg); int receiveMessage(json &msg, int maxAttempts); - void sendHolePunchMsg(std::string remoteIp, u16 remotePort, u16 localPort); - void startMatchmaking(); void handleMatchmaking(); void handleConnecting(); diff --git a/Source/Core/Core/Slippi/SlippiNetplay.cpp b/Source/Core/Core/Slippi/SlippiNetplay.cpp index 8fb73a8f08..5161dd5f79 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.cpp +++ b/Source/Core/Core/Slippi/SlippiNetplay.cpp @@ -17,6 +17,10 @@ #include #include +#ifndef _WIN32 +#include +#endif + //#include "Common/MD5.h" //#include "Common/Common.h" //#include "Common/CommonPaths.h" @@ -35,6 +39,19 @@ static std::mutex pad_mutex; static std::mutex ack_mutex; +static slippi_endpoint toSlippiEndpoint(ENetAddress enetAddress) +{ + slippi_endpoint out = enetAddress.host; + out = out << 16; + out += enetAddress.port; + return out; +} + +static enet_uint32 getHost(slippi_endpoint slippiEndpoint) +{ + return static_cast(slippiEndpoint >> 16); +} + SlippiNetplayClient *SLIPPI_NETPLAY = nullptr; // called from ---GUI--- thread @@ -44,7 +61,7 @@ SlippiNetplayClient::~SlippiNetplayClient() if (m_thread.joinable()) m_thread.join(); - if (!m_server.empty()) + if (!m_connectedPeers.empty()) { Disconnect(); } @@ -65,8 +82,8 @@ SlippiNetplayClient::~SlippiNetplayClient() } // called from ---SLIPPI EXI--- thread -SlippiNetplayClient::SlippiNetplayClient(std::vector addrs, std::vector ports, - const u8 remotePlayerCount, const u16 localPort, bool isDecider, u8 playerIdx) +SlippiNetplayClient::SlippiNetplayClient(std::vector remotePlayers, enet_uint32 ownExternalAddress, + const u16 localPort, bool isDecider, u8 playerIdx) #ifdef _WIN32 : m_qos_handle(nullptr) , m_qos_flow_id(0) @@ -75,7 +92,7 @@ SlippiNetplayClient::SlippiNetplayClient(std::vector addrs, std::ve WARN_LOG(SLIPPI_ONLINE, "Initializing Slippi Netplay for port: %d, with host: %s, player idx: %d", localPort, isDecider ? "true" : "false", playerIdx); this->isDecider = isDecider; - this->m_remotePlayerCount = remotePlayerCount; + this->m_remotePlayerCount = static_cast(remotePlayers.size()); this->playerIdx = playerIdx; // Set up remote player data structures @@ -113,39 +130,50 @@ SlippiNetplayClient::SlippiNetplayClient(std::vector addrs, std::ve localAddr = &localAddrDef; } - // TODO: Figure out how to use a local port when not hosting without accepting incoming connections - m_client = enet_host_create(localAddr, 10, 3, 0, 0); - - if (m_client == nullptr) + int numAttemptedConnections = 0; + for (auto remotePlayer : remotePlayers) { - PanicAlertT("Couldn't Create Client"); + if (remotePlayer.localAddress.host && ownExternalAddress == remotePlayer.externalAddress.host) + numAttemptedConnections++; + if (remotePlayer.externalAddress.host) + numAttemptedConnections++; } + // Connections are attempted bidirectionally to satisfy firewalls, so *2. Then +1 so that if we somehow recieve an + // unexpected connection (we will immediately disconnect) we won't run out of peers. + int numMaxPeers = numAttemptedConnections * 2 + 1; + m_client = enet_host_create(localAddr, numMaxPeers, 3, 0, 0); + if (m_client == nullptr) + PanicAlertT("Couldn't Create Client"); - for (int i = 0; i < remotePlayerCount; i++) + for (auto remotePlayer : remotePlayers) { - ENetAddress addr; - enet_address_set_host(&addr, addrs[i].c_str()); - addr.port = ports[i]; - // INFO_LOG(SLIPPI_ONLINE, "Set ENet host, addr = %x, port = %d", addr.host, addr.port); - - ENetPeer *peer = enet_host_connect(m_client, &addr, 3, 0); - m_server.push_back(peer); - - // Store this connection - std::stringstream keyStrm; - keyStrm << addr.host << "-" << addr.port; - activeConnections[keyStrm.str()][peer] = true; - INFO_LOG(SLIPPI_ONLINE, "New connection (constr): %s, %X", keyStrm.str().c_str(), peer); - - if (peer == nullptr) + bool useLocalEndpoint = + remotePlayer.localAddress.host && ownExternalAddress == remotePlayer.externalAddress.host; + if (useLocalEndpoint) { - PanicAlertT("Couldn't create peer."); + ENetPeer *peer = enet_host_connect(m_client, &remotePlayer.localAddress, 3, 0); + if (peer == nullptr) + PanicAlertT("Couldn't create peer."); + endpointToIndexAndType[toSlippiEndpoint(remotePlayer.localAddress)] = { + remotePlayer.index, SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL}; + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Connecting to peer (local): %s:%d, %X", + inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); } - else + if (remotePlayer.externalAddress.host) { - // INFO_LOG(SLIPPI_ONLINE, "Connecting to ENet host, addr = %x, port = %d", peer->address.host, - // peer->address.port); + ENetPeer *peer = enet_host_connect(m_client, &remotePlayer.externalAddress, 3, 0); + if (peer == nullptr) + PanicAlertT("Couldn't create peer."); + endpointToIndexAndType[toSlippiEndpoint(remotePlayer.externalAddress)] = { + remotePlayer.index, SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL}; + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Connecting to peer (external): %s:%d, %X", + inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); } + indexToConnectionManager.insert( + {remotePlayer.index, + SlippiConnectionManager(useLocalEndpoint + ? SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL + : SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL)}); } slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_INITIATED; @@ -229,44 +257,6 @@ unsigned int SlippiNetplayClient::OnData(sf::Packet &packet, ENetPeer *peer) //ERROR_LOG(SLIPPI_ONLINE, "Received Checksum. CurFrame: %d, ChkFrame: %d, Chk: %08x", frame, checksumFrame, checksum); - // This fetches the m_server index that stores the connection we want to overwrite (if necessary). Note that - // this index is not necessarily the same as the pIdx because if we have users connecting with the same - // WAN, the m_server indices might not match - int connIdx = 0; - for (int i = 0; i < m_server.size(); i++) - { - if (peer->address.host == m_server[i]->address.host && peer->address.port == m_server[i]->address.port) - { - connIdx = i; - break; - } - } - - // Here we check if we have more than 1 connection for a specific player, this can happen because both - // players try to connect to each other at the same time to increase the odds that one direction might - // work and for hole punching. That said there's no point in keeping more than 1 connection alive. I - // think they might use bandwidth with keep alives or something. Only the lower port player will - // initiate the disconnect - std::stringstream keyStrm; - keyStrm << peer->address.host << "-" << peer->address.port; - if (activeConnections[keyStrm.str()].size() > 1 && playerIdx <= pIdx) - { - m_server[connIdx] = peer; - INFO_LOG(SLIPPI_ONLINE, - "Multiple connections detected for single peer. %x:%d. %x. Disconnecting superfluous " - "connections. oppIdx: %d. pIdx: %d", - peer->address.host, peer->address.port, peer, pIdx, playerIdx); - - for (auto activeConn : activeConnections[keyStrm.str()]) - { - if (activeConn.first == peer) - continue; - - // Tell our peer to terminate this connection - enet_peer_disconnect(activeConn.first, 0); - } - } - // Pad received, try to guess what our local time was when the frame was sent by our opponent // before we initialized // We can compare this to when we sent a pad for last frame to figure out how far/behind we @@ -644,23 +634,19 @@ std::unique_ptr SlippiNetplayClient::readSelectionsFromP void SlippiNetplayClient::Send(sf::Packet &packet) { - enet_uint32 flags = ENET_PACKET_FLAG_RELIABLE; - u8 channelId = 0; + MessageId mid = ((u8 *)packet.getData())[0]; - for (int i = 0; i < m_server.size(); i++) - { - MessageId mid = ((u8 *)packet.getData())[0]; - if (mid == NP_MSG_SLIPPI_PAD || mid == NP_MSG_SLIPPI_PAD_ACK) - { - // Slippi communications do not need reliable connection and do not need to - // be received in order. Channel is changed so that other reliable communications - // do not block anything. This may not be necessary if order is not maintained? - flags = ENET_PACKET_FLAG_UNSEQUENCED; - channelId = 1; - } + // Slippi communications do not need reliable connection and do not need to + // be received in order. Channel is changed so that other reliable communications + // do not block anything. This may not be necessary if order is not maintained? + bool isUnsequenced = mid == NP_MSG_SLIPPI_PAD || mid == NP_MSG_SLIPPI_PAD_ACK; + enet_uint32 flags = isUnsequenced ? ENET_PACKET_FLAG_UNSEQUENCED : ENET_PACKET_FLAG_RELIABLE; + u8 channelId = isUnsequenced ? 1 : 0; + for (auto peer : m_connectedPeers) + { ENetPacket *epac = enet_packet_create(packet.getData(), packet.getDataSize(), flags); - int sendResult = enet_peer_send(m_server[i], channelId, epac); + int sendResult = enet_peer_send(peer, channelId, epac); } } @@ -668,18 +654,13 @@ void SlippiNetplayClient::Disconnect() { ENetEvent netEvent; slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_DISCONNECTED; - if (activeConnections.empty()) - { + if (m_connectedPeers.empty()) return; - } - for (auto conn : activeConnections) + for (auto peer : m_connectedPeers) { - for (auto peer : conn.second) - { - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Disconnecting peer %d", peer.first->address.port); - enet_peer_disconnect(peer.first, 0); - } + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Disconnecting peer %d", peer->address.port); + enet_peer_disconnect(peer, 0); } while (enet_host_service(m_client, &netEvent, 3000) > 0) @@ -698,15 +679,9 @@ void SlippiNetplayClient::Disconnect() } // didn't disconnect gracefully force disconnect - for (auto conn : activeConnections) - { - for (auto peer : conn.second) - { - enet_peer_reset(peer.first); - } - } - activeConnections.clear(); - m_server.clear(); + for (auto peer : m_connectedPeers) + enet_peer_reset(peer); + m_connectedPeers.clear(); SLIPPI_NETPLAY = nullptr; } @@ -720,19 +695,46 @@ void SlippiNetplayClient::SendAsync(std::unique_ptr packet) } // called from ---NETPLAY--- thread -void SlippiNetplayClient::ThreadFunc() +void SlippiNetplayClient::SelectConnectedPeers() { - // Let client die 1 second before host such that after a swap, the client won't be connected to - u64 startTime = Common::Timer::GetTimeMs(); - u64 timeout = 8000; + if (LogTypes::LINFO <= MAX_LOGLEVEL) + { + for (int i = 0; i < m_client->peerCount; i++) + { + auto peer = &m_client->peers[i]; + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Peer: %s:%d, %X (state: %d)", inet_ntoa(*(in_addr *)&peer->address.host), + peer->address.port, peer, peer->state); + } + } - std::vector connections; - std::vector remoteAddrs; - for (int i = 0; i < m_remotePlayerCount; i++) + for (auto indexAndConnectionManager : indexToConnectionManager) { - remoteAddrs.push_back(m_server[i]->address); - connections.push_back(false); + SlippiConnectionManager connectionManager = indexAndConnectionManager.second; + // By convention, the lower number port disconnects any superfluous connections. Connections will be initially + // duplicate most of the time since the connection should always be attempted from both ends. + if (playerIdx >= indexAndConnectionManager.first) + connectionManager.SelectAllConnections(m_connectedPeers); + else + connectionManager.SelectOneConnection(m_connectedPeers); } + m_client->intercept = ENetUtil::InterceptCallback; + + if (LogTypes::LINFO <= MAX_LOGLEVEL) + { + for (auto peer : m_connectedPeers) + { + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer: %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), + peer->address.port, peer); + } + } + INFO_LOG(SLIPPI_ONLINE, "Slippi online connection successful!"); +} + +// called from ---NETPLAY--- thread +void SlippiNetplayClient::ThreadFunc() +{ + u64 startTime = Common::Timer::GetTimeMs(); + u64 timeout = 8000; while (slippiConnectStatus == SlippiConnectStatus::NET_CONNECT_STATUS_INITIATED) { @@ -741,148 +743,162 @@ void SlippiNetplayClient::ThreadFunc() int net = enet_host_service(m_client, &netEvent, 500); if (net > 0) { + auto peer = netEvent.peer; sf::Packet rpac; switch (netEvent.type) { case ENET_EVENT_TYPE_RECEIVE: - if (!netEvent.peer) + { + if (!peer) { INFO_LOG(SLIPPI_ONLINE, "[Netplay] got receive event with nil peer"); continue; } - INFO_LOG(SLIPPI_ONLINE, "[Netplay] got receive event with peer addr %x:%d", netEvent.peer->address.host, - netEvent.peer->address.port); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] got receive event with peer addr %x:%d", peer->address.host, + peer->address.port); rpac.append(netEvent.packet->data, netEvent.packet->dataLength); - OnData(rpac, netEvent.peer); + OnData(rpac, peer); enet_packet_destroy(netEvent.packet); break; - + } case ENET_EVENT_TYPE_DISCONNECT: - if (!netEvent.peer) + { + if (!peer) { - INFO_LOG(SLIPPI_ONLINE, "[Netplay] got disconnect event with nil peer"); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] got disconnect event (early) with nil peer"); continue; } - INFO_LOG(SLIPPI_ONLINE, "[Netplay] got disconnect event with peer addr %x:%d. %x", - netEvent.peer->address.host, netEvent.peer->address.port, netEvent.peer); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Disconnect (early) %x:%d, %X", + inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); break; - + } case ENET_EVENT_TYPE_CONNECT: { - if (!netEvent.peer) + if (!peer) { INFO_LOG(SLIPPI_ONLINE, "[Netplay] got connect event with nil peer"); continue; } - std::stringstream keyStrm; - keyStrm << netEvent.peer->address.host << "-" << netEvent.peer->address.port; - activeConnections[keyStrm.str()][netEvent.peer] = true; - INFO_LOG(SLIPPI_ONLINE, "New connection (early): %s, %X", keyStrm.str().c_str(), netEvent.peer); - - for (auto pair : activeConnections) - { - INFO_LOG(SLIPPI_ONLINE, "%s: %d", pair.first.c_str(), pair.second.size()); - } - - INFO_LOG(SLIPPI_ONLINE, "[Netplay] got connect event with peer addr %x:%d. %x", - netEvent.peer->address.host, netEvent.peer->address.port, netEvent.peer); - - auto isAlreadyConnected = false; - for (int i = 0; i < m_server.size(); i++) + auto host = peer->address.host; + auto port = peer->address.port; + char *addressStr = inet_ntoa(*(in_addr *)&host); + slippi_endpoint endpoint = toSlippiEndpoint(peer->address); + auto found = endpointToIndexAndType.find(endpoint); + if (found == endpointToIndexAndType.end()) { - if (connections[i] && netEvent.peer->address.host == m_server[i]->address.host && - netEvent.peer->address.port == m_server[i]->address.port) + // It's possible (ie. symmetric NAT + permissive firewall) for connections from remote players to + // originate from a different port than we expect, so let's check if we're expecting remote players + // at this address + std::vector> potentialMatches; + for (auto endpointAndIndexAndType : endpointToIndexAndType) + if (getHost(endpointAndIndexAndType.first) == host) + potentialMatches.push_back(endpointAndIndexAndType.second); + switch (potentialMatches.size()) { - m_server[i] = netEvent.peer; - isAlreadyConnected = true; - break; + case 0: + { + // Not just the port, we don't even know about this address. Reject to be safe. + enet_peer_disconnect(peer, 0); + ERROR_LOG(SLIPPI_ONLINE, "[Netplay] Unexpected connection from unexpected address %s:%d, %X", + addressStr, port, peer); + continue; } - } - - if (isAlreadyConnected) - { - // Don't add this person again if they are already connected. Not doing this can cause one person to - // take up 2 or more spots, denying one or more players from connecting and thus getting stuck on - // the "Waiting" step - INFO_LOG(SLIPPI_ONLINE, "Already connected!"); - break; // Breaks out of case - } - - for (int i = 0; i < m_server.size(); i++) - { - // This check used to check for port as well as host. The problem was that for some people, their - // internet will switch the port they're sending from. This means these people struggle to connect - // to others but they sometimes do succeed. When we were checking for port here though we would get - // into a state where the person they succeeded to connect to would not accept the connection with - // them, this would lead the player with this internet issue to get stuck waiting for the other - // player. The only downside to this that I can guess is that if you fail to connect to one person - // out of two that are on your LAN, it might report that you failed to connect to the wrong person. - // There might be more problems tho, not sure - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Comparing connection address: %x - %x", remoteAddrs[i].host, - netEvent.peer->address.host); - if (remoteAddrs[i].host == netEvent.peer->address.host && !connections[i]) + case 1: { - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Overwriting ENetPeer for address: %x:%d", - netEvent.peer->address.host, netEvent.peer->address.port); - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Overwriting ENetPeer with id (%d) with new peer of id %d", - m_server[i]->connectID, netEvent.peer->connectID); - m_server[i] = netEvent.peer; - connections[i] = true; - break; + // It's clear which remote player this new port corresponds to + endpointToIndexAndType.insert({endpoint, potentialMatches[0]}); + indexToConnectionManager[potentialMatches[0].first].InsertConnection( + SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL, peer); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Matched unexpected endpoint %s:%d, %X to player index %d", + addressStr, port, peer, potentialMatches[0].first); } + default: + { + // Multiple remote players share the same external IP address. We don't actually need to know + // which one is which. Just keep track of them to make sure the total number of connections is + // correct. + unexpectedPeers.push_back(peer); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Unexpected connection from %s:%d, %X", addressStr, port, peer); + } + } + } + else + { + indexToConnectionManager[found->second.first].InsertConnection(found->second.second, peer); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] New connection (ontime) %s:%d, %X", addressStr, port, peer); } break; } } } - bool allConnected = true; - for (int i = 0; i < m_remotePlayerCount; i++) + // We can early terminate this phase IFF we've connected to every remote player's highest priority endpoint + bool enoughConnected = true; + for (auto indexAndConnectionManager : indexToConnectionManager) + if (!indexAndConnectionManager.second.HasHighestPriorityConnection()) + enoughConnected = false; + if (enoughConnected) { - if (!connections[i]) - allConnected = false; - } - - if (allConnected) - { - m_client->intercept = ENetUtil::InterceptCallback; - INFO_LOG(SLIPPI_ONLINE, "Slippi online connection successful!"); + SelectConnectedPeers(); slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_CONNECTED; break; } - for (int i = 0; i < m_remotePlayerCount; i++) - { - INFO_LOG(SLIPPI_ONLINE, "m_client peer %d state: %d", i, m_client->peers[i].state); - } INFO_LOG(SLIPPI_ONLINE, "[Netplay] Not yet connected. Res: %d, Type: %d", net, netEvent.type); // Time out after enough time has passed u64 curTime = Common::Timer::GetTimeMs(); if ((curTime - startTime) >= timeout || !m_do_loop.IsSet()) { - for (int i = 0; i < m_remotePlayerCount; i++) + bool enoughConnected = true; + for (auto indexAndConnectionManager : indexToConnectionManager) { - if (!connections[i]) + if (!indexAndConnectionManager.second.HasAnyConnection()) { - failedConnections.push_back(i); + enoughConnected = false; + failedConnections.push_back(indexAndConnectionManager.first); } } - slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_FAILED; - INFO_LOG(SLIPPI_ONLINE, "Slippi online connection failed"); - return; + const auto numUnexpectedPeers = unexpectedPeers.size(); + if (!enoughConnected && numUnexpectedPeers > 0) + { + const auto numFailedConnections = failedConnections.size(); + if (numFailedConnections == numUnexpectedPeers) + { + enoughConnected = true; + failedConnections.clear(); + m_connectedPeers.insert(m_connectedPeers.end(), unexpectedPeers.begin(), unexpectedPeers.end()); + } + else if (numFailedConnections < numUnexpectedPeers) + { + // This would imply that at least one remote player connected to us from multiple unexpected + // endpoints. This shouldn't be possible as players should only have one endpoint that could be + // subject to symmetric NAT (external). + PanicAlertT("Number of unexpected peers exceeds number of failed connections"); + } + // if numFailedConnections > numUnexpectedPeers then at least one remote player really did fail to connect. + } + + if (enoughConnected) + { + SelectConnectedPeers(); + slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_CONNECTED; + break; + } + else + { + slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_FAILED; + INFO_LOG(SLIPPI_ONLINE, "Slippi online connection failed"); + return; + } } } - INFO_LOG(SLIPPI_ONLINE, "Successfully initialized %d connections", m_server.size()); - for (int i = 0; i < m_server.size(); i++) - { - INFO_LOG(SLIPPI_ONLINE, "Connection %d: %d, %d", i, m_server[i]->address.host, m_server[i]->address.port); - } + INFO_LOG(SLIPPI_ONLINE, "Successfully initialized %d connections", m_connectedPeers.size()); bool qos_success = false; #ifdef _WIN32 @@ -890,16 +906,16 @@ void SlippiNetplayClient::ThreadFunc() if (SConfig::GetInstance().bQoSEnabled && QOSCreateHandle(&ver, &m_qos_handle)) { - for (int i = 0; i < m_server.size(); i++) + for (auto peer : m_connectedPeers) { // from win32.c struct sockaddr_in sin = {0}; sin.sin_family = AF_INET; - sin.sin_port = ENET_HOST_TO_NET_16(m_server[i]->host->address.port); - sin.sin_addr.s_addr = m_server[i]->host->address.host; + sin.sin_port = ENET_HOST_TO_NET_16(peer->host->address.port); + sin.sin_addr.s_addr = peer->host->address.host; - if (QOSAddSocketToFlow(m_qos_handle, m_server[i]->host->socket, reinterpret_cast(&sin), + if (QOSAddSocketToFlow(m_qos_handle, peer->host->socket, reinterpret_cast(&sin), // this is 0x38 QOSTrafficTypeControl, QOS_NON_ADAPTIVE_FLOW, &m_qos_flow_id)) { @@ -916,18 +932,18 @@ void SlippiNetplayClient::ThreadFunc() #else if (SConfig::GetInstance().bQoSEnabled) { - for (int i = 0; i < m_server.size(); i++) + for (auto peer : m_connectedPeers) { #ifdef __linux__ // highest priority int priority = 7; - setsockopt(m_server[i]->host->socket, SOL_SOCKET, SO_PRIORITY, &priority, sizeof(priority)); + setsockopt(peer->host->socket, SOL_SOCKET, SO_PRIORITY, &priority, sizeof(priority)); #endif // https://www.tucny.com/Home/dscp-tos // ef is better than cs7 int tos_val = 0xb8; - qos_success = setsockopt(m_server[i]->host->socket, IPPROTO_IP, IP_TOS, &tos_val, sizeof(tos_val)) == 0; + qos_success = setsockopt(peer->host->socket, IPPROTO_IP, IP_TOS, &tos_val, sizeof(tos_val)) == 0; } } #endif @@ -945,6 +961,7 @@ void SlippiNetplayClient::ThreadFunc() if (net > 0) { + auto peer = netEvent.peer; sf::Packet rpac; bool isConnectedClient = false; switch (netEvent.type) @@ -952,61 +969,60 @@ void SlippiNetplayClient::ThreadFunc() case ENET_EVENT_TYPE_RECEIVE: { rpac.append(netEvent.packet->data, netEvent.packet->dataLength); - OnData(rpac, netEvent.peer); + OnData(rpac, peer); enet_packet_destroy(netEvent.packet); break; } case ENET_EVENT_TYPE_DISCONNECT: { - std::stringstream keyStrm; - keyStrm << netEvent.peer->address.host << "-" << netEvent.peer->address.port; - activeConnections[keyStrm.str()].erase(netEvent.peer); - - for (auto pair : activeConnections) - { - INFO_LOG(SLIPPI_ONLINE, "%s: %d", pair.first.c_str(), pair.second.size()); - } - - // Check to make sure this address+port are one of the ones we are actually connected to. - // When connecting to someone that randomizes ports, you can get one valid connection from - // one port and a failed connection on another port. We don't want to cause a real disconnect - // if we receive a disconnect message from the port we never connected to - bool isConnectedClient = false; - for (int i = 0; i < m_server.size(); i++) + auto host = peer->address.host; + auto port = peer->address.port; + for (auto it = m_connectedPeers.begin(); it != m_connectedPeers.end(); it++) { - if (netEvent.peer->address.host == m_server[i]->address.host && - netEvent.peer->address.port == m_server[i]->address.port) + if (*it == peer) { - isConnectedClient = true; + m_connectedPeers.erase(it); break; } } - INFO_LOG(SLIPPI_ONLINE, - "[Netplay] Disconnect late %x:%d. %x. Remaining connections: %d. Is connected client: %s", - netEvent.peer->address.host, netEvent.peer->address.port, netEvent.peer, - activeConnections[keyStrm.str()].size(), isConnectedClient ? "true" : "false"); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Disconnected %s:%d, %X", inet_ntoa(*(in_addr *)&host), port, peer); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Remaining peers:"); + for (auto peer : m_connectedPeers) + { + INFO_LOG(SLIPPI_ONLINE, "%s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), + peer->address.port, peer); + } - // If the disconnect event doesn't come from the client we are actually listening to, - // it can be safely ignored - if (isConnectedClient && activeConnections[keyStrm.str()].empty()) + if (m_connectedPeers.size() < m_remotePlayerCount) { - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Final disconnect received for a client."); + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Final disconnect received for a client."); m_do_loop.Clear(); // Stop the loop, will trigger a disconnect } break; } case ENET_EVENT_TYPE_CONNECT: { - std::stringstream keyStrm; - keyStrm << netEvent.peer->address.host << "-" << netEvent.peer->address.port; - activeConnections[keyStrm.str()][netEvent.peer] = true; - INFO_LOG(SLIPPI_ONLINE, "New connection (late): %s, %X", keyStrm.str().c_str(), netEvent.peer); - for (auto pair : activeConnections) + auto host = peer->address.host; + auto port = peer->address.port; + slippi_endpoint endpoint = toSlippiEndpoint(peer->address); + char *addressStr = inet_ntoa(*(in_addr *)&host); + auto found = endpointToIndexAndType.find(endpoint); + if (found == endpointToIndexAndType.end()) { - INFO_LOG(SLIPPI_ONLINE, "%s: %d", pair.first.c_str(), pair.second.size()); + ERROR_LOG(SLIPPI_ONLINE, "[Netplay] Unexpected connection (late) from %s:%d", addressStr, port); + enet_peer_disconnect(peer, 0); + continue; + } + + // Disregard late connections and disconnect if we are lower number port + // There's very little chance a higher priority connection would be late + WARN_LOG(SLIPPI_ONLINE, "[Netplay] Superfluous connection (late): %s:%d, %X", addressStr, port, peer); + if (playerIdx < found->second.first) + { + enet_peer_disconnect(peer, 0); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Requesting disconnect: %s:%d, %X", addressStr, port, peer); } - break; } default: break; @@ -1018,12 +1034,8 @@ void SlippiNetplayClient::ThreadFunc() if (m_qos_handle != 0) { if (m_qos_flow_id != 0) - { - for (int i = 0; i < m_server.size(); i++) - { - QOSRemoveSocketFromFlow(m_qos_handle, m_server[i]->host->socket, m_qos_flow_id, 0); - } - } + for (auto peer : m_connectedPeers) + QOSRemoveSocketFromFlow(m_qos_handle, peer->host->socket, m_qos_flow_id, 0); QOSCloseHandle(m_qos_handle); } @@ -1552,3 +1564,82 @@ SlippiDesyncRecoveryResp SlippiNetplayClient::GetDesyncRecoveryState() return result; } + +SlippiConnectionManager::SlippiConnectionManager() +{ + m_highestPriority = SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL; +} + +SlippiConnectionManager::SlippiConnectionManager(SlippiConnectionManager::SlippiEndpointType highestPriority) +{ + m_highestPriority = highestPriority; +} + +bool SlippiConnectionManager::HasAnyConnection() +{ + return !m_localPeers.empty() || !m_externalPeers.empty(); +} + +bool SlippiConnectionManager::HasHighestPriorityConnection() +{ + switch (m_highestPriority) + { + case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL: + return !m_localPeers.empty(); + case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL: + return !m_externalPeers.empty(); + default: + return false; + } +} + +void SlippiConnectionManager::InsertConnection(SlippiConnectionManager::SlippiEndpointType endpointType, ENetPeer* peer) +{ + switch (endpointType) + { + case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL: + { + m_localPeers.push_back(peer); + break; + } + case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL: + { + m_externalPeers.push_back(peer); + break; + } + } +} + +void SlippiConnectionManager::SelectAllConnections(std::vector& connections) +{ + connections.insert(connections.end(), m_localPeers.begin(), m_localPeers.end()); + connections.insert(connections.end(), m_externalPeers.begin(), m_externalPeers.end()); +} + +void SlippiConnectionManager::SelectOneConnection(std::vector& connections) { + bool found = false; + for (auto peer : m_localPeers) + { + if (found) + { + enet_peer_disconnect(peer, 0); + } + else + { + connections.push_back(peer); + found = true; + } + } + for (auto peer : m_externalPeers) + { + if (found) + { + enet_peer_disconnect(peer, 0); + } + else + { + connections.push_back(peer); + found = true; + } + } +} diff --git a/Source/Core/Core/Slippi/SlippiNetplay.h b/Source/Core/Core/Slippi/SlippiNetplay.h index ac90d56bd7..1b85891284 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.h +++ b/Source/Core/Core/Slippi/SlippiNetplay.h @@ -128,6 +128,45 @@ struct ChecksumEntry u32 value; }; +struct RemotePlayer +{ + u8 index; // port - 1 + ENetAddress externalAddress; + ENetAddress localAddress; +}; + +class SlippiConnectionManager +{ + public: + enum class SlippiEndpointType + { + ENDPOINT_TYPE_LOCAL, + ENDPOINT_TYPE_EXTERNAL, + }; + + SlippiConnectionManager(); + SlippiConnectionManager(SlippiEndpointType highestPriority); + bool HasAnyConnection(); + void InsertConnection(SlippiEndpointType endpointType, ENetPeer *peer); + + // Whether there are any connections of the highest priority type + bool HasHighestPriorityConnection(); + + // Add all connections to the input vector + void SelectAllConnections(std::vector& connections); + + // Add the most preferred connection to the input vector, disconnect any others + void SelectOneConnection(std::vector& connections); + + protected: + SlippiEndpointType m_highestPriority; + std::vector m_localPeers; + std::vector m_externalPeers; +}; + +// For use in std containers, we shove the u32 address and u16 port into one u64 +typedef u64 slippi_endpoint; + class SlippiMatchInfo { public: @@ -152,7 +191,7 @@ class SlippiNetplayClient void SendAsync(std::unique_ptr packet); SlippiNetplayClient(bool isDecider); // Make a dummy client - SlippiNetplayClient(std::vector addrs, std::vector ports, const u8 remotePlayerCount, + SlippiNetplayClient(std::vector remotePlayers, enet_uint32 ownExternalAddress, const u16 localPort, bool isDecider, u8 playerIdx); ~SlippiNetplayClient(); @@ -208,7 +247,7 @@ class SlippiNetplayClient Common::FifoQueue, false> m_async_queue; ENetHost *m_client = nullptr; - std::vector m_server; + std::vector m_connectedPeers; std::thread m_thread; u8 m_remotePlayerCount = 0; @@ -239,7 +278,10 @@ class SlippiNetplayClient bool hasGameStarted = false; u8 playerIdx = 0; - std::unordered_map> activeConnections; + std::unordered_map> + endpointToIndexAndType; + std::unordered_map indexToConnectionManager; + std::vector unexpectedPeers; std::deque> localPadQueue; // most recent inputs at start of deque std::deque> @@ -272,6 +314,7 @@ class SlippiNetplayClient unsigned int OnData(sf::Packet &packet, ENetPeer *peer); void Send(sf::Packet &packet); void Disconnect(); + void SelectConnectedPeers(); bool m_is_connected = false; From f4900c1697aacdce66014c6c27c5841e2296af31 Mon Sep 17 00:00:00 2001 From: jmlee337 Date: Sat, 15 Jul 2023 20:04:33 +0900 Subject: [PATCH 2/4] SlippiConnectionManager methods filter out non-connected peers this is necessary because early disconnects are possible (in case of network failures, physical disconnect, etc) --- Source/Core/Core/Slippi/SlippiNetplay.cpp | 33 ++++++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/Source/Core/Core/Slippi/SlippiNetplay.cpp b/Source/Core/Core/Slippi/SlippiNetplay.cpp index 5161dd5f79..54faf37cb6 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.cpp +++ b/Source/Core/Core/Slippi/SlippiNetplay.cpp @@ -1577,7 +1577,13 @@ SlippiConnectionManager::SlippiConnectionManager(SlippiConnectionManager::Slippi bool SlippiConnectionManager::HasAnyConnection() { - return !m_localPeers.empty() || !m_externalPeers.empty(); + for (auto peer : m_localPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + return true; + for (auto peer : m_externalPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + return true; + return false; } bool SlippiConnectionManager::HasHighestPriorityConnection() @@ -1585,12 +1591,17 @@ bool SlippiConnectionManager::HasHighestPriorityConnection() switch (m_highestPriority) { case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL: - return !m_localPeers.empty(); + for (auto peer : m_localPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + return true; + break; case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL: - return !m_externalPeers.empty(); - default: - return false; + for (auto peer : m_externalPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + return true; + break; } + return false; } void SlippiConnectionManager::InsertConnection(SlippiConnectionManager::SlippiEndpointType endpointType, ENetPeer* peer) @@ -1612,14 +1623,20 @@ void SlippiConnectionManager::InsertConnection(SlippiConnectionManager::SlippiEn void SlippiConnectionManager::SelectAllConnections(std::vector& connections) { - connections.insert(connections.end(), m_localPeers.begin(), m_localPeers.end()); - connections.insert(connections.end(), m_externalPeers.begin(), m_externalPeers.end()); + for (auto peer : m_localPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + connections.push_back(peer); + for (auto peer : m_externalPeers) + if (peer->state == ENET_PEER_STATE_CONNECTED) + connections.push_back(peer); } void SlippiConnectionManager::SelectOneConnection(std::vector& connections) { bool found = false; for (auto peer : m_localPeers) { + if (peer->state != ENET_PEER_STATE_CONNECTED) + continue; if (found) { enet_peer_disconnect(peer, 0); @@ -1632,6 +1649,8 @@ void SlippiConnectionManager::SelectOneConnection(std::vector& connec } for (auto peer : m_externalPeers) { + if (peer->state != ENET_PEER_STATE_CONNECTED) + continue; if (found) { enet_peer_disconnect(peer, 0); From 440856d32e16edd6be1973b9f69f1cad628caa89 Mon Sep 17 00:00:00 2001 From: jmlee337 Date: Sat, 15 Jul 2023 22:09:10 +0900 Subject: [PATCH 3/4] Move SlippiEndpointType and SlippiConnectionManager inside SlippiNetplayClient --- Source/Core/Core/Slippi/SlippiNetplay.cpp | 41 +++++++-------- Source/Core/Core/Slippi/SlippiNetplay.h | 63 +++++++++++------------ 2 files changed, 51 insertions(+), 53 deletions(-) diff --git a/Source/Core/Core/Slippi/SlippiNetplay.cpp b/Source/Core/Core/Slippi/SlippiNetplay.cpp index 54faf37cb6..f9b74a6ecc 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.cpp +++ b/Source/Core/Core/Slippi/SlippiNetplay.cpp @@ -155,7 +155,7 @@ SlippiNetplayClient::SlippiNetplayClient(std::vector remote if (peer == nullptr) PanicAlertT("Couldn't create peer."); endpointToIndexAndType[toSlippiEndpoint(remotePlayer.localAddress)] = { - remotePlayer.index, SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL}; + remotePlayer.index, EndpointType::ENDPOINT_TYPE_LOCAL}; WARN_LOG(SLIPPI_ONLINE, "[Netplay] Connecting to peer (local): %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); } @@ -165,15 +165,13 @@ SlippiNetplayClient::SlippiNetplayClient(std::vector remote if (peer == nullptr) PanicAlertT("Couldn't create peer."); endpointToIndexAndType[toSlippiEndpoint(remotePlayer.externalAddress)] = { - remotePlayer.index, SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL}; + remotePlayer.index, EndpointType::ENDPOINT_TYPE_EXTERNAL}; WARN_LOG(SLIPPI_ONLINE, "[Netplay] Connecting to peer (external): %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); } indexToConnectionManager.insert( - {remotePlayer.index, - SlippiConnectionManager(useLocalEndpoint - ? SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL - : SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL)}); + {remotePlayer.index, ConnectionManager(useLocalEndpoint ? EndpointType::ENDPOINT_TYPE_LOCAL + : EndpointType::ENDPOINT_TYPE_EXTERNAL)}); } slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_INITIATED; @@ -709,7 +707,7 @@ void SlippiNetplayClient::SelectConnectedPeers() for (auto indexAndConnectionManager : indexToConnectionManager) { - SlippiConnectionManager connectionManager = indexAndConnectionManager.second; + ConnectionManager connectionManager = indexAndConnectionManager.second; // By convention, the lower number port disconnects any superfluous connections. Connections will be initially // duplicate most of the time since the connection should always be attempted from both ends. if (playerIdx >= indexAndConnectionManager.first) @@ -792,7 +790,7 @@ void SlippiNetplayClient::ThreadFunc() // It's possible (ie. symmetric NAT + permissive firewall) for connections from remote players to // originate from a different port than we expect, so let's check if we're expecting remote players // at this address - std::vector> potentialMatches; + std::vector> potentialMatches; for (auto endpointAndIndexAndType : endpointToIndexAndType) if (getHost(endpointAndIndexAndType.first) == host) potentialMatches.push_back(endpointAndIndexAndType.second); @@ -811,7 +809,7 @@ void SlippiNetplayClient::ThreadFunc() // It's clear which remote player this new port corresponds to endpointToIndexAndType.insert({endpoint, potentialMatches[0]}); indexToConnectionManager[potentialMatches[0].first].InsertConnection( - SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL, peer); + EndpointType::ENDPOINT_TYPE_EXTERNAL, peer); WARN_LOG(SLIPPI_ONLINE, "[Netplay] Matched unexpected endpoint %s:%d, %X to player index %d", addressStr, port, peer, potentialMatches[0].first); } @@ -1565,17 +1563,17 @@ SlippiDesyncRecoveryResp SlippiNetplayClient::GetDesyncRecoveryState() return result; } -SlippiConnectionManager::SlippiConnectionManager() +SlippiNetplayClient::ConnectionManager::ConnectionManager() { - m_highestPriority = SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL; + m_highestPriority = EndpointType::ENDPOINT_TYPE_EXTERNAL; } -SlippiConnectionManager::SlippiConnectionManager(SlippiConnectionManager::SlippiEndpointType highestPriority) +SlippiNetplayClient::ConnectionManager::ConnectionManager(EndpointType highestPriority) { m_highestPriority = highestPriority; } -bool SlippiConnectionManager::HasAnyConnection() +bool SlippiNetplayClient::ConnectionManager::HasAnyConnection() { for (auto peer : m_localPeers) if (peer->state == ENET_PEER_STATE_CONNECTED) @@ -1586,16 +1584,16 @@ bool SlippiConnectionManager::HasAnyConnection() return false; } -bool SlippiConnectionManager::HasHighestPriorityConnection() +bool SlippiNetplayClient::ConnectionManager::HasHighestPriorityConnection() { switch (m_highestPriority) { - case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL: + case EndpointType::ENDPOINT_TYPE_LOCAL: for (auto peer : m_localPeers) if (peer->state == ENET_PEER_STATE_CONNECTED) return true; break; - case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL: + case EndpointType::ENDPOINT_TYPE_EXTERNAL: for (auto peer : m_externalPeers) if (peer->state == ENET_PEER_STATE_CONNECTED) return true; @@ -1604,16 +1602,16 @@ bool SlippiConnectionManager::HasHighestPriorityConnection() return false; } -void SlippiConnectionManager::InsertConnection(SlippiConnectionManager::SlippiEndpointType endpointType, ENetPeer* peer) +void SlippiNetplayClient::ConnectionManager::InsertConnection(EndpointType endpointType, ENetPeer *peer) { switch (endpointType) { - case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_LOCAL: + case EndpointType::ENDPOINT_TYPE_LOCAL: { m_localPeers.push_back(peer); break; } - case SlippiConnectionManager::SlippiEndpointType::ENDPOINT_TYPE_EXTERNAL: + case EndpointType::ENDPOINT_TYPE_EXTERNAL: { m_externalPeers.push_back(peer); break; @@ -1621,7 +1619,7 @@ void SlippiConnectionManager::InsertConnection(SlippiConnectionManager::SlippiEn } } -void SlippiConnectionManager::SelectAllConnections(std::vector& connections) +void SlippiNetplayClient::ConnectionManager::SelectAllConnections(std::vector &connections) { for (auto peer : m_localPeers) if (peer->state == ENET_PEER_STATE_CONNECTED) @@ -1631,7 +1629,8 @@ void SlippiConnectionManager::SelectAllConnections(std::vector& conne connections.push_back(peer); } -void SlippiConnectionManager::SelectOneConnection(std::vector& connections) { +void SlippiNetplayClient::ConnectionManager::SelectOneConnection(std::vector &connections) +{ bool found = false; for (auto peer : m_localPeers) { diff --git a/Source/Core/Core/Slippi/SlippiNetplay.h b/Source/Core/Core/Slippi/SlippiNetplay.h index 1b85891284..c88b3ecc71 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.h +++ b/Source/Core/Core/Slippi/SlippiNetplay.h @@ -135,35 +135,6 @@ struct RemotePlayer ENetAddress localAddress; }; -class SlippiConnectionManager -{ - public: - enum class SlippiEndpointType - { - ENDPOINT_TYPE_LOCAL, - ENDPOINT_TYPE_EXTERNAL, - }; - - SlippiConnectionManager(); - SlippiConnectionManager(SlippiEndpointType highestPriority); - bool HasAnyConnection(); - void InsertConnection(SlippiEndpointType endpointType, ENetPeer *peer); - - // Whether there are any connections of the highest priority type - bool HasHighestPriorityConnection(); - - // Add all connections to the input vector - void SelectAllConnections(std::vector& connections); - - // Add the most preferred connection to the input vector, disconnect any others - void SelectOneConnection(std::vector& connections); - - protected: - SlippiEndpointType m_highestPriority; - std::vector m_localPeers; - std::vector m_externalPeers; -}; - // For use in std containers, we shove the u32 address and u16 port into one u64 typedef u64 slippi_endpoint; @@ -236,6 +207,35 @@ class SlippiNetplayClient u8 remoteSentChatMessageId = 0; // most recent chat message id that current player sent protected: + enum class EndpointType + { + ENDPOINT_TYPE_LOCAL, + ENDPOINT_TYPE_EXTERNAL, + }; + + class ConnectionManager + { + public: + ConnectionManager(); + ConnectionManager(EndpointType highestPriority); + bool HasAnyConnection(); + void InsertConnection(EndpointType endpointType, ENetPeer *peer); + + // Whether there are any connections of the highest priority type + bool HasHighestPriorityConnection(); + + // Add all connections to the input vector + void SelectAllConnections(std::vector &connections); + + // Add the most preferred connection to the input vector, disconnect any others + void SelectOneConnection(std::vector &connections); + + protected: + EndpointType m_highestPriority; + std::vector m_localPeers; + std::vector m_externalPeers; + }; + struct { std::recursive_mutex game; @@ -278,9 +278,8 @@ class SlippiNetplayClient bool hasGameStarted = false; u8 playerIdx = 0; - std::unordered_map> - endpointToIndexAndType; - std::unordered_map indexToConnectionManager; + std::unordered_map> endpointToIndexAndType; + std::unordered_map indexToConnectionManager; std::vector unexpectedPeers; std::deque> localPadQueue; // most recent inputs at start of deque From 123e33d4b1f707bddcdd8e57ebda642feb157954 Mon Sep 17 00:00:00 2001 From: jmlee337 Date: Sat, 15 Jul 2023 23:01:03 +0900 Subject: [PATCH 4/4] clean up before PR --- Source/Core/Core/Slippi/SlippiNetplay.cpp | 80 ++++++++++++----------- Source/Core/Core/Slippi/SlippiNetplay.h | 20 +++--- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/Source/Core/Core/Slippi/SlippiNetplay.cpp b/Source/Core/Core/Slippi/SlippiNetplay.cpp index f9b74a6ecc..55381adb44 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.cpp +++ b/Source/Core/Core/Slippi/SlippiNetplay.cpp @@ -169,9 +169,9 @@ SlippiNetplayClient::SlippiNetplayClient(std::vector remote WARN_LOG(SLIPPI_ONLINE, "[Netplay] Connecting to peer (external): %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); } - indexToConnectionManager.insert( - {remotePlayer.index, ConnectionManager(useLocalEndpoint ? EndpointType::ENDPOINT_TYPE_LOCAL - : EndpointType::ENDPOINT_TYPE_EXTERNAL)}); + indexToPeerManager.insert( + {remotePlayer.index, + PeerManager(useLocalEndpoint ? EndpointType::ENDPOINT_TYPE_LOCAL : EndpointType::ENDPOINT_TYPE_EXTERNAL)}); } slippiConnectStatus = SlippiConnectStatus::NET_CONNECT_STATUS_INITIATED; @@ -695,36 +695,18 @@ void SlippiNetplayClient::SendAsync(std::unique_ptr packet) // called from ---NETPLAY--- thread void SlippiNetplayClient::SelectConnectedPeers() { - if (LogTypes::LINFO <= MAX_LOGLEVEL) + for (auto indexAndConnectionManager : indexToPeerManager) { - for (int i = 0; i < m_client->peerCount; i++) - { - auto peer = &m_client->peers[i]; - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Peer: %s:%d, %X (state: %d)", inet_ntoa(*(in_addr *)&peer->address.host), - peer->address.port, peer, peer->state); - } - } - - for (auto indexAndConnectionManager : indexToConnectionManager) - { - ConnectionManager connectionManager = indexAndConnectionManager.second; + PeerManager connectionManager = indexAndConnectionManager.second; // By convention, the lower number port disconnects any superfluous connections. Connections will be initially // duplicate most of the time since the connection should always be attempted from both ends. if (playerIdx >= indexAndConnectionManager.first) - connectionManager.SelectAllConnections(m_connectedPeers); + connectionManager.SelectAllPeers(m_connectedPeers); else - connectionManager.SelectOneConnection(m_connectedPeers); + connectionManager.SelectOnePeer(m_connectedPeers); } m_client->intercept = ENetUtil::InterceptCallback; - if (LogTypes::LINFO <= MAX_LOGLEVEL) - { - for (auto peer : m_connectedPeers) - { - INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer: %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), - peer->address.port, peer); - } - } INFO_LOG(SLIPPI_ONLINE, "Slippi online connection successful!"); } @@ -808,8 +790,8 @@ void SlippiNetplayClient::ThreadFunc() { // It's clear which remote player this new port corresponds to endpointToIndexAndType.insert({endpoint, potentialMatches[0]}); - indexToConnectionManager[potentialMatches[0].first].InsertConnection( - EndpointType::ENDPOINT_TYPE_EXTERNAL, peer); + indexToPeerManager[potentialMatches[0].first].AddPeer(EndpointType::ENDPOINT_TYPE_EXTERNAL, + peer); WARN_LOG(SLIPPI_ONLINE, "[Netplay] Matched unexpected endpoint %s:%d, %X to player index %d", addressStr, port, peer, potentialMatches[0].first); } @@ -825,7 +807,7 @@ void SlippiNetplayClient::ThreadFunc() } else { - indexToConnectionManager[found->second.first].InsertConnection(found->second.second, peer); + indexToPeerManager[found->second.first].AddPeer(found->second.second, peer); WARN_LOG(SLIPPI_ONLINE, "[Netplay] New connection (ontime) %s:%d, %X", addressStr, port, peer); } break; @@ -835,8 +817,8 @@ void SlippiNetplayClient::ThreadFunc() // We can early terminate this phase IFF we've connected to every remote player's highest priority endpoint bool enoughConnected = true; - for (auto indexAndConnectionManager : indexToConnectionManager) - if (!indexAndConnectionManager.second.HasHighestPriorityConnection()) + for (auto indexAndConnectionManager : indexToPeerManager) + if (!indexAndConnectionManager.second.HasAnyHighestPriorityPeers()) enoughConnected = false; if (enoughConnected) { @@ -852,9 +834,9 @@ void SlippiNetplayClient::ThreadFunc() if ((curTime - startTime) >= timeout || !m_do_loop.IsSet()) { bool enoughConnected = true; - for (auto indexAndConnectionManager : indexToConnectionManager) + for (auto indexAndConnectionManager : indexToPeerManager) { - if (!indexAndConnectionManager.second.HasAnyConnection()) + if (!indexAndConnectionManager.second.HasAnyPeers()) { enoughConnected = false; failedConnections.push_back(indexAndConnectionManager.first); @@ -1563,17 +1545,17 @@ SlippiDesyncRecoveryResp SlippiNetplayClient::GetDesyncRecoveryState() return result; } -SlippiNetplayClient::ConnectionManager::ConnectionManager() +SlippiNetplayClient::PeerManager::PeerManager() { m_highestPriority = EndpointType::ENDPOINT_TYPE_EXTERNAL; } -SlippiNetplayClient::ConnectionManager::ConnectionManager(EndpointType highestPriority) +SlippiNetplayClient::PeerManager::PeerManager(EndpointType highestPriority) { m_highestPriority = highestPriority; } -bool SlippiNetplayClient::ConnectionManager::HasAnyConnection() +bool SlippiNetplayClient::PeerManager::HasAnyPeers() { for (auto peer : m_localPeers) if (peer->state == ENET_PEER_STATE_CONNECTED) @@ -1584,7 +1566,7 @@ bool SlippiNetplayClient::ConnectionManager::HasAnyConnection() return false; } -bool SlippiNetplayClient::ConnectionManager::HasHighestPriorityConnection() +bool SlippiNetplayClient::PeerManager::HasAnyHighestPriorityPeers() { switch (m_highestPriority) { @@ -1602,7 +1584,7 @@ bool SlippiNetplayClient::ConnectionManager::HasHighestPriorityConnection() return false; } -void SlippiNetplayClient::ConnectionManager::InsertConnection(EndpointType endpointType, ENetPeer *peer) +void SlippiNetplayClient::PeerManager::AddPeer(EndpointType endpointType, ENetPeer *peer) { switch (endpointType) { @@ -1619,17 +1601,29 @@ void SlippiNetplayClient::ConnectionManager::InsertConnection(EndpointType endpo } } -void SlippiNetplayClient::ConnectionManager::SelectAllConnections(std::vector &connections) +void SlippiNetplayClient::PeerManager::SelectAllPeers(std::vector &connections) { for (auto peer : m_localPeers) + { if (peer->state == ENET_PEER_STATE_CONNECTED) + { connections.push_back(peer); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer (all): %s:%d, %X", + inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); + } + } for (auto peer : m_externalPeers) + { if (peer->state == ENET_PEER_STATE_CONNECTED) + { connections.push_back(peer); + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer (all): %s:%d, %X", + inet_ntoa(*(in_addr *)&peer->address.host), peer->address.port, peer); + } + } } -void SlippiNetplayClient::ConnectionManager::SelectOneConnection(std::vector &connections) +void SlippiNetplayClient::PeerManager::SelectOnePeer(std::vector &connections) { bool found = false; for (auto peer : m_localPeers) @@ -1639,11 +1633,15 @@ void SlippiNetplayClient::ConnectionManager::SelectOneConnection(std::vectoraddress.host), + peer->address.port, peer); } else { connections.push_back(peer); found = true; + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer: %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), + peer->address.port, peer); } } for (auto peer : m_externalPeers) @@ -1653,11 +1651,15 @@ void SlippiNetplayClient::ConnectionManager::SelectOneConnection(std::vectoraddress.host), + peer->address.port, peer); } else { connections.push_back(peer); found = true; + INFO_LOG(SLIPPI_ONLINE, "[Netplay] Selected peer: %s:%d, %X", inet_ntoa(*(in_addr *)&peer->address.host), + peer->address.port, peer); } } } diff --git a/Source/Core/Core/Slippi/SlippiNetplay.h b/Source/Core/Core/Slippi/SlippiNetplay.h index c88b3ecc71..dcf4eb54d7 100644 --- a/Source/Core/Core/Slippi/SlippiNetplay.h +++ b/Source/Core/Core/Slippi/SlippiNetplay.h @@ -213,22 +213,24 @@ class SlippiNetplayClient ENDPOINT_TYPE_EXTERNAL, }; - class ConnectionManager + class PeerManager { public: - ConnectionManager(); - ConnectionManager(EndpointType highestPriority); - bool HasAnyConnection(); - void InsertConnection(EndpointType endpointType, ENetPeer *peer); + PeerManager(EndpointType highestPriority); + bool HasAnyPeers(); + void AddPeer(EndpointType endpointType, ENetPeer *peer); + + // Default constructor to satisfy use as std::map key. Do not use! + PeerManager(); // Whether there are any connections of the highest priority type - bool HasHighestPriorityConnection(); + bool HasAnyHighestPriorityPeers(); // Add all connections to the input vector - void SelectAllConnections(std::vector &connections); + void SelectAllPeers(std::vector &connections); // Add the most preferred connection to the input vector, disconnect any others - void SelectOneConnection(std::vector &connections); + void SelectOnePeer(std::vector &connections); protected: EndpointType m_highestPriority; @@ -279,7 +281,7 @@ class SlippiNetplayClient u8 playerIdx = 0; std::unordered_map> endpointToIndexAndType; - std::unordered_map indexToConnectionManager; + std::unordered_map indexToPeerManager; std::vector unexpectedPeers; std::deque> localPadQueue; // most recent inputs at start of deque