tweak(particlesys): Decouple Particles render update from logic step by xezon · Pull Request #2709 · TheSuperHackers/GeneralsGameCode

xezon · 2026-05-14T17:04:48Z

Merge with Rebase

Merge after fix(particlesys): Simplify ParticleSystemManagerDummy setup #2740

This change decouples the Particles render update from the logic step.

Split into 6 commits for ease of understanding and review.

TODO

Replicate in Generals
Add pull ids to commit titles
Investigate plane trail particles
~~Initialize particle templates with RETAIL_COMPATIBLE_CRC~~

greptile-apps · 2026-05-14T17:11:15Z

Greptile Summary

This PR decouples client particle rendering and motion from fixed logic updates while retaining lifetime and emission management on the logic step.

Adds render-frame timing and logic-frame phase tracking to FramePacer.
Moves particle motion, damping, wind, gravity, size, color, and alpha sampling into a render update.
Adds particle render-update calls to both Generals and Generals Zero Hour display paths.
Preserves particle save compatibility while validating keyframes and unsupported damping values.
Updates attachment, bone-effect, editor, and color utility code to support the new particle flow.
Changes made since the previous review do not modify the files belonging to this PR.

Confidence Score: 5/5

The PR appears safe to merge because no new actionable issue was introduced by the changes made since the previous review, and no previous finding remains outstanding.

The dangling RGBColor operator references were corrected to return values and that thread is resolved. The headless-particle finding was withdrawn after confirming that supported headless execution uses the dummy particle manager, and its thread is resolved. No mounted-rule violations or new merge-blocking failures remain.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp	Splits particle logic and render updates, adds frame-rate-aware integration and keyframe validation, and preserves legacy save fields where required.
Core/GameEngine/Source/Common/FramePacer.cpp	Tracks fractional progress through the current logic frame for render interpolation.
Core/GameEngine/Include/GameClient/ParticleSys.h	Extends particle and manager interfaces with render-update, validation, and timing support.
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp	Runs the particle render update from the Generals display path.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp	Separates the logic-gated particle update from the per-render particle draw update in Zero Hour.
Core/Libraries/Include/Lib/BaseType.h	Adds value-safe arithmetic operators for `RGBColor`; the prior dangling-reference defect is fixed.
Generals/Code/GameEngine/Source/Common/GameEngine.cpp	Signals the frame pacer after each new Generals logic frame.
GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp	Signals the frame pacer after each new Zero Hour logic frame.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Logic[GameLogic frame] --> Phase[Reset logic-frame phase]
    Logic --> ParticleUpdate[Particle logic update]
    ParticleUpdate --> Lifetime[Lifetime and keyframe progression]
    ParticleUpdate --> Emission[Particle emission and attachments]
    Render[Display frame] --> ParticleDraw[Particle render update]
    Phase --> ParticleDraw
    ParticleDraw --> Motion[Motion, damping, gravity, and wind]
    ParticleDraw --> Appearance[Alpha, color, and size]
    Motion --> W3D[W3D particle rendering]
    Appearance --> W3D

_{Reviews (11): Last reviewed commit: "tweak(particlesys): Decouple Particles r..." | Re-trigger Greptile}

Caball009 · 2026-05-14T20:22:47Z

I presume this PR fixes #2467.

xezon · 2026-05-14T17:05:42Z

-		psys->attachToObject(building);
-		Drawable *drawable = building->getDrawable();
+		psys->attachToObject(object);
+		Drawable *drawable = object->getDrawable();


This rename was just a side quest from when looking around particle things.

xezon · 2026-05-14T17:09:37Z

-	ParticleSystemInfo::WindMotion windMotion = m_system->getWindMotion();
+	// monitor lifetime
+	if (m_lifetimeLeft && --m_lifetimeLeft == 0)
+		return false;


I moved the lifetime check from the bottom of the update to the very top, because there is no reason to go through all the trouble of updating the particle when the lifetime hits zero anyway. So this is a very minor performance improvement maybe.

xezon · 2026-05-17T12:48:07Z

The plane trails do not fade out gracefully. That needs looking into.

Caball009 · 2026-05-17T15:07:26Z

The changes in this PR don't seem remotely retail compatible. Like many other replays, this PR makes GR1 mismatch with headless mode for me. The mismatch happens at frame 14227, but only with headless mode. It looks like the CI replay checker is silently broken.

xezon · 2026-05-21T08:13:48Z

I will break the dummy particle manager changes off of this pull.

xezon · 2026-05-21T19:34:36Z

ParticleSystemManagerDummy changes split off to #2740

xezon · 2026-06-16T20:56:07Z

This still needs work. Plane trail issue I have locally fixed but there was another issue that I already forgot. Maybe need to watch old videos.

Edit: I remember it was plane exhaust and low frame rate (USA 02 mission intro)

Skyaero42 · 2026-06-17T07:52:06Z

This still needs work. Plane trail issue I have locally fixed but there was another issue that I already forgot. Maybe need to watch old videos.

Edit: I remember it was plane exhaust and low frame rate (USA 02 mission intro)

Would have liked this PR in draft then, so I didn't spend time reviewing it.
I cannot withdraw my review until branch conflicts are resolved - long live Github

xezon · 2026-07-11T09:28:23Z

@greptileai
With this change, JetLenzflare does not render at render frame rate 25 or lower. Why?

Particle definition:

ParticleSystem JetLenzflare
  Priority = WEAPON_EXPLOSION
  IsOneShot = NO
  Shader = ADDITIVE
  Type = PARTICLE
  ParticleName = EXLnzFlar2.tga
  AngleX = 0.00 0.00
  AngleY = 0.00 0.00
  AngleZ = 0.00 0.00
  AngularRateX = 0.00 0.00
  AngularRateY = 0.00 0.00
  AngularRateZ = 0.00 0.00
  AngularDamping = 1.00 1.00
  VelocityDamping = 1.00 1.00
  Gravity = 0.00
  SlaveSystem = JetLenzflareExhaust
  SlavePosOffset = X:0.00 Y:0.00 Z:0.00
  Lifetime = 3.00 3.00
  SystemLifetime = 0
  Size = 20.00 20.00
  StartSizeRate = 0.00 0.00
  SizeRate = 0.20 0.20
  SizeRateDamping = 1.00 1.00
  Alpha1 = 1.00 1.00 0
  Alpha2 = 0.00 0.00 2
  Alpha3 = 0.00 0.00 0
  Alpha4 = 0.00 0.00 0
  Alpha5 = 0.00 0.00 0
  Alpha6 = 0.00 0.00 0
  Alpha7 = 0.00 0.00 0
  Alpha8 = 0.00 0.00 0
  Color1 = R:0 G:0 B:0 0
  Color2 = R:255 G:255 B:255 1
  Color3 = R:0 G:0 B:0 3
  Color4 = R:0 G:0 B:0 0
  Color5 = R:0 G:0 B:0 0
  Color6 = R:0 G:0 B:0 0
  Color7 = R:0 G:0 B:0 0
  Color8 = R:0 G:0 B:0 0
  ColorScale = 0.00 0.00
  BurstDelay = 0.00 0.00
  BurstCount = 1.00 1.00
  InitialDelay = 0.00 0.00
  DriftVelocity = X:0.00 Y:0.00 Z:0.25
  VelocityType = ORTHO
  VelOrthoX = -1.00 0.00
  VelOrthoY = 0.00 0.00
  VelOrthoZ = 0.00 0.00
  VolumeType = LINE
  VolLineStart = X:0.00 Y:0.00 Z:0.00
  VolLineEnd = X:0.00 Y:0.00 Z:0.00
  IsHollow = YES
  IsGroundAligned = NO
  IsEmitAboveGroundOnly = NO
  IsParticleUpTowardsEmitter = NO
  WindMotion = UNUSED
  WindAngleChangeMin = 0.149924
  WindAngleChangeMax = 0.449946
  WindPingPongStartAngleMin = 0.000000
  WindPingPongStartAngleMax = 0.785398
  WindPingPongEndAngleMin = 5.497787
  WindPingPongEndAngleMax = 6.283185
End

greptile-apps · 2026-07-11T09:32:12Z

The root cause is the interaction between the decoupled logic/render updates, the initial black color of JetLenzflare particles, and the isInvisible() check in Particle::update().

Why it happens:

JetLenzflare uses Shader = ADDITIVE with:

Color1 = R:0 G:0 B:0 0   ← starts BLACK
Color2 = R:255 G:255 B:255 1

With the decoupled design, m_color is only advanced in Particle::draw() (the render step), not in Particle::update() (the logic step). The logic step only manages key frame index advancement and the isInvisible() death check.

At logic = 30 FPS, render = 25 FPS (or lower), the timing is:

Time	Event
t=0ms	Logic frame N: particle created, `m_color = (0,0,0)` (black)
t=33ms	Logic frame N+1: `update()` runs with `frameCount=1`. `draw()` has NOT been called yet — render frame isn't due until t=40ms. `isInvisible()` sees color is still black → particle dies
t=40ms	Render frame: `draw()` would have made the color white — but the particle is already gone

The frameCount == 0 guard in update() was intended to protect against this:

if (frameCount == 0)
{
    // Pass one full logic frame before trying to update and delete this potentially now
    // invisible particle, because the later render update may fade it in and make it visible.
    return true;
}

