Skip to content

Conversation

@xezon
Copy link

@xezon xezon commented Jan 19, 2026

Merge with Rebase

This change merges all Particle System related code, including Particle Editor code. All individual changes are split into separate commits for best visibility.

Zero Hour gets

Generals gets

  • Removal of unused Drawable Particle Attachments
  • Addition of ParticleSystem::setSkipParentXfrm
  • Compile out of superfluous X and Y particle angles
  • Addition of SMUDGE particle type (used for Microwave Tank effect)
  • Optimization for particle update
  • Optimization for wind motion particle update when disabled
  • Change of math for aligning particle Z rotation with emitter direction

TODO

  • Add pull id to commits

@xezon xezon added this to the Code foundation build up milestone Jan 19, 2026
@xezon xezon added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Jan 19, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 19, 2026

Greptile Summary

This PR unifies particle system code between Generals and Zero Hour by merging features and optimizations.

Major Changes

  • Removed Drawable Particle Attachments - Generals particle system no longer supports attaching Drawable objects to particles, aligning with Zero Hour architecture
  • Compiled out X/Y rotation - Both versions now use PARTICLE_USE_XY_ROTATION=0 macro since particles only support billboard (camera-facing) and ground-aligned modes which only use Z-axis rotation
  • Added SMUDGE particle type - New particle type for Microwave Tank effects in Generals
  • Added setSkipParentXfrm() method - Allows particle systems already in world space to skip parent transform concatenation
  • Fixed particle visibility thresholds - Zero Hour's isInvisible() now uses correct thresholds (0.01 instead of 0.02/0.06 for sum/product checks, changed to per-component checks)
  • Optimized wind motion - Only processes wind motion when windMotion != WIND_MOTION_NOT_USED
  • Optimized alpha updates - Skips alpha computation for ADDITIVE shader type particles
  • Changed angle calculation - Replaced inline math with angleBetween() helper function for computing particle Z rotation aligned with emitter direction

Issues Found

  • Performance issue in angleBetween() function: calls length() multiple times unnecessarily (2 calls in null check + 2 calls for actual calculation = 4 total sqrt operations instead of 2)

Confidence Score: 4/5

  • This PR is safe to merge after fixing the performance issue in angleBetween()
  • The changes are well-structured with clear separation into commits, and most are straightforward refactorings (removing unused drawable support, conditional compilation of unused X/Y rotation). The particle visibility threshold fix addresses a real bug. However, the angleBetween() function contains an inefficiency that calls expensive sqrt operations 4 times instead of 2, which impacts performance in every frame for particles with m_particleUpTowardsEmitter enabled.
  • Both ParticleSys.cpp files need the angleBetween() performance fix before merge

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Include/GameClient/ParticleSys.h Added PARTICLE_USE_XY_ROTATION macro, conditionally compiled X/Y rotation fields, added SMUDGE particle type, and setSkipParentXfrm method
Generals/Code/GameEngine/Source/GameClient/System/ParticleSys.cpp Removed drawable particle support, conditionally compiled X/Y rotation code, optimized wind motion and alpha updates, changed angle calculation math - contains logic issue in angleBetween function
Generals/Code/GameEngine/Include/GameClient/Drawable.h Removed particle system attachment methods and m_particle member field
GeneralsMD/Code/GameEngine/Source/GameClient/System/ParticleSys.cpp Conditionally compiled X/Y rotation code, fixed particle visibility thresholds in isInvisible, improved whitespace formatting - contains logic issue in angleBetween function

Sequence Diagram

sequenceDiagram
    participant PS as ParticleSystem
    participant P as Particle
    participant PI as ParticleInfo
    participant D as Drawable (removed)
    
    Note over PS,D: Before: Particle System with Drawable Support
    PS->>PI: generateParticleInfo()
    PI->>P: new Particle(system, info)
    P->>D: Create Drawable if type==DRAWABLE
    D-->>P: Attach to particle
    P->>P: update() - update drawable position/rotation
    P->>D: setInstanceMatrix(), setPosition()
    
    Note over PS,D: After: Unified Particle System (No Drawables)
    PS->>PI: generateParticleInfo()
    Note over PI: X/Y rotation compiled out (PARTICLE_USE_XY_ROTATION=0)
    PI->>P: new Particle(system, info)
    Note over P,D: No drawable creation
    P->>P: update() - optimize wind motion check
    P->>P: update() - optimize alpha update (skip if ADDITIVE)
    P->>P: angleBetween() - calculate Z rotation for emitter direction
    
    Note over PS: Additional Optimizations
    PS->>PS: setSkipParentXfrm() - skip parent transform if in world space
    P->>P: isInvisible() - fixed thresholds (0.01 vs 0.02/0.06)
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (2)

  1. Generals/Code/GameEngine/Source/GameClient/System/ParticleSys.cpp, line 217-220 (link)

    logic: The null check calls length() twice per vector unnecessarily, then calls it again below. Since length() involves a square root calculation (sqrt(x*x + y*y)), this is wasteful. Calculate once and check for zero length:

  2. GeneralsMD/Code/GameEngine/Source/GameClient/System/ParticleSys.cpp, line 217-220 (link)

    logic: The null check calls length() twice per vector unnecessarily, then calls it again below. Since length() involves a square root calculation (sqrt(x*x + y*y)), this is wasteful. Calculate once and check for zero length:

14 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@xezon
Copy link
Author

xezon commented Jan 19, 2026

Very fair comment, but this is EA code and outside the scope of this change.

Edit: I have prepared an optimization for later.

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 Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Unify Unifies code between Generals and Zero Hour ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant