Skip to content

unify(controlbar): Merge and move ControlBar and related code to core#2849

Open
stephanmeesters wants to merge 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:unify/merge-move-controlbar
Open

unify(controlbar): Merge and move ControlBar and related code to core#2849
stephanmeesters wants to merge 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:unify/merge-move-controlbar

Conversation

@stephanmeesters

@stephanmeesters stephanmeesters commented Jul 1, 2026

Copy link
Copy Markdown

Merge by rebase

Rebased and diffs checked in WinMerge. Re-applied the move with the script.

Changes to Generals

  • Fixed a Base Generals HUD regression where the Generals Powers shortcut bar could disappear.
  • Added support for dynamic General's Power purchase images.
  • Resized the General's Power purchase panel from its configured image.
  • Unified special-power shortcut handling for Generals and Zero Hour.
  • Preserved Base Generals command-center behavior while supporting Zero Hour shortcut powers.
  • Let OCL timer tech buildings show rally-point controls when supported.
  • Hid tooltip cost text for free items and sciences.
  • Showed building-specific max-count tooltip text.
  • Listed missing General's Promotions in upgrade tooltips.

Todo

  • Add pull id's
  • Test

@stephanmeesters stephanmeesters force-pushed the unify/merge-move-controlbar branch from 95571e0 to da28ca7 Compare July 1, 2026 11:50
@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown

Greptile Summary

This PR unifies the ControlBar subsystem by moving its headers and source files from the game-specific Generals/ and GeneralsMD/ directories into the shared Core/ tree. CMakeLists.txt files for both games are updated to comment out their local ControlBar entries, the Core CMakeLists.txt files are updated to enable the moved files, and the Generals-specific copies are deleted. The automation script is also updated to mark these operations as complete.

  • The CMakeLists.txt changes correctly use CMake's INTERFACE propagation model: Core files are registered on corei_gameengine_private and corei_gameenginedevice_private, which are then linked by both g_gameengine (Generals) and z_gameengine (Zero Hour), so both games will compile ControlBar from the single Core location.
  • The functional improvements described in the PR (shortcut-bar regression fix, dynamic purchase images, OCL-timer rally-point controls, hidden free-item costs, building-specific max-count tooltips) live in the pre-existing Core files and are now active for both builds.
  • The PR checklist explicitly marks "Test" as still outstanding; given the breadth of UI behaviour covered, this is worth confirming before merge.

Confidence Score: 4/5

The build-system wiring is correct and both games will compile from the single Core ControlBar location; the main concern is that the PR's own checklist marks testing as not yet done.

The CMakeLists.txt changes are mechanically straightforward and the INTERFACE-propagation model is used correctly. The Core files themselves are well-formed (pragma once, nullptr throughout). The only code-level finding is a pair of duplicate forward declarations in ControlBar.h that are harmless but untidy. The more significant concern is that the PR description explicitly leaves the 'Test' box unchecked, and the changed subsystem drives a significant portion of the in-game HUD for both titles — shortcut bars, tooltips, purchase UI, OCL timers, and rally-point controls are all affected. A full play-through in both Generals and Zero Hour before merge would be prudent.

Core/GameEngine/Include/GameClient/ControlBar.h (duplicate forward declarations). Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp should be exercised in a Generals build to confirm the shortcut-bar regression fix holds.

Important Files Changed

