tweak(gui): Decouple GUI transition and world animation time step from render update

[bobtista]

Summary

• GUI window transitions now advance at a consistent rate regardless of frame rate
• 2D world animation icons (healing, promotion, crate collection effects) rise at consistent speed regardless of frame rate

Changes

• TransitionGroup::m_currentFrame changed from Int to Real to support fractional frame accumulation
• TransitionGroup::update() scales frame advancement by TheFramePacer->getActualLogicTimeScaleOverFpsRatio()
• InGameUI world animation Z-rise calculation now scales by the same time factor

Test plan

• Verify GUI transitions (menu fades, button flashes) play at the same speed at 30fps and higher frame rates
• Verify 2D world icons (healing icons, veteran stars, crate pickups) rise at consistent speed regardless of frame rate

[greptile-apps]

Greptile Summary

This PR decouples GUI window transition timing and 2D world animation Z-rise from the render frame rate by introducing a Real accumulator for TransitionGroup::m_currentFrame and scaling per-render-frame increments by FramePacer ratios.

• TransitionGroup::update() now accumulates fractional frames using getBaseOverUpdateFpsRatio() (30/renderFps) and steps through any skipped integer states via a loop, ensuring transitions advance at a constant 30fps-equivalent wall-clock rate.
• InGameUI::updateAndDrawWorldAnimations() scales Z-rise per render frame by getActualLogicTimeScaleOverFpsRatio(), which returns 0 during single-player game pause (preserving the original no-rise-while-paused behavior) and reduces to logicFps/renderFps when the logic time-scale feature is active.

Confidence Score: 5/5

Safe to merge — the fractional accumulator and integer-frame stepping logic are correct for both forward and reverse transitions, and the Z-rise pause behaviour is preserved through the FramePacer's halt-aware ratio.

The discrete-state-machine design of transitions is handled correctly: fractional accumulation preserves progress across render frames, the loop prevents skipping states at low FPS, and the early-return at high FPS correctly holds state. For world animations, getActualLogicTimeScaleOverFpsRatio() returns 0 when the game is halted (single-player pause), matching the original isGamePaused() guard, and the Anim2D completion check is also gated on TheGameLogic->getFrame() so it cannot fire during pause. The only finding is a comment clarification.

No files require special attention; all four changed files implement their respective time-scale strategies correctly.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/GameWindowTransitions.h	Changed m_currentFrame from Int to Real and updated the comment to document 30fps-equivalent frame units; clean and correct.
Core/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp	Replaces single-frame-per-render-update advance with a fractional accumulator scaled by getBaseOverUpdateFpsRatio(); adds a loop to step every integer state so discrete states cannot be skipped at low FPS. Logic is correct for both forward and reverse directions.
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp	Removes the isGamePaused() outer guard and scales Z-rise by getActualLogicTimeScaleOverFpsRatio(); the scale returns 0 when game-halted so pause behavior is preserved. The two different FramePacer functions used across this file vs transitions are intentional.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Identical change to the Zero Hour variant of InGameUI; same analysis applies as Generals.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Render Frame] --> B[TransitionGroup::update]
    A --> C[InGameUI::preDraw]
    B --> D["timeScale = getBaseOverUpdateFpsRatio()\n= BaseFps / renderFps"]
    D --> E["prevFrame = int(m_currentFrame)\nm_currentFrame += dir * timeScale\nnewFrame = int(m_currentFrame)"]
    E --> F{newFrame == prevFrame?}
    F -- Yes / high FPS --> G[return — hold current state]
    F -- No --> H["Loop: tWin->update(frame)\nfor each integer frame crossed"]
    H --> I{isFinished?}
    I -- Yes --> J[break early]
    I -- No --> H
    C --> K[updateAndDrawWorldAnimations]
    K --> L["zRiseTimeScale =\ngetActualLogicTimeScaleOverFpsRatio()\n= min(1, logicFps/renderFps)\n= 0 if game halted"]
    L --> M{Game paused / halted?}
    M -- Yes, zRiseTimeScale=0 --> N[Z-rise = 0, expiry check still runs]
    M -- No --> O["worldPos.z +=\nzRisePerSecond / LOGICFRAMES_PER_SECOND\n* zRiseTimeScale"]
    N --> P[Draw animation]
    O --> P

Loading

%%{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[Render Frame] --> B[TransitionGroup::update]
    A --> C[InGameUI::preDraw]
    B --> D["timeScale = getBaseOverUpdateFpsRatio()\n= BaseFps / renderFps"]
    D --> E["prevFrame = int(m_currentFrame)\nm_currentFrame += dir * timeScale\nnewFrame = int(m_currentFrame)"]
    E --> F{newFrame == prevFrame?}
    F -- Yes / high FPS --> G[return — hold current state]
    F -- No --> H["Loop: tWin->update(frame)\nfor each integer frame crossed"]
    H --> I{isFinished?}
    I -- Yes --> J[break early]
    I -- No --> H
    C --> K[updateAndDrawWorldAnimations]
    K --> L["zRiseTimeScale =\ngetActualLogicTimeScaleOverFpsRatio()\n= min(1, logicFps/renderFps)\n= 0 if game halted"]
    L --> M{Game paused / halted?}
    M -- Yes, zRiseTimeScale=0 --> N[Z-rise = 0, expiry check still runs]
    M -- No --> O["worldPos.z +=\nzRisePerSecond / LOGICFRAMES_PER_SECOND\n* zRiseTimeScale"]
    N --> P[Draw animation]
    O --> P

Loading

_{Reviews (7): Last reviewed commit: "fix(gui): Replicate to Generals" | Re-trigger Greptile}

[greptile-apps]

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp
[P2] This file still uses NULL (TheTransitionHandler = NULL;). New code in this PR also adds FramePacer usage; consider switching to nullptr for null pointer literals to match the repo’s C++ style.

GameWindowTransitionsHandler *TheTransitionHandler = nullptr;

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp
Line: 62:62

Comment:
[P2] This file still uses `NULL` (`TheTransitionHandler = NULL;`). New code in this PR also adds `FramePacer` usage; consider switching to `nullptr` for null pointer literals to match the repo’s C++ style.

```suggestion
GameWindowTransitionsHandler *TheTransitionHandler = nullptr;
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[Caball009]

I frequently find myself increasing the render frame rate at the menu because I find the window transitions too slow at 30 fps, and would probably like it better if they played close to ~1.75x the original speed. Is there anything this PR can do in that regard?

[ElTioRata]

I agree with Caball. It would be nice to have a setting to adjust GUI speed (x1.0... x1.25... x1.5x... x1.75... x2.00... and so on).

[xezon]

Feature for scaling animation speed is unrelated to decoupling step and better be follow up change.

[githubawn]

Looks good.
Maybe fully skip animations under -quickstart in the follow-up, cleaner than adding explicit speed controls.

[Caball009]

There are still a couple of to-dos in the PR description.

[Caball009]

Looks good to me.

I'm still in favor of making the float assignments explicit for m_currentFrame, but it makes no functional difference.

[xezon]

I tested this and something appears to be broken.

Sometimes the UI just gets stuck. I was testing with long SSD load times because Visual Studio was parsing which bogs down the SSD.

After clicking Options menu

After clicking Skirmish Menu

After clicking Credits Menu

[Caball009]

Sometimes the UI just gets stuck.

Is this something you can reproduce? I don't think I've seen anything like this during my testing.

[xezon]

I ran into this several times when testing. My SSD was slow, but still it was unusual behavior. I can retest if this is not reproducible for others.

[Caball009]

I can retest if this is not reproducible for others.

Please do.

[Caball009]

I ran into this several times when testing. My SSD was slow, but still it was unusual behavior. I can retest if this is not reproducible for others.

Can you take a look at this? I cannot reproduce it, and this PR probably shouldn't be blocked from merging if you can't reproduce it either

[githubawn]

I replicated Xezon's bug by running this on a networked drive.
This would also probably break on SD installs.

The logic seems to fail on long IO stalls.

[githubawn]

Added a single sleep to replicate this, the menu won't even launch on this. But it does on main.

GameEngine.cpp

void GameEngine::execute()
{
#if defined(RTS_DEBUG)
	DWORD startTime = timeGetTime() / 1000;
#endif

	// pretty basic for now
	while( !m_quitting )
	{
		::Sleep(100);

30 will randomly bug out the menu after a few switches.

Implementation needs changes.

[Caball009]

Added a single sleep to replicate this, the menu won't even launch on this. But it does on main.

I just tried with the main branch + sleep and the menu doesn't load, so it doesn't seem like this PR is at fault.
Edit: I can confirm that main + sleep still works, but PR + sleep does not.

I suspect that the window transition code is too brittle to handle the sleep as it probably skips over hard-coded transitions.

[xezon]

Yes the bug still happens. Got stuck here:

 	generalsv.exe!TransitionGroup::isFinished() Line 293	C++
>	generalsv.exe!GameLogic::startNewGame(bool saveGame) Line 1895	C++
 	generalsv.exe!GameLogic::update() Line 3108	C++
 	generalsv.exe!SubsystemInterface::UPDATE() Line 74	C++
 	generalsv.exe!GameEngine::update() Line 749	C++
 	generalsv.exe!Win32GameEngine::update() Line 94	C++
 	generalsv.exe!GameEngine::execute() Line 804	C++
 	generalsv.exe!GameMain() Line 59	C++
 	generalsv.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 874	C++
 	[Inline Frame] generalsv.exe!invoke_main() Line 102	C++
 	generalsv.exe!__scrt_common_main_seh() Line 288	C++
 	kernel32.dll!76385d49()	Unknown
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	ntdll.dll!7767d83b()	Unknown
 	ntdll.dll!7767d7c1()	Unknown

I suspect it has something to do with FlashTransition::update where if the frame does not perfectly hit the expected value, the transition never finishes. This certainly is possible when the frame rate is not stable.

[Caball009]

@bobtista Can you take a look at the stuck menu issue?

[bobtista]

I've addressed the remaining review comments and tested the menu-stall repro locally:

• Tested generalsv.exe with the reviewer repro hook, ::Sleep(100), in GameEngine::execute().
• Clicked through the shell map menu buttons in both Generals and Zero Hour
  Ready for re-review as needed.

[xezon]

Works in principle. I see no more hangs. 2 minor improvements noted.

What about the Window move transition? It is not decoupled. Is that a different piece of code? Can be tested with the diplomacy screen.

[xezon]

I expect this test can now be removed because it is implicit in getActualLogicTimeScaleOverFpsRatio ?

[bobtista]

Yes but this block also gates animation cleanup/removal, not only the scaled Z movement. I'm not sure if that would ever make a difference though

[xezon]

m_expireFrame and ANIM_2D_STATUS_COMPLETE are bound to logic frame number. As long as game is paused frame number will not advance. So this should be stable when paused. Please test.

[bobtista]

I just tested and was able to pause while a unit was being promoted, the animation stopped while paused, and continued when I resumed.

[bobtista]

Wait, the main menu is not rendering when I hit escape with fixed logic fps and uncapped render fps, the game just pauses, then it renders briefly when i resume.

[xezon]

So it does not work and needs more work?

[bobtista]

Never mind, that was on another branch build unrelated. I rebuilt this PR and retested, 30fps render and faster, main menu animates fine, and the veterancy chevron animates consistently and stops and resumes normally while paused.

[bobtista]

Next thing I noticed was the "you have been promoted" stars blink faster at higher render fps, which is for another PR

Hang is fixed

[bobtista]

Works in principle. I see no more hangs. 2 minor improvements noted.

What about the Window move transition? It is not decoupled. Is that a different piece of code? Can be tested with the diplomacy screen.

Addressed in #2833

[Evulant]

Good change, however I have a suggestion. I for my part like the menus moving a bit faster. Maybe a setting can be added to scale the GUI menu transition speeds to one's liking?