tweak(gui): Decouple GUI transition and world animation timing 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 transition animations and 2D world animation Z-rise from the raw render frame rate, ensuring consistent visual speeds at any FPS by scaling per-frame increments with TheFramePacer->getBaseOverUpdateFpsRatio() (i.e. BaseFps / actualUpdateFps).

Key changes:

• TransitionGroup::m_currentFrame promoted from Int to Real so fractional frame accumulation is preserved across render frames; the implicit Real → Int truncation at tWin->update(m_currentFrame) is intentional and correct — the discrete state machine advances to the next integer threshold at the right wall-clock rate regardless of FPS.
• TransitionGroup::update() multiplies the per-frame step by getBaseOverUpdateFpsRatio(): at 30 fps the ratio is 1.0 (unchanged behaviour), at 60 fps it is 0.5 (two render frames per logical step).
• InGameUI::updateAndDrawWorldAnimations() (both Generals/ and GeneralsMD/ variants) applies the same ratio to the Z-rise delta. Because BaseFps == LOGICFRAMES_PER_SECOND == 30, the expression reduces cleanly to zRisePerSecond / actualUpdateFps — the mathematically correct frame-rate–independent delta.
• getBaseOverUpdateFpsRatio() caps the minimum update FPS at 5 fps, preventing runaway scaling during frame spikes.

Minor note: The PR description references getActualLogicTimeScaleOverFpsRatio(), but the implementation correctly uses getBaseOverUpdateFpsRatio(). The former returns min(1.0f, logicTimeScaleFps / updateFps), which collapses to 1.0f in the normal (non-logic-scaled) case and would not achieve frame-rate independence; getBaseOverUpdateFpsRatio() is the right API for purely visual, render-side updates.

Confidence Score: 5/5

• Safe to merge — changes are purely visual, scoped to render-path updates, and mathematically correct at all frame rates.
• All four changed sites apply the same well-understood BaseFps / actualFps scaling pattern already used elsewhere in the codebase (e.g. camera rotation in InGameUI.cpp). The prior P0 concern about Real → Int truncation has been thoroughly addressed and confirmed correct. No logic, networking, or save-game state is touched. The implementation matches the project's existing conventions for frame-rate–independent render updates.
• No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/GameWindowTransitions.h	Single field type change: m_currentFrame from Int to Real with updated comment. Clean and correct.
Core/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp	Adds FramePacer.h include, initialises m_currentFrame as 0.0f, and scales the per-frame increment by getBaseOverUpdateFpsRatio(). The Real → Int implicit conversion at the tWin->update() call site is intentional (confirmed in prior review thread) – the state machine advances to the next integer threshold at the correct wall-clock rate.
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp	Z-rise delta now multiplied by getBaseOverUpdateFpsRatio(). Since BaseFps == LOGICFRAMES_PER_SECOND == 30, the expression simplifies to zRisePerSecond / updateFps — correct frame-rate–independent delta. FramePacer.h was already included. Braces added around the new block body, consistent with project style rules.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Identical fix to the Generals variant; mirrors the same Z-rise scaling with getBaseOverUpdateFpsRatio(). No issues found.

Sequence Diagram

sequenceDiagram
    participant RenderLoop
    participant FramePacer
    participant TransitionGroup
    participant TransitionWindow
    participant InGameUI

    RenderLoop->>FramePacer: update() [measures actual frame time]
    RenderLoop->>TransitionGroup: update()
    TransitionGroup->>FramePacer: getBaseOverUpdateFpsRatio()
    FramePacer-->>TransitionGroup: BaseFps / actualFps (e.g. 0.5 at 60fps)
    TransitionGroup->>TransitionGroup: m_currentFrame += directionMultiplier * ratio
    TransitionGroup->>TransitionWindow: update(Int(m_currentFrame))
    Note over TransitionWindow: Discrete state machine selects<br/>visual state at integer thresholds

    RenderLoop->>InGameUI: draw() → updateAndDrawWorldAnimations()
    InGameUI->>FramePacer: getBaseOverUpdateFpsRatio()
    FramePacer-->>InGameUI: BaseFps / actualFps
    InGameUI->>InGameUI: wad.z += zRisePerSecond / LOGICFRAMES_PER_SECOND * ratio
    Note over InGameUI: Equivalent to zRisePerSecond / actualFps<br/>(frame-rate independent)

Loading

_{Reviews (3): Last reviewed commit: "style: Use explicit float assignments fo..." | 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.