bugfix(drawable): Decouple fade timing from render rate by bobtista · Pull Request #3303 · TheSuperHackers/GeneralsGameCode

bobtista · 2026-09-15T16:22:42Z

Related to Enemy stealthed units/buildings reveal animation flicker or animate too quickly at higher render FPS #1719
Split from tweak(drawable): Interpolate physics transforms #2055

Drawable fade in/out and terrain decal opacity currently advance once per render frame, so they run faster at higher render rates.

This scales their progress by the logic/render ratio. At 120 render FPS and 30 logic FPS, a Rebel Ambush authored with FadeTime = 3000 currently finishes in 750 ms. With this change it takes 3 seconds as authored.

m_timeElapsedFade changes from UnsignedInt to Real to accumulate fractional steps during play. Saves keep the existing integer representation, so save compatibility is unchanged; loading may discard less than one 30 Hz frame of fade progress.

Rebel Ambush comparison:

pr2055_sidebyside.mp4

To reproduce: load a save, Ctrl+KP_Plus to raise the render cap to 120, Ctrl+Shift+KP_Minus until the logic time scale reads 30, then fire Rebel Ambush.

Todo:

Compare Rebel Ambush fade at high FPS
Before/after video
Replicate to Generals

qodo-free-for-open-source-projects · 2026-09-15T16:24:24Z

PR Summary by Qodo

Decouple drawable and decal fades from render rate

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Scale drawable fades by the logic-to-render update ratio.
• Apply identical timing correction to terrain decal opacity transitions.
• Preserve integer save serialization while accumulating fractional fade progress at runtime.

Diagram

graph TD
  A["Render Loop"] -->|"invokes"| B["Drawable Update"] -->|"queries"| C["Frame Pacer"] -->|"returns ratio"| D["Scaled Step"] -->|"advances"| E["Drawable Fade"] -->|"serializes integer"| G["Save Transfer"]
  D -->|"scales delta"| F["Decal Opacity"]

High-Level Assessment

The chosen approach is appropriate: it reuses the engine's established FramePacer ratio, preserves authored 30 FPS timing semantics, and retains the existing binary save representation. Using raw elapsed seconds would require broader unit conversions, while changing the serialized field to Real would introduce an unnecessary save-format migration.

Files changed (4) +26 / -8

Bug fix (4) +26 / -8

Drawable.hAllow fractional drawable fade progress in Generals +1/-1

Allow fractional drawable fade progress in Generals
• Changes elapsed fade progress from UnsignedInt to Real so render updates can accumulate fractional 30 FPS-equivalent steps.
Generals/Code/GameEngine/Include/GameClient/Drawable.h

Drawable.cppScale Generals fade progression by logic timing +12/-3

Scale Generals fade progression by logic timing
• Scales drawable and terrain decal fades using the logic-to-render FPS ratio. Converts fractional drawable progress through an UnsignedInt intermediary during save transfer to preserve the existing serialized layout.
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp

Drawable.hAllow fractional drawable fade progress in Zero Hour +1/-1

Allow fractional drawable fade progress in Zero Hour
• Changes elapsed fade progress from UnsignedInt to Real so Zero Hour can retain sub-frame timing increments.
GeneralsMD/Code/GameEngine/Include/GameClient/Drawable.h

Drawable.cppScale Zero Hour fade progression by logic timing +12/-3

Scale Zero Hour fade progression by logic timing
• Replicates logic-rate-scaled drawable and terrain decal fades in Zero Hour. Maintains save compatibility by serializing elapsed drawable fade progress as an unsigned integer.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp

greptile-apps · 2026-09-15T16:25:23Z

Greptile Summary

This PR makes drawable and terrain-decal fades advance according to the logic/render timing ratio rather than once per rendered frame while preserving the existing save representation.

Changes elapsed drawable fade state from an integer to a fractional value.
Scales drawable and terrain-decal fade increments through the frame pacer.
Serializes fractional runtime progress through the existing unsigned-integer save field.
Applies equivalent changes to both Generals and GeneralsMD.

Confidence Score: 5/5

The PR appears safe to merge, with both game variants updated consistently and no actionable correctness or compatibility issue identified.

The frame-pacer ratio is bounded, terminal fade behavior remains reachable, runtime initialization guarantees the new dependency, and save files retain their previous integer representation.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/GameClient/Drawable.h	Changes elapsed fade progress to a fractional frame-equivalent value.
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp	Scales visual fade progression by the frame-pacer ratio and preserves the existing serialized integer format.
GeneralsMD/Code/GameEngine/Include/GameClient/Drawable.h	Mirrors the fractional fade-state definition for Zero Hour.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp	Mirrors the timing and save-compatibility changes for Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  R[Render update] --> P[Read logic/render ratio]
  P --> F[Advance drawable fade]
  P --> D[Advance terrain-decal opacity]
  F --> O[Apply visual opacity]
  D --> O
  F --> S[Convert elapsed fade to integer for saves]
  S --> L[Restore integer progress as Real on load]

_{Reviews (1): Last reviewed commit: "bugfix(drawable): Replicate fade timing ..." | Re-trigger Greptile}

qodo-free-for-open-source-projects · 2026-09-15T16:26:40Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Objects remain translucent after fades 🐞 Bug ≡ Correctness

Description

Drawable::updateDrawable() applies opacity from the old m_timeElapsedFade, then adds a
fractional step and clears m_fadeMode without applying the terminal opacity. When the step crosses
rather than lands on m_timeToFade, fade-ins retain opacity below 1 and fade-outs retain opacity
above 0 in both game variants.

Code

Generals/Code/GameEngine/Source/GameClient/Drawable.cpp[1152]
+			m_timeElapsedFade += fadeTimeScale;

Evidence
The new FramePacer ratio can be fractional, while opacity is calculated before that value is added.
If the addition crosses the integer duration, the existing greater-than check immediately disables
further updates, and neither implementation subsequently assigns the skipped endpoint.
Core/GameEngine/Source/Common/FramePacer.cpp[205-210]
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp[1044-1060]
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp[1147-1155]
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[1149-1157]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Fractional fade steps can cross the duration after the current opacity has already been applied, causing the fade mode to stop without ever assigning its exact final opacity.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameClient/Drawable.cpp[1147-1155]
- GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[1149-1157]

## Recommended Fix
When the updated elapsed time exceeds the fade duration, explicitly set opacity to `1.0f` for a fade-in or `0.0f` for a fade-out before clearing the fade mode. Apply the same correction to both game variants.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Tickets: 🎫 Enemy stealthed units/buildings reveal animation flicker or animate too quickly at higher render FPS

Review mode: ⚖️ Balanced: This is a localized runtime timing and save-serialization change affecting gameplay behavior in two mirrored codepaths, warranting a careful single-pass review but not the density needed for extended review.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

qodo-free-for-open-source-projects · 2026-09-15T16:26:41Z

-			++m_timeElapsedFade;
+			// TheSuperHackers @bugfix bobtista 15/09/2026 Decouple Drawable fade timing from render updates.
+			const Real fadeTimeScale = TheFramePacer->getActualLogicTimeScaleOverFpsRatio();
+			m_timeElapsedFade += fadeTimeScale;


1. Objects remain translucent after fades 🐞 Bug ≡ Correctness

Drawable::updateDrawable() applies opacity from the old m_timeElapsedFade, then adds a fractional step and clears m_fadeMode without applying the terminal opacity. When the step crosses rather than lands on m_timeToFade, fade-ins retain opacity below 1 and fade-outs retain opacity above 0 in both game variants.

Agent Prompt

## Issue description Fractional fade steps can cross the duration after the current opacity has already been applied, causing the fade mode to stop without ever assigning its exact final opacity. ## Fix Focus Areas - Generals/Code/GameEngine/Source/GameClient/Drawable.cpp[1147-1155] - GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp[1149-1157] ## Recommended Fix When the updated elapsed time exceeds the fade duration, explicitly set opacity to `1.0f` for a fade-in or `0.0f` for a fade-out before clearing the fade mode. Apply the same correction to both game variants.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Skyaero42 · 2026-09-15T20:16:21Z


 	// time elapsed fade
-	xfer->xferUnsignedInt( &m_timeElapsedFade );
+	UnsignedInt timeElapsedFadeFrames = static_cast<UnsignedInt>(m_timeElapsedFade);


What is the risk - if any - if the cast rounds the value to an integer value?

Skyaero42 · 2026-09-15T20:24:21Z

 	// time elapsed fade
-	xfer->xferUnsignedInt( &m_timeElapsedFade );
+	UnsignedInt timeElapsedFadeFrames = static_cast<UnsignedInt>(m_timeElapsedFade);
+	xfer->xferUnsignedInt( &timeElapsedFadeFrames );


Wouldn't it be better to only keep the cast in retail compatible?

Skyaero42 · 2026-09-15T20:28:25Z

-			++m_timeElapsedFade;
+			// TheSuperHackers @bugfix bobtista 15/09/2026 Decouple Drawable fade timing from render updates.
+			const Real fadeTimeScale = TheFramePacer->getActualLogicTimeScaleOverFpsRatio();
+			m_timeElapsedFade += fadeTimeScale;


Can be reduced to a single line expression

Skyaero42 · 2026-09-15T20:28:34Z

-				m_decalOpacity += m_decalOpacityFadeRate;
+				// TheSuperHackers @bugfix bobtista 15/09/2026 Decouple decal opacity fade timing from render updates.
+				const Real decalFadeTimeScale = TheFramePacer->getActualLogicTimeScaleOverFpsRatio();
+				m_decalOpacity += m_decalOpacityFadeRate * decalFadeTimeScale;


Can be reduced to a single line expression

bobtista added 2 commits September 15, 2026 12:16

bugfix(drawable): Decouple fade timing from render rate

bb8610d

bugfix(drawable): Replicate fade timing fix to Generals

0ef50c6

qodo-free-for-open-source-projects Bot reviewed Sep 15, 2026

View reviewed changes

bobtista mentioned this pull request Sep 15, 2026

tweak(drawable): Interpolate physics transforms #2055

Open

2 tasks

Skyaero42 reviewed Sep 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

bugfix(drawable): Decouple fade timing from render rate - #3303

bugfix(drawable): Decouple fade timing from render rate#3303
bobtista wants to merge 2 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/bugfix/drawable-fade-timing

bobtista commented Sep 15, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 15, 2026

Uh oh!

greptile-apps Bot commented Sep 15, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 15, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Sep 15, 2026

Uh oh!

Skyaero42 Sep 15, 2026

Uh oh!

Skyaero42 Sep 15, 2026

Uh oh!

Skyaero42 Sep 15, 2026

Uh oh!

Skyaero42 Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bobtista commented Sep 15, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 15, 2026

PR Summary by Qodo

Uh oh!

greptile-apps Bot commented Sep 15, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 15, 2026

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants