Skip to content

fix(particlesys): Simplify ParticleSystemManagerDummy setup #2740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xezon wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(particlesys): Simplify ParticleSystemManagerDummy setup #2740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6796abb
fix(particlesys): Simplify ParticleSystemManagerDummy setup
xezon May 15, 2026
00d7bdc
Pass false
xezon May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions Core/GameEngine/Include/GameClient/ParticleSys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,8 @@ class ParticleSystemManager : public SubsystemInterface,
virtual void reset() override; ///< reset the manager and all particle systems
virtual void update() override; ///< update all particle systems

virtual Bool isDummy() const { return false; }

virtual Int getOnScreenParticleCount() = 0; ///< returns the number of particles on screen
virtual void setOnScreenParticleCount(int count);

Expand All @@ -761,8 +763,7 @@ class ParticleSystemManager : public SubsystemInterface,
ParticleSystemTemplate *newTemplate( const AsciiString &name );

/// given a template, instantiate a particle system
ParticleSystem *createParticleSystem( const ParticleSystemTemplate *sysTemplate,
Bool createSlaves = TRUE );
virtual ParticleSystem *createParticleSystem( const ParticleSystemTemplate *sysTemplate, Bool createSlaves = TRUE );

/** given a template, instantiate a particle system.
if attachTo is not null, attach the particle system to the given object.
Expand Down Expand Up @@ -835,15 +836,53 @@ class ParticleSystemManager : public SubsystemInterface,
ParticleSystemIDMap m_systemMap; ///< a hash map of all particle systems
};


// TheSuperHackers @feature bobtista 31/01/2026
// ParticleSystemManager that does nothing. Used for Headless Mode.
// ParticleSystemManager that does nothing. Cannot create particle systems and templates. Used for Headless Mode.
class ParticleSystemManagerDummy : public ParticleSystemManager
{
#if RETAIL_COMPATIBLE_CRC
struct StaticParticleSystemTemplate : public ParticleSystemTemplate
{
StaticParticleSystemTemplate()
: ParticleSystemTemplate("dummy") {}
};
struct StaticParticleSystem : public ParticleSystem
{
StaticParticleSystem(const StaticParticleSystemTemplate *sysTemplate)
: ParticleSystem(sysTemplate, ParticleSystemID(0), false) {}
};
#endif

public:
#if RETAIL_COMPATIBLE_CRC
// Must not overload init to keep loading the particle system templates,
// which are unfortunately needed to preserve the correct logic crc.
#else
virtual void init() override {}
virtual void reset() override {}
#endif
virtual void update() override {}

virtual Bool isDummy() const override { return true; }

virtual Int getOnScreenParticleCount() override { return 0; }
virtual void doParticles(RenderInfoClass &rinfo) override {}
virtual void queueParticleRender() override {}

virtual ParticleSystem *createParticleSystem(const ParticleSystemTemplate *sysTemplate, Bool createSlaves = TRUE) override
{
#if RETAIL_COMPATIBLE_CRC
if (sysTemplate == nullptr)
return nullptr;
static StaticParticleSystemTemplate dummyTemplate;
static StaticParticleSystem dummySystem(&dummyTemplate);
return &dummySystem;
Copy link
Copy Markdown
Author

@xezon xezon May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#else
return nullptr;
#endif
}
Comment on lines +873 to +884
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

@xezon xezon May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


protected:
virtual void crc( Xfer *xfer ) override {}
virtual void xfer( Xfer *xfer ) override {}
Expand Down
2 changes: 0 additions & 2 deletions Core/GameEngine/Source/Common/ReplaySimulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ int ReplaySimulation::simulateReplaysInThisProcess(const std::vector<AsciiString
UnsignedInt totalTimeSec = TheRecorder->getPlaybackFrameCount() / LOGICFRAMES_PER_SECOND;
while (TheRecorder->isPlaybackInProgress())
{
TheGameClient->updateHeadless();

const int progressFrameInterval = 10*60*LOGICFRAMES_PER_SECOND;
if (TheGameLogic->getFrame() != 0 && TheGameLogic->getFrame() % progressFrameInterval == 0)
{
Expand Down
6 changes: 2 additions & 4 deletions Core/GameEngine/Source/GameClient/Drawable/Update/BeaconClientUpdate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ static ParticleSystem* createParticleSystem( Drawable *draw )
AsciiString templateName;
templateName.format("BeaconSmoke%6.6X", (0xffffff & obj->getIndicatorColor()));
const ParticleSystemTemplate *particleTemplate = TheParticleSystemManager->findTemplate( templateName );

DEBUG_ASSERTCRASH(particleTemplate, ("Could not find particle system %s", templateName.str()));

DEBUG_ASSERTCRASH(TheParticleSystemManager->isDummy() || particleTemplate, ("Could not find particle system %s", templateName.str()));
if (particleTemplate)
{
system = TheParticleSystemManager->createParticleSystem( particleTemplate );
Expand All @@ -107,7 +105,7 @@ static ParticleSystem* createParticleSystem( Drawable *draw )
{// THis this will whip up a new particle system to match the house color provided
templateName.format("BeaconSmokeFFFFFF");
const ParticleSystemTemplate *failsafeTemplate = TheParticleSystemManager->findTemplate( templateName );
DEBUG_ASSERTCRASH(failsafeTemplate, ("Doh, this is bad \n I Could not even find the white particle system to make a failsafe system out of."));
DEBUG_ASSERTCRASH(TheParticleSystemManager->isDummy() || failsafeTemplate, ("Doh, this is bad \n I Could not even find the white particle system to make a failsafe system out of."));
system = TheParticleSystemManager->createParticleSystem( failsafeTemplate );
if (system)
{
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngine/Source/GameClient/FXList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ class ParticleSystemFXNugget : public FXNugget
}

const ParticleSystemTemplate *tmp = TheParticleSystemManager->findTemplate(m_name);
DEBUG_ASSERTCRASH(tmp, ("ParticleSystem %s not found",m_name.str()));
DEBUG_ASSERTCRASH(TheParticleSystemManager->isDummy() || tmp, ("ParticleSystem %s not found",m_name.str()));
if (tmp)
{
for (Int i = 0; i < m_count; i++ )
Expand Down
2 changes: 0 additions & 2 deletions GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ class GameClient : public SubsystemInterface,

void step(); ///< Do one fixed time step

void updateHeadless();

void addDrawableToLookupTable( Drawable *draw ); ///< add drawable ID to hash lookup table
void removeDrawableFromLookupTable( Drawable *draw ); ///< remove drawable ID from hash lookup table

Expand Down
9 changes: 0 additions & 9 deletions GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,15 +787,6 @@ void GameClient::step()
TheDisplay->step();
}

void GameClient::updateHeadless()
{
// TheSuperHackers @info helmutbuhler 03/05/2025 bobtista 02/02/2026
// Update particles to prevent accumulation in headless mode. 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().
TheParticleSystemManager->update();
}

Bool GameClient::isMovieAbortRequested()
{
if (TheGameEngine)
Expand Down
Loading