refactor(particlesys): Cleanup retail volume particle depth handling by Mauller · Pull Request #3188 · TheSuperHackers/GeneralsGameCode

Mauller · 2026-08-22T09:02:32Z

This PR is a refactor to cleanup the handling of volume depth for volume type and normal particles.

The particle system class originally returned a hard coded value from getVolumeParticleDepth() instead of returning the variable m_volumeParticleDepth.

This value is now retrieved from the particle template and exposed to configuration through the ini field of VolParticleDepth.

To preserve the retail particle behaviour, we identify uninitialised volume particles and set their particle depth to the original hard coded value. Otherwise the configured by ini value will be used.

EDIT - For normal particles we now also initialise their depth to 1.

qodo-free-for-open-source-projects · 2026-08-22T09:03:54Z

PR Summary by Qodo

ParticleSys: Make volume particle depth configurable while preserving retail defaults

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Read volume particle depth from particle templates instead of using a hard-coded constant.
• Add INI support for configuring volume particle depth via "VolParticleDepth".
• Preserve retail behavior by defaulting legacy volume particles to the original depth.

Diagram

graph TD
  A[/"INI: VolParticleDepth"/] --> B["ParticleSystemTemplate"] --> C["ParticleSystem ctor"] --> D["m_volumeParticleDepth"] --> E["getVolumeParticleDepth()"] --> F["Volume particle render"]
  C --> G{"PRESERVE_RETAIL_PARTICLES && depth==DEFAULT?"} --> H["Force OPTIMUM depth (6)"] --> D
  subgraph Legend
    direction LR
    _in[/"INI field"/] ~~~ _proc["Runtime/template" ] ~~~ _dec{"Decision"}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Move retail defaulting into template initialization/parsing

➕ Keeps runtime constructor logic simpler (no retail-specific override in ParticleSystem ctor).
➕ Ensures any other users of the template value see the same retail default consistently.
➖ May require careful ordering to know particle type/name at parse time.
➖ Could be harder to reason about if parsing and compatibility logic become intertwined.

2. Use OPTIMUM as the default and treat explicit INI as override

➕ Eliminates the special-case check for DEFAULT implying retail.
➕ Simplifies behavior: depth is always sensible unless explicitly changed.
➖ Risky if DEFAULT has meaning beyond volume particles elsewhere.
➖ Could unintentionally change non-retail assets that relied on DEFAULT semantics.

Recommendation: Current approach is reasonable for a low-risk refactor: it makes depth configurable while explicitly guarding retail parity. If retail-compatibility logic grows, consider relocating the defaulting into template initialization/parsing to keep ParticleSystem construction focused on value transfer.

Files changed (2) +10 / -2

Bug fix (1) +1 / -1

ParticleSys.hUse configured volume particle depth in accessor +1/-1

Use configured volume particle depth in accessor

• Updates ParticleSystem::getVolumeParticleDepth() to return the instance member m_volumeParticleDepth for VOLUME_PARTICLE instead of a hard-coded constant. Non-volume particles continue to return the default depth value.

Core/GameEngine/Include/GameClient/ParticleSys.h

Other (1) +9 / -1

ParticleSys.cppWire volume depth from template/INI and preserve retail default +9/-1

Wire volume depth from template/INI and preserve retail default

• Initializes ParticleSystem::m_volumeParticleDepth from the template value rather than forcing the default. Adds a retail-compatibility fallback that restores the legacy OPTIMUM depth when no INI depth is configured, and introduces the new INI field parse entry "VolParticleDepth" to populate ParticleSystemTemplate::m_volumeParticleDepth.

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

qodo-free-for-open-source-projects · 2026-08-22T09:05:47Z

Code Review by Qodo

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

1. Vol depth 0 ignored 🐞 Bug ≡ Correctness

Description

When PRESERVE_RETAIL_PARTICLES is enabled, any VOLUME_PARTICLE with m_volumeParticleDepth equal to
DEFAULT_VOLUME_PARTICLE_DEPTH (0) is forcibly changed to OPTIMUM_VOLUME_PARTICLE_DEPTH (6), so an
INI value of VolParticleDepth=0 cannot be honored. This makes it impossible to explicitly configure
depth 0 for volume particles and can unintentionally enable expensive volume rendering when authors
expected depth 0 to disable it.

Code

Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[R1201-1204]

+	// In retail, volume particle depth was not setup through ini and was hard coded to a particle depth of 6
+	if (m_particleType == ParticleType::VOLUME_PARTICLE && m_volumeParticleDepth == DEFAULT_VOLUME_PARTICLE_DEPTH)
+	{
+		m_volumeParticleDepth = OPTIMUM_VOLUME_PARTICLE_DEPTH;

Evidence
The PR’s new retail-compatibility block forces depth 0 to 6 for all volume particles, while the
codebase documents 0 as the default “disable volume” depth and the renderer only uses volume
rendering when depth > 1. Because INI parsing cannot distinguish “unset” from an explicit 0, the
override prevents intentionally configuring depth 0.
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1193-1205]
Core/GameEngine/Include/GameClient/ParticleSys.h[60-62]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[328-334]

Agent prompt

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

### Issue description
`VolParticleDepth` is parsed into `m_volumeParticleDepth` with a default of `DEFAULT_VOLUME_PARTICLE_DEPTH` (0). Under `PRESERVE_RETAIL_PARTICLES`, the constructor treats `m_volumeParticleDepth == 0` as “retail/unset” and overwrites it to 6, which also overwrites an explicit INI configuration of `VolParticleDepth=0`.

### Issue Context
- `DEFAULT_VOLUME_PARTICLE_DEPTH` is documented as “not to do the volume thing” (0).
- Volume rendering is only used when depth > 1.
- Current logic cannot tell whether depth==0 came from “field missing” or “field explicitly set to 0”.

### Fix Focus Areas
- Implement an explicit "was specified" signal (or sentinel value) for `VolParticleDepth`, and only apply the retail override when the field was not specified.
- file: Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[2756-2760]
- file: Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[1193-1205]
- file: Core/GameEngine/Include/GameClient/ParticleSys.h[60-63]

### Suggested approach
- Add a `bool m_volumeParticleDepthSpecified` (or similar) to `ParticleSystemTemplate` (or `ParticleSystemInfo` if appropriate).
- Replace the parse-table entry for `VolParticleDepth` with a custom parser that sets both `m_volumeParticleDepth` and `m_volumeParticleDepthSpecified=true`.
- Change the retail-compat block to:
 - if volume particle AND `!m_volumeParticleDepthSpecified` then set to `OPTIMUM_VOLUME_PARTICLE_DEPTH`.
 - otherwise honor the configured value, including 0.

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

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

xezon · 2026-08-22T16:06:32Z

+	}
+
+	// In retail, volume particle depth was not setup through ini and was hard coded to a particle depth of 6
+	if (sysTemplate->m_particleType == ParticleSystemInfo::VOLUME_PARTICLE && sysTemplate->m_volumeParticleDepth == DEFAULT_VOLUME_PARTICLE_DEPTH)


This means if someone now sets VolParticleDepth to 0 in the INI explicitly, then it overwrites it here. That does not seem right. Perhaps it should only set it if the INI field was not set. Or is 0 an invalid setting for Volume particles?

Zero and One are invalid settings anyway.

There are tests in the render code for the volume depth being greater than One.

The value is also initialised to zero in the template.

If the particle depth is set to zero or one, then the volume particle just gets rendered as a standard particle in this instance.

void PointGroupClass::RenderVolumeParticle(RenderInfoClass &rinfo, unsigned int depth ) { if ( depth <= 1 ) //oops,wrong number { Render( rinfo ); return; }

Which kind of voids the point of it being a volume particle. But seems more like a safety net.

When looking further into the RenderVolumeParticle code the reciprocal of the depth is take which would cause a divide by zero error if 0 was a valid depth. Not sure why One is not considered though from the quick glance i took.

How about do

#define INVALID_VOLUME_PARTICLE_DEPTH ( 0 ) #define DEFAULT_VOLUME_PARTICLE_DEPTH ( 1 ) // The Default is not to do the volume thing! #define OPTIMUM_VOLUME_PARTICLE_DEPTH ( 6 )

Then initialize particle template depth with invalid, and then depending on the particle type parsed from ini, choose 1 or 6, regardless of retail.

Well the problem is that we only consider getting as far as the rendering if the volume is 2 or higher anyway.

That's where i added the MIN_VOLUME_PARTICLE_DEPTH ( 2 ) in the particle batching PR since that is what the retail code tests in doParticles to determine if it handles a volume particle.

So even set to 1 the volume particle is never rendered. Even if it the rendering code has a failsafe to render as a regular particle.

The workaround was always more about catching the non configured particles from retail.

For non retail and mod's the particle editor should always put a minimum of 2 as the particle depth if a volume particle is selected. Otherwise it should be considered misconfigured and not render etc.

This is more just a hack to keep retail particles working that lack the configuration essentially while opening up the particle depth option for mods and future etc.

I dont quite follow.

The proposed flow is to initialize with invalid, and then set 1 or 6 depending on the type wehn invalid. This way it always works the same way reliably and needs to retail guarding.

OmarAglan · 2026-08-22T20:34:30Z

The overall direction looks good, but I think two issues should be addressed before merging:

The retail fallback cannot distinguish an omitted VolParticleDepth from an explicitly configured value of 0. Depths 0 and 1 are handled safely by the rendering path by falling back to standard particle rendering, so 0 appears to be a meaningful way to disable volume rendering. It may be cleaner to default volume templates to OPTIMUM_VOLUME_PARTICLE_DEPTH and allow the INI value to override it, including with 0.
VolParticleDepth is parsed, but the Generals and Zero Hour _writeSingleParticleSystem() implementations do not write it. Because the particle editor regenerates ParticleSystem.ini, saving the file would silently discard a configured depth.

Other than these configuration and round-trip concerns, the change looks clean and the CI results are good.

xezon · 2026-08-23T07:54:01Z

+	}
+
+	// In retail, volume particle depth was not setup through ini and was hard coded to a particle depth of 6
+	if (sysTemplate->m_particleType == ParticleSystemInfo::VOLUME_PARTICLE && sysTemplate->m_volumeParticleDepth == DEFAULT_VOLUME_PARTICLE_DEPTH)


How about do

#define INVALID_VOLUME_PARTICLE_DEPTH ( 0 ) #define DEFAULT_VOLUME_PARTICLE_DEPTH ( 1 ) // The Default is not to do the volume thing! #define OPTIMUM_VOLUME_PARTICLE_DEPTH ( 6 )

Then initialize particle template depth with invalid, and then depending on the particle type parsed from ini, choose 1 or 6, regardless of retail.

Mauller · 2026-08-23T09:00:29Z

Here's a test showing different levels of volume particle depth.

One appears to not work very well in this instance if you let the volume particle code work. but this could also be down to placement of the particle emitter on the microwave tank. It looks like the particle is there but it pretty much renders billboard and flat, you can partly see the line of it across the top of the flat portion of the microwave emitter / turret.

Screenshots

This is as volume particle set to 1

This is letting it render as a normal particle in this instance using the escape code.

Two you can minimally start to see the particle effect

Three is significantly more noticeable

depth four

depth five

Retail depth of "Optimal" at 6

I am going to jump a bit for the next ones to every second level to 16 which is considered the max.

Depth 8

Depth 10

Depth 12

Going to jump to 16 here as i think people get the point

Brighter than the SUN! if only we had HDR for this we could blind people.

Mauller · 2026-08-23T13:38:18Z

Fixed based on feedback and did a little more cleanup around it as well.

xezon · 2026-08-23T17:31:34Z

 	Bool isUsingStreak() { return (m_particleType == STREAK) ? true : false; }
 	Bool isUsingSmudge() { return (m_particleType == SMUDGE) ? true : false; }
-	UnsignedInt getVolumeParticleDepth() { return ( m_particleType == VOLUME_PARTICLE ) ? OPTIMUM_VOLUME_PARTICLE_DEPTH : 0; }
+	UnsignedInt getVolumeParticleDepth() { return ( m_particleType == VOLUME_PARTICLE ) ? m_volumeParticleDepth : DEFAULT_VOLUME_PARTICLE_DEPTH; }


Assuming the template now holds a validated depth value, the condition here should no longer be needed?

I was considering that, maybe i split it into two functions, one like the others that checks for the particle type of VOLUME_PARTICLE then have the original return m_volumeParticleDepth.

xezon · 2026-08-23T17:36:26Z

+
+	// TheSuperHackers @info Initialise all volume particles that lack ini configuration to the optimum depth of 6
+	// In retail, volume particle depth was not configurable through ini and was hard coded to a particle depth of 6
+	if (sysTemplate->m_particleType == ParticleSystemInfo::VOLUME_PARTICLE && sysTemplate->m_volumeParticleDepth == INVALID_VOLUME_PARTICLE_DEPTH)


How about move these validation steps int a new function inside the template class? I have seen the same approach with LocomotorTemplate::validate.

I can do, then call validate after the load.

Mauller · 2026-08-23T21:24:03Z

Updated with recent suggestions along with a little extra cleanup around the same area.

Mauller · 2026-08-23T21:25:16Z

+	Bool isUsingDrawables() { return m_particleType == DRAWABLE; }
+	Bool isUsingStreak() { return m_particleType == STREAK; }
+	Bool isUsingSmudge() { return m_particleType == SMUDGE; }
+	BOOL isUsingVolumeParticles() { return m_particleType == VOLUME_PARTICLE; }


Only just noticed that i used BOOL instead of bool but i will fix this after the next review so the diff can be seen.

Mauller self-assigned this Aug 22, 2026

Mauller added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Refactor Edits the code with insignificant behavior changes, is never user facing labels Aug 22, 2026

qodo-free-for-open-source-projects Bot reviewed Aug 22, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Outdated

xezon reviewed Aug 22, 2026

View reviewed changes

Mauller force-pushed the Mauller/chore-cleanup-vol-particle-depth-handling branch from bbbc607 to a596261 Compare August 22, 2026 14:53

xezon reviewed Aug 22, 2026

View reviewed changes

Mauller force-pushed the Mauller/chore-cleanup-vol-particle-depth-handling branch from a596261 to 291a2e3 Compare August 22, 2026 21:48

xezon reviewed Aug 23, 2026

View reviewed changes

Mauller force-pushed the Mauller/chore-cleanup-vol-particle-depth-handling branch from 291a2e3 to 53eef90 Compare August 23, 2026 08:38

Mauller force-pushed the Mauller/chore-cleanup-vol-particle-depth-handling branch from 53eef90 to c32d92c Compare August 23, 2026 13:37

xezon reviewed Aug 23, 2026

View reviewed changes

refactor(particlesys): Cleanup retail volume particle depth handling

ffa3dc6

Mauller force-pushed the Mauller/chore-cleanup-vol-particle-depth-handling branch from c32d92c to ffa3dc6 Compare August 23, 2026 21:23

Mauller commented Aug 23, 2026

View reviewed changes

Uh oh!

Conversation

Mauller commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 22, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 22, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmarAglan commented Aug 22, 2026

Uh oh!

Uh oh!

xezon Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller commented Aug 23, 2026 • edited by Caball009 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mauller commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mauller commented Aug 22, 2026 •

edited

Loading

Mauller Aug 22, 2026 •

edited

Loading

xezon Aug 23, 2026 •

edited

Loading

Mauller Aug 23, 2026 •

edited

Loading

xezon Aug 23, 2026 •

edited

Loading

Mauller commented Aug 23, 2026 •

edited by Caball009

Loading