From 08c8f8b19bed86ac6b653272705ca9522f717f02 Mon Sep 17 00:00:00 2001 From: Bobby Battista Date: Thu, 9 Oct 2025 23:19:10 -0500 Subject: [PATCH 1/2] refactor(network): convert some if-else chains to switch statements --- .../Source/GameNetwork/ConnectionManager.cpp | 149 ++++++++---------- .../Source/GameNetwork/DisconnectManager.cpp | 46 ++++-- .../Source/GameNetwork/FirewallHelper.cpp | 73 ++++----- 3 files changed, 134 insertions(+), 134 deletions(-) diff --git a/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp index 66b4e0f2b40..e8469b68f2f 100644 --- a/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -420,25 +420,14 @@ void ConnectionManager::doRelay() { */ Bool ConnectionManager::processNetCommand(NetCommandRef *ref) { NetCommandMsg *msg = ref->getCommand(); + NetCommandType cmdType = msg->getNetCommandType(); - if ((msg->getNetCommandType() == NETCOMMANDTYPE_ACKSTAGE1) || - (msg->getNetCommandType() == NETCOMMANDTYPE_ACKSTAGE2) || - (msg->getNetCommandType() == NETCOMMANDTYPE_ACKBOTH)) { - - processAck(msg); - return FALSE; - } - + // Early validation checks if ((m_connections[msg->getPlayerID()] == NULL) && (msg->getPlayerID() != m_localSlot)) { // if this is from a player that is no longer in the game, then ignore them. return TRUE; } - if (msg->getNetCommandType() == NETCOMMANDTYPE_WRAPPER) { - processWrapper(ref); // need to send the NetCommandRef since we have to construct the relay for the wrapped command. - return FALSE; - } - if ((msg->getPlayerID() >= 0) && (msg->getPlayerID() < MAX_SLOTS) && (msg->getPlayerID() != m_localSlot)) { if (m_connections[msg->getPlayerID()] == NULL) { return TRUE; @@ -451,93 +440,93 @@ Bool ConnectionManager::processNetCommand(NetCommandRef *ref) { // This was a fix for a command count bug where a command would be // executed, then a command for that old frame would be added to the // FrameData for that frame + 256, and would screw up the command count. - - if (IsCommandSynchronized(msg->getNetCommandType())) { + if (IsCommandSynchronized(cmdType)) { if (ref->getCommand()->getExecutionFrame() < TheGameLogic->getFrame()) { return TRUE; } } - if (msg->getNetCommandType() == NETCOMMANDTYPE_FRAMEINFO) { - processFrameInfo((NetFrameCommandMsg *)msg); - - // need to set the relay so we don't send it to ourselves. - UnsignedByte relay = ref->getRelay(); - relay = relay & (0xff ^ (1 << m_localSlot)); - ref->setRelay(relay); - return FALSE; + // Handle disconnect commands as a range + if ((cmdType > NETCOMMANDTYPE_DISCONNECTSTART) && (cmdType < NETCOMMANDTYPE_DISCONNECTEND)) { + m_disconnectManager->processDisconnectCommand(ref, this); + return TRUE; } - if (msg->getNetCommandType() == NETCOMMANDTYPE_PROGRESS) - { - //DEBUG_LOG(("ConnectionManager::processNetCommand - got a progress net command from player %d", msg->getPlayerID())); - processProgress((NetProgressCommandMsg *) msg); + // Process command by type + switch (cmdType) { + case NETCOMMANDTYPE_ACKSTAGE1: + case NETCOMMANDTYPE_ACKSTAGE2: + case NETCOMMANDTYPE_ACKBOTH: + processAck(msg); + return FALSE; - // need to set the relay so we don't send it to ourselves. - UnsignedByte relay = ref->getRelay(); - relay = relay & (0xff ^ (1 << m_localSlot)); - ref->setRelay(relay); - return FALSE; - } + case NETCOMMANDTYPE_WRAPPER: + processWrapper(ref); // need to send the NetCommandRef since we have to construct the relay for the wrapped command. + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_TIMEOUTSTART) - { - DEBUG_LOG(("ConnectionManager::processNetCommand - got a TimeOut GameStart net command from player %d", msg->getPlayerID())); - processTimeOutGameStart(msg); - return FALSE; - } + case NETCOMMANDTYPE_FRAMEINFO: { + processFrameInfo((NetFrameCommandMsg *)msg); + // need to set the relay so we don't send it to ourselves. + UnsignedByte relay = ref->getRelay(); + relay = relay & (0xff ^ (1 << m_localSlot)); + ref->setRelay(relay); + return FALSE; + } - if (msg->getNetCommandType() == NETCOMMANDTYPE_RUNAHEADMETRICS) { - processRunAheadMetrics((NetRunAheadMetricsCommandMsg *)msg); - return TRUE; - } + case NETCOMMANDTYPE_PROGRESS: { + //DEBUG_LOG(("ConnectionManager::processNetCommand - got a progress net command from player %d", msg->getPlayerID())); + processProgress((NetProgressCommandMsg *) msg); + // need to set the relay so we don't send it to ourselves. + UnsignedByte relay = ref->getRelay(); + relay = relay & (0xff ^ (1 << m_localSlot)); + ref->setRelay(relay); + return FALSE; + } - if (msg->getNetCommandType() == NETCOMMANDTYPE_KEEPALIVE) { - return TRUE; - } + case NETCOMMANDTYPE_TIMEOUTSTART: + DEBUG_LOG(("ConnectionManager::processNetCommand - got a TimeOut GameStart net command from player %d", msg->getPlayerID())); + processTimeOutGameStart(msg); + return FALSE; - if ((msg->getNetCommandType() > NETCOMMANDTYPE_DISCONNECTSTART) && (msg->getNetCommandType() < NETCOMMANDTYPE_DISCONNECTEND)) { - m_disconnectManager->processDisconnectCommand(ref, this); - return TRUE; - } + case NETCOMMANDTYPE_RUNAHEADMETRICS: + processRunAheadMetrics((NetRunAheadMetricsCommandMsg *)msg); + return TRUE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTCHAT) { - processDisconnectChat((NetDisconnectChatCommandMsg *)msg); - } + case NETCOMMANDTYPE_KEEPALIVE: + return TRUE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_LOADCOMPLETE) - { - DEBUG_LOG(("ConnectionManager::processNetCommand - got a Load Complete net command from player %d", msg->getPlayerID())); - processLoadComplete(msg); - return FALSE; - } + case NETCOMMANDTYPE_DISCONNECTCHAT: + processDisconnectChat((NetDisconnectChatCommandMsg *)msg); + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_CHAT) { - processChat((NetChatCommandMsg *)msg); - return FALSE; - } + case NETCOMMANDTYPE_LOADCOMPLETE: + DEBUG_LOG(("ConnectionManager::processNetCommand - got a Load Complete net command from player %d", msg->getPlayerID())); + processLoadComplete(msg); + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_FILE) { - processFile((NetFileCommandMsg *)msg); - return FALSE; - } + case NETCOMMANDTYPE_CHAT: + processChat((NetChatCommandMsg *)msg); + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_FILEANNOUNCE) { - processFileAnnounce((NetFileAnnounceCommandMsg *)msg); - return FALSE; - } + case NETCOMMANDTYPE_FILE: + processFile((NetFileCommandMsg *)msg); + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_FILEPROGRESS) { - processFileProgress((NetFileProgressCommandMsg *)msg); - return FALSE; - } + case NETCOMMANDTYPE_FILEANNOUNCE: + processFileAnnounce((NetFileAnnounceCommandMsg *)msg); + return FALSE; - if (msg->getNetCommandType() == NETCOMMANDTYPE_FRAMERESENDREQUEST) { - processFrameResendRequest((NetFrameResendRequestCommandMsg *)msg); - return TRUE; - } + case NETCOMMANDTYPE_FILEPROGRESS: + processFileProgress((NetFileProgressCommandMsg *)msg); + return FALSE; + + case NETCOMMANDTYPE_FRAMERESENDREQUEST: + processFrameResendRequest((NetFrameResendRequestCommandMsg *)msg); + return TRUE; - return FALSE; + default: + return FALSE; + } } void ConnectionManager::processFrameResendRequest(NetFrameResendRequestCommandMsg *msg) { diff --git a/Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp b/Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp index 94268e63863..cd00da753c9 100644 --- a/Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp +++ b/Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp @@ -268,20 +268,38 @@ void DisconnectManager::updateWaitForPacketRouter(ConnectionManager *conMgr) { void DisconnectManager::processDisconnectCommand(NetCommandRef *ref, ConnectionManager *conMgr) { NetCommandMsg *msg = ref->getCommand(); - if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTKEEPALIVE) { - processDisconnectKeepAlive(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTPLAYER) { - processDisconnectPlayer(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_PACKETROUTERQUERY) { - processPacketRouterQuery(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_PACKETROUTERACK) { - processPacketRouterAck(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTVOTE) { - processDisconnectVote(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTFRAME) { - processDisconnectFrame(msg, conMgr); - } else if (msg->getNetCommandType() == NETCOMMANDTYPE_DISCONNECTSCREENOFF) { - processDisconnectScreenOff(msg, conMgr); + + switch (msg->getNetCommandType()) { + case NETCOMMANDTYPE_DISCONNECTKEEPALIVE: + processDisconnectKeepAlive(msg, conMgr); + break; + + case NETCOMMANDTYPE_DISCONNECTPLAYER: + processDisconnectPlayer(msg, conMgr); + break; + + case NETCOMMANDTYPE_PACKETROUTERQUERY: + processPacketRouterQuery(msg, conMgr); + break; + + case NETCOMMANDTYPE_PACKETROUTERACK: + processPacketRouterAck(msg, conMgr); + break; + + case NETCOMMANDTYPE_DISCONNECTVOTE: + processDisconnectVote(msg, conMgr); + break; + + case NETCOMMANDTYPE_DISCONNECTFRAME: + processDisconnectFrame(msg, conMgr); + break; + + case NETCOMMANDTYPE_DISCONNECTSCREENOFF: + processDisconnectScreenOff(msg, conMgr); + break; + + default: + break; } } diff --git a/Core/GameEngine/Source/GameNetwork/FirewallHelper.cpp b/Core/GameEngine/Source/GameNetwork/FirewallHelper.cpp index d8ffb64f340..f173050b636 100644 --- a/Core/GameEngine/Source/GameNetwork/FirewallHelper.cpp +++ b/Core/GameEngine/Source/GameNetwork/FirewallHelper.cpp @@ -204,47 +204,40 @@ Bool FirewallHelperClass::detectFirewall(void) Bool FirewallHelperClass::behaviorDetectionUpdate() { - if (m_currentState == DETECTIONSTATE_IDLE) { - return FALSE; - } - - if (m_currentState == DETECTIONSTATE_DONE) { - return TRUE; - } - - if (m_currentState == DETECTIONSTATE_BEGIN) { - return detectionBeginUpdate(); - } - - if (m_currentState == DETECTIONSTATE_TEST1) { - return detectionTest1Update(); - } - - if (m_currentState == DETECTIONSTATE_TEST2) { - return detectionTest2Update(); - } - - if (m_currentState == DETECTIONSTATE_TEST3) { - return detectionTest3Update(); - } - - if (m_currentState == DETECTIONSTATE_TEST3_WAITFORRESPONSES) { - return detectionTest3WaitForResponsesUpdate(); - } - - if (m_currentState == DETECTIONSTATE_TEST4_1) { - return detectionTest4Stage1Update(); - } - - if (m_currentState == DETECTIONSTATE_TEST4_2) { - return detectionTest4Stage2Update(); - } - - if (m_currentState == DETECTIONSTATE_TEST5) { - return detectionTest5Update(); + switch (m_currentState) { + case DETECTIONSTATE_IDLE: + return FALSE; + + case DETECTIONSTATE_DONE: + return TRUE; + + case DETECTIONSTATE_BEGIN: + return detectionBeginUpdate(); + + case DETECTIONSTATE_TEST1: + return detectionTest1Update(); + + case DETECTIONSTATE_TEST2: + return detectionTest2Update(); + + case DETECTIONSTATE_TEST3: + return detectionTest3Update(); + + case DETECTIONSTATE_TEST3_WAITFORRESPONSES: + return detectionTest3WaitForResponsesUpdate(); + + case DETECTIONSTATE_TEST4_1: + return detectionTest4Stage1Update(); + + case DETECTIONSTATE_TEST4_2: + return detectionTest4Stage2Update(); + + case DETECTIONSTATE_TEST5: + return detectionTest5Update(); + + default: + return TRUE; } - - return TRUE; } /*********************************************************************************************** From 2a8cf0bbe33e03845a7eac494d2fdf7250261044 Mon Sep 17 00:00:00 2001 From: Bobby Battista Date: Fri, 10 Oct 2025 13:56:04 -0400 Subject: [PATCH 2/2] fix(network): preserve original execution order for ACK and WRAPPER commands --- .../Source/GameNetwork/ConnectionManager.cpp | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp index e8469b68f2f..6bd869f1bff 100644 --- a/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -422,12 +422,26 @@ Bool ConnectionManager::processNetCommand(NetCommandRef *ref) { NetCommandMsg *msg = ref->getCommand(); NetCommandType cmdType = msg->getNetCommandType(); + // Handle ACK commands first (before connection validation) + if ((cmdType == NETCOMMANDTYPE_ACKSTAGE1) || + (cmdType == NETCOMMANDTYPE_ACKSTAGE2) || + (cmdType == NETCOMMANDTYPE_ACKBOTH)) { + processAck(msg); + return FALSE; + } + // Early validation checks if ((m_connections[msg->getPlayerID()] == NULL) && (msg->getPlayerID() != m_localSlot)) { // if this is from a player that is no longer in the game, then ignore them. return TRUE; } + // Handle WRAPPER commands (before second connection validation) + if (cmdType == NETCOMMANDTYPE_WRAPPER) { + processWrapper(ref); // need to send the NetCommandRef since we have to construct the relay for the wrapped command. + return FALSE; + } + if ((msg->getPlayerID() >= 0) && (msg->getPlayerID() < MAX_SLOTS) && (msg->getPlayerID() != m_localSlot)) { if (m_connections[msg->getPlayerID()] == NULL) { return TRUE; @@ -454,15 +468,6 @@ Bool ConnectionManager::processNetCommand(NetCommandRef *ref) { // Process command by type switch (cmdType) { - case NETCOMMANDTYPE_ACKSTAGE1: - case NETCOMMANDTYPE_ACKSTAGE2: - case NETCOMMANDTYPE_ACKBOTH: - processAck(msg); - return FALSE; - - case NETCOMMANDTYPE_WRAPPER: - processWrapper(ref); // need to send the NetCommandRef since we have to construct the relay for the wrapped command. - return FALSE; case NETCOMMANDTYPE_FRAMEINFO: { processFrameInfo((NetFrameCommandMsg *)msg);