Skip to content

bugfix(object): Preserve max-health upgrade order #3285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tintinhamans wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

bugfix(object): Preserve max-health upgrade order #3285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Generals/Code/GameEngine/Include/GameLogic/Object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ class Object : public Thing, public Snapshot
ExperienceTracker* getExperienceTracker() {return m_experienceTracker;}
const ExperienceTracker* getExperienceTracker() const {return m_experienceTracker;}
VeterancyLevel getVeterancyLevel() const;
Real getMaxHealthUpgradeScale() const;

inline const AsciiString& getName() const { return m_name; }
inline void setName( const AsciiString& newName ) { m_name = newName; }
Expand Down
6 changes: 6 additions & 0 deletions Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2727,6 +2727,12 @@ VeterancyLevel Object::getVeterancyLevel() const
return m_experienceTracker ? m_experienceTracker->getVeterancyLevel() : LEVEL_REGULAR;
}

//-------------------------------------------------------------------------------------------------
Real Object::getMaxHealthUpgradeScale() const
{
return TheGlobalData->m_healthBonus[getVeterancyLevel()];
}

//-------------------------------------------------------------------------------------------------
void Object::friend_bindToDrawable( Drawable *draw )
{
Expand Down
6 changes: 5 additions & 1 deletion Generals/Code/GameEngine/Source/GameLogic/Object/Upgrade/MaxHealthUpgrade.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ void MaxHealthUpgrade::upgradeImplementation()
{
const MaxHealthUpgradeModuleData *data = getMaxHealthUpgradeModuleData();

//Simply add the xp scalar to the xp tracker!
Object *obj = getObject();

BodyModuleInterface *body = obj->getBodyModule();
if( body )
{
#if RETAIL_COMPATIBLE_CRC
body->setMaxHealth( body->getMaxHealth() + data->m_addMaxHealth, data->m_maxHealthChangeType );
#else
Real maxHealth = body->getMaxHealth() + data->m_addMaxHealth * obj->getMaxHealthUpgradeScale();
Comment on lines +89 to +92

@greptile-apps greptile-apps Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Fix Disabled by Default

RETAIL_COMPATIBLE_CRC defaults to 1, so normal builds take this branch and keep adding the raw AddMaxHealth value. The new veterancy scaling runs only when that macro is explicitly disabled. As a result, applying veterancy before the armor upgrade still produces 750 health instead of 780 in default builds. The Zero Hour implementation has the same guard and behavior.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameLogic/Object/Upgrade/MaxHealthUpgrade.cpp
Line: 89-92

Comment:
**Fix Disabled by Default**

`RETAIL_COMPATIBLE_CRC` defaults to `1`, so normal builds take this branch and keep adding the raw `AddMaxHealth` value. The new veterancy scaling runs only when that macro is explicitly disabled. As a result, applying veterancy before the armor upgrade still produces 750 health instead of 780 in default builds. The Zero Hour implementation has the same guard and behavior.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@tintinhamans tintinhamans Sep 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is by design, this fix is not compatible with non-retail builds.

@greptile-apps greptile-apps Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Understood—thanks for clarifying. Since this fix is intentionally limited to non-retail builds, the RETAIL_COMPATIBLE_CRC guard is by design and my finding does not apply. I'll withdraw the comment.

Comment thread
tintinhamans marked this conversation as resolved.
body->setMaxHealth( maxHealth, data->m_maxHealthChangeType );
Comment on lines +92 to +93

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. New veterans receive too much health 🐞 Bug ≡ Correctness

MaxHealthUpgrade::upgradeImplementation uses the tracker's new veterancy bonus even when
Object::onVeterancyLevelChanged runs upgrade modules before the body applies the old-to-new health
multiplier. If a player health upgrade is already completed when a unit is created with production
veterancy, or the module is triggered by promotion, the addition is multiplied at lines 92-93 and
again by ActiveBody::onVeterancyLevelChanged, so a +100 elite upgrade at 1.3× contributes 169
health instead of 130 in both games.
Agent Prompt 
## Issue description
Max-health upgrades can execute after the experience tracker has entered its new veterancy level but before the body has applied that veterancy transition. The new scaling therefore applies the new bonus to the addition before the body multiplies the same addition again.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[2835-2851]
- Generals/Code/GameEngine/Source/GameLogic/Object/Upgrade/MaxHealthUpgrade.cpp[89-94]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[3140-3156]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Upgrade/MaxHealthUpgrade.cpp[89-94]

## Recommended Fix
Apply the body's old-to-new veterancy health transition before any upgrade-module pass can execute against the new veterancy level, then grant and process the corresponding upgrade. Mirror the ordering change in both games and cover a unit created with non-regular production veterancy while its player already owns a max-health upgrade.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

#endif
}
}

Expand Down
1 change: 1 addition & 0 deletions GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ class Object : public Thing, public Snapshot
ExperienceTracker* getExperienceTracker() {return m_experienceTracker;}
const ExperienceTracker* getExperienceTracker() const {return m_experienceTracker;}
VeterancyLevel getVeterancyLevel() const;
Real getMaxHealthUpgradeScale() const;

inline const AsciiString& getName() const { return m_name; }
inline void setName( const AsciiString& newName ) { m_name = newName; }
Expand Down
6 changes: 6 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3015,6 +3015,12 @@ VeterancyLevel Object::getVeterancyLevel() const
return m_experienceTracker ? m_experienceTracker->getVeterancyLevel() : LEVEL_REGULAR;
}

//-------------------------------------------------------------------------------------------------
Real Object::getMaxHealthUpgradeScale() const
{
return TheGlobalData->m_healthBonus[getVeterancyLevel()];
}

//-------------------------------------------------------------------------------------------------
void Object::friend_bindToDrawable( Drawable *draw )
{
Expand Down
6 changes: 5 additions & 1 deletion GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Upgrade/MaxHealthUpgrade.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ void MaxHealthUpgrade::upgradeImplementation()
{
const MaxHealthUpgradeModuleData *data = getMaxHealthUpgradeModuleData();

//Simply add the xp scalar to the xp tracker!
Object *obj = getObject();

BodyModuleInterface *body = obj->getBodyModule();
if( body )
{
#if RETAIL_COMPATIBLE_CRC
body->setMaxHealth( body->getMaxHealth() + data->m_addMaxHealth, data->m_maxHealthChangeType );
#else
Real maxHealth = body->getMaxHealth() + data->m_addMaxHealth * obj->getMaxHealthUpgradeScale();
body->setMaxHealth( maxHealth, data->m_maxHealthChangeType );
#endif
}
}

Expand Down