refactor(player): Use slot indices for player conversions - #3033
Open
CryoTheRenegade wants to merge 1 commit into
Open
refactor(player): Use slot indices for player conversions#3033CryoTheRenegade wants to merge 1 commit into
CryoTheRenegade wants to merge 1 commit into
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp | Converts the CRC message sender’s player index to the network slot domain before checking connectivity. |
| Generals/Code/GameEngine/Source/Common/Recorder.cpp | Compares the converted sender slot with the replay header’s local slot instead of reconstructing player identity from a name key. |
| Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp | Resolves defeated AI players through PlayerList’s established slot mapping before updating slot metadata. |
| GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp | Mirrors the domain-correct replay CRC player-to-slot conversion for Zero Hour. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp | Mirrors the defeated-AI slot lookup update for Zero Hour. |
Reviews (1): Last reviewed commit: "refactor(player): Use slot indices for p..." | Re-trigger Greptile
CryoTheRenegade
changed the title
|
Is 2987 related, or does it actually resolve it? If it doesnt resolve it, what (part) of it does it solve? |
Author
CryoTheRenegade
force-pushed
the
refactor/player-index-to-slot-index
branch
from
August 4, 2026 17:36
7623f98 to
b2024a6
Compare
xezon
reviewed
Aug 5, 2026
| } | ||
| } | ||
| const Int slotIndex = msgPlayer->getPlayerType() == PLAYER_HUMAN | ||
| ? ThePlayerList->getSlotIndex(msgPlayer->getPlayerIndex()) |
There was a problem hiding this comment.
Can simplify by first returning false when player is not human.
| if (!p || (p->getPlayerNameKey() == NAMEKEY(playerName))) | ||
| samePlayer = TRUE; | ||
| const Int slotIndex = ThePlayerList->getSlotIndex(playerIndex); | ||
| const Bool samePlayer = !p || slotIndex == localPlayerIndex; |
There was a problem hiding this comment.
Cheaper condition:
if (!p || localPlayerIndex < 0 || ThePlayerList->getSlotIndex(playerIndex) == localPlayerIndex)
| 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()) |
xezon
added
Minor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the remaining player-index-to-slot-index reconstruction with
PlayerList::getSlotIndex.Update
GameLogic::onLogicCrc,RecorderClass::handleCRCMessage, andVictoryConditions::update.Related: #2897
Merge before #3034