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 7 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 particle physics integration (velocity, position, color, alpha) from the fixed logic step and moves it into a new ParticleSystem::draw(Real timeScale) render-path call, while ParticleSystem::update() is demoted to keyframe advancement and lifetime management only. The logic update is now driven from GameLogic::update() instead of W3DDisplay, and ParticleSystemManagerDummy is extended with empty draw()/update() overrides and a createParticleSystem stub that returns a well-guarded sentinel (ID = INVALID_PARTICLE_SYSTEM_ID) for headless replay.

Introduces frame-rate–independent damping helpers (scaleDamping / scaleAccelDamping) using pow(damping, timeScale) and the correct geometric series formula; the PRESERVE_RETAIL_PARTICLES flag preserves original keyframe timing to avoid known green-flame visual glitches.
m_pos/m_lastPos on both Particle and ParticleSystem are renamed to m_logicalPos/m_lastLogicalPos, particle m_lastPos is dropped and xfer versioned accordingly, and timestamps are moved from client-frame to logic-frame counters.
GameClient::update() retains a dead, empty block with a commented-out draw() call after the real call was moved to W3DDisplay and the set-up was moved to GameLogic.

Confidence Score: 5/5

The decoupling is architecturally sound and the change is safe to merge.

The frame-rate-independent damping math is correct, the dummy manager ID=0 sentinel prevents phantom registration, keyframe delay flag preserves retail visual behavior, and xfer versioning is properly bumped for the removed m_lastPos field.

No files require special attention; the empty block in GameClient.cpp around line 1261 is a minor cleanup item.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp	Core of the PR: split into update() (keyframes/lifetime) and draw(timeScale) (physics integration with frame-rate-independent damping). New helper functions scaleDamping/scaleAccelDamping are mathematically correct.
Core/GameEngine/Include/GameClient/ParticleSys.h	Adds draw()/updateWindMotion()/updateTransform() family of methods; drops m_lastPos from Particle. ParticleSystemManagerDummy extended with guarded createParticleSystem stub (ID=0 sentinel, never registered in manager map).
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Moves setLocalPlayerIndex + ParticleSystemManager::update() call here from GameClient, correctly preceding the logic-step particle update.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp	Removes updateHeadless() and the old update block from the client path. Leaves a dead-code empty block with a commented-out draw() call.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp	Single-line change: replaces legacy update() with new draw() in the render path.
Core/Libraries/Include/Lib/BaseType.h	Adds compound assignment and binary operators for RGBColor. All binary operators return by value, fixing the dangling-reference issue from a previous review.

Sequence Diagram

sequenceDiagram
    participant GL as GameLogic::update()
    participant PSM_U as ParticleSystemManager::update()
    participant PS_U as ParticleSystem::update()
    participant W3D as W3DDisplay (render path)
    participant PSM_D as ParticleSystemManager::draw()
    participant PS_D as ParticleSystem::draw(timeScale)

    Note over GL,PS_D: New decoupled flow (this PR)

    GL->>PSM_U: setLocalPlayerIndex + update()
    PSM_U->>PS_U: update() — keyframes, lifetime, isInvisible
    PS_U-->>PSM_U: alive / dead

    W3D->>PSM_D: draw() — render path
    PSM_D->>PS_D: applyForce(gravity) + draw(timeScale)
    PS_D-->>PSM_D: physics integrated (vel, pos, color, alpha)

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
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp:1261-1267
**Dead code block left behind**

The `setLocalPlayerIndex` + `update()` call was moved to `GameLogic::update()` and the draw call is now in `W3DDisplay`, leaving an empty block with only a commented-out line. The surrounding braces and comment have no functional effect and should be removed to keep the call site clean.

_{Reviews (4): Last reviewed commit: "fixup! fix(particlesys): Simplify Partic..." | 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:07:36Z

+//   vel += accel;
+//   vel *= damping;
+//
+static inline Real scaleAccelDamping(Real damping, Real timeScale)


This function was crafted by prompting Chat Gippy many times. I do not understand exactly why it works but it works.

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.

Skyaero42 · 2026-05-18T10:09:24Z


+	RGBColor& operator+=(const RGBColor& c)
+	{
+		red += c.red;


Can this cause colors to go out of bound?

same for the other operations

Yes it can. The caller needs to clamp then.

wouldn't it be preferable that the operator deals with the clamp? Makes it less error-prone

I dont think so. Because it is up to the color user what to do with it. For example the user can normalize the color, or average it like it does in W3DRadar:

// set the color to an average of the colors read color.red = sampleColor.red / (Real)samples; color.green = sampleColor.green / (Real)samples; color.blue = sampleColor.blue / (Real)samples;

oof, this is such a poor design. It takes away the meaning of red, green and blue, which always should have a value between 0 and 255 (even as intermediate result). If we would want an average of colors, it would be better to have a static RGBColor AverageColor(RGBColor[] colors) method that does this, rather then have color implementations in other classes. Otherwise it makes it error prone and fails to adhere to srp.

This is an issue well beyond this PR, so I'll leave this as resolved.

We can think of this class like a Vector. Vectors are also manipulated in various ways, for example for creating a unit vector we do normalize. I think that is why they made it 3x float to begin with, instead of just 4x byte.

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

…icle::draw() (#2709)

…2709)

… systems (#2709)

…(#2709)

…2709)

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 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

Open

Skyaero42 reviewed May 18, 2026

View reviewed changes

stephanmeesters mentioned this pull request May 18, 2026

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

Closed

xezon added 7 commits May 20, 2026 21:56

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

1e31ab4

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

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

df72d36

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

9d24c73

…icle::draw() (#2709)

fix(particlesys): Simplify ParticleSystemManagerDummy setup (#2709)

0ddccbb

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

e5c7987

…2709)

tweak(particlesys): Simplify and improve transform update of particle…

72e926e

… systems (#2709)

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

278f319

…(#2709)

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

fixup! fix(particlesys): Simplify ParticleSystemManagerDummy setup (#…

d08094c

…2709)

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

Sequence Diagram

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!

Choose a reason for hiding this comment

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!

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

Uh oh!

xezon commented May 21, 2026

Uh oh!

xezon commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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