But it only protects the creation frame. It does not protect against the case where render FPS < logic FPS and no draw() call has yet occurred by frameCount=1. At 25 FPS render, draw() simply hasn't been called between frameCount=0 and frameCount=1, so the particle is still black when isInvisible() fires.

The existing todo in the code already captures this:

// TheSuperHackers @todo This check is shady for particles that fade in first. A more robust logic would be good.
if (isInvisible())
    return false;

A robust fix would need to track whether at least one draw() call has occurred before enabling the isInvisible() early-exit in update(). For example, a m_hasBeenDrawn flag on the particle: skip the isInvisible() check until the flag is set by the first draw() call.

xezon · 2026-07-11T09:36:22Z

@greptileai
This is not the cause of the issue. The Logic step cannot run faster than the Render step. And I already tested without the isInvisible() test and it the issue still happens. Please check for other reasons.

greptile-apps · 2026-07-11T09:39:55Z

You're right, I was wrong on both counts — the logic step can't run faster than render in this engine's design, and the isInvisible() check being removed didn't reproduce the fix. I apologize for the incorrect diagnosis.

Looking more carefully at the decoupling for other potential causes: one thing that stands out is that m_alpha and m_color are now only accumulated in draw(), but the keyframe rate recomputation (computeAlphaRate() / computeColorRate()) happens in update(). So there's an asymmetry — the rate changes on logic ticks, but the accumulated value only advances on render ticks. If a keyframe boundary is crossed in update() and then draw() overshoots because of a large timeScale, the alpha/color could drift past the intended keyframe target value with no correction, since the direct-snap m_alpha = m_alphaKey[...].value was removed.

Could the issue be related to particle position or alpha/color values going out of expected range due to this rate/accumulation split, rather than the lifetime check? If you can describe what specific visual artifact you're seeing, I can narrow down the analysis.

xezon · 2026-07-11T14:11:07Z

I found the hidden particles at sub 30 fps are an explicit feature in ParticleSystem::createParticle

		//
		// Check if particle is below priorities we allow for this FPS or if it being skipped because
		// all particles are being skipped (excluding special fps independent particles at
		// getMinDynamicParticleSkipPriority())
		//
		if( priority < TheGameLODManager->getMinDynamicParticlePriority() ||
				(priority < TheGameLODManager->getMinDynamicParticleSkipPriority() &&
				 TheGameLODManager->isParticleSkipped()) )
			return nullptr;

xezon · 2026-07-11T14:50:35Z

I cannot figure out why the USA02 Cine JetLenzflare is lagging behind.

Jet Lenzflare in normal gameplay looks normal.

xezon · 2026-07-16T07:08:49Z

Brother @greptileai, please list potential reasons why this change could have introduced lagging-behind particles on fast moving objects or drawables.

It is not observable with all particles, but is 100% with JetLenzflare in a cutscene, that runs at 22 fps.

The effect(s):

ParticleSystem JetLenzflare
  Priority = WEAPON_EXPLOSION
  IsOneShot = NO
  Shader = ADDITIVE
  Type = PARTICLE
  ParticleName = EXLnzFlar2.tga
  AngleZ = 0.00 0.00
  AngularRateZ = 0.00 0.00
  AngularDamping = 1.00 1.00
  VelocityDamping = 1.00 1.00
  Gravity = 0.00
  SlaveSystem = JetLenzflareExhaust
  SlavePosOffset = X:0.00 Y:0.00 Z:0.00
  Lifetime = 3.00 3.00
  SystemLifetime = 0
  Size = 20.00 20.00
  StartSizeRate = 0.00 0.00
  SizeRate = 0.20 0.20
  SizeRateDamping = 1.00 1.00
  Alpha1 = 1.00 1.00 0
  Alpha2 = 0.00 0.00 2
  Alpha3 = 0.00 0.00 0
  Alpha4 = 0.00 0.00 0
  Alpha5 = 0.00 0.00 0
  Alpha6 = 0.00 0.00 0
  Alpha7 = 0.00 0.00 0
  Alpha8 = 0.00 0.00 0
  Color1 = R:0 G:0 B:0 0
  Color2 = R:255 G:255 B:255 1
  Color3 = R:0 G:0 B:0 3
  Color4 = R:0 G:0 B:0 0
  Color5 = R:0 G:0 B:0 0
  Color6 = R:0 G:0 B:0 0
  Color7 = R:0 G:0 B:0 0
  Color8 = R:0 G:0 B:0 0
  ColorScale = 0.00 0.00
  BurstDelay = 0.00 0.00
  BurstCount = 1.00 1.00
  InitialDelay = 0.00 0.00
  DriftVelocity = X:0.00 Y:0.00 Z:0.25
  VelocityType = ORTHO
  VelOrthoX = -1.00 0.00
  VelOrthoY = 0.00 0.00
  VelOrthoZ = 0.00 0.00
  VolumeType = LINE
  VolLineStart = X:0.00 Y:0.00 Z:0.00
  VolLineEnd = X:0.00 Y:0.00 Z:0.00
  IsHollow = YES
  IsGroundAligned = NO
  IsEmitAboveGroundOnly = NO
  IsParticleUpTowardsEmitter = NO
