Repository navigation
bugfix(CacheButtonImage):Allow player to edit images with map/solo.ini - #3446
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughButton image access and caching now use final override data. Map loading post-processes control-bar commands and resets the upgrade center after loading ChangesOverride image handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Suggested reviewers: Merge Risk: 🔵 Low · up to In Zero Hour, a map’s upgrade image may remain unchanged when its MappedImage block follows its Upgrade block. This is a bounded feature gap rather than a broader gameplay failure, but it leaves the map/solo image fix incomplete for that variant. Pre-merge checks |
 |
| 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 CacheButtonImage bug fix and the support for editing images through map/solo.ini. |
| Description check | |
The description directly explains the image-update issue, the intended fix, affected objects, and testing context. |
- Autopilot · 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.
Comment @coderabbitai help to get the list of available commands.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES) | ||
| { | ||
| button->cacheButtonImage(); | ||
| } |
There was a problem hiding this comment.
🟡 Medium INI/INICommandButton.cpp:77
An override whose ButtonImage is declared later in the INI loses its image name and ends up with a null or stale image. cacheButtonImage() runs before that MappedImage exists and clears m_buttonImageName, so postProcessCommands() cannot retry; defer this cache call until after mapped images are loaded.
- if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES)
- {
- button->cacheButtonImage();
- }🤖 Copy this AI Prompt to have your agent fix this:
In file @Core/GameEngine/Source/Common/INI/INICommandButton.cpp around lines 77-80:
An override whose `ButtonImage` is declared later in the INI loses its image name and ends up with a null or stale image. `cacheButtonImage()` runs before that `MappedImage` exists and clears `m_buttonImageName`, so `postProcessCommands()` cannot retry; defer this cache call until after mapped images are loaded.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES) | ||
| { | ||
| button->cacheButtonImage(); | ||
| } |
There was a problem hiding this comment.
When a CommandButton names a MappedImage defined later in map.ini or solo.ini, this new call looks it up before it exists. cacheButtonImage() then clears m_buttonImageName even though the lookup failed. Loading the image later cannot repair the blank button.
The new calls in both versions of Upgrade.cpp have the same problem. Defer these lookups until both map files finish loading, or preserve failed names for a later retry.
|
|
||
| const Image *getSelectedPortraitImage() const { return m_selectedPortraitImage; } | ||
| const Image *getButtonImage() const { return m_buttonImage; } | ||
| const Image* getButtonImage() const { return ((const ThingTemplate*)getFinalOverride())->m_buttonImage; } |
There was a problem hiding this comment.
This getter still leaves map-defined unit images broken in Generals. Its ThingFactory::parseThingTemplate() parses and validates map overrides but never calls resolveNames(), unlike the Zero Hour version. Reading the final override therefore returns the copied base image when ButtonImage changes, or no image for a new unit.
Resolve the override's image in the Generals load path before this getter reads it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
30f424e0-fcee-4412-b1ed-205fed84417d
📒 Files selected for processing (6)
Core/GameEngine/Include/Common/ThingTemplate.hCore/GameEngine/Include/GameClient/ControlBar.hCore/GameEngine/Source/Common/INI/INICommandButton.cppCore/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cppGenerals/Code/GameEngine/Source/Common/System/Upgrade.cppGeneralsMD/Code/GameEngine/Source/Common/System/Upgrade.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.
| CommandButton* finalOverride = (CommandButton*)getFinalOverride(); | ||
| if (finalOverride->m_buttonImageName.isNotEmpty()) | ||
| const_cast<CommandButton*>(finalOverride)->cacheButtonImage(); | ||
| return finalOverride->m_buttonImage; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n -C 5 'copyImagesFrom\(|getButtonImage\(|getFinalOverride\(' Core/GameEngine/Source/GameClient/GUI/ControlBar Core/GameEngine/Include/GameClient/ControlBar.hRepository: TheSuperHackers/GeneralsGameCode
Length of output: 25663
🏁 Script executed:
set -eu
printf '%s\n' '--- shortcut branches in ControlBarCommand.cpp ---'
sed -n '360,475p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp
printf '%s\n' '--- shortcut branches in ControlBar.cpp ---'
sed -n '3400,3520p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
printf '%s\n' '--- CommandButton override declarations and helpers ---'
rg -n -C 8 --glob '*.[ch]' -- 'getFinalOverride\(|friend_getFinalOverride|m_override|override' Core/GameEngine/Include/GameClient/ControlBar.h Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | sed -n '1,260p'
printf '%s\n' '--- getter diff against merge base ---'
git diff 1b1212f0de1bab9997f86bb1fa7e3c9cb2815674 45d8e2011617b3831e44733d53501b63209ec5f6 -- Core/GameEngine/Include/GameClient/ControlBar.h Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cppRepository: TheSuperHackers/GeneralsGameCode
Length of output: 23377
🏁 Script executed:
set -eu
printf '%s\n' '--- ControlBarCommand.cpp enclosing shortcut function ---'
sed -n '1,390p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp
printf '%s\n' '--- ControlBar.cpp enclosing shortcut function ---'
sed -n '3280,3425p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
printf '%s\n' '--- all getFinalOverride declarations and definitions ---'
rg -n -C 6 --glob '*.{h,hpp,c,cpp,inc}' -- 'getFinalOverride|friend_getFinalOverride|setNextOverride|markAsOverride' Core | sed -n '1,360p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 44644
🏁 Script executed:
set -eu
printf '%s\n' '--- CommandButton and CommandSet declarations ---'
rg -n -C 20 --glob '*.{h,hpp}' -- 'class CommandButton|class CommandSet|getCommandButton\(' Core/GameEngine/Include Core/GameClient/Include 2>/dev/null | sed -n '1,360p'
printf '%s\n' '--- getCommandButton definitions and callers ---'
rg -n -C 10 --glob '*.{c,cpp,h,hpp}' -- 'CommandSet::getCommandButton|getCommandButton\( *Int|getCommandButton\( *[a-zA-Z_]+ *\)' Core | sed -n '1,360p'
printf '%s\n' '--- override base declarations ---'
rg -n -C 12 --glob '*.{h,hpp}' -- 'friend_getFinalOverride|setNextOverride|markAsOverride|class Overridable|Overridable' Core/GameEngine/Include Core/GameClient/Include 2>/dev/null | sed -n '1,360p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 41651
🏁 Script executed:
set -eu
printf '%s\n' '--- exact CommandSet::getCommandButton ---'
sed -n '820,870p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
printf '%s\n' '--- findControlBarOverride declarations and definitions ---'
rg -n -C 14 --glob '*.{h,hpp,c,cpp}' -- 'findControlBarOverride' Core | sed -n '1,300p'
printf '%s\n' '--- Overridable files and methods ---'
rg --files | rg '(^|/)(Overridable|Override)\.(h|hpp|c|cpp)$'
rg -n -C 12 --glob '*.{h,hpp,c,cpp}' -- 'class Overridable|Overridable::|getFinalOverride\(' . | sed -n '1,320p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 36858
🏁 Script executed:
set -eu
printf '%s\n' '--- all tracked findControlBarOverride references ---'
git grep -n -C 10 -i -- 'findControlBarOverride' || true
printf '%s\n' '--- command-button and command-set override creation/parsing ---'
sed -n '1938,2025p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
sed -n '1,180p' Core/GameEngine/Source/Common/INI/INICommandButton.cpp
sed -n '1,180p' Core/GameEngine/Source/Common/INI/INICommandSet.cpp 2>/dev/null || true
git grep -n -C 8 -- 'newCommandButtonOverride\|newCommandSetOverride\|parseCommandButton\|setNextOverride' -- Core/GameEngine/Source Core/GameEngine/Include | sed -n '1,360p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 41501
🏁 Script executed:
set -eu
printf '%s\n' '--- setControlBarOverride callers and lifecycle ---'
git grep -n -C 12 -- 'setControlBarOverride' -- Core Generals GeneralsMD | sed -n '1,360p'
printf '%s\n' '--- command-set lookup and override chain callers ---'
git grep -n -C 8 -- 'findCommandSet(' -- Core/GameEngine/Source/GameClient/GUI/ControlBar Core/GameEngine/Source/GameLogic | sed -n '1,300p'
printf '%s\n' '--- relevant INI load ordering references ---'
git grep -n -C 8 -- 'parseCommandButtonDefinition\|parseCommandSetDefinition\|INI_LOAD_CREATE_OVERRIDES' -- Core/GameEngine/Source/Common Core/GameEngine/Source/GameClient/GUI/ControlBar | sed -n '1,360p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 41713
🏁 Script executed:
set -eu
printf '%s\n' '--- map/solo INI loading sequence ---'
git grep -n -C 12 -E 'solo\.ini|map\.ini|load.*INI|INI_LOAD_CREATE_OVERRIDES' -- Core/GameEngine/Source/GameLogic Core/GameEngine/Source/GameClient Core/GameEngine/Source/Common | sed -n '1,420p'
printf '%s\n' '--- exact shortcut setup and consumer flow ---'
rg -n -C 18 -- 'populateSpecialPowerShortcut\(|setControlCommand\(|commandButton = commandSet->getCommandButton|copyImagesFrom\( command' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp | sed -n '1,420p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 41664
🏁 Script executed:
set -eu
sed -n '748,792p' Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cppRepository: TheSuperHackers/GeneralsGameCode
Length of output: 1791
Write copied images to the final command-button override.
When a command-set slot still points to the base button and a later map/solo override exists, the special-power shortcut updates the base image with copyImagesFrom(). getButtonImage() then reads the final override image. The shortcut can display the old override image instead of the science-selected image.
Resolve the receiver to its final override inside copyImagesFrom() and preserve the existing dirty-state behavior.
Suggested fix
void CommandButton::copyImagesFrom( const CommandButton *button, Bool markUIDirtyIfChanged ) const
{
- if( m_buttonImage != button->getButtonImage() )
+ CommandButton* finalOverride = (CommandButton*)getFinalOverride();
+ if( finalOverride->m_buttonImage != button->getButtonImage() )
{
- m_buttonImage = button->getButtonImage();
+ finalOverride->m_buttonImage = button->getButtonImage();
//Code renderer handles these states now.|
|
||
| if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES) | ||
| { | ||
| button->cacheButtonImage(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
printf '%s\n' '--- changed hunk ---'
git diff 1b1212f0de1bab9997f86bb1fa7e3c9cb2815674 45d8e2011617b3831e44733d53501b63209ec5f6 -- Core/GameEngine/Source/Common/INI/INICommandButton.cpp
printf '%s\n' '--- parser and cache blocks ---'
nl -ba Core/GameEngine/Source/Common/INI/INICommandButton.cpp | sed -n '45,95p'
nl -ba Core/GameEngine/Source/Common/INI/INI.cpp | sed -n '380,415p'
nl -ba Core/GameEngine/Source/Common/INI/INIMappedImage.cpp | sed -n '45,75p'
nl -ba Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | sed -n '2530,2575p'
printf '%s\n' '--- postProcessCommands callers ---'
rg -n -F -- 'postProcessCommands(' Core Generals GeneralsMD || test "$?" -eq 1
printf '%s\n' '--- map override load and client image initialization ---'
nl -ba Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | sed -n '1170,1195p'
nl -ba Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | sed -n '2180,2210p'
nl -ba Generals/Code/GameEngine/Source/GameClient/GameClient.cpp | sed -n '260,280p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 11019
🏁 Script executed:
set -eu
printf '%s\n' '--- ControlBar initialization around postProcessCommands ---'
nl -ba Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | sed -n '1000,1090p'
printf '%s\n' '--- ControlBar initialization callers ---'
rg -n -F -- 'TheControlBar' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source | head -80
printf '%s\n' '--- GameClient initialization entrypoint ---'
rg -n -F -- 'GameClient::init' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
rg -n -F -- 'GameClient::' Core/GameEngine/Source/GameClient/GameClient.cpp | head -40
printf '%s\n' '--- map-loading caller context ---'
nl -ba Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | sed -n '1100,1190p'Repository: TheSuperHackers/GeneralsGameCode
Length of output: 18331
🏁 Script executed:
set -eu
printf '%s\n' '--- ControlBar init call ---'
rg -n -F -- 'TheControlBar->init' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- GameClient initialization context ---'
nl -ba Generals/Code/GameEngine/Source/GameClient/GameClient.cpp | sed -n '225,310p'
nl -ba GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp | sed -n '235,320p'
printf '%s\n' '--- ControlBar construction context ---'
rg -n -F -- 'new ControlBar' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
rg -n -F -- 'MSGNEW("GameClientSubsystem") ControlBar' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1Repository: TheSuperHackers/GeneralsGameCode
Length of output: 10250
🏁 Script executed:
set -eu
printf '%s\n' '--- InGameUI ControlBar init call sites ---'
nl -ba Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp | sed -n '1340,1405p'
nl -ba Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp | sed -n '5860,5925p'
printf '%s\n' '--- surrounding InGameUI lifecycle declarations ---'
rg -n '^(void|Bool|Int|InGameUI::)[A-Za-z_].*' Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp | sed -n '1,80p'
printf '%s\n' '--- map-start and UI setup references ---'
rg -n -F -- 'initInGameUI' Generals/Code/GameEngine/Source Core/GameEngine/Source || test "$?" -eq 1
rg -n -F -- 'TheInGameUI' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp | head -80Repository: TheSuperHackers/GeneralsGameCode
Length of output: 9856
🏁 Script executed:
set -eu
printf '%s\n' '--- recreateControlBar callers ---'
rg -n -F -- 'recreateControlBar(' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- all postProcessCommands references ---'
rg -n -F -- 'postProcessCommands' Core/GameEngine/Source Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- map-load references to control-bar initialization ---'
rg -n -F -- 'TheControlBar->init()' Generals/Code/GameEngine/Source/GameLogic GeneralsMD/Code/GameEngine/Source/GameLogic Core/GameEngine/Source/GameLogic || test "$?" -eq 1
rg -n -F -- 'recreateControlBar' Generals/Code/GameEngine/Source/GameLogic GeneralsMD/Code/GameEngine/Source/GameLogic Core/GameEngine/Source/GameLogic || test "$?" -eq 1Repository: TheSuperHackers/GeneralsGameCode
Length of output: 1197
🏁 Script executed:
set -eu
printf '%s\n' '--- Generals loadMapINI tail ---'
nl -ba Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | sed -n '2140,2245p'
printf '%s\n' '--- GeneralsMD loadMapINI tail ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | sed -n '2445,2545p'
printf '%s\n' '--- GameLogic includes and ControlBar visibility ---'
sed -n '1,90p' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
rg -n -F -- 'TheControlBar' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | head -20Repository: TheSuperHackers/GeneralsGameCode
Length of output: 12823
Cache command-button images after loading all map overrides.
INI::load() processes blocks in file order. A CommandButton can therefore reference a later MappedImage. The new immediate cache call fails, clears m_buttonImageName, and leaves m_buttonImage null. No normal postProcessCommands() call runs after map.ini and solo.ini load.
Remove the per-block cache call and run one cache pass after both override files finish loading.
Suggested fix
diff --git a/Core/GameEngine/Source/Common/INI/INICommandButton.cpp b/Core/GameEngine/Source/Common/INI/INICommandButton.cpp
@@
- if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES)
- {
- button->cacheButtonImage();
- }
-
const SpecialPowerTemplate *spTemplate = button->getSpecialPowerTemplate();
diff --git a/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp b/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
@@
if (TheFileSystem->doesFileExist(fullFledgeFilename)) {
DEBUG_LOG(("Loading solo.ini"));
INI ini;
ini.load( AsciiString(fullFledgeFilename), INI_LOAD_CREATE_OVERRIDES, nullptr );
}
+ if (TheControlBar)
+ TheControlBar->postProcessCommands();
+
// No error here. There could've just *not* been a map.ini file.
diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
@@
if (TheFileSystem->doesFileExist(fullFledgeFilename)) {
DEBUG_LOG(("Loading solo.ini"));
INI ini;
ini.load( AsciiString(fullFledgeFilename), INI_LOAD_CREATE_OVERRIDES, nullptr );
}
+ if (TheControlBar)
+ TheControlBar->postProcessCommands();
+
// No error here. There could've just *not* been a map.ini file.|
|
||
| // TheSuperHackers @bugfix DrGoldFish 21/02/2026 Keeps map.ini upgrade cameos scoped to the map override lifetime. | ||
| if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES) | ||
| upgrade->cacheButtonImage(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '190,230p' Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp
sed -n '270,310p' Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp
sed -n '2140,2220p' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
rg -n 'cacheButtonImage\(|buttonImagesCached|TheUpgradeCenter->reset|TheUpgradeCenter->' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Generals/Code/GameEngine/Source/Common/System/Upgrade.cppRepository: TheSuperHackers/GeneralsGameCode
Length of output: 6926
🏁 Script executed:
set -u
for f in \
Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp \
GeneralsMD/Code/GameEngine/Source/Common/System/Upgrade.cpp \
Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp \
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
do
echo "===== $f: parser/reset references ====="
rg -n -F -- 'parseUpgradeDefinition' "$f" || true
rg -n -F -- 'TheUpgradeCenter->reset' "$f" || true
rg -n -F -- 'UpgradeCenter::reset' "$f" || true
done
echo "===== Generals parser block ====="
sed -n '450,515p' Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp
echo "===== GeneralsMD parser block ====="
sed -n '450,515p' GeneralsMD/Code/GameEngine/Source/Common/System/Upgrade.cpp
echo "===== all reset call sites in game code ====="
rg -n -F --glob '*.{cpp,h}' -- 'TheUpgradeCenter->reset' Generals GeneralsMD Core || true
echo "===== map load tails ====="
sed -n '2180,2260p' Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
sed -n '2485,2555p' GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cppRepository: TheSuperHackers/GeneralsGameCode
Length of output: 10265
🏁 Script executed:
set -u
echo "===== UpgradeCenter declarations and inheritance ====="
rg -n -F --glob '*.{h,cpp}' -- 'class UpgradeCenter' Generals GeneralsMD Core || true
rg -n -F --glob '*.{h,cpp}' -- 'buttonImagesCached' Generals GeneralsMD Core || true
rg -n -F --glob '*.{h,cpp}' -- 'UpgradeCenter::reset' . || true
rg -n -F --glob '*.{h,cpp}' -- 'TheUpgradeCenter->reset' . || true
echo "===== reset declarations and subsystem lifecycle candidates ====="
rg -n -F --glob '*.{h,cpp}' -- 'void reset()' Generals/Code/GameEngine/Source/Common GeneralsMD/Code/GameEngine/Source/Common Core/GameEngine/Source/Common 2>/dev/null | rg 'Upgrade|INI|System|Subsystem' || true
rg -n -F --glob '*.{h,cpp}' -- 'TheUpgradeCenter' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source Core/GameEngine/Source 2>/dev/null | head -200 || true
echo "===== upgrade header context ====="
rg --files Generals/Code/GameEngine/Source/Common GeneralsMD/Code/GameEngine/Source/Common Core/GameEngine/Source/Common 2>/dev/null | rg '/Upgrade\.h$|/Upgrade\.cpp$' || trueRepository: TheSuperHackers/GeneralsGameCode
Length of output: 13402
🏁 Script executed:
set -u
echo "===== SubsystemInterface declarations ====="
rg -n -F --glob '*.{h,cpp}' -- 'class SubsystemInterface' Core Generals GeneralsMD || true
rg -n -F --glob '*.{h,cpp}' -- 'SubsystemInterface::reset' Core Generals GeneralsMD || true
echo "===== reset dispatch candidates ====="
rg -n -F --glob '*.{h,cpp}' -- 'resetSubsystem' Core Generals GeneralsMD || true
rg -n -F --glob '*.{h,cpp}' -- 'resetAll' Core Generals GeneralsMD || true
rg -n -F --glob '*.{h,cpp}' -- '.reset()' Core/GameEngine Generals/Code/GameEngine GeneralsMD/Code/GameEngine || true
rg -n -F --glob '*.{h,cpp}' -- 'reset(' Core/GameEngine/Source/Common/GameEngine.cpp Generals/Code/GameEngine/Source/Common/GameEngine.cpp GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp || true
echo "===== subsystem lifecycle excerpts ====="
rg -n -F --glob '*.{h,cpp}' -- 'SubsystemInterface' Core/GameEngine Generals/Code/GameEngine GeneralsMD/Code/GameEngine | head -120 || trueRepository: TheSuperHackers/GeneralsGameCode
Length of output: 19270
🏁 Script executed:
set -u
echo "===== shared subsystem reset implementation ====="
sed -n '150,195p' Core/GameEngine/Source/Common/System/SubsystemInterface.cpp
echo "===== Generals engine reset lifecycle ====="
sed -n '585,665p' Generals/Code/GameEngine/Source/Common/GameEngine.cpp
echo "===== GeneralsMD engine reset lifecycle ====="
sed -n '748,828p' GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp
echo "===== resetSubsystems call context ====="
sed -n '590,655p' Generals/Code/GameEngine/Source/Common/GameEngine.cpp
sed -n '755,818p' GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cppRepository: TheSuperHackers/GeneralsGameCode
Length of output: 9628
Retry upgrade image caching after map overrides load.
INI::load parses blocks in file order. An Upgrade block can call cacheButtonImage() before its map-local MappedImage block is registered. The helper then clears m_buttonImageName after the failed lookup.
UpgradeCenter::reset() cannot recover this case. Its cache gate is already set during the engine reset, and loadMapINI() does not call it after map.ini and solo.ini.
Defer the override lookup until both files finish loading. Retain m_buttonImageName when lookup fails. Apply the same localized changes in both Generals variants.
Suggested fix
diff --git a/Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp b/Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp
@@
- m_buttonImageName.clear(); // we're done with this, so nuke it
+ if (m_buttonImage)
+ m_buttonImageName.clear(); // we're done with this, so nuke it
@@
- if( TheMappedImageCollection && !buttonImagesCached )
+ if( TheMappedImageCollection )
diff --git a/Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp b/Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp
@@
- if (ini->getLoadType() == INI_LOAD_CREATE_OVERRIDES)
- upgrade->cacheButtonImage();
-
diff --git a/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp b/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
@@
ini.load( AsciiString(fullFledgeFilename), INI_LOAD_CREATE_OVERRIDES, nullptr );
}
+ TheUpgradeCenter->reset();
+
// No error here. There could've just *not* been a map.ini file.Apply the equivalent changes to GeneralsMD/Code/GameEngine/Source/Common/System/Upgrade.cpp and GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| upgrade->cacheButtonImage(); |
|
Related #3343 |
|
How is this related? |
Sorry, typo. Corrected the link to the issue. |
bcf76fd to
d46e94d
Compare
|
Made some changes to what the bots suggested, I did test the changes and they seem to work like intended. |
3 participants
The images dont change for existing command buttons, upgrades, units and promotions when changed in the map/solo.ini.
The only issue that occurs now is that the image of the upgrades you changed does not get changed back to the original image, this is due to a different bug that does not reset any changes made to upgrades at all, and therefor unrelated to this issue.
Pictures dont show for NEW command buttons, upgrades, units and promotions when newly made in the map/solo.ini.
Before
After
These changes will allow map makers to create new units, promotions, upgrades and command buttons with an actual image to show for it, so this can be used for making maps easier and to prevent the current map.ini missmatch issues, by making new upgrades and stuff instead of changing the existing upgrades.
Testing can be done on my new zombie map for newly made objects and upgrades,
[Zombies] Wheel Of Death V7-55.zip
This map does change the icon of the power
[Testing] Bugs (2).zip
plant upgrade and the image of the ranger.