Skip to content

feat(gui): Add configurable menu transition speed#2840

Open
bobtista wants to merge 7 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/feat/configurable-menu-transition-speed
Open

feat(gui): Add configurable menu transition speed#2840
bobtista wants to merge 7 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/feat/configurable-menu-transition-speed

Conversation

@bobtista

@bobtista bobtista commented Jun 28, 2026

Copy link
Copy Markdown

Closes #2839

Adds a per-user MenuTransitionSpeed option (in Options.ini) that scales the speed of menu/window transition animations, as requested following #2056. Defaults to 1.0 (unchanged behavior); values range from 1.0 (1× slower) to 1000.0 (1000× faster).

The multiplier is applied to the existing frame-rate-decoupled time step from #2056, so transitions still step through every state and never skip.

Todo

  • testing
  • replicate to Generals

@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a per-user GameWindowTransitionSpeedMultiplier option read from Options.ini and applies it as a scalar to the existing FPS-decoupled transition time step in TransitionGroup::update(). The field is initialised, declared, and wired up symmetrically for both Generals and Zero Hour.

  • OptionPreferences::getGameWindowTransitionSpeedMultiplier() reads and clamps the INI value, but the clamp bounds are [1.0, 1000.0] rather than the [0.1, 10.0] range advertised in the PR description — see inline comment.
  • GlobalData and its constructors are updated identically for both game targets, and the GameWindowTransitions.cpp change is a clean one-line multiplication.

Confidence Score: 4/5

The feature wiring is correct end-to-end, but the clamp bounds in OptionPreferences.cpp are wrong, preventing users from setting any speed slower than the default.

The only change that reads user input and enforces the documented range is getGameWindowTransitionSpeedMultiplier(), and it clamps to [1.0, 1000.0] instead of the [0.1, 10.0] range the PR describes. Every user who enters a value below 1.0 will silently get 1.0, making the slow-down transitions half of the feature non-functional.

Core/GameEngine/Source/Common/OptionPreferences.cpp — the clamp bounds need to match the documented range.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/OptionPreferences.cpp Adds getGameWindowTransitionSpeedMultiplier() but clamps the range to [1.0, 1000.0] instead of the documented [0.1, 10.0], making slow-transition values impossible and the upper bound excessively large.
Core/GameEngine/Include/Common/OptionPreferences.h Adds const declaration for getGameWindowTransitionSpeedMultiplier(); straightforward header change.
Core/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp Multiplies the existing FPS-decoupled timeScale by the GlobalData multiplier; logic is correct assuming the multiplier value is valid.
Generals/Code/GameEngine/Include/Common/GlobalData.h Adds m_gameWindowTransitionSpeedMultiplier field with a proper comment; mirrors GeneralsMD correctly.
Generals/Code/GameEngine/Source/Common/GlobalData.cpp Initialises and populates the new multiplier field from OptionPreferences; mirrors GeneralsMD correctly.
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Adds m_gameWindowTransitionSpeedMultiplier field; identical change to Generals counterpart.
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp Initialises and populates the new multiplier field from OptionPreferences; identical change to Generals counterpart.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant INI as Options.ini
    participant OP as OptionPreferences
    participant GD as GlobalData
    participant TG as TransitionGroup::update()
    participant FP as FramePacer

    INI->>OP: GameWindowTransitionSpeedMultiplier value
    OP->>OP: clamp(min, value, max) — bounds bug here
    OP->>GD: parseGameDataDefinition() sets m_gameWindowTransitionSpeedMultiplier
    FP->>TG: getBaseOverUpdateFpsRatio()
    GD->>TG: m_gameWindowTransitionSpeedMultiplier
    TG->>TG: "timeScale = ratio * multiplier"
    TG->>TG: "m_currentFrame += directionMultiplier * timeScale"
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"}}}%%
sequenceDiagram
    participant INI as Options.ini
    participant OP as OptionPreferences
    participant GD as GlobalData
    participant TG as TransitionGroup::update()
    participant FP as FramePacer

    INI->>OP: GameWindowTransitionSpeedMultiplier value
    OP->>OP: clamp(min, value, max) — bounds bug here
    OP->>GD: parseGameDataDefinition() sets m_gameWindowTransitionSpeedMultiplier
    FP->>TG: getBaseOverUpdateFpsRatio()
    GD->>TG: m_gameWindowTransitionSpeedMultiplier
    TG->>TG: "timeScale = ratio * multiplier"
    TG->>TG: "m_currentFrame += directionMultiplier * timeScale"