End

ParticleSystem JetLenzflareExhaust
  Priority = WEAPON_EXPLOSION
  IsOneShot = NO
  Shader = ALPHA
  Type = PARTICLE
  ParticleName = EXCloud01.tga
  AngleZ = 0.00 1.00
  AngularRateZ = 0.00 0.00
  AngularDamping = 1.00 1.00
  VelocityDamping = 0.00 0.00
  Gravity = 0.00
  Lifetime = 35.00 35.00
  SystemLifetime = 0
  Size = 0.50 0.50
  StartSizeRate = 0.00 0.00
  SizeRate = 0.60 2.00
  SizeRateDamping = 0.93 0.94
  Alpha1 = 0.50 0.50 0
  Alpha2 = 0.00 0.50 25
  Alpha3 = 0.00 0.00 35
  Alpha4 = 0.00 0.00 0
  Alpha5 = 0.00 0.00 0
  Alpha6 = 0.00 0.00 0
  Alpha7 = 0.00 0.00 0
  Alpha8 = 0.00 0.00 0
  Color1 = R:245 G:204 B:141 0
  Color2 = R:128 G:128 B:128 10
  Color3 = R:0 G:0 B:0 0
  Color4 = R:0 G:0 B:0 0
  Color5 = R:0 G:0 B:0 0
  Color6 = R:0 G:0 B:0 0
  Color7 = R:0 G:0 B:0 0
  Color8 = R:0 G:0 B:0 0
  ColorScale = 0.00 0.00
  BurstDelay = 2.00 2.00
  BurstCount = 1.00 1.00
  InitialDelay = 0.00 0.00
  DriftVelocity = X:0.00 Y:0.00 Z:0.00
  VelocityType = ORTHO
  VelOrthoX = 0.00 0.00
  VelOrthoY = 0.00 0.00
  VelOrthoZ = 0.00 0.00
  VolumeType = POINT
  IsHollow = NO
  IsGroundAligned = NO
  IsEmitAboveGroundOnly = NO
  IsParticleUpTowardsEmitter = NO
  WindMotion = UNUSED
  WindAngleChangeMin = 0.149924
  WindAngleChangeMax = 0.449946
  WindPingPongStartAngleMin = 0.000000
  WindPingPongStartAngleMax = 0.785398
  WindPingPongEndAngleMin = 5.497787
  WindPingPongEndAngleMax = 6.283185
End

greptile-apps · 2026-07-16T07:13:49Z

Here are the potential root causes, ranked by likelihood:

1. Spawn position lag due to update-order change (most likely)

In the old code, ParticleSystemManager::update() (which includes spawning) ran from W3DDisplay::draw() — after GameClient::update() had already refreshed all drawable bone/animation transforms. New particles therefore spawned at the current render-frame transform.

Now update() runs from GameLogic::update() — before GameClient::update(). When updateTransform() calls attachedTo->getTransformMatrix(), it reads the transform from the previous client frame. Particles spawn one render-frame behind the drawable's true visual position. For a fast jet this is a visible displacement, especially because JetLenzflare particles have near-zero own velocity (VelOrthoX = -1.00 0.00, DriftVelocity = 0.25 Z), so they essentially mark where the jet was, not where it is.

This was literally the reason Lorenzen & Wilczynski originally moved particle update into W3DDisplay::draw() — to capture the render-object's transform "while it is still valid."

2. Bone/animated transform vs. logical object position

The particle system is attached to a drawable whose bone positions are resolved during the W3D client pipeline, not during game logic. m_logicalPos captures the object's raw world-space origin, but the lensflare should track the exhaust bone. If the drawable's getTransformMatrix() returns the bone-resolved transform only after client update, then logic-time updateTransform() will get an outdated bone position regardless of object speed.

3. timeScale doesn't represent actual elapsed wall-clock time per render frame

getActualLogicTimeScaleOverFpsRatio() appears to be a fixed-ratio value (logic step duration / target FPS frame duration), not a measurement of actual elapsed time since the last draw() call. At genuinely variable fps (like 22fps in a heavy cutscene), the actual render-frame wall-clock time fluctuates. If timeScale is a fixed ratio (e.g., logicHz / targetRenderHz), particle velocity integration will over- or under-shoot the correct world displacement each frame, and the error accumulates as visible lag against the emitter.

4. Multiple logic steps between render frames are not compensated

At 22fps render / 30fps logic, there will be render frames where 1 logic step fires and others where 2 fire before the next draw() call. If draw() is called once per render frame with a fixed timeScale that assumes 1 logic step, and 2 logic steps occurred, particles integrate only half the needed physics displacement for that frame, lagging behind the emitter.

5. emissionAdjustment interpolation uses only the logic-frame delta

frameDeltaPos = m_logicalPos - m_lastLogicalPos spreads burst particles along the path between two consecutive logic positions. At 22fps render / 30fps logic, this is fine for the spread itself, but the spread starts at m_lastLogicalPos (one full logic frame back) rather than at the emitter's current render-frame position. For fast-moving emitters the entire spawn cluster is displaced backward.

In short: The most actionable cause is #1 — particles now spawn with one client-update-cycle of position lag that the original design deliberately avoided by running update() inside the render pipeline. The fix would be to either run updateTransform() / updateLogicalPos() a second time at the start of draw() before any new particles are spawned (but spawning already happened), or to record and use the render-time emitter position separately from m_logicalPos for the purpose of computing the visual emitter origin in draw().

qodo-free-for-open-source-projects · 2026-08-29T17:17:51Z

PR Summary by Qodo

Decouple particle rendering from fixed logic updates

✨ Enhancement 🐞 Bug fix 🕐 40+ Minutes

AI Description

• Separates particle lifetime logic from frame-paced visual simulation.
• Makes motion, damping, wind, color, and size updates frame-rate independent.
• Preserves attachment, save compatibility, and particle behavior across both game variants.

Diagram

sequenceDiagram
    participant GL as Game Logic
    participant GC as Game Client
    participant PM as Particle Manager
    participant PS as Particle System
    participant FP as Frame Pacer
    participant RD as W3D Display
    participant P as Particle
    GL->>GC: Complete logic frame
    GC->>PM: Logic update
    PM->>PS: Update lifetime
    PS->>P: Advance keys
    RD->>FP: Read time scale
    FP-->>RD: Delta ratio
    RD->>PM: Render update
    PM->>PS: Draw scaled
    PS->>P: Integrate visuals

High-Level Assessment

The following are alternative approaches to this PR:

1. Fixed-step simulation with interpolation

➕ Keeps rendering side-effect free and simulation deterministic.
➕ Avoids visual state changing multiple times per logic frame.
➖ Requires previous/current particle snapshots and interpolation support.
➖ Adds memory and complexity for short-lived particles, emissions, and keyframes.

2. Accumulated fixed render substeps

➕ Reuses original fixed-step equations with predictable behavior.
➕ Avoids fractional powers for damping calculations.
➖ Can perform multiple particle traversals per display frame.
➖ Needs remainder handling and may produce visible stepping.

Recommendation: The PR's frame-scaled render update is pragmatic because particles are visual effects and avoids snapshot storage or repeated substeps. Keep this approach, but validate unusual frame ratios, paused rendering, negative damping inputs, and retail keyframe behavior; interpolation would be preferable only if render-side mutation later causes determinism or multi-view rendering issues.

Files changed (10) +602 / -272

Enhancement (6) +589 / -251

ParticleSys.hDefine separate particle logic and render update interfaces +35/-6

Define separate particle logic and render update interfaces
• Adds draw paths for particles, systems, and the manager; extracts transform and visibility helpers; and removes the unused particle position member. Declares damping validation and updates the dummy manager interface.
Core/GameEngine/Include/GameClient/ParticleSys.h

ParticleSys.cppImplement frame-paced particle rendering and compatibility safeguards +414/-235

Implement frame-paced particle rendering and compatibility safeguards

• Moves motion, gravity, wind, rotation, size, alpha, and color integration into render updates using frame-rate-independent damping. Retains lifetime and keyframe progression in logic updates, extracts attachment transform and visibility handling, validates damping, and versions serialization after removing the unused field.

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

BaseType.hAdd RGBColor arithmetic operators +120/-0

Add RGBColor arithmetic operators
• Adds component-wise and scalar arithmetic operators used to simplify frame-scaled particle color updates.
Core/Libraries/Include/Lib/BaseType.h

GameClient.cppGate Generals particle logic updates on completed logic frames +2/-2

Gate Generals particle logic updates on completed logic frames
• Runs the particle manager logic update only when game logic advances, using the subsystem update wrapper.
Generals/Code/GameEngine/Source/GameClient/GameClient.cpp

W3DDisplay.cppRun Generals particle render updates from W3D draw +2/-0

Run Generals particle render updates from W3D draw
• Invokes the particle manager render path after view updates so visual particle state advances each display frame.
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

W3DDisplay.cppSeparate GeneralsMD particle logic and render dispatch +16/-8

Separate GeneralsMD particle logic and render dispatch
• Runs particle logic only after game logic advances, resolves the observed player for shroud checks, and dispatches render updates every display frame.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

Bug fix (1) +6 / -6

ParticleEditorDialog.cppPrevent negative particle damping in editor inputs +6/-6

Prevent negative particle damping in editor inputs
• Clamps size-rate, angular, and velocity damping values to non-negative ranges when applying editor changes.
Core/Tools/ParticleEditor/ParticleEditorDialog.cpp

Refactor (3) +7 / -15

BoneFXUpdate.cppClarify bone effect object attachment +3/-4

Clarify bone effect object attachment
• Renames the local attached object variable while preserving particle attachment and hidden-drawable stopping behavior.
Generals/Code/GameEngine/Source/GameLogic/Object/Update/BoneFXUpdate.cpp

GameClient.cppRemove obsolete GeneralsMD client particle update block +1/-7

Remove obsolete GeneralsMD client particle update block
• Removes the inactive client-side update scaffold because particle logic and rendering are coordinated in W3DDisplay.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

BoneFXUpdate.cppClarify GeneralsMD bone effect object attachment +3/-4

Clarify GeneralsMD bone effect object attachment
• Renames the local attached object variable while preserving particle attachment and hidden-drawable stopping behavior.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BoneFXUpdate.cpp

xezon · 2026-08-29T17:18:33Z

After several review rounds with Claude Opus I have now updated and polished this change further. It is ready for human re-review.

qodo-free-for-open-source-projects · 2026-08-29T17:21:05Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Particle logic updates repeatedly~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new GeneralsMD display-loop call invokes ParticleSystemManager::UPDATE() on every iteration
while hasUpdated() remains true, and removal of the manager's logic-frame guard allows one game
frame to age and emit particles multiple times during tactical-camera freeze loops. This shortens
particle/system lifetimes and consumes burst delays without corresponding logic advancement.

Code

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp[1961]
+				TheParticleSystemManager->UPDATE();

Evidence
The display code places the update inside a repeating freeze loop, while m_hasUpdated is only
cleared by the next logic pre-update. Each manager update advances emission delays, particle
lifetimes, and system lifetime.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp[1886-1886]
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp[1940-1963]
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp[2100-2105]
GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp[316-320]
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp[3403-3416]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2043-2055]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2091-2115]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2124-2129]

Agent prompt

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

## Issue description
Prevent particle logic from updating multiple times during one game-logic frame when the GeneralsMD render loop repeats.
## Issue Context
`GameLogic::hasUpdated()` stays true until the next logic `preUpdate()`, so it is not a once-only consumption flag. Restore a per-logic-frame guard in the particle manager or move the update to a location that executes exactly once per logic frame.
## Fix Focus Areas
- GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp[1957-1962]
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[3183-3195]

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

2. ~~Gravity displacement remains FPS-dependent~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Particle::draw() updates velocity from acceleration and then integrates position from that
post-acceleration velocity, so subdividing one logic interval changes the endpoint. With damping 1,
zero initial velocity, and gravity a, one step of scale 1 moves by a, while two half-steps move
by only 0.75a, making particle trajectories depend on render FPS.

Code

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[R499-501]
+	m_pos.x += (m_vel.x + driftVel->x) * timeScale;
+	m_pos.y += (m_vel.y + driftVel->y) * timeScale;
+	m_pos.z += (m_vel.z + driftVel->z) * timeScale;

Evidence
FramePacer supplies a logic-FPS/render-FPS ratio, gravity is reapplied before every fractional draw,
and acceleration is reset afterward. The added position equation therefore yields the demonstrated
1.0 versus 0.75 displacement discrepancy solely from render subdivision.
Core/GameEngine/Source/Common/FramePacer.cpp[205-210]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[457-482]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[491-501]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[562-565]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2300-2310]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[3237-3248]

Agent prompt

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

## Issue description
Make acceleration-driven particle displacement produce the same endpoint regardless of render-step subdivision.
## Issue Context
The velocity damping transition is subdivision-compatible, but position currently uses only the post-update velocity. Implement the matching closed-form/semigroup displacement transition, including acceleration and damping edge cases, and test one full step against multiple fractional steps.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[491-501]
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2300-2310]

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

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

bobtista · 2026-08-30T17:43:07Z

 			{
-				m_alpha = m_alphaKey[ m_alphaTargetKey ].value;
 				m_alphaTargetKey++;
 				computeAlphaRate();


Was removing the alpha key snap intentional? Retail sets m_alpha to the reached key value before computing the next rate. computeAlphaRate() is key-to-key, so any render-step drift now carries into later segments.

I think it was intentional but I also cannot remember all the details anymore. I was unable to observe bugs from it.

I dug into it more. This is a one-frame phase lead, not drift. We could restore m_alpha = m_alphaKey[m_alphaTargetKey].value before incrementing the target, then have draw() skip alpha integration when m_alphaTargetKey > 1 && frameCount == m_alphaKey[m_alphaTargetKey - 1].frame.
frameCount can be derived from TheGameClient->getFrame() - m_createTimestamp. This holds the snapped value for the boundary frame and starts the new rate on the next frame without adding state.
I don't think this is materially noticeable in normal play. The fix would be more retail-compatible, but I'm happy to approve either way.

I am not convinced the above observations are correct. I remember I extensively tested this and the + KeyFrameDelay on one of these advances and not on the other already correctly migrated it.

If there are observable issues with the code as presented, please say with which particle to observe this with.

I measured this with ToxinSpray in three builds: the base, this branch, and this branch with the snap restored:

age base head head+snap
4 0.312500 0.312500 0.312500
5 0.250000 0.305357 0.242857
20 0.142857 0.198214 0.135714
38 0.014286 0.069643 0.007143

The missing snap leaves alpha 0.055357 above the base for the remaining 34 frames. Restoring it removes that persistent offset; the remaining 0.007143 is the next rate being applied in draw() on the boundary frame, which the skip described above handles.
The same alpha keys are used by ToxinBuildingClearSpray, ToxinUpgradedBuildingClearSpray, AnthraxSpray, AnthraxGammaSpray and ScudStormBuildingGoo. The KeyFrameDelay comment exempts alpha because retail overwrote the accumulated value at the key—the line removed here.

Ok. I did a long chat session with Claude Opus about this and we settled on a solution that moves the alpha frame computation entirely to the render update, which then solves this problem. The Particle class loses its m_alphaRate member, but is preserved for RETAIL_COMPATIBLE_XFER_SAVE. The implementations for alpha rate and color rate are now different, but it was not easily possible to do the same for the color rate and not introduce a visual divergence. Maybe it can be revisited in the future.

The logic for key frames not starting at 0 has changed for not PRESERVER_RETAIL_PARTICLES. They now legitimately hold the value before reaching the first frame instead of advancing to the first key frame and then holding its value towards the second frame. This is more true to what the INI setup says.

It's all part of the last fixup commit.

The code is only getting more complicated with every new comment added to this pull 😆

Skyaero42

Looks good to me.

…rticle::update() into additional functions (#2709)

…icle::draw() (#2709)

…2709)

…(#2709)

xezon · 2026-09-05T08:24:39Z

Fixup commits squashed

xezon · 2026-09-05T08:47:12Z

Any final words?

chatgpt-codex-connector · 2026-09-05T20:21:09Z

To use Codex here, create an environment for this repo.

OmarAglan · 2026-09-05T20:23:13Z

@codex Review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07c95d1142

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-05T20:34:46Z

+		ParticleSystem* sys = *it++;
+		DEBUG_ASSERTCRASH(sys != nullptr, ("ParticleSystemManager::draw: ParticleSystem is null"));
+
+		sys->draw(timeScale);


Refresh smudge snapshots after advancing particles

When the render rate exceeds the logic rate, ParticleSystemManager::update() rebuilds each Smudge from the particle's position, size, and opacity only on logic ticks, but this new call subsequently changes those properties on every render tick. W3DParticleSystemManager::doParticles() merely marks the stored smudge for drawing, and W3DSmudge renders its stored values, so heat-distortion particles remain at the logic rate and visibly lag the corresponding particle motion. Synchronize the smudge properties after the render-step particle updates.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-05T20:34:46Z

+	// TheSuperHackers @info The acceleration pushes the position for only half of the step it is added
+	// in, on average, whereas the original single step integration pushes it for the whole step. This
+	// gain interpolates between both and remains the original value at time scale 1.
+	const Real accelGain = velGain * (1.0f + timeScale) * 0.5f;


Make the acceleration gain compose across render steps

When a logic frame is split across multiple render steps and gravity is nonzero, this heuristic does not reproduce the original full-step position update. For example, with VelocityDamping = 0.5, zero initial velocity, and two timeScale = 0.5 draws, the accumulated acceleration displacement is about 0.5251 * accel, whereas one timeScale = 1 draw produces 0.5 * accel; velocity composes correctly, but the trajectory still changes with render FPS. Derive the acceleration position gain from the same composable state transition instead of the half-step average.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-05T20:34:46Z

+	if (m_windMotion != ParticleSystemInfo::WIND_MOTION_NOT_USED )
+		updateWindMotion(timeScale);


Keep wind RNG transitions on the logic update

For ping-pong wind systems, updateWindMotion() rerolls the angle change and endpoints through the shared GameClientRandomValueReal stream whenever an endpoint is crossed. Moving that call here means particle emission now consumes random values before the wind reroll instead of after it even at the original frame rate, and at higher render rates the reroll can also occur between logic emissions; subsequent particle positions, velocities, and other randomized attributes therefore change with render FPS and no longer preserve the retail sequence. Keep the RNG-consuming state transition logic-timed and interpolate only the resulting wind motion during draws.

Useful? React with 👍 / 👎.

xezon added Enhancement Is new feature or request Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Rendering Is Rendering related labels May 14, 2026

greptile-apps Bot reviewed May 14, 2026

View reviewed changes

Comment thread Core/Libraries/Include/Lib/BaseType.h Outdated

xezon commented May 15, 2026

View reviewed changes

xezon force-pushed the xezon/decouple-particle-update branch from b94750d to b86a504 Compare May 17, 2026 11:40

xezon mentioned this pull request May 17, 2026

Particle systems cause game logic / CRC changes #2717

Closed

Skyaero42 reviewed May 18, 2026

View reviewed changes

Comment thread Core/Libraries/Include/Lib/BaseType.h

stephanmeesters mentioned this pull request May 18, 2026

fix(particlesys): Decouple creation of particle systems from game logic stephanmeesters/GeneralsGameCode#1

Closed

xezon force-pushed the xezon/decouple-particle-update branch from b86a504 to 278f319 Compare May 20, 2026 20:02

Skyaero42 previously approved these changes Jun 16, 2026

View reviewed changes

Comment thread Core/Libraries/Include/Lib/BaseType.h

xezon marked this pull request as draft June 17, 2026 07:53

qodo-free-for-open-source-projects Bot reviewed Aug 29, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated

bobtista reviewed Aug 29, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

Skyaero42 reviewed Aug 29, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated

greptile-apps Bot reviewed Aug 30, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

xezon requested review from Skyaero42 and bobtista August 30, 2026 10:48

bobtista reviewed Aug 30, 2026

View reviewed changes

Skyaero42 approved these changes Aug 30, 2026

View reviewed changes

xezon force-pushed the xezon/decouple-particle-update branch 2 times, most recently from 439c4c9 to 6143a7a Compare September 2, 2026 15:32

xezon commented Sep 2, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

xezon commented Sep 2, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

xezon added 6 commits September 5, 2026 10:21

refactor(particlesys): Split parts of ParticleSystem::update() and Pa…

779e61d

…rticle::update() into additional functions (#2709)

fix(particlesys): Fix wrong color clamp in Particle::draw() (#2709)

469fc80

refactor(particlesys): Simplify color arithmetic and clamping in Part…

5609fb1

…icle::draw() (#2709)

tweak(particlesys): Remove unused class member Particle::m_lastPos (#…

1457565

…2709)

refactor(particlesys): Simplify transform update of particle systems …

0e6f55a

…(#2709)

tweak(particlesys): Decouple Particles render update from logic step …

07c95d1

…(#2709)

xezon force-pushed the xezon/decouple-particle-update branch from 4c0cbb0 to 07c95d1 Compare September 5, 2026 08:24

xezon mentioned this pull request Sep 5, 2026

bugfix(drawable): Decouple visual render from projectile's logic position #2598

Draft

stephanmeesters reviewed Sep 5, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

chatgpt-codex-connector Bot reviewed Sep 5, 2026

View reviewed changes

stephanmeesters mentioned this pull request Sep 6, 2026

unify(w3ddisplay): Merge most of W3DDisplay #3260

Open

		if (m_windMotion != ParticleSystemInfo::WIND_MOTION_NOT_USED )
		updateWindMotion(timeScale);

Uh oh!

Conversation

xezon commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

greptile-apps Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Caball009 commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon commented May 17, 2026

Uh oh!

Caball009 commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

xezon commented May 21, 2026

Uh oh!

xezon commented May 21, 2026

Uh oh!

Uh oh!

xezon commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Skyaero42 commented Jun 17, 2026

Uh oh!

xezon commented Jul 11, 2026

Uh oh!

greptile-apps Bot commented Jul 11, 2026

Uh oh!

xezon commented Jul 11, 2026

Uh oh!

greptile-apps Bot commented Jul 11, 2026

Uh oh!

xezon commented Jul 11, 2026

Uh oh!

xezon commented Jul 11, 2026

Uh oh!

xezon commented Jul 16, 2026

Uh oh!

greptile-apps Bot commented Jul 16, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 29, 2026

PR Summary by Qodo

Uh oh!

xezon commented Aug 29, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xezon commented May 14, 2026 •

edited

Loading

greptile-apps Bot commented May 14, 2026 •

edited

Loading

Caball009 commented May 14, 2026 •

edited

Loading

Caball009 commented May 17, 2026 •

edited

Loading

xezon commented Jun 16, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 29, 2026 •

edited

Loading