tweak: decouple the general promotion star blink from the render frame rate#2835
tweak: decouple the general promotion star blink from the render frame rate#2835bobtista wants to merge 4 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | Replaces frame-counter-based star blink with timeGetTime() wall-clock blink; the !isGamePaused() guard stops blinking during pause, contradicting the PR's stated goal of keeping the blink alive while paused. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | Identical change to the Zero Hour counterpart; carries the same paused-state behavior discrepancy. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[getStarImage called] --> B{m_genStarFlash?}
B -- No --> C[SetEnabledImage: generalButtonEnable\nreturn nullptr]
B -- Yes --> D{isGamePaused?}
D -- Yes --> E[SetEnabledImage: generalButtonEnable\nreturn nullptr\n⚠️ blink suppressed]
D -- No --> F{"timeGetTime() % 1000\n>= 500?"}
F -- Yes --> G[SetEnabledImage: generalButtonHighlight\nreturn nullptr]
F -- No --> H[SetEnabledImage: generalButtonEnable\nreturn nullptr]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[getStarImage called] --> B{m_genStarFlash?}
B -- No --> C[SetEnabledImage: generalButtonEnable\nreturn nullptr]
B -- Yes --> D{isGamePaused?}
D -- Yes --> E[SetEnabledImage: generalButtonEnable\nreturn nullptr\n⚠️ blink suppressed]
D -- No --> F{"timeGetTime() % 1000\n>= 500?"}
F -- Yes --> G[SetEnabledImage: generalButtonHighlight\nreturn nullptr]
F -- No --> H[SetEnabledImage: generalButtonEnable\nreturn nullptr]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
Generals/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp:1688
**Paused-state behavior contradicts the PR's stated goal**
The PR description says the fix "keeps blinking while paused," but `!TheGameLogic->isGamePaused()` short-circuits the whole expression to `false` when the game is paused, meaning the star always shows the non-highlighted (normal) image during pause — blinking is fully suppressed. The code comment even acknowledges this by saying "freezes while the game is paused." This doesn't fix the original problem of the stars stopping their blink during pause; it just replaces a stuck-on-highlight state with a stuck-on-normal state. If the intent truly is to keep the star blinking while paused, the `!TheGameLogic->isGamePaused() &&` guard should be removed so `timeGetTime()` drives the blink unconditionally. The same issue is present in `GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp` line 1688.
Reviews (2): Last reviewed commit: "tweak: freeze general promotion star bli..." | Re-trigger Greptile
| if(TheGameLogic->getFrame()% LOGICFRAMES_PER_SECOND > LOGICFRAMES_PER_SECOND/2) | ||
| // TheSuperHackers @tweak bobtista 27/06/2026 Blink the general-promotion star on a | ||
| // wall-clock cycle so the blink rate is independent of the render frame rate and the | ||
| // logic time scale, and keeps blinking while the game is paused. |
There was a problem hiding this comment.
Looking at #2484, it seems the project convention is to freeze visual elements on pause. Should these stars match that behavior?
There was a problem hiding this comment.
Yeah I was thinking about that. I'm fine either way. I think the only thing that needs to be moving while paused is the world clock really
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // wall-clock cycle so the blink rate is independent of the render frame rate and the | ||
| // logic time scale, and freezes while the game is paused. | ||
| const UnsignedInt blinkPeriodMs = 1000; | ||
| if( !TheGameLogic->isGamePaused() && timeGetTime() % blinkPeriodMs >= blinkPeriodMs / 2 ) |
There was a problem hiding this comment.
The PR description says the fix "keeps blinking while paused," but !TheGameLogic->isGamePaused() short-circuits the whole expression to false when the game is paused, meaning the star always shows the non-highlighted (normal) image during pause — blinking is fully suppressed. The code comment even acknowledges this by saying "freezes while the game is paused." This doesn't fix the original problem of the stars stopping their blink during pause; it just replaces a stuck-on-highlight state with a stuck-on-normal state. If the intent truly is to keep the star blinking while paused, the !TheGameLogic->isGamePaused() && guard should be removed so timeGetTime() drives the blink unconditionally. The same issue is present in GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp line 1688.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
Line: 1688
Comment:
**Paused-state behavior contradicts the PR's stated goal**
The PR description says the fix "keeps blinking while paused," but `!TheGameLogic->isGamePaused()` short-circuits the whole expression to `false` when the game is paused, meaning the star always shows the non-highlighted (normal) image during pause — blinking is fully suppressed. The code comment even acknowledges this by saying "freezes while the game is paused." This doesn't fix the original problem of the stars stopping their blink during pause; it just replaces a stuck-on-highlight state with a stuck-on-normal state. If the intent truly is to keep the star blinking while paused, the `!TheGameLogic->isGamePaused() &&` guard should be removed so `timeGetTime()` drives the blink unconditionally. The same issue is present in `GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp` line 1688.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Addresses #2834, same idea as #2833 / #2056.
Problem
The general's promote-button stars blink faster as you raise render FPS, because the blink is driven off the logic frame (
TheGameLogic->getFrame()) and re-checked every render frame. The blink rate should be constant regardless of render FPS / logic time scale.Approach
ControlBar::getStarImage()now toggles the star highlight on a wall-clock cycle (timeGetTime, 1s period) instead ofTheGameLogic->getFrame(), so the blink rate is constant regardless of render FPS / logic time scale. The toggle is gated onTheGameLogic->isGamePaused()so the star freezes (no blinking) while the game is paused, preserving the original pause behavior.TODO:
[x] Testing
[x] Replicate to Generals