Skip to content

feat(client): Add HealthBarDisplayMode, NumericalHealth and SmartPips options - #3214

Open
triatomic wants to merge 5 commits into
TheSuperHackers:mainfrom
triatomic:qol/health-bar-display
Open

feat(client): Add HealthBarDisplayMode, NumericalHealth and SmartPips options#3214
triatomic wants to merge 5 commits into
TheSuperHackers:mainfrom
triatomic:qol/health-bar-display

Conversation

@triatomic

Copy link
Copy Markdown

Adds three opt-in client options for health bar and pip display. All default to retail behavior when unset in Options.ini.

HealthBarDisplayMode — controls when health bars appear:

  • Classic — selected and moused-over objects only (retail, default)
  • Damaged — the above, plus anything below full health
  • Always — the above, plus every undamaged unit and structure

A meta-event (default Ctrl+Shift+`) cycles the mode in-game; the choice is not persisted, Options.ini remains the source of truth.

NumericalHealth (= Yes) — prints hit points beside the health bar, following HealthBarDisplayMode so the number appears exactly where a bar does.

SmartPips (= Yes) — keeps ammo and passenger pips on screen whenever there is something to report, rather than only on selection/hover. Own units only — not allies, not enemies — so it reveals nothing the game otherwise withholds. Drawing pips every frame for every owned unit exposed three latent faults in the container pip path (null entries in the contained list, numFull/list disagreement via OverlordContain, unguarded pip image dereference), which are guarded as part of the change.

The in-game features are Zero Hour only per the contribution guidelines (the shared meta-event handler and key binding are guarded with RTS_ZEROHOUR; both titles compile). A Generals replica can follow after review.

These options ship in the Contra mod's engine fork and have been played there; this PR is the port onto current main. Code was written with LLM assistance and human-reviewed, adapted and playtested by the author.

Prepared for Squash and Merge.

Adds an Options.ini client preference controlling when health bars appear:

  HealthBarDisplayMode = Classic   ; selected and moused over only (default)
  HealthBarDisplayMode = Damaged   ; the above, plus anything below full health
  HealthBarDisplayMode = Always    ; the above, plus all undamaged units/structures

A plain index (0, 1, 2) is also accepted. Default is Classic, so behavior is
unchanged unless the option is set.

The mode is read into GlobalData alongside the other Options.ini overrides, so
it refreshes whenever game data is parsed rather than only at process start.

Deliberately a separate setting rather than an extension of m_showObjectHealth,
which stays the master switch and remains bound to the existing CHEAT_SHOW_HEALTH
and DEMO_SHOW_HEALTH keys.

Bars are suppressed for corpses, projectiles, shrubbery, trees, mines, inert,
unattackable, drawable only and non selectable objects, so the wider modes do
not label scenery. Always mode is further limited to structures and objects
with an AI module, i.e. real combatants and buildings. Hidden, stealthed and
shrouded objects are already filtered upstream in drawablePostDraw, so no mode
can reveal something the local player cannot see.

Entirely client side: the new code only reads state and draws, so it cannot
affect game logic, multiplayer sync or replays.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add configurable health bars, numerical health, and smart pips

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds configurable classic, damaged, and always-on health-bar visibility modes.
• Shows optional numerical health and owner-only persistent ammo/passenger pips.
• Adds session mode cycling and hardens container pip rendering against invalid state.
Diagram

graph TD
  A["Options.ini"] --> B["Option Preferences"] --> C["Global Display State"] --> D{"UI Draw Pass"} --> E["Health Bars"]
  D --> F["Numeric Health"]
  D --> G["Smart Pips"]
  H["Cycle Hotkey"] --> C
Loading
High-Level Assessment

The approach fits the existing OptionsPreferences-to-GlobalData pipeline and keeps all behavior client-side. Separate options are preferable to overloading the existing health master switch, while localized drawable guards preserve retail defaults and avoid a broader UI-policy refactor.

Files changed (10) +331 / -15

Enhancement (5) +54 / -0
MessageStream.hDeclare the health-bar mode cycle meta-message +1/-0

Declare the health-bar mode cycle meta-message

• Adds a shared message type for cycling the active health-bar display mode.

Core/GameEngine/Include/Common/MessageStream.h

MessageStream.cppExpose the cycle message name +1/-0

Expose the cycle message name

• Adds the health-bar cycle message to command-type string conversion for diagnostics and message tooling.

Core/GameEngine/Source/Common/MessageStream.cpp

CommandXlat.cppHandle session-only health-bar mode cycling +40/-0

Handle session-only health-bar mode cycling

• Handles the Zero Hour cycle command by rotating through all display modes and showing localized feedback. The selected mode updates client state without modifying Options.ini.

Core/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp

GlobalData.hStore active client overlay preferences +9/-0

Store active client overlay preferences

• Adds global client state for health-bar mode, numerical health, and smart pips.

GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h

Drawable.hDeclare numerical health rendering +3/-0

Declare numerical health rendering

• Adds the drawable helper used to render hit-point text beside a visible health bar.

GeneralsMD/Code/GameEngine/Include/GameClient/Drawable.h

Other (5) +277 / -15
OptionPreferences.hDefine health display preferences and accessors +15/-0

Define health display preferences and accessors

• Introduces the three-state HealthBarDisplayMode enum and preference accessors for numerical health and smart pips.

Core/GameEngine/Include/Common/OptionPreferences.h

OptionPreferences.cppParse the new Options.ini display settings +53/-0

Parse the new Options.ini display settings

• Parses named or numeric health-bar modes with Classic fallback. Reads NumericalHealth and SmartPips as opt-in Yes values.

Core/GameEngine/Source/Common/OptionPreferences.cpp

MetaEvent.cppBind the health-bar cycle meta-event +18/-0

Bind the health-bar cycle meta-event

• Registers the cycle event name and assigns Ctrl+Shift+tick in Zero Hour gameplay and observer contexts.

Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

GlobalData.cppInitialize display defaults from Options.ini +7/-0

Initialize display defaults from Options.ini

• Defaults all new behavior to retail presentation and refreshes the active values whenever game data preferences are parsed.

GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp

Drawable.cppRender configurable bars, health text, and smart pips +184/-15

Render configurable bars, health text, and smart pips

• Applies Classic, Damaged, and Always visibility rules, renders optional hit-point text, and keeps meaningful owner-only ammo and passenger pips visible. Guards null contained entries, mismatched container counts, absent pip images, and missing health-bar regions.

GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Smart ammo pips dereference null ✓ Resolved 🐞 Bug ☼ Reliability
Description
With SmartPips enabled, drawAmmo lets every owned ammo-bearing object pass without ensuring
healthBarRegion exists, then dereferences healthBarRegion->lo.x. computeHealthRegion can
legitimately fail, so rendering such an object crashes instead of skipping its pips.
Code

GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[R2866-2869]

+	else
+	{
+		if (obj->getControllingPlayer() != rts::getObservedOrLocalPlayer())
+			return;
Evidence
drawIconUI leaves the pointer null when computeHealthRegion fails, and that computation returns
false when projection or health-box lookup fails. The new SmartPips branch permits owned objects
through, while the unchanged placement code dereferences the pointer; the sibling container path
demonstrates the required guard.

GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2650-2669]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2727-2734]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2866-2875]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2901-2913]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2931-2936]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
SmartPips bypasses the selection/hover gate in `drawAmmo`, but the function later dereferences a potentially null `healthBarRegion`.
## Issue Context
`drawIconUI` passes null when health-region computation fails; `drawContained` already handles the same condition explicitly.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[2846-2913]

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



Remediation recommended

2. Numerical health breaks teardown ✓ Resolved 🐞 Bug ☼ Reliability
Description
drawNumericalHealth registers a function-static DisplayString with TheDisplayStringManager but
provides no path to free or unregister it. After NumericalHealth has drawn once, manager destruction
encounters a non-empty string list and asserts in debug builds, while release builds leak the
display string.
Code

GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[R3881-3885]

+	static DisplayString *s_healthString = nullptr;
+
+	if( s_healthString == nullptr )
+	{
+		s_healthString = TheDisplayStringManager->newDisplayString();
Evidence
The new code allocates the static string from the manager and never releases it. The manager links
every allocation into m_stringList, its destructor asserts that list is empty, and established
Drawable-owned strings are explicitly freed during destruction.

GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[3881-3895]
Core/GameEngine/Source/GameClient/DisplayStringManager.cpp[43-60]
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayStringManager.cpp[101-133]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[521-531]
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp[225-232]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The function-static numerical-health display string remains linked to the display-string manager through shutdown.
## Issue Context
Display strings allocated by the manager must be released with `freeDisplayString`; the manager destructor requires an empty tracked-string list.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[3872-3917]
- GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp[225-232]

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


Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp
Comment thread GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp Outdated
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds configurable health-bar visibility, numerical health text, persistent own-unit pips, and an in-game health-bar mode cycle.

  • Parses the three opt-in display preferences from Options.ini.
  • Extends Zero Hour client rendering for health bars, numerical values, and smart pips.
  • Adds lifecycle cleanup for the shared numerical-health display string.
  • Guards ammo and container pip rendering against absent regions, images, and contained objects.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported ammo-pip crash is prevented by returning before the only health-bar-region dereference.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/OptionPreferences.cpp Parses the new health-bar mode, numerical-health, and smart-pip preferences with retail-compatible defaults.
Core/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp Handles the Zero Hour client-only event that cycles the active health-bar display mode.
Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp Registers and binds the health-bar mode cycling command for gameplay and observer contexts.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp Implements the display modes, numerical health rendering, smart pips, and the null-region guard that resolves the previously reported crash.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp Releases the shared numerical-health display string before destroying its manager.

Reviews (3): Last reviewed commit: "fix(client): Guard the ammo pip health r..." | Re-trigger Greptile

@Skyaero42

Copy link
Copy Markdown

Ctrl+Shift+``` is tied to Discord and not a very handy shortcut.
The shortcut can also not be changed.

Also would have been nice to add an actual screen shot.
image

The code itself has too many comments and has lots of AI-related issues. I'm questioning how much of a human review/supervision has taken place here.

Last, I'm not sure of TSH wants such a feature and if we want to look at these type of features in the current phase of development.

@Mauller

Mauller commented Aug 27, 2026

Copy link
Copy Markdown

I also have a unit info scaling PR in the works that i need to get back to.

This change touches a lot of code in that area as well and is far too noisy for what it is doing. likely due to the development being AI driven.

@triatomic

Copy link
Copy Markdown
Author

I also have a unit info scaling PR in the works that i need to get back to.

This change touches a lot of code in that area as well and is far too noisy for what it is doing. likely due to the development being AI driven.

I probably should have split these three into separate PRs

HealthBarDisplayMode could only be changed by editing Options.ini and
restarting. This cycles it in game -- Classic, Damaged, Always -- with
an on screen line naming the mode it landed on.

Bound through generateMetaMap rather than requiring a CommandMap.ini
entry, the same way pause, fast forward and the fps controls default
themselves. The binding only installs if the message is still unbound,
so a mod that maps CYCLE_HEALTH_BAR_MODE itself still wins. This matters
here because Contra ships its command map inside a .big, where a loose
override file would replace every binding rather than merge.

Ctrl+` is unbound in retail and in both the debug and demo command
maps. The tick key is layout dependent -- it prints something other than
` outside a US keyboard -- but the binding is by scancode, so the same
physical key works everywhere.

Purely a client side display setting. Nothing reaches the simulation, so
it is safe in multiplayer and replays. The value is not written back to
Options.ini, so it lasts for the session and the configured mode returns
on the next load.
Shows current and max hit points to the right of the bar, so exact values
are readable without inferring them from bar length.

Hooked at the end of drawHealthBar rather than as a parallel draw path,
which means every rule that already decides whether a bar appears applies
unchanged -- selection, mouseover, HealthBarDisplayMode and the dead and
scenery exclusions. The number therefore shows exactly where a bar shows,
in all three modes, with no duplicated visibility logic to drift out of
step.

It reuses the bar's own colour, so it carries the same red to green
reading at a glance, including the blue to cyan variant for objects under
construction or disabled, and the damaged tinting.

Drawn small and unbold: in Always mode this lands over every unit on
screen at once, so it has to annotate the bar rather than compete with
it. A drop shadow rather than a backdrop plate, since this sits over the
battlefield and not over a cameo. Values are rounded rather than
truncated, so a unit with a sliver of health left does not read as 0.

Options.ini: NumericalHealth = Yes
Both pip types normally appear only while a unit is selected or moused
over, and only with the ShowObjectHealth debug flag on. SmartPips shows
them whenever there is something to report, so remaining ammo and loaded
transports read at a glance.

Own units only -- not allies, not enemies -- since standing pips over
units the player does not control would give away information the game
otherwise withholds.

Nothing is drawn when there is nothing to report. Passenger pips already
bailed on an empty transport; ammo pips needed a new early out, placed
before the style switch so every style behaves the same, because the
default style deliberately draws empty boxes for spent shots.

Drawing these every frame for every owned unit rather than only for a
selected one exposed assumptions the container path was quietly making,
so three latent faults are guarded here as well. The contained items list
can hold a null and entries can be mid removal while a transport unloads.
numFull comes from getContainerPipsToShow, which counts extra slots in
use and may be redirected to another module entirely -- OverlordContain
forwards it to its sub container -- while the list is always the outer
container's own, so the two need not agree. And the pip images were
dereferenced unguarded, where drawAmmo has always checked its own.

Options.ini: SmartPips = Yes
…lth string

Addresses both review findings:

- drawAmmo dereferenced healthBarRegion unchecked. The selection test made
  a valid region implicit, but SmartPips draws without that test, so an
  owned unit whose health region cannot be computed crashed instead of
  skipping its pips. Checked outright now, matching drawContained.

- The shared numerical health DisplayString stayed registered with
  TheDisplayStringManager through shutdown, asserting in debug builds and
  leaking in release. It is now file scoped with a teardown hook that
  GameClient's destructor runs right before deleting the manager - the
  existing killStaticImages runs after the manager is gone, so it could
  not take this job.
@triatomic
triatomic force-pushed the qol/health-bar-display branch from 77fdead to b05927f Compare August 27, 2026 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants