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 that headless/replay mode no longer creates or updates real particle systems. The updateHeadless() method is removed entirely, and the dummy manager now overrides createParticleSystem and update to be inert — with a RETAIL_COMPATIBLE_CRC path that keeps INI template loading active (to preserve logic CRC) while still skipping actual particle instantiation.

• isDummy() is added as a virtual method on the base class and overridden in the dummy, allowing FXList.cpp and BeaconClientUpdate.cpp to suppress template-not-found asserts that are expected when no templates are loaded.
• In RETAIL_COMPATIBLE_CRC mode, createParticleSystem returns a shared static sentinel (dummySystem) instead of null; the sentinel is safe because ParticleSystem's constructor skips friend_addParticleSystem when the ID equals INVALID_PARTICLE_SYSTEM_ID(0), and its destructor does the same.
• updateHeadless() is removed from GameClient and its call site in ReplaySimulation.cpp, since the dummy's update() override is now a no-op.

Confidence Score: 5/5

Safe to merge — the change correctly eliminates dead particle work in headless mode without touching logic-CRC-relevant state.

The dummy manager's override of createParticleSystem is carefully guarded: the returned static sentinel is zero-initialized, and the ParticleSystem constructor skips both friend_addParticleSystem and friend_removeParticleSystem when the ID is INVALID_PARTICLE_SYSTEM_ID. The assert suppressions in FXList and BeaconClientUpdate are narrowly scoped via isDummy() and leave the non-headless path unchanged. The RETAIL_COMPATIBLE_CRC split is well-motivated: template loading (which affects logic CRC) is preserved; particle instantiation (client-side, does not affect logic CRC) is skipped.

No files require special attention. The replay simulation testing noted in the PR TODO is the main remaining validation gap, but the code itself is correct.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/ParticleSys.h	Core change: adds isDummy() virtual method, makes createParticleSystem virtual, and reworks ParticleSystemManagerDummy with conditional RETAIL_COMPATIBLE_CRC logic — well-structured and safe.
Core/GameEngine/Source/Common/ReplaySimulation.cpp	Removes the now-unnecessary updateHeadless() call from the replay simulation loop; correct given that the dummy manager's update() is now a no-op.
Core/GameEngine/Source/GameClient/Drawable/Update/BeaconClientUpdate.cpp	Suppresses two DEBUG_ASSERTCRASH calls when isDummy() is true, preventing spurious asserts in non-RETAIL_COMPATIBLE_CRC headless mode where no templates are loaded.
Core/GameEngine/Source/GameClient/FXList.cpp	Same isDummy() guard pattern as BeaconClientUpdate.cpp — assert is suppressed when in dummy mode and template lookup returns null.
GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h	Removes the updateHeadless() declaration — clean deletion matching the GameClient.cpp removal.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp	Removes the updateHeadless() implementation that previously called TheParticleSystemManager->update() on each replay frame — no longer needed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ReplaySimulation loop] -->|before PR| B[TheGameClient->updateHeadless]
    B --> C[TheParticleSystemManager->update real update on base class]
    A -->|after PR| D[No updateHeadless call]

    E[createParticleSystem called] --> F{isDummy?}
    F -->|No| G[Real ParticleSystemManager creates tracked ParticleSystem]
    F -->|Yes - RETAIL_COMPATIBLE_CRC| H[Return static dummySystem ID=INVALID not registered]
    F -->|Yes - no RETAIL_COMPATIBLE_CRC| I[Return nullptr]

    J[DEBUG_ASSERTCRASH] --> K{isDummy OR template found?}
    K -->|No| L[Assert fires]
    K -->|Yes| M[Continue silently]

Loading

_{Reviews (2): Last reviewed commit: "Pass false" | 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.

[xezon]

After #2742 is merged, we should be able to remove this path, because particle systems will no longer be needed for correct CRC.

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

[Caball009]

This is going to change after #2742, right?

[xezon]

Loading particle system templates will still be required under RETAIL CRC because calls to logic random value is conditioned by existence of some templates.