bugfix(particlesys): Delay creation of particle emitters until ParticleSystemManager is xfer-loaded

[stephanmeesters]

• Closes W3DTankDraw::createEmitters create new particle system before XFER_LOAD of TheParticleSystemManager cause save crash.. #754
• Relates to bugfix(particlesys): Restore Particle System IDs on save load before registering to ParticleSystemManager #2316

During loading a savegame, some particle systems were created before ParticleSystemManager had the opportunity to xfer-load the saved particle systems, which led to the possibility that some of these early created particle systems could be overwritten in m_systemMap of ParticleSystemManager. When that happens they would no longer reliably be able to use master/slave systems. The retail game does not use master/slave on the involved effects but it could appear as an issue with some mods (as the bug report describes too).

Misc findings

W3DTankDraw and W3DTankTruckDraw would create particle systems in the constructor, only to purge and recreate them again in postProcess, this has been changed to look if we're loading a savegame and then only create it once.

GrantStealthBehavior triggers when you do a GPS scrambler, but the behavior and associated particle effect will only last for one frame. The particle effect references a texture that's available in Generals but not in Zero Hour. So I think that so far nobody has been able to see this particle effect in action.

Todo

• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR fixes a savegame-loading race condition where particle systems for AutoHealBehavior, GrantStealthBehavior, W3DTankDraw, and W3DTankTruckDraw were created in their constructors before ParticleSystemManager had finished xfer-loading the saved particle systems, potentially clobbering entries in m_systemMap and breaking master/slave system lookup. The fix defers emitter creation to loadPostProcess() (for the savegame path) and to the first update() / doDrawModule() call (for the normal gameplay path), using an INVALID_PARTICLE_SYSTEM_ID guard to prevent double-creation.

• The core fix is correct and well-targeted: all four affected classes now create their particle emitters lazily, eliminating the constructor-time race.
• The new createParticleSystem static helper in W3DTankDraw.cpp and W3DTankTruckDraw.cpp is defined identically in both files; extracting it to a shared utility would improve maintainability.
• Removal of the original if (sysTemplate) / if (d->m_radiusParticleSystemTmpl) null guards before calling createParticleSystem is safe — ParticleSystemManager::createParticleSystem(nullptr) explicitly returns nullptr at line 3022–3023 of ParticleSys.cpp.
• The tossTreadEmitters() call removed from both loadPostProcess() implementations was effectively a no-op (IDs are never serialized, so they are always INVALID_PARTICLE_SYSTEM_ID at that point), making the removal safe.

Confidence Score: 4/5

• This PR is safe to merge; it correctly resolves the savegame-load race condition with no introduced regressions.
• The fix is logically sound, all destructors already clean up the particle system IDs, createParticleSystem(nullptr) is confirmed safe, and the double-creation guard (INVALID_PARTICLE_SYSTEM_ID check) is consistent across all changed classes. The only deduction is the duplicated static helper function which is a maintenance concern but not a correctness issue.
• No files require special attention beyond the noted code duplication between W3DTankDraw.cpp and W3DTankTruckDraw.cpp.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DTankDraw.cpp	Removes constructor-time emitter creation, defers it to doDrawModule() and loadPostProcess() via a new createParticleSystem static helper; removes tossTreadEmitters() call from loadPostProcess() (safe since IDs are never serialized). Introduces a duplicate static helper that also appears in W3DTankTruckDraw.cpp.
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DTankTruckDraw.cpp	Mirrors W3DTankDraw.cpp changes: defers tread emitter creation to doDrawModule() and loadPostProcess(), removes the constructor call and the tossTreadEmitters() pre-clear in loadPostProcess(). Contains the same duplicate static helper.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/AutoHealBehavior.cpp	Particle-system creation moved from the constructor to a new createEmitters() helper called from loadPostProcess() and from update() (when upgrade is active). The original if (d->m_radiusParticleSystemTmpl) null guard before calling createParticleSystem was removed, but ParticleSystemManager::createParticleSystem handles null gracefully so this is safe.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/GrantStealthBehavior.cpp	Same pattern as AutoHealBehavior: emitter creation moved from the constructor to a createEmitters() helper called from loadPostProcess() and update(). Cosmetic fix: removed an extra blank line and reindented setWakeFrame call in the constructor.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/AutoHealBehavior.h	Adds createEmitters() private method declaration; minor cosmetic change to the class opening-brace placement.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/GrantStealthBehavior.h	Adds createEmitters() private method declaration. No other structural changes.

Sequence Diagram

sequenceDiagram
    participant Ctor as Constructor
    participant Xfer as xfer() / XferLoad
    participant PSM as ParticleSystemManager
    participant LPP as loadPostProcess()
    participant Update as update() / doDrawModule()

    Note over Ctor,Update: Old flow (buggy)
    Ctor->>PSM: createParticleSystem() — ID saved
    PSM-->>Ctor: systemID (e.g. 42)
    Xfer->>PSM: xfer-load saved systems (may overwrite ID 42!)
    Xfer->>Ctor: restore m_radiusParticleSystemID (stale ID)
    LPP->>PSM: tossTreadEmitters + createParticleSystem (redundant)

    Note over Ctor,Update: New flow (fixed)
    Ctor->>Ctor: m_radiusParticleSystemID = INVALID
    Xfer->>PSM: xfer-load saved systems (safe, no conflict)
    Xfer->>Ctor: restore m_radiusParticleSystemID (valid or INVALID)
    LPP->>LPP: createEmitters() — only if ID == INVALID
    LPP->>PSM: createParticleSystem()
    PSM-->>LPP: new systemID
    Update->>Update: createEmitters() — no-op if already created

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DTankDraw.cpp
Line: 128-142

Comment:
**Duplicated static helper function**

The `createParticleSystem` static helper introduced here is identical to the one added at line 184 of `W3DTankTruckDraw.cpp`. Since both files live in the same subsystem and the function body is byte-for-byte the same, any future change (e.g. adding a `setSaveable` flag, or adjusting attachment logic) would need to be applied in both places. Extracting this into a shared inline helper (e.g. in a local utility header or a shared `.cpp`) would eliminate the duplication and reduce maintenance risk.

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

_{Last reviewed commit: 3605bfe}

[greptile-apps]

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{6 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[xezon]

This is calling a function that is commented with "brutal hack", indicating that using this function is undesired. Can we solve this another way perhaps? Perhaps create particle effects later?

[stephanmeesters]

Particle effects are now created later, in update or doDrawModule. When loading from savegame these happen after loadPostProcess.

I've reworked the create emitters functions a bit to be more performant and readable

[xezon]

So my first thought here is that this is a workaround for a limitation of the particle system manager; changing the particle creation on object creation to a lazy update creation. This is similar to what the wheel emitters do. However, it does not fix the root issue with the particle system manager. So if someone created particles on object creation in the future again, then the issue returns. Is there a way to make the particle system manager robust against this? Or at least add an assert that signals that it is invalid?