Filename Overview
Core/GameEngine/CMakeLists.txt Uncomments 15 ControlBar source/header entries in GAMEENGINE_SRC. Aligns correctly with the complementary removals in Generals and GeneralsMD CMakeLists files.
Core/GameEngineDevice/CMakeLists.txt Uncomments W3DControlBar.cpp in the Core device layer, mirroring the removal from Generals and GeneralsMD device CMakeLists files.
Generals/Code/GameEngine/CMakeLists.txt Comments out the 15 Generals-specific ControlBar entries from GAMEENGINE_SRC so that the Core versions are used instead.
GeneralsMD/Code/GameEngine/CMakeLists.txt Comments out the 15 Zero Hour-specific ControlBar entries from GAMEENGINE_SRC to switch to the shared Core version.
Core/GameEngine/Include/GameClient/ControlBar.h Moved from Generals/. Uses #pragma once correctly. Contains duplicate forward declarations for WindowVideoManager and GameWindow that are harmless but should be cleaned up.
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp Main ControlBar implementation moved to Core. Contains unified Generals/Zero Hour shortcut bar handling. Inner scope re-declaration of loop variable i inside populateSpecialPowerShortcut is a pre-existing minor shadow issue.
Core/GameEngine/Source/GameClient/GUI/GUICallbacks/ControlBarPopupDescription.cpp Tooltip description logic moved to Core. Implements new features: free-item cost hiding, building-specific max-count text, and promotion listing in upgrade tooltips. Logic looks correct.
scripts/cpp/unify_move_files.py Marks previously active CommandXlat and related unify_file calls as done (commented out), and documents the completed ControlBar move as also done. No functional script logic changed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Core ["Core (shared)"]
        CE["Core/GameEngine/CMakeLists.txt\n(ControlBar sources NOW enabled)"]
        CED["Core/GameEngineDevice/CMakeLists.txt\n(W3DControlBar.cpp NOW enabled)"]
        CB["Core/.../ControlBar.cpp\nControlBar.h, ControlBarScheme, etc."]
        W3D["Core/.../W3DControlBar.cpp"]
    end

    subgraph Generals ["Generals (game-specific)"]
        GE["Generals/Code/GameEngine/CMakeLists.txt\n(ControlBar entries commented out)"]
        GDEL["Generals ControlBar source files\n(DELETED)"]
    end

    subgraph GeneralsMD ["GeneralsMD / Zero Hour (game-specific)"]
        ZE["GeneralsMD/Code/GameEngine/CMakeLists.txt\n(ControlBar entries commented out)"]
    end

    CE -->|"INTERFACE sources via corei_gameengine_private"| g_gameengine["g_gameengine (Generals build)"]
    CE -->|"INTERFACE sources via corei_gameengine_private"| z_gameengine["z_gameengine (Zero Hour build)"]
    CED -->|"INTERFACE sources"| g_gameengine
    CED -->|"INTERFACE sources"| z_gameengine
    GE -->|"game-specific files"| g_gameengine
    ZE -->|"game-specific files"| z_gameengine
    CB -.->|"compiled into"| CE
    W3D -.->|"compiled into"| CED
    GDEL -.-|"replaced by Core"| CB
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    subgraph Core ["Core (shared)"]
        CE["Core/GameEngine/CMakeLists.txt\n(ControlBar sources NOW enabled)"]
        CED["Core/GameEngineDevice/CMakeLists.txt\n(W3DControlBar.cpp NOW enabled)"]
        CB["Core/.../ControlBar.cpp\nControlBar.h, ControlBarScheme, etc."]
        W3D["Core/.../W3DControlBar.cpp"]
    end

    subgraph Generals ["Generals (game-specific)"]
        GE["Generals/Code/GameEngine/CMakeLists.txt\n(ControlBar entries commented out)"]
        GDEL["Generals ControlBar source files\n(DELETED)"]
    end

    subgraph GeneralsMD ["GeneralsMD / Zero Hour (game-specific)"]
        ZE["GeneralsMD/Code/GameEngine/CMakeLists.txt\n(ControlBar entries commented out)"]
    end

    CE -->|"INTERFACE sources via corei_gameengine_private"| g_gameengine["g_gameengine (Generals build)"]
    CE -->|"INTERFACE sources via corei_gameengine_private"| z_gameengine["z_gameengine (Zero Hour build)"]
    CED -->|"INTERFACE sources"| g_gameengine
    CED -->|"INTERFACE sources"| z_gameengine
    GE -->|"game-specific files"| g_gameengine
    ZE -->|"game-specific files"| z_gameengine
    CB -.->|"compiled into"| CE
    W3D -.->|"compiled into"| CED
    GDEL -.-|"replaced by Core"| CB
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/GameEngine/Include/GameClient/ControlBar.h:41-50
**Duplicate forward declarations**

`class WindowVideoManager;` is declared on both line 47 and line 48, and `class GameWindow;` is declared on both line 41 and line 50. These are harmless in C++ but are clear copy-paste artifacts that may generate compiler warnings and add noise to the header. Since this file is now actively compiled for both games, it is a good time to clean them up.

Reviews (1): Last reviewed commit: "unify(controlbar): Move ControlBar files..." | Re-trigger Greptile

@stephanmeesters stephanmeesters added Gen Relates to Generals ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Jul 1, 2026
@xezon xezon added ZH Relates to Zero Hour and removed ZH Relates to Zero Hour labels Jul 1, 2026
@xezon xezon added this to the Code foundation build up milestone Jul 1, 2026

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a Base Generals HUD regression where the Generals Powers shortcut bar could disappear

How did Generals regress?

descrip.concat( TheGameText->fetch( "TOOLTIP:TooltipCannotBuildUnitBecauseMaximumNumber" ) );
if ( thingTemplate->isKindOf( KINDOF_STRUCTURE ) )
{
descrip.concat( TheGameText->fetch( "TOOLTIP:TooltipCannotBuildBuildingBecauseMaximumNumber" ) );

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tool tip string does not exist in Generals and therefore will show placeholder text now.

win = TheWindowManager->winGetWindowFromId( nullptr, TheNameKeyGenerator->nameToKey( "GeneralsExpPoints.wnd:GenExpParent" ) );
if(win)
{
win->winSetEnabledImage(0,m_powerPurchaseImage);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_powerPurchaseImage will be null here in Generals. Is this ok?

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

Labels

Gen Relates to Generals Unify Unifies code between Generals and Zero Hour ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants