Skip to content

bugfix(particlesys): Delay particle system destruction to prevent orphaning slaved finite-lifetime particle systems - #3238

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
Mauller:Mauller/fix-particle-system-lifetime-handling
Aug 31, 2026
Merged

bugfix(particlesys): Delay particle system destruction to prevent orphaning slaved finite-lifetime particle systems#3238
xezon merged 1 commit into
TheSuperHackers:mainfrom
Mauller:Mauller/fix-particle-system-lifetime-handling

Conversation

@Mauller

@Mauller Mauller commented Aug 30, 2026

Copy link
Copy Markdown

Alternative to: #3071

Closes: #3071
Closed: #2645

This PR alters the lifetime handling of linked particle systems. Preventing a master particle system destroying itself before any slaved particle systems have been destroyed.

The result of a slave system losing its master are the following:

  • Positional data is lost as this comes from the master system, so slave system particles render at the origin.
  • The slave systems m_isDestroyed variable gets set which cuts the systems lifetime short.

The above occurs due to the random nature of particle lifetimes. And when a master particle system destroys itself,
the destructor clears the master of the slave system and calls destroy() which sets the slave systems m_isDestroyed.

With this fix a master particle system checks if it has a finite-lifetime slaved particle system and will not destroy itself till the slaved particle system has been destroyed.
This allows the slaved particle system to fully run the course of its lifetime and render properly.

EDIT - This only applies to finite lifetime particle systems now, it has been tweaked to destroy infinite lifetime systems like it did prior to this change.

@Mauller Mauller self-assigned this Aug 30, 2026
@Mauller Mauller added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Aug 30, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Prevent master particle systems outliving linked slave cleanup

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Keeps destroyed master systems alive until linked slave systems finish destruction.
• Prevents orphaned slave particles from rendering at the world origin.
• Routes lifetime expiry through normal deferred destruction handling.
Diagram

stateDiagram-v2
  direction TD
  [*] --> Active
  Active --> DestroyPending: expiry or destroy
  DestroyPending --> Draining: particles remain
  Draining --> SlaveCheck: particles cleared
  DestroyPending --> SlaveCheck: no particles
  SlaveCheck --> Waiting: slave linked
  Waiting --> SlaveCheck: slave clears link
  SlaveCheck --> Removed: no slave
  Removed --> [*]
Loading
High-Level Assessment

The targeted lifecycle guard is the appropriate approach because it preserves the existing bidirectional master-slave ownership contract and deferred particle cleanup. A broader ownership redesign would add disproportionate risk for this isolated lifetime bug.

Files changed (1) +10 / -3

Bug fix (1) +10 / -3
ParticleSys.cppDefer master removal until slave destruction completes +10/-3

Defer master removal until slave destruction completes

• The update lifecycle now keeps a destroyed master alive while a valid slave remains linked, allowing the slave destructor to clear the relationship first. Natural lifetime expiry marks the system destroyed instead of immediately returning false, routing cleanup through the same deferred path.

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Forever slave retains master 🐞 Bug ☼ Reliability
Description
Natural lifetime expiry now sets only the master's m_isDestroyed, even though destroy() is the
path that propagates destruction to its slave. A linked slave with the supported default
SystemLifetime of zero is forever-lived, so it never clears the link and the master's new wait
guard returns true indefinitely, leaking and updating both systems for the rest of the session.
Code

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2152]

+			m_isDestroyed = true;
Evidence
The template default is a zero system lifetime, and construction explicitly interprets zero as
forever. Slave systems are instantiated from independently configured templates, while only
destroy() propagates destruction to the slave; the changed natural-expiry branch bypasses that
propagation, and the added wait then retains the master as long as the forever slave remains linked.

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[752-765]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1139-1144]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1195-1210]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1320-1328]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2128-2153]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2666-2692]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Natural lifetime expiry marks only the master particle system as destroyed. If its slave has the supported default zero system lifetime, the slave never destroys itself, so the master waits on the slave forever and both systems remain registered and updated.

## Issue Context
`ParticleSystem::destroy()` already marks the current system destroyed and propagates destruction to its slave. A zero `SystemLifetime` is initialized by default and is interpreted as `m_isForever = true`, while the newly added master guard refuses deletion until the slave clears the relationship.

## Fix Focus Areas
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1320-1328]
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2139-2153]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

// check if time is up
if (m_systemLifetimeLeft == 0)
return false;
m_isDestroyed = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Forever slave retains master 🐞 Bug ☼ Reliability

Natural lifetime expiry now sets only the master's m_isDestroyed, even though destroy() is the
path that propagates destruction to its slave. A linked slave with the supported default
SystemLifetime of zero is forever-lived, so it never clears the link and the master's new wait
guard returns true indefinitely, leaking and updating both systems for the rest of the session.
Agent Prompt
## Issue description
Natural lifetime expiry marks only the master particle system as destroyed. If its slave has the supported default zero system lifetime, the slave never destroys itself, so the master waits on the slave forever and both systems remain registered and updated.

## Issue Context
`ParticleSystem::destroy()` already marks the current system destroyed and propagates destruction to its slave. A zero `SystemLifetime` is initialized by default and is interpreted as `m_isForever = true`, while the newly added master guard refuses deletion until the slave clears the relationship.

## Fix Focus Areas
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1320-1328]
- Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2139-2153]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed by checking for forever-lived particles and acting like before the change.

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

The PR changes linked particle-system lifetime handling so a destroyed or naturally expired master remains alive while a finite-lifetime slave finishes.

  • Retains masters that still own finite-lifetime slaves.
  • Preserves immediate master removal for forever-lived slaves.
  • Adds comments documenting the intended lifecycle behavior.

Confidence Score: 4/5

The PR does not appear safe to merge because a forever-lived slave remains registered after its finite master expires.

The prior correction claimed that the master destructor calls destroy() on its slave, but the current destructor only clears their links; the forever-lived slave therefore survives the master and has no natural expiry path.

Files Needing Attention: Core/GameEngine/Source/GameClient/System/ParticleSys.cpp

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Adds finite-slave retention, but the forever-slave exemption still deletes the master while merely detaching and retaining the slave.

Reviews (5): Last reviewed commit: "fix(particlesys): Prevent master particl..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
@Mauller
Mauller force-pushed the Mauller/fix-particle-system-lifetime-handling branch from 18a5e53 to 7a25825 Compare August 30, 2026 17:20
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp
@Mauller

Mauller commented Aug 30, 2026

Copy link
Copy Markdown
Author

Fixed the initial lifetime issue of forever particle systems brought up by the bots.

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does not look bullet proof yet.

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp
@Mauller
Mauller force-pushed the Mauller/fix-particle-system-lifetime-handling branch from 7a25825 to bba9374 Compare August 30, 2026 21:01
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
@Mauller
Mauller force-pushed the Mauller/fix-particle-system-lifetime-handling branch from bba9374 to bfca22f Compare August 31, 2026 10:01
@Mauller

Mauller commented Aug 31, 2026

Copy link
Copy Markdown
Author

Tweaked from recent feedback, should be good now.

@Mauller Mauller changed the title fix(particlesys): Prevent master particle systems destroying themselves before their slave system has destroyed itself bugfix(particlesys): Delay particle system destruction to prevent orphaning slaved finite-lifetime particle systems Aug 31, 2026
Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated
@Mauller
Mauller force-pushed the Mauller/fix-particle-system-lifetime-handling branch from bfca22f to 43450ea Compare August 31, 2026 11:49
@xezon
xezon merged commit b8756d3 into TheSuperHackers:main Aug 31, 2026
23 checks passed
@xezon
xezon deleted the Mauller/fix-particle-system-lifetime-handling branch August 31, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explosions in map corner around coordinates 0, 0, 0

2 participants