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]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: e2dcd03

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The changes update build-placement and unit-production checks, add player-template fields and lookup, wire Challenge menu callbacks, adjust save handling and 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 results and path checks Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp	Placement checks propagate detailed clearance 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 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	Unit-production checks use configuration-specific simultaneous-unit limits. The Generals retail-compatible path counts existing objects and queued units; other configurations use the player unit-cap check and special-power construction shortcut.

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 conditionally registers its callbacks. PopupHostGameUpdate is declared, implemented with an empty body, and registered. The GeneralsMD build excludes ChallengeMenu.cpp. The file-move script contains a commented-out invocation.

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 searches template names without case sensitivity. GeneralsMD field entries receive Zero Hour comments.
Player simultaneous-build limit Generals/Code/GameEngine/Include/Common/Player.h, Generals/Code/GameEngine/Source/Common/RTS/Player.cpp	Player::canBuildMoreOfType counts living matching objects and, for non-structures, queued units against the configured maximum.

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.

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: ⬆️ High

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

Change: Feature

Suggested reviewers: bobtista

Pre-merge checks: 4

Check name	Status	Explanation
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.
Title check		The title clearly identifies the common-system merge and names the primary affected components. It is specific and related to the changeset.
Description check		The description directly covers the common-system merge, ChallengeMenu relocation, BuildAssistant changes, PlayerTemplate updates, and replay testing.

• 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 PR appears safe to merge based on the code review; replay compatibility has not been tested.

Summary

Prepares common engine code for sharing between Generals and Zero Hour.

• The Challenge Menu sets up a match from the chosen general.
• Build limits count matching units that are built or queued.
• Building checks return clearer reasons when a site is blocked.
• Save files keep the game setup and use game-specific list colors.
• Registry string lookups prefer the current user's settings.
• Small wording and formatting edits leave behavior unchanged.

Previous-thread context:

• xezon dismissed the failed-load quit-menu finding as unrelated to this merge.
• The missing Generals canBuildMoreOfType declaration and implementation are now present.
• xezon accepted the quick-path behavior provided RETAIL_COMPATIBLE_PATHFINDING=1 keeps retail behavior.

The ancestry and merge-base checks did not pass, so this review used the full PR diff. No build, replay, or visual test was run.

_{Reviews (3) · Last reviewed commit: "Exclude FunctionLexicon changes from Gen..." · 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 5a588351cda521fca6926f8149da0d0281e131ad.

📒 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

[Caball009]

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

Ok that suckssss

It's the additions to "FunctionLexicon.cpp". NameKeyGenerator::m_nextID strikes again...

[xezon]

It's the additions to "FunctionLexicon.cpp". NameKeyGenerator::m_nextID strikes again...

Ok fixed that. Can you retest that?

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 75a46317-e998-47af-8c8a-29aa3ab9249d

📥 Commits

Reviewing files that changed from the base of the PR and between 7566178eee17a47cdc5770566f247ba1c013faf7 and e2dcd03.

📒 Files selected for processing: 20

• 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/Player.h
• Generals/Code/GameEngine/Include/Common/PlayerTemplate.h
• Generals/Code/GameEngine/Include/GameClient/GUICallbacks.h
• Generals/Code/GameEngine/Source/Common/RTS/Player.cpp
• 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

💤 Files with no reviewable changes: 1

• Core/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ChallengeMenu.cpp

🚧 Files skipped from review as they are similar to previous changes: 2

• scripts/cpp/unify_move_files.py
• GeneralsMD/Code/GameEngine/Source/Common/RTS/PlayerTemplate.cpp

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

Fixes since review.

[Caball009]

Looks safe now. I checked 3000 Generals replays and didn't see a mismatch because of these changes.

I have not looked closely at the changes themselves.

[Skyaero42]

Code looks good