unify(controlbar): Merge and move ControlBar and related code to core#2675
unify(controlbar): Merge and move ControlBar and related code to core#2675DevGeniusCode wants to merge 2 commits intoTheSuperHackers:mainfrom
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | New Core file (Zero Hour baseline + regression fix): adds hasAnyShortcutSelection() and #if RTS_GENERALS conditional blocks to restore original Generals shortcut-bar visibility logic (command-center check) separate from the Zero Hour path (shortcut power/selection check). |
| scripts/cpp/unify_move_files.py | Newly added developer utility script for moving/unifying files between Generals, GeneralsMD and Core; main() is fully commented-out and serves as documentation. Minor issues: type parameter shadowing built-in and non-idempotent ADD_COMMENT path. |
| Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp | Zero Hour baseline introduced into Core; key fix hides all m_commandWindows slots before rebuilding the structure inventory, eliminating stale-button UI artifacts. |
| Core/GameEngine/CMakeLists.txt | Uncomments all ControlBar headers and source files so they participate in the Core build; mirrors the corresponding removals in Generals and GeneralsMD CMakeLists. |
| GeneralsMD/Code/GameEngine/CMakeLists.txt | Comments out all ControlBar source and header entries now provided by Core; no source files deleted from GeneralsMD (they were already moved in a prior step). |
| Generals/Code/GameEngine/CMakeLists.txt | Comments out ControlBar entries as the source files are deleted from Generals/ in this PR. |
| Core/GameEngine/Include/GameClient/ControlBarScheme.h | Zero Hour baseline moved to Core; adds m_powerPurchaseImage member for dynamic General's Power purchase icons. Uses #pragma once correctly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[updateSpecialPowerShortcut] --> B{RTS_GENERALS?}
B -- Yes --> C[findNaturalCommandCenter != nullptr]
B -- No --> D[hasAnyShortcutSpecialPower OR hasAnyShortcutSelection]
C --> E{hasValidShortcutButton}
D --> E
E -- true AND bar hidden --> F[showSpecialPowerShortcut\nanimateSpecialPowerShortcut TRUE]
E -- false AND bar visible --> G[hideSpecialPowerShortcut\nanimateSpecialPowerShortcut FALSE]
F --> H[showSpecialPowerShortcut]
H --> I{dontAnimate?}
I -- Yes --> J[return early]
I -- No --> K{RTS_GENERALS?}
K -- Yes --> L{findNaturalCommandCenter?}
L -- No --> J
L -- Yes --> M[winHide FALSE\npopulateSpecialPowerShortcut]
K -- No --> N{hasShortcutPower OR\nhasShortcutSelection?}
N -- No --> J
N -- Yes --> M
Comments Outside Diff (1)
-
scripts/cpp/unify_move_files.py, line 65-68 (link)Double-comment risk in
ADD_COMMENTpathif searchString in linematches regardless of whether the line is already commented out. Ifmodify_cmakelistsis called twice withADD_COMMENTon the same file/search string (e.g. if the script is re-run), the entry becomes## Source/...which CMake will not recognise as a commented line that can be cleanly un-commented. Adding an early-continue guard likeif line.lstrip().startswith('#'): continuebefore the modification would make the function idempotent.Prompt To Fix With AI
This is a comment left during a code review. Path: scripts/cpp/unify_move_files.py Line: 65-68 Comment: **Double-comment risk in `ADD_COMMENT` path** `if searchString in line` matches regardless of whether the line is already commented out. If `modify_cmakelists` is called twice with `ADD_COMMENT` on the same file/search string (e.g. if the script is re-run), the entry becomes `## Source/...` which CMake will not recognise as a commented line that can be cleanly un-commented. Adding an early-continue guard like `if line.lstrip().startswith('#'): continue` before the modification would make the function idempotent. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
scripts/cpp/unify_move_files.py:58
**`type` parameter shadows Python built-in**
The parameter name `type` in `modify_cmakelists` shadows Python's built-in `type()`. While this doesn't cause a runtime bug here, it is a well-known Python anti-pattern that can cause confusing behavior if `type()` is later needed inside the function. Consider renaming to `modify_type` or `operation`.
### Issue 2 of 2
scripts/cpp/unify_move_files.py:65-68
**Double-comment risk in `ADD_COMMENT` path**
`if searchString in line` matches regardless of whether the line is already commented out. If `modify_cmakelists` is called twice with `ADD_COMMENT` on the same file/search string (e.g. if the script is re-run), the entry becomes `## Source/...` which CMake will not recognise as a commented line that can be cleanly un-commented. Adding an early-continue guard like `if line.lstrip().startswith('#'): continue` before the modification would make the function idempotent.
Reviews (1): Last reviewed commit: "unify(controlbar): Move ControlBar files..." | Re-trigger Greptile
| || !ThePlayerList || !ThePlayerList->getLocalPlayer()) | ||
| return; | ||
|
|
||
| #if RTS_GENERALS |
There was a problem hiding this comment.
This looks incorrect.
The original code in generals is the same as in Zero Hour, so I don't understand the #if statement
There was a problem hiding this comment.
This is not the original code, it was changed to be like this after PR #2655
| break; | ||
| } | ||
| } | ||
| #if RTS_GENERALS |
|
|
||
| #if RTS_GENERALS | ||
| // Base Generals just needs a Command Center to show the shortcut bar | ||
| Bool hasValidShortcutButton = (ThePlayerList->getLocalPlayer()->findNaturalCommandCenter() != nullptr); |
There was a problem hiding this comment.
I missed this. I have prepared a fix with #2680
This PR unifies the core ControlBar GUI system, standardizing the GameEngine logic and W3D rendering device files against the Zero Hour (GeneralsMD) baseline. 18 files.
The merged is by
ControlBar.hthat we defined the command:Fixes & Regressions
ControlBar/UI Features (Zero Hour Port):
m_powerPurchaseImagetoControlBarSchemeto support dynamic General's Power purchase icons.ControlBarScheme::repositionControlBarto automatically resize theGeneralsExpPoints.wnd:GenExpParentwindow usinggetImageWidth()andgetImageHeight()from the scheme.GUI_COMMAND_SPECIAL_POWER_FROM_SHORTCUT(aliased toGUI_COMMAND_SPECIAL_POWER_FROM_COMMAND_CENTERfor Generals), utilizingfindNaturalCommandCenter()for legacy behavior andhasAnyShortcutSelection()/hasAnyShortcutSpecialPower()for Zero Hour behavior.ControlBarOCLTimerto detectKINDOF_TECH_BUILDINGandKINDOF_AUTO_RALLYPOINT, enablingExitInterface::getRallyPointandshowRallyPoint()functionality on objects that previously only supported a hardcoded "Sell" button.ControlBarPopupDescriptiontooltip logic to dynamically callwinHide(TRUE)on theStaticTextCostwindow ifcalcCostToBuild()orgetSciencePurchaseCost()returns 0.KINDOF_STRUCTUREcheck duringCANMAKE_MAXED_OUT_FOR_PLAYERvalidation to display theTOOLTIP:TooltipCannotBuildBuildingBecauseMaximumNumberstring instead of the generic unit error.missingScienceforGUI_COMMAND_PLAYER_UPGRADEandGUI_COMMAND_OBJECT_UPGRADEtypes to explicitly list required General's Promotions in the description.ControlBarStructureInventoryby implementing awinHide(TRUE)loop acrossm_commandWindowsfor allMAX_COMMANDS_PER_SETslots before rebuilding the inventory.