fix(particlesys): Simplify ParticleSystemManagerDummy setup

[xezon]

• Merge after fix(particlesys): Decouple particle systems from logic crc #2742

This change simplifies the ParticleSystemManagerDummy setup. It no longer creates and updates particle systems in headless mode with the dummy implementation. With RETAIL_COMPATIBLE_CRC it still INI parses the particle system templates to make sure the Logic CRC is not affected.

TODO

• Mass test Replay simulations
• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR simplifies ParticleSystemManagerDummy so headless replay mode no longer ticks or creates particle systems, removing the updateHeadless() call-site and the old shared-static dummy return from createParticleSystem. Assert guards in FXList.cpp and BeaconClientUpdate.cpp are updated to short-circuit on isDummy() instead of crashing when templates are absent.

• ParticleSystemManagerDummy gains conditional init()/reset() no-ops (skipped when RETAIL_COMPATIBLE_CRC is defined so template loading still runs for CRC parity) and an always-no-op update().
• updateHeadless() is deleted from both Generals and GeneralsMD GameClient and removed from the ReplaySimulation loop.
• A new isDummy() virtual is added to the base ParticleSystemManager class, defaulting to false, so callsites can skip assert-crashes when no real manager is present.

Confidence Score: 4/5

Safe to merge for non-RETAIL_COMPATIBLE_CRC builds; RETAIL_COMPATIBLE_CRC replay runs should be tested for memory growth before relying on this path.

In RETAIL_COMPATIBLE_CRC builds, templates are loaded by the base-class init() but createParticleSystem is not overridden in the dummy, so every FXList/BeaconClientUpdate call that resolves a template allocates a real ParticleSystem that registers itself into m_allParticleSystemList and m_systemMap. Since update() is now always a no-op, those systems never expire and accumulate until reset() is called at end-of-replay — the exact unbounded-growth scenario the deleted updateHeadless() comment explicitly described as the problem being solved.

Core/GameEngine/Include/GameClient/ParticleSys.h — the ParticleSystemManagerDummy class needs a createParticleSystem override that returns nullptr to prevent accumulation in RETAIL_COMPATIBLE_CRC builds.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/ParticleSys.h	Adds isDummy() virtual to base class; expands ParticleSystemManagerDummy with conditional init/reset no-ops and always-no-op update — but createParticleSystem is not overridden, so RETAIL_COMPATIBLE_CRC builds still allocate real particle systems that accumulate until reset().
Core/GameEngine/Source/Common/ReplaySimulation.cpp	Removes the updateHeadless() call from the replay loop; safe for non-RETAIL_COMPATIBLE_CRC mode but interacts with the accumulation issue in RETAIL_COMPATIBLE_CRC builds.
Core/GameEngine/Source/GameClient/Drawable/Update/BeaconClientUpdate.cpp	Suppresses assert crashes when isDummy() is true; null template still results in createParticleSystem(nullptr) which safely returns nullptr via base class null-check.
Core/GameEngine/Source/GameClient/FXList.cpp	Same isDummy() short-circuit pattern as BeaconClientUpdate; suppresses the template-not-found assert for headless mode.
Generals/Code/GameEngine/Source/GameClient/GameClient.cpp	Removes updateHeadless() definition; clean removal matching the ReplaySimulation.cpp change.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp	Mirrors the Generals GameClient.cpp removal of updateHeadless(); clean deletion.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    RS[ReplaySimulation loop] --> GL[TheGameLogic UPDATE]
    GL --> FX[FXList / BeaconClientUpdate]
    FX --> isDummy{isDummy?}
    isDummy -- "true: skip assert" --> findT[findTemplate]
    isDummy -- "false: normal assert" --> findT
    findT -- "non-RETAIL: nullptr" --> safe[createParticleSystem with null returns nullptr immediately]
    findT -- "RETAIL_COMPATIBLE_CRC: valid" --> create[createParticleSystem creates real object]
    create --> leak[System added to m_systemMap, update is no-op, accumulates until reset]
    RS -. "removed in this PR" .-> old[updateHeadless was calling update]

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/Include/GameClient/ParticleSys.h:846-864
**Particle systems accumulate unboundedly in `RETAIL_COMPATIBLE_CRC` builds**

When `RETAIL_COMPATIBLE_CRC` is defined, `init()` and `reset()` are not overridden, so templates are loaded. `createParticleSystem` is also not overridden, so every `FXList` or `BeaconClientUpdate` call that obtains a valid template from `findTemplate` allocates a real `ParticleSystem` — which immediately registers itself into `m_allParticleSystemList` and `m_systemMap` via `friend_addParticleSystem` in the constructor. Since `update()` is always a no-op, those systems are never ticked, never expire, and never removed from those lists. The removed `updateHeadless()` comment explicitly documented this problem: *"Particles are generated during GameLogic and only cleaned up during rendering. update() lets particles finish their lifecycle naturally instead of abruptly removing them with reset()."* `reset()` (base class) will free them, but only at end-of-replay, so a long replay causes unbounded growth that contradicts both the stated intent and the header comment "Certainly does not create particle systems." Overriding `createParticleSystem` in the dummy class to return `nullptr` would close this gap for both build configurations.

_{Reviews (3): Last reviewed commit: "Replicate in Generals" | Re-trigger Greptile}

[greptile-apps]

Shared mutable static singleton returned to callers

createParticleSystem returns &dummySystem — a single static object shared by every caller. Code in FXList.cpp calls setLocalTransform, rotateLocalTransformX/Y/Z, and setPosition on the returned pointer inside a loop where each iteration overwrites the previous iteration's state on the same object. BeaconClientUpdate.cpp calls attachToDrawable and tintAllColors on it, and stores system->getSystemID() which always equals INVALID_PARTICLE_SYSTEM_ID (0) — causing the beacon to re-enter the creation path on every subsequent call.

This is benign in single-threaded headless mode since update() is a no-op and nothing is rendered, but a brief comment explaining that the returned pointer is a non-owning, non-unique token (and that callers must not delete or track it by ID) would prevent future misuse when this class is re-used.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/GameEngine/Include/GameClient/ParticleSys.h
Line: 873-884

Comment:
**Shared mutable static singleton returned to callers**

`createParticleSystem` returns `&dummySystem` — a single static object shared by every caller. Code in `FXList.cpp` calls `setLocalTransform`, `rotateLocalTransformX/Y/Z`, and `setPosition` on the returned pointer inside a loop where each iteration overwrites the previous iteration's state on the same object. `BeaconClientUpdate.cpp` calls `attachToDrawable` and `tintAllColors` on it, and stores `system->getSystemID()` which always equals `INVALID_PARTICLE_SYSTEM_ID (0)` — causing the beacon to re-enter the creation path on every subsequent call.

This is benign in single-threaded headless mode since `update()` is a no-op and nothing is rendered, but a brief comment explaining that the returned pointer is a non-owning, non-unique token (and that callers must not delete or track it by ID) would prevent future misuse when this class is re-used.

How can I resolve this? If you propose a fix, please make it concise.

[xezon]

We expect single threaded use right now.

We could add thread_local for non VC6 builds to protect future multithreaded users, but it is useless because ParticleSystemManager::createParticleSystem is also not thread safe.

[Caball009]

I tested this against ~2500 (short) replays and saw no mismatches because of this change. I can check again after #2742 is merged just to be sure.

[xezon]

Replicated in Generals without conflicts.

[Caball009]

Looks good to me.

I tested this against ~2500 (short) replays and saw no mismatches because of this change. I can check again after #2742 is merged just to be sure.

Tested again and no mismatch.