Skip to content

bugfix(controlbar): Fix possible divisions by zero in Control Bar code#2867

Open
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_zero_division_sciences_controlbar
Open

bugfix(controlbar): Fix possible divisions by zero in Control Bar code#2867
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_zero_division_sciences_controlbar

Conversation

@Caball009

@Caball009 Caball009 commented Jul 12, 2026

Copy link
Copy Markdown

Using the modified rank file in #797, there are two more places the game would crash because of a division by zero; in ControlBar::populatePurchaseScience and ControlBar::updateContextPurchaseScience.

Issue reproduction steps:

  1. Add the modified rank ini data to a custom map.
  2. Start a match and click on the special powers menu.
  3. Game crashes due to a division by zero.

Callstacks:

generalszh.exe!ControlBar::populatePurchaseScience(Player * player) Line 430
generalszh.exe!ControlBar::showPurchaseScience() Line 2977
generalszh.exe!ControlBar::togglePurchaseScience() Line 3016
generalszh.exe!ControlBarSystem(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2) Line 418
generalszh.exe!GameWindowManager::winSendSystemMsg(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2) Line 705
generalszh.exe!GadgetPushButtonInput(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2) Line 218
generalszh.exe!GameWindowManager::winSendInputMsg(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2) Line 724
generalszh.exe!GameWindowManager::winProcessMouseEvent(GameWindowMessage msg, ICoord2D * mousePos, void * data) Line 919
generalszh.exe!WindowTranslator::translateGameMessage(const GameMessage * msg) Line 242
generalszh.exe!MessageStream::propagateMessages() Line 1090
generalszh.exe!GameEngine::update() Line 910
generalszh.exe!Win32GameEngine::update() Line 91
generalszh.exe!GameEngine::execute() Line 983
generalszh.exe!GameMain() Line 55
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 929
generalszh.exe!ControlBar::updateContextPurchaseScience() Line 495
generalszh.exe!ControlBar::update() Line 1530
generalszh.exe!InGameUI::update() Line 2093
generalszh.exe!W3DInGameUI::update() Line 370
generalszh.exe!SubsystemInterface::UPDATE() Line 131
generalszh.exe!GameClient::update() Line 781
generalszh.exe!W3DGameClient::update() Line 94
generalszh.exe!SubsystemInterface::UPDATE() Line 131
generalszh.exe!GameEngine::update() Line 910
generalszh.exe!Win32GameEngine::update() Line 91
generalszh.exe!GameEngine::execute() Line 983
generalszh.exe!GameMain() Line 55
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 929

@Caball009 Caball009 added Bug Something is not working right, typically is user facing GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Crash This is a crash, very bad labels Jul 12, 2026
@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates rank threshold handling to avoid zero-width progress intervals. The main changes are:

  • Normalize zero or equal rank thresholds in both Player implementations.
  • Update purchase-science progress calculations.
  • Route general experience progress through the normalized player thresholds.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp Makes the purchase-science progress locals immutable while retaining the existing calculation.
Core/GameEngineDevice/Source/W3DDevice/GameClient/GUI/GUICallbacks/W3DControlBar.cpp Uses the player rank interval directly for general experience progress rendering.
Generals/Code/GameEngine/Source/Common/RTS/Player.cpp Normalizes invalid rank thresholds during rank reset and rank changes.
GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp Mirrors the rank threshold normalization for the Zero Hour player implementation.

Reviews (6): Last reviewed commit: "bugfix(controlbar): Fix possible divisio..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp Outdated
@Caball009 Caball009 force-pushed the fix_zero_division_sciences_controlbar branch from 47b4cdc to 2ac70a7 Compare July 12, 2026 12:07
@Caball009 Caball009 changed the title bugfix(controlbar): Fix possible divisions by zero in ControlBar code bugfix(controlbar): Fix possible divisions by zero in Control Bar code Jul 12, 2026
@Skyaero42

Copy link
Copy Markdown

If skillPointsRequired is 0, than this implies that a rank up didn't go through - after all the player has enough points to rank up to the next level. So I wonder if the fix is at the wrong place

// TheSuperHackers @bugfix Caball009 12/07/2026 Prevent possible division by zero.
if (skillPointsRequired > 0)
{
progress = ((player->getSkillPoints() - player->getSkillPointsLevelDown()) * 100) / skillPointsRequired;

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 code now exists at 3 locations. Maybe consolidate it to one?

// TheSuperHackers @bugfix Caball009 12/07/2026 Prevent possible division by zero.
if (skillPointsRequired > 0)
{
progress = ((player->getSkillPoints() - player->getSkillPointsLevelDown()) * 100) / skillPointsRequired;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I wonder if this should be even a bit smarter and set progress to zero if the max skill points have been reached. Currently It looks like getSkillPointsLevelUp will become INT_MAX after the last level so it is not totally clean.

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.

Not sure if the new implementation is any better.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Check the bot complaints first.

@Caball009 Caball009 force-pushed the fix_zero_division_sciences_controlbar branch from 2ac70a7 to ed174e2 Compare July 15, 2026 12:09
@Caball009 Caball009 force-pushed the fix_zero_division_sciences_controlbar branch from ed174e2 to ede4635 Compare July 15, 2026 12:46
@Caball009 Caball009 closed this Jul 15, 2026
@Caball009 Caball009 force-pushed the fix_zero_division_sciences_controlbar branch from ede4635 to 480a9a7 Compare July 15, 2026 21:51
@Caball009 Caball009 reopened this Jul 15, 2026
@Caball009

Copy link
Copy Markdown
Author

@greptileai re-review this pull request.

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

Labels

Bug Something is not working right, typically is user facing Crash This is a crash, very bad Gen Relates to Generals GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants