unify: Merge common system; BuildAssistant, PlayerTemplate, FunctionLexicon, GameState, GameStateMap

[xezon]

This change merges all of GameEngine/Source/Common/System and its includes and related code. ChallengeMenu, a dependency, is moved to Core.

BuildAssistant is CRC critical and requires careful review.

Generals gets

• ChallengeMenu GUI function callbacks
• Some BuildAssistant fixes
• Challenge setup in PlayerTemplate

TODO

• Test against Generals replays

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 530fb1d3-5eff-4b70-b2e0-e217a9c8aa9b

---

📥 Commits

Reviewing files that changed from the base of the PR and between 5a58835 and 7566178.

---

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Include/Common/Player.h
• Generals/Code/GameEngine/Source/Common/RTS/Player.cpp

---

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

---

Walkthrough

The changes update build validation and unit-production checks, add Zero Hour menu and player-template support, adjust save loading and save-list colors, and change registry lookup order.

Changes

Build rules

Layer / File(s)	Summary
Build collision results Generals/Code/GameEngine/Include/Common/BuildAssistant.h, Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp	isLocationClearOfObjects returns LegalBuildCode. Collision handling distinguishes obstruction and shroud results, with configuration-specific handling for stealth, busy allies, and disabled objects.
Placement errors and clear-path checks Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp	Placement checks preserve detailed collision results outside Generals retail-compatible builds. That configuration maps failures to LBC_OBJECTS_IN_THE_WAY. Clear-path checks use quick pathfinding and apply configuration-specific builder rules.
Construction validation and clearing Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp	buildObjectNow validates build eligibility when a constructor exists. Construction clearing also removes terrain trees and props. The sale-animation duration call no longer casts its argument to unsigned.
Unit-production limits Generals/Code/GameEngine/Include/Common/Player.h, Generals/Code/GameEngine/Source/Common/RTS/Player.cpp, Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp	The Generals retail-compatible path counts existing and queued units against simultaneous-unit limits. Other configurations apply the special-power construction shortcut and player unit-cap check.

Challenge menu callbacks

Layer / File(s)

Summary

Challenge menu compilation and callback wiring Core/GameEngine/CMakeLists.txt, Generals/Code/GameEngine/Include/GameClient/GUICallbacks.h, Generals/Code/GameEngine/Source/Common/System/FunctionLexicon.cpp, Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp, GeneralsMD/Code/GameEngine/CMakeLists.txt, GeneralsMD/Code/GameEngine/Source/Common/System/FunctionLexicon.cpp, scripts/cpp/unify_move_files.py

The Generals build includes ChallengeMenu.cpp and registers its callbacks. PopupHostGameUpdate is declared, implemented as an empty callback, and registered. The GeneralsMD build excludes ChallengeMenu.cpp; its callback mappings receive Zero Hour comments.

Player-template data

Layer / File(s)	Summary
Template fields and lookup Generals/Code/GameEngine/Include/Common/PlayerTemplate.h, Generals/Code/GameEngine/Source/Common/RTS/PlayerTemplate.cpp, GeneralsMD/Code/GameEngine/Source/Common/RTS/PlayerTemplate.cpp	Player templates parse and expose score-screen music, general image, general features, and medallion fields. The store looks up template names without case sensitivity and triggers DEBUG_CRASH if no match exists. GeneralsMD field entries receive Zero Hour comments.

Save handling and save-list colors

Layer / File(s)	Summary
Save loading and save-list display Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp, Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp	GameStateMap sets the save-loading flag before transferring load data and clears it after starting the game. Non-mission save entries use different alternating colors by game configuration. A map-path failure comment also describes relative paths outside the permitted base directory.

Registry lookup order

Layer / File(s)	Summary
Registry hive precedence Generals/Code/GameEngine/Source/Common/System/registry.cpp	GetStringFromRegistry checks HKEY_CURRENT_USER first and falls back to HKEY_LOCAL_MACHINE.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Merge Risk: 🟡 Moderate · up to 75661

Unit limits match the original game behavior. However, a failed save load can leave the game marked as still loading a save. An unexpected registry value can also stop the game from falling back to the machine-wide language or SKU setting. Address these before merging.

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title accurately identifies the common-system merge and names the main affected components. It is specific enough for repository history.
Description check		The description directly covers the common-system merge, ChallengeMenu move, BuildAssistant changes, PlayerTemplate changes, and planned testing.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[High impact] The reviewed changes appear safe to merge, with the missing Generals method now supplied.

Summary

This PR brings common engine code closer together and moves ChallengeMenu.cpp into Core. The latest change adds Generals' missing Player::canBuildMoreOfType method.

• Building checks return clearer results and enforce player-wide unit limits.
• Player templates supply more details for game screens.
• The Challenge menu now comes from the shared engine.
• Save loading tracks its transfer state and keeps save-list colors game-specific.
• Popup host layouts can resolve an update callback.
• Registry lookups check the current user before the machine-wide store.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Core["Core: ChallengeMenu.cpp"] --> Generals["Generals"]
    Core --> ZeroHour["Zero Hour"]
    Build["Generals: BuildAssistant::canMakeUnit"] --> Flag{"Retail-compatible CRC?"}
    Flag -->|Yes|Existing["Existing build-limit check"]
    Flag -->|No|Added["Player::canBuildMoreOfType"]

Loading

_{Reviews (2) · Last reviewed commit: "Merge Player::canBuildMoreOfType" · Reviewed by Greptile}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b40501cc-0eea-44dc-aa63-c25af24db822

📥 Commits

Reviewing files that changed from the base of the PR and between 6463b0d and 5a58835.

📒 Files selected for processing (18)

• Core/GameEngine/CMakeLists.txt
• Core/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ChallengeMenu.cpp
• Generals/Code/GameEngine/Include/Common/BuildAssistant.h
• Generals/Code/GameEngine/Include/Common/PlayerTemplate.h
• Generals/Code/GameEngine/Include/GameClient/GUICallbacks.h
• Generals/Code/GameEngine/Source/Common/RTS/PlayerTemplate.cpp
• Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp
• Generals/Code/GameEngine/Source/Common/System/FunctionLexicon.cpp
• Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
• Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp
• Generals/Code/GameEngine/Source/Common/System/registry.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupHostGame.cpp
• GeneralsMD/Code/GameEngine/CMakeLists.txt
• GeneralsMD/Code/GameEngine/Source/Common/RTS/PlayerTemplate.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/FunctionLexicon.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
• scripts/cpp/unify_move_files.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[tintinhamans]

Looks good to me.

In terms of BuildAssistant, I think isLocationClearOfObjects has no other callers so the Bool to LegalBuildCode is probably fine.

Could add to "Generals gets" that registry now reads HKCU before HKLM, and the loading save flag is set while loading a save.

[xezon]

In terms of BuildAssistant, I think isLocationClearOfObjects has no other callers so the Bool to LegalBuildCode is probably fine.

Yes

Could add to "Generals gets" that registry now reads HKCU before HKLM, and the loading save flag is set while loading a save.

I omitted that because it was only part of this merge because bugfix(registry): Prioritize HKEY_CURRENT_USER registry reads and writes over HKEY_LOCAL_MACHINE to prevent inaccessible data (#1844) did not correctly replicate to Generals.

[xezon]

@Caball009 @Skyaero42 Can you please run a few Generals replays against this branch?

[Caball009]

I'm seeing a ~95% mismatch rate for Generals replays :)

[xezon]

I'm seeing a ~95% mismatch rate for Generals replays :)

Ok that suckssss