From b2024a6f2a46992a01f911235ff2dc7fdd7bf21c Mon Sep 17 00:00:00 2001 From: Jacob Ledbetter Date: Wed, 29 Jul 2026 09:46:16 -0600 Subject: [PATCH 1/5] refactor(player): Use slot indices for player conversions (#2897) --- .../GameLogic/System/GameLogicDispatch.cpp | 12 +++--------- .../Code/GameEngine/Source/Common/Recorder.cpp | 7 ++----- .../ScriptEngine/VictoryConditions.cpp | 17 +++++------------ .../Code/GameEngine/Source/Common/Recorder.cpp | 7 ++----- .../ScriptEngine/VictoryConditions.cpp | 17 +++++------------ 5 files changed, 17 insertions(+), 43 deletions(-) diff --git a/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp b/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp index c9c7cbb7602..32d6665719e 100644 --- a/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp +++ b/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp @@ -2383,15 +2383,9 @@ bool GameLogic::onLogicCrc(MAYBE_UNUSED GameMessage *msg) Player *msgPlayer = getMessagePlayer(msg); if (TheNetwork) { - Int slotIndex = -1; - for (Int i=0; igetPlayerType() == PLAYER_HUMAN && TheNetwork->getPlayerName(i) == msgPlayer->getPlayerDisplayName()) - { - slotIndex = i; - break; - } - } + const Int slotIndex = msgPlayer->getPlayerType() == PLAYER_HUMAN + ? ThePlayerList->getSlotIndex(msgPlayer->getPlayerIndex()) + : -1; if (slotIndex < 0 || !TheNetwork->isPlayerConnected(slotIndex)) return false; diff --git a/Generals/Code/GameEngine/Source/Common/Recorder.cpp b/Generals/Code/GameEngine/Source/Common/Recorder.cpp index a6f51ade405..256b75a6639 100644 --- a/Generals/Code/GameEngine/Source/Common/Recorder.cpp +++ b/Generals/Code/GameEngine/Source/Common/Recorder.cpp @@ -989,12 +989,9 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f } Int localPlayerIndex = m_crcInfo.getLocalPlayer(); - Bool samePlayer = FALSE; - AsciiString playerName; - playerName.format("player%d", localPlayerIndex); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - if (!p || (p->getPlayerNameKey() == NAMEKEY(playerName))) - samePlayer = TRUE; + const Int slotIndex = ThePlayerList->getSlotIndex(playerIndex); + const Bool samePlayer = !p || slotIndex == localPlayerIndex; if (samePlayer || (localPlayerIndex < 0)) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index f101b226542..8e1b358ab6f 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -211,19 +211,12 @@ void VictoryConditions::update() TheAudio->addAudioEvent(&leftGameSound); } - for (Int idx = 0; idx < MAX_SLOTS; ++idx) + const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); + GameSlot *slot = TheGameInfo && slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + if (slot && slot->isAI()) { - AsciiString pName; - pName.format("player%d", idx); - if (p->getPlayerNameKey() == NAMEKEY(pName)) - { - GameSlot *slot = (TheGameInfo)?TheGameInfo->getSlot(idx):nullptr; - if (slot && slot->isAI()) - { - DEBUG_LOG(("Marking AI player %s as defeated", pName.str())); - slot->setLastFrameInGame(TheGameLogic->getFrame()); - } - } + DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); + slot->setLastFrameInGame(TheGameLogic->getFrame()); } // destroy any remaining units (infantry if its a short game, for example) diff --git a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp index 2d9bfea3a16..eedce53950a 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp @@ -991,12 +991,9 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f } Int localPlayerIndex = m_crcInfo.getLocalPlayer(); - Bool samePlayer = FALSE; - AsciiString playerName; - playerName.format("player%d", localPlayerIndex); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - if (!p || (p->getPlayerNameKey() == NAMEKEY(playerName))) - samePlayer = TRUE; + const Int slotIndex = ThePlayerList->getSlotIndex(playerIndex); + const Bool samePlayer = !p || slotIndex == localPlayerIndex; if (samePlayer || (localPlayerIndex < 0)) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index 76fc18d6e41..5717943cc20 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -213,19 +213,12 @@ void VictoryConditions::update() TheAudio->addAudioEvent(&leftGameSound); } - for (Int idx = 0; idx < MAX_SLOTS; ++idx) + const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); + GameSlot *slot = TheGameInfo && slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + if (slot && slot->isAI()) { - AsciiString pName; - pName.format("player%d", idx); - if (p->getPlayerNameKey() == NAMEKEY(pName)) - { - GameSlot *slot = (TheGameInfo)?TheGameInfo->getSlot(idx):nullptr; - if (slot && slot->isAI()) - { - DEBUG_LOG(("Marking AI player %s as defeated", pName.str())); - slot->setLastFrameInGame(TheGameLogic->getFrame()); - } - } + DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); + slot->setLastFrameInGame(TheGameLogic->getFrame()); } // destroy any remaining units (infantry if its a short game, for example) From 8e5ec745ddee39a2ec65620b74b9484d3229b5db Mon Sep 17 00:00:00 2001 From: Jacob Ledbetter Date: Thu, 6 Aug 2026 11:22:50 -0600 Subject: [PATCH 2/5] refactor(player): Simplify slot index conversion checks --- .../Source/GameLogic/System/GameLogicDispatch.cpp | 10 +++++++--- .../Code/GameEngine/Source/Common/Recorder.cpp | 5 ++--- .../GameLogic/ScriptEngine/VictoryConditions.cpp | 14 +++++++++----- .../Code/GameEngine/Source/Common/Recorder.cpp | 5 ++--- .../GameLogic/ScriptEngine/VictoryConditions.cpp | 14 +++++++++----- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp b/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp index 32d6665719e..fd9733f25ac 100644 --- a/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp +++ b/Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp @@ -2383,12 +2383,16 @@ bool GameLogic::onLogicCrc(MAYBE_UNUSED GameMessage *msg) Player *msgPlayer = getMessagePlayer(msg); if (TheNetwork) { - const Int slotIndex = msgPlayer->getPlayerType() == PLAYER_HUMAN - ? ThePlayerList->getSlotIndex(msgPlayer->getPlayerIndex()) - : -1; + if (msgPlayer->getPlayerType() != PLAYER_HUMAN) + { + return false; + } + const Int slotIndex = ThePlayerList->getSlotIndex(msgPlayer->getPlayerIndex()); if (slotIndex < 0 || !TheNetwork->isPlayerConnected(slotIndex)) + { return false; + } if (msgPlayer->isLocalPlayer()) { diff --git a/Generals/Code/GameEngine/Source/Common/Recorder.cpp b/Generals/Code/GameEngine/Source/Common/Recorder.cpp index 256b75a6639..a254b42e0a2 100644 --- a/Generals/Code/GameEngine/Source/Common/Recorder.cpp +++ b/Generals/Code/GameEngine/Source/Common/Recorder.cpp @@ -990,9 +990,8 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Int slotIndex = ThePlayerList->getSlotIndex(playerIndex); - const Bool samePlayer = !p || slotIndex == localPlayerIndex; - if (samePlayer || (localPlayerIndex < 0)) + const Bool samePlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + if (samePlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); //DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d", diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index 8e1b358ab6f..78f2101deed 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -211,12 +211,16 @@ void VictoryConditions::update() TheAudio->addAudioEvent(&leftGameSound); } - const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); - GameSlot *slot = TheGameInfo && slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; - if (slot && slot->isAI()) + if (TheGameInfo) { - DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); - slot->setLastFrameInGame(TheGameLogic->getFrame()); + const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); + GameSlot *slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + + if (slot && slot->isAI()) + { + DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); + slot->setLastFrameInGame(TheGameLogic->getFrame()); + } } // destroy any remaining units (infantry if its a short game, for example) diff --git a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp index eedce53950a..003006acd22 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp @@ -992,9 +992,8 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Int slotIndex = ThePlayerList->getSlotIndex(playerIndex); - const Bool samePlayer = !p || slotIndex == localPlayerIndex; - if (samePlayer || (localPlayerIndex < 0)) + const Bool samePlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + if (samePlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); //DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d", diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index 5717943cc20..ab34ddf756a 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -213,12 +213,16 @@ void VictoryConditions::update() TheAudio->addAudioEvent(&leftGameSound); } - const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); - GameSlot *slot = TheGameInfo && slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; - if (slot && slot->isAI()) + if (TheGameInfo) { - DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); - slot->setLastFrameInGame(TheGameLogic->getFrame()); + const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); + GameSlot *slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + + if (slot && slot->isAI()) + { + DEBUG_LOG(("Marking AI player %s as defeated", TheNameKeyGenerator->keyToName(p->getPlayerNameKey()).str())); + slot->setLastFrameInGame(TheGameLogic->getFrame()); + } } // destroy any remaining units (infantry if its a short game, for example) From f53c37839d9855d146a99969a0671c4e605c8e3c Mon Sep 17 00:00:00 2001 From: Jacob Ledbetter Date: Thu, 6 Aug 2026 13:56:21 -0600 Subject: [PATCH 3/5] refactor(player): Clarify local player and slot constness --- Generals/Code/GameEngine/Source/Common/Recorder.cpp | 4 ++-- .../Source/GameLogic/ScriptEngine/VictoryConditions.cpp | 2 +- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp | 4 ++-- .../Source/GameLogic/ScriptEngine/VictoryConditions.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Generals/Code/GameEngine/Source/Common/Recorder.cpp b/Generals/Code/GameEngine/Source/Common/Recorder.cpp index a254b42e0a2..c25b725e668 100644 --- a/Generals/Code/GameEngine/Source/Common/Recorder.cpp +++ b/Generals/Code/GameEngine/Source/Common/Recorder.cpp @@ -990,8 +990,8 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Bool samePlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; - if (samePlayer) + const Bool isLocalPlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + if (isLocalPlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); //DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d", diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index 78f2101deed..322e05275f0 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -214,7 +214,7 @@ void VictoryConditions::update() if (TheGameInfo) { const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); - GameSlot *slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + GameSlot *const slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; if (slot && slot->isAI()) { diff --git a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp index 003006acd22..4b31581cb7d 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp @@ -992,8 +992,8 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Bool samePlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; - if (samePlayer) + const Bool isLocalPlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + if (isLocalPlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); //DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d", diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index ab34ddf756a..d8e217f6d35 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -216,7 +216,7 @@ void VictoryConditions::update() if (TheGameInfo) { const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); - GameSlot *slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + GameSlot *const slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; if (slot && slot->isAI()) { From ad022f18bbb3bd09cd0affb954d0c82058b14406 Mon Sep 17 00:00:00 2001 From: Jacob Ledbetter Date: Thu, 6 Aug 2026 14:04:50 -0600 Subject: [PATCH 4/5] refactor(player): Drop redundant local player bounds check --- Generals/Code/GameEngine/Source/Common/Recorder.cpp | 2 +- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Generals/Code/GameEngine/Source/Common/Recorder.cpp b/Generals/Code/GameEngine/Source/Common/Recorder.cpp index c25b725e668..7a955895827 100644 --- a/Generals/Code/GameEngine/Source/Common/Recorder.cpp +++ b/Generals/Code/GameEngine/Source/Common/Recorder.cpp @@ -990,7 +990,7 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Bool isLocalPlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + const Bool isLocalPlayer = !p || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; if (isLocalPlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); diff --git a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp index 4b31581cb7d..9d71eb45b1b 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp @@ -992,7 +992,7 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f Int localPlayerIndex = m_crcInfo.getLocalPlayer(); const Player *p = ThePlayerList->getNthPlayer(playerIndex); - const Bool isLocalPlayer = !p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; + const Bool isLocalPlayer = !p || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex; if (isLocalPlayer) { UnsignedInt playbackCRC = m_crcInfo.readCRC(); From 7da1cd17d85ffdd8f84bf22ea8dae1792b5930e8 Mon Sep 17 00:00:00 2001 From: Jacob Lane Ledbetter <23038070+CryoTheRenegade@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:16:45 -0600 Subject: [PATCH 5/5] fix const order --- .../Source/GameLogic/ScriptEngine/VictoryConditions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp index 322e05275f0..f27c9f93368 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp @@ -214,7 +214,7 @@ void VictoryConditions::update() if (TheGameInfo) { const Int slotIndex = ThePlayerList->getSlotIndex(p->getPlayerIndex()); - GameSlot *const slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; + const GameSlot* slot = slotIndex >= 0 ? TheGameInfo->getSlot(slotIndex) : nullptr; if (slot && slot->isAI()) {