Skip to content

bugfix(fxlist): Fix unreliable terrain layer selection of particles using CreateAtGroundHeight - #3004

Merged
xezon merged 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:bugfix/particles-createatgroundheight-layer
Jul 23, 2026
Merged

bugfix(fxlist): Fix unreliable terrain layer selection of particles using CreateAtGroundHeight#3004
xezon merged 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:bugfix/particles-createatgroundheight-layer

Conversation

@stephanmeesters

Copy link
Copy Markdown

When creating a particle system that uses the CreateAtGroundHeight option in the INI, the current terrain layer (for example GROUND, or BRIDGE) was not reliably determined because of an uninitialized variable.

This mostly affects toxin pools on a bridge which end up on the ground layer instead, see the images.

Before (note the particles on the ground):
Screenshot From 2026-07-22 21-56-56

After (most of the particles are now on the bridge, some are on the edge and float):
Screenshot From 2026-07-22 21-54-52

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes an uninitialized variable bug in ParticleSystemFXNugget::reallyDoFX where newPos.z was not set before being passed to getLayerForDestination, causing unreliable terrain layer selection for particles using CreateAtGroundHeight.

  • newPos.z is now initialized to primary->z + offset.z before the m_createAtGroundHeight branch, giving getLayerForDestination a meaningful z-coordinate to determine whether the spawn point is on a bridge or ground layer.
  • The else branch is refactored from a full assignment (= primary->z + offset.z + m_height.getValue()) to an in-place add (+= m_height.getValue()), which is mathematically equivalent and preserves existing non-ground-height behavior.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted fix for an uninitialized variable with no behavioral side effects on non-ground-height paths.

The fix correctly initializes newPos.z before it is read by getLayerForDestination, eliminating the undefined behavior. The else-branch rewrite is mathematically identical to the original. No other paths are affected.

No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/FXList.cpp Initializes newPos.z before calling getLayerForDestination to fix uninitialized variable causing wrong terrain layer selection; else-branch refactored to equivalent += expression.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["reallyDoFX called"] --> B["Compute newPos.x, newPos.y via offset + radius * cos/sin"]
    B --> C["newPos.z = primary->z + offset.z\n(NEW: initialized here)"]
    C --> D{"m_createAtGroundHeight\n&& TheTerrainLogic?"}
    D -- Yes --> E["getLayerForDestination(&newPos)\n(now uses valid z for layer detection)"]
    E --> F["layer = GROUND or BRIDGE"]
    F --> G["newPos.z = getLayerHeight(x, y, layer)"]
    D -- No --> H["newPos.z += m_height.getValue()\n(equivalent to old assignment)"]
    G --> I["setPosition(&newPos)"]
    H --> I
Loading

Reviews (2): Last reviewed commit: "Process review comment" | Re-trigger Greptile

@Mauller

Mauller commented Jul 22, 2026

Copy link
Copy Markdown

seems quite simple, does this also affect burn marks being applied to the bridge?

@stephanmeesters

Copy link
Copy Markdown
Author

seems quite simple, does this also affect burn marks being applied to the bridge?

Scorch marks are different as they are not particles and have their own render path (because they persist the entire match, and you have a max number of scorches)

@Mauller

Mauller commented Jul 23, 2026

Copy link
Copy Markdown

Scorch marks are different as they are not particles and have their own render path (because they persist the entire match, and you have a max number of scorches)

Ah i see, that's cool. Does this also affect how particles are placed over uneven terrain? Or is that a different problem?

@bobtista

Copy link
Copy Markdown

Good find! Looks good to me

@stephanmeesters stephanmeesters added Bug Something is not working right, typically is user facing Gen Relates to Generals ZH Relates to Zero Hour Rendering Is Rendering related labels Jul 23, 2026

@bobtista bobtista 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.

LGTM 👍

@stephanmeesters

stephanmeesters commented Jul 23, 2026

Copy link
Copy Markdown
Author

Ah i see, that's cool. Does this also affect how particles are placed over uneven terrain? Or is that a different problem?

This change should not affect particle placement at locations where there is only one terrain layer because getLayerForDestination will return GROUND regardless of the Z value.

For particles on uneven terrain I'm working on terrain conforming particles, which instead of the regular method of drawing quads will make tiny meshes by sampling the terrain map, unless that particle is on a bridge in which case its probably best to just draw as a quad

@Mauller

Mauller commented Jul 23, 2026

Copy link
Copy Markdown

This change should not affect particle placement at locations where there is only one terrain layer because getLayerForDestination will return GROUND regardless of the Z value.

For particles on uneven terrain I'm working on terrain conforming particles, which instead of the regular method of drawing quads will make tiny meshes by sampling the terrain map, unless that particle is on a bridge in which case its probably best to just draw as a quad

That's cool, from your pictures showing the terrain conforming particles i thought this might have been part of it.

@Mauller Mauller 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.

Looks good to me.

@stephanmeesters stephanmeesters added the Minor Severity: Minor < Major < Critical < Blocker label Jul 23, 2026
Comment thread Core/GameEngine/Source/GameClient/FXList.cpp Outdated
@xezon
xezon merged commit fad7935 into TheSuperHackers:main Jul 23, 2026
17 checks passed
@stephanmeesters
stephanmeesters deleted the bugfix/particles-createatgroundheight-layer branch July 26, 2026 17:23
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 Rendering Is Rendering related ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants