tweak(particlesys): Decouple Particles render update from logic step#2709
tweak(particlesys): Decouple Particles render update from logic step#2709xezon wants to merge 7 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameClient/System/ParticleSys.cpp | Major refactor splitting particle logic into update() (logic step: lifetime/keyframes) and draw() (render step: physics/color/alpha). Introduces scaleDamping/scaleAccelDamping helpers for frame-rate-independent physics. Contains a duplicate m_velCoeff.zero() call introduced by the diff. |
| Core/GameEngine/Include/GameClient/ParticleSys.h | Adds draw(Real timeScale) methods to Particle, ParticleSystem, and ParticleSystemManager; replaces m_pos/m_lastPos with m_logicalPos/m_lastLogicalPos; introduces VisibilityState and refactored transform helpers; adds isDummy() virtual to ParticleSystemManagerDummy. |
| Core/Libraries/Include/Lib/BaseType.h | Adds a full set of arithmetic operators for RGBColor. All binary operators correctly return by value. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | Moves TheParticleSystemManager->update() from GameClient to GameLogic::update(), anchoring particle lifetime management to the logic step. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Switches call from ->update() to ->draw() in the render path, completing the logic/render decoupling. |
Sequence Diagram
sequenceDiagram
participant GL as GameLogic::update()
participant PSM as ParticleSystemManager
participant PS as ParticleSystem
participant P as Particle
participant W3D as W3DDisplay (render)
GL->>PSM: setLocalPlayerIndex()
GL->>PSM: update()
PSM->>PS: update(localPlayerIndex)
PS->>PS: updateVisibility()
PS->>PS: updateLastLogicalPos()
PS->>PS: updateTransform()
PS->>PS: updateLogicalPos()
PS->>PS: generateParticles()
PS->>P: update()
Note over P: lifetime decrement / keyframe advance / isInvisible() cull
W3D->>PSM: draw()
Note over PSM: timeScale = FramePacer ratio
PSM->>PS: draw(timeScale)
PS->>PS: updateWindMotion(timeScale)
PS->>PS: updateTransform()
PS->>P: applyForce(gravity)
PS->>P: draw(timeScale)
Note over P: velocity/position integration / alpha/colour accumulation / accel reset
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/GameClient/System/ParticleSys.cpp:1125-1128
Duplicate `m_velCoeff.zero()` call introduced by this diff. The line was already present in the original constructor body; the new `+` hunk added a second identical call immediately before it. The later assignment to `m_velCoeff.x/y/z = 1.0f` means both zeroing calls are effectively dead, but having two consecutive identical statements is misleading noise worth removing.
```suggestion
m_logicalPos.zero();
m_lastLogicalPos.zero();
m_velCoeff.zero();
```
Reviews (2): Last reviewed commit: "tweak(particlesys): Decouple Particles r..." | Re-trigger Greptile
|
I presume this PR fixes #2467. |
| psys->attachToObject(building); | ||
| Drawable *drawable = building->getDrawable(); | ||
| psys->attachToObject(object); | ||
| Drawable *drawable = object->getDrawable(); |
There was a problem hiding this comment.
This rename was just a side quest from when looking around particle things.
| // vel += accel; | ||
| // vel *= damping; | ||
| // | ||
| static inline Real scaleAccelDamping(Real damping, Real timeScale) |
There was a problem hiding this comment.
This function was crafted by prompting Chat Gippy many times. I do not understand exactly why it works but it works.
| ParticleSystemInfo::WindMotion windMotion = m_system->getWindMotion(); | ||
| // monitor lifetime | ||
| if (m_lifetimeLeft && --m_lifetimeLeft == 0) | ||
| return false; |
There was a problem hiding this comment.
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.
…rticle::update() into additional functions (#2709)
…icle::draw() (#2709)
b94750d to
b86a504
Compare
|
The plane trails do not fade out gracefully. That needs looking into. |
|
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. |
Merge with Rebase
This change decouples the Particles render update from the logic step.
Split into 7 commits for ease of understanding and review.
TODO