Loading
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
Core/GameEngine/Source/Common/OptionPreferences.cpp:895
The `clamp` call uses wrong bounds: `clamp(min, value, max)` in this codebase means speed is clamped to `[1.0, 1000.0]`, not `[0.1, 10.0]` as stated in the PR description. Any user who sets `MenuTransitionSpeed=0.5` expecting slower transitions will silently have their value overridden to `1.0`, and the upper cap of 1000× allows a wildly excessive speed that could make transitions instantaneous in a single frame loop iteration.

```suggestion
	return clamp(0.1f, speed, 10.0f);
```

Reviews (4): Last reviewed commit: "refactor(gui): Replicate GameWindowTrans..." | Re-trigger Greptile

@bobtista bobtista self-assigned this Jun 28, 2026

@Caball009 Caball009 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you lower this Sleep? As far as I'm concerned it can be set to 1 msec.

Comment thread Core/GameEngine/Source/Common/OptionPreferences.cpp Outdated
// transitions cannot skip a state when the render frame rate dips below the base rate.
const Real timeScale = TheFramePacer->getBaseOverUpdateFpsRatio();
// TheSuperHackers @feature bobtista 28/06/2026 Scale by the user menu transition speed preference.
const Real timeScale = TheFramePacer->getBaseOverUpdateFpsRatio() * TheGlobalData->m_menuTransitionSpeed;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe also apply this to the movement speed of Control Bar, Diplomacy Screen? Or does that need to be a separate setting?

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.

Those don't go through TransitionGroup they use the sliding/animate-window path. Could make another PR for those - would you want it to be a separate setting or controlled by this one?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe make it a separate setting GameWindowMovementSpeedMultiplier

Comment thread Core/GameEngine/Include/Common/OptionPreferences.h Outdated
@bobtista

Copy link
Copy Markdown
Author

Can you lower this Sleep? As far as I'm concerned it can be set to 1ms.

It's iteration-bound (frame-pacer frozen in that loop), so lowering Sleep speeds the fade up rather than smoothing it, is that what you want? It would be like 30x faster right? Maybe the proper change is to route the loop through the frame pacer, which would presumably be in its own PR.

@Caball009

Caball009 commented Jun 28, 2026

Copy link
Copy Markdown

It's iteration-bound (frame-pacer frozen in that loop), so lowering Sleep speeds the fade up rather than smoothing it, is that what you want? It would be like 30x faster right?

Right, perhaps 1000.0f / (30 * TheWritableGlobalData->m_menuTransitionSpeed) as duration is better. Having a hardcoded 33 msec delay makes no sense anymore now that the transition speed is variable.

Maybe the proper change is to route the loop through the frame pacer, which would presumably be in its own PR.

I don't think that's necessary.

Comment thread Core/GameEngine/Source/Common/OptionPreferences.cpp Outdated

Real OptionPreferences::getGameWindowTransitionSpeed() const
{
OptionPreferences::const_iterator it = find("GameWindowTransitionSpeed");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe call it GameWindowTransitionSpeedMultiplier or GameWindowTransitionSpeedScale because it is not clear what "Speed" refers to, but "Multiplier", "Scale" tells that it scales the speed.

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.

done

@bobtista

Copy link
Copy Markdown
Author

It's iteration-bound (frame-pacer frozen in that loop), so lowering Sleep speeds the fade up rather than smoothing it, is that what you want? It would be like 30x faster right?

Right, perhaps 1000.0f / (30 * TheWritableGlobalData->m_menuTransitionSpeed) as duration is better. Having a hardcoded 33 msec delay makes no sense anymore now that the transition speed is variable.

Maybe the proper change is to route the loop through the frame pacer, which would presumably be in its own PR.

I don't think that's necessary.

Let's handle this in another PR. If we use that formula it will double count the denominator, duration becomes 1/speed². That 33 msec delay just makes the whole fade loop have 30fps pacing. We could do something like

TheTransitionHandler->setGroup("FadeWholeScreen");
while (!TheTransitionHandler->isFinished())
{
    TheWindowManager->update();
    if (!TheTransitionHandler->isFinished())
    {
        TheDisplay->draw();
        setFPMode();
        TheFramePacer->update();   // caps the framerate and measures real elapsed time
    }
}

Comment thread Core/GameEngine/Source/Common/OptionPreferences.cpp
@Caball009

Copy link
Copy Markdown

Let's handle this in another PR.

Would you mind doing a follow-up PR for this, then?

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.

Make menu transition animation speed configurable

4 participants