Skip to content

refactor(player): Add slot-to-player lookup - #3034

Open
CryoTheRenegade wants to merge 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:refactor/slot-index-to-player
Open

refactor(player): Add slot-to-player lookup#3034
CryoTheRenegade wants to merge 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:refactor/slot-index-to-player

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Jul 31, 2026

Copy link
Copy Markdown

Add PlayerList::getPlayerFromSlotIndex for reverse slot-to-player lookup.
Replace synthetic "player%d" associations
Preserve multiplayer slot ordering in result generation and UI paths.
Update the additional GameSpyGameInfo::generateGameResultsPacket implementation

Closes #2897
Merge After #3033

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown

Greptile Summary

Adds a canonical reverse lookup from multiplayer slot index to Player and migrates networking, game-result, startup, chat, diplomacy, score-screen, observer, and player-list UI paths away from synthetic playerN name lookups.

  • Implements the lookup consistently for both Generals and Zero Hour.
  • Preserves lobby-slot ordering when resolving players for multiplayer output and presentation.
  • Converts network slot identifiers to internal player indices where required.

Confidence Score: 5/5

The PR appears safe to merge, with the changed paths consistently translating multiplayer slot identifiers through the new reverse lookup.

The lookup is bounds-checked, derives its result from the existing slot assignment mapping, and the migrated callers use identifiers whose slot semantics align with that mapping; no actionable regression was identified.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/Common/RTS/PlayerList.cpp Adds a bounds-checked reverse slot lookup over the existing player-to-slot mapping.
GeneralsMD/Code/GameEngine/Source/Common/RTS/PlayerList.cpp Mirrors the reverse lookup implementation for the independently built Zero Hour target.
Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp Converts the command sender’s network slot into the corresponding internal player index.
Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp Resolves chat senders using the slot-index semantics of the command header.
Core/GameEngine/Source/GameNetwork/Network.cpp Resolves destroy-player command targets from the slot value carried by the command-specific field.
Core/GameEngine/Source/GameNetwork/GameSpy/StagingRoomGameInfo.cpp Generates results in lobby-slot order using the canonical reverse lookup.
Generals/Code/GameEngine/Source/GameNetwork/GameSpyGameInfo.cpp Associates Generals result records with players through stable multiplayer slot mappings.
GeneralsMD/Code/GameEngine/Source/GameNetwork/GameSpyGameInfo.cpp Applies the same slot-ordered result association to Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Slot["Lobby / network slot index"] --> Lookup["PlayerList::getPlayerFromSlotIndex"]
  Lookup --> Mapping["playerIndex → slotIndex mapping"]
  Mapping --> Player["Player instance"]
  Player --> Network["Network command ownership"]
  Player --> Results["GameSpy results"]
  Player --> UI["Chat, diplomacy, scores, observers"]
Loading

Reviews (1): Last reviewed commit: "refactor(player): Add slot-to-player loo..." | Re-trigger Greptile

@Skyaero42

Copy link
Copy Markdown

It does not reference the correct issue. Can you explain in 2 lines what problem this PR is trying to resolve?

@CryoTheRenegade

CryoTheRenegade commented Jul 31, 2026

Copy link
Copy Markdown
Author

It does not reference the correct issue

Fixed

Can you explain in 2 lines what problem this PR is trying to resolve?

Player indices and multiplayer slot indices can differ, so reconstructing their relationship through "player%d" names can select the wrong player or slot. This change uses the stored PlayerList mapping for reliable conversions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use slot indices in the PlayerList for conversions from player index to slot index

2 participants