Skip to content

fix(particlesys): Decouple particle systems from logic crc #2742

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 3 commits into
base: main
Choose a base branch
from
Open

fix(particlesys): Decouple particle systems from logic crc #2742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f355793
fix(particlesys): Decouple particle systems from logic crc
xezon May 22, 2026
edfee4a
Simplify names
xezon May 22, 2026
bbdce66
Fix macro arguments
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
20 changes: 20 additions & 0 deletions Core/GameEngine/Include/Common/RandomValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,24 @@ extern void InitRandom( UnsignedInt seed );
extern UnsignedInt GetGameLogicRandomSeed(); ///< Get the seed (used for replays)
extern UnsignedInt GetGameLogicRandomSeedCRC();///< Get the seed (used for CRCs)

struct RandomValueClass
Copy link
Copy Markdown

@Caball009 Caball009 May 24, 2026

Choose a reason for hiding this comment

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

These structs aren't necessary anymore after we break retail compatibility, right? Perhaps there could be a TSH todo here to remove them when we do.

{
virtual Int GetRandomValueInt( Int lo, Int hi, const char *file, Int line ) const = 0;
virtual Real GetRandomValueReal( Real lo, Real hi, const char *file, Int line ) const = 0;
};
struct LogicRandomValueClass final : RandomValueClass
{
virtual Int GetRandomValueInt( Int lo, Int hi, const char *file, Int line ) const override;
Copy link
Copy Markdown

@Caball009 Caball009 May 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Could make these virtual functions final, but I doubt it'll make a difference for modern compilers (considering the struct is already marked final).

Copy link
Copy Markdown
Author

@xezon xezon May 24, 2026

Choose a reason for hiding this comment

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

The class itself is marked final, which has the same effect.

virtual Real GetRandomValueReal( Real lo, Real hi, const char *file, Int line ) const override;
};
struct ClientRandomValueClass final : RandomValueClass
{
virtual Int GetRandomValueInt( Int lo, Int hi, const char *file, Int line ) const override;
virtual Real GetRandomValueReal( Real lo, Real hi, const char *file, Int line ) const override;
};

// use these macros to access the random value functions
#define RandomValueInt(randomValueClass, lo, hi) randomValueClass.GetRandomValueInt( lo, hi, __FILE__, __LINE__ )
#define RandomValueReal(randomValueClass, lo, hi) randomValueClass.GetRandomValueReal( lo, hi, __FILE__, __LINE__ )

//--------------------------------------------------------------------------------------------------------------
21 changes: 21 additions & 0 deletions Core/GameEngine/Source/Common/RandomValue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,24 @@ Real GameLogicRandomVariable::getValue() const
return 0.0f;
}
}


Int LogicRandomValueClass::GetRandomValueInt( Int lo, Int hi, const char *file, Int line ) const
{
return GetGameLogicRandomValue(lo, hi, file, line);
}

Real LogicRandomValueClass::GetRandomValueReal( Real lo, Real hi, const char *file, Int line ) const
{
return GetGameLogicRandomValueReal(lo, hi, file, line);
}

Int ClientRandomValueClass::GetRandomValueInt( Int lo, Int hi, const char *file, Int line ) const
{
return GetGameClientRandomValue(lo, hi, file, line);
}

Real ClientRandomValueClass::GetRandomValueReal( Real lo, Real hi, const char *file, Int line ) const
{
return GetGameClientRandomValueReal(lo, hi, file, line);
}
3 changes: 2 additions & 1 deletion GeneralsMD/Code/GameEngine/Include/Common/Geometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

#include "Lib/BaseType.h"
#include "Common/AsciiString.h"
#include "Common/RandomValue.h"
#include "Common/Snapshot.h"

class INI;
Expand Down Expand Up @@ -172,7 +173,7 @@ class GeometryInfo : public Snapshot
void get2DBounds(const Coord3D& geomCenter, Real angle, Region2D& bounds ) const;

/// note that the pt is generated using game logic random, not game client random!
Copy link
Copy Markdown

@Caball009 Caball009 May 24, 2026

Choose a reason for hiding this comment

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

This comment is now inaccurate, I think.

void makeRandomOffsetWithinFootprint(Coord3D& pt) const;
void makeRandomOffsetWithinFootprint(Coord3D& pt, const RandomValueClass& random = LogicRandomValueClass()) const;
void makeRandomOffsetOnPerimeter(Coord3D& pt) const; //Chooses a random point on the extent border.

void clipPointToFootprint(const Coord3D& geomCenter, Coord3D& ptToClip) const;
Expand Down
14 changes: 7 additions & 7 deletions GeneralsMD/Code/GameEngine/Source/Common/System/Geometry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ Bool GeometryInfo::isPointInFootprint(const Coord3D& geomCenter, const Coord3D&
}

//=============================================================================
void GeometryInfo::makeRandomOffsetWithinFootprint(Coord3D& pt) const
void GeometryInfo::makeRandomOffsetWithinFootprint(Coord3D& pt, const RandomValueClass& random) const
{
switch(m_type)
{
Expand All @@ -390,14 +390,14 @@ void GeometryInfo::makeRandomOffsetWithinFootprint(Coord3D& pt) const
Real distSqr;
do
{
pt.x = GameLogicRandomValueReal(-m_majorRadius, m_majorRadius);
pt.y = GameLogicRandomValueReal(-m_majorRadius, m_majorRadius);
pt.x = RandomValueReal(random, -m_majorRadius, m_majorRadius);
pt.y = RandomValueReal(random, -m_majorRadius, m_majorRadius);
pt.z = 0.0f;
distSqr = sqr(pt.x) + sqr(pt.y);
} while (distSqr > maxDistSqr);
#else
Real radius = GameLogicRandomValueReal(0.0f, m_boundingCircleRadius);
Real angle = GameLogicRandomValueReal(-PI, PI);
Real radius = RandomValueReal(random, 0.0f, m_boundingCircleRadius);
Real angle = RandomValueReal(random, -PI, PI);
pt.x = radius * Cos(angle);
pt.y = radius * Sin(angle);
pt.z = 0.0f;
Expand All @@ -407,8 +407,8 @@ void GeometryInfo::makeRandomOffsetWithinFootprint(Coord3D& pt) const

case GEOMETRY_BOX:
{
pt.x = GameLogicRandomValueReal(-m_majorRadius, m_majorRadius);
pt.y = GameLogicRandomValueReal(-m_minorRadius, m_minorRadius);
pt.x = RandomValueReal(random, -m_majorRadius, m_majorRadius);
pt.y = RandomValueReal(random, -m_minorRadius, m_minorRadius);
pt.z = 0.0f;
break;
}
Expand Down
11 changes: 8 additions & 3 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Damage/TransitionDamageFX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ void TransitionDamageFX::onDelete()
/** Given an FXLoc info struct, return the effect position that we are supposed to use.
* The position is local to to the object */
//-------------------------------------------------------------------------------------------------
static Coord3D getLocalEffectPos( const FXLocInfo *locInfo, Drawable *draw )
static Coord3D getLocalEffectPos( const FXLocInfo *locInfo, Drawable *draw, const RandomValueClass &random = LogicRandomValueClass() )
{

DEBUG_ASSERTCRASH( locInfo, ("getLocalEffectPos: locInfo is null") );
Expand Down Expand Up @@ -290,7 +290,7 @@ static Coord3D getLocalEffectPos( const FXLocInfo *locInfo, Drawable *draw )
return locInfo->loc;

// pick one of the bone positions
Int pick = GameLogicRandomValue( 0, boneCount - 1 );
Int pick = RandomValueInt( random, 0, boneCount - 1 );
return positions[ pick ];

}
Expand Down Expand Up @@ -387,14 +387,19 @@ void TransitionDamageFX::onBodyDamageStateChange( const DamageInfo* damageInfo,
if( lastDamageInfo == nullptr ||
getDamageTypeFlag( modData->m_damageParticleTypes, lastDamageInfo->in.m_damageType ) )
{
#if RETAIL_COMPATIBLE_CRC
// TheSuperHackers @fix The particle system is now decoupled from the logic crc
// and the legacy logic random values are preserved here.
Copy link
Copy Markdown

@Caball009 Caball009 May 24, 2026

Choose a reason for hiding this comment

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

// and the legacy logic random values are preserved here.

I don't think this explains to an uninformed reader what's going on here. Perhaps "and the side effects on the logical seed values are preserved here for retail compatibility" ?

getLocalEffectPos( &modData->m_particleSystem[ newState ][ i ].locInfo, draw, LogicRandomValueClass() );
#endif

// create a new particle system based on the template provided
ParticleSystem* pSystem = TheParticleSystemManager->createParticleSystem( pSystemT );
if( pSystem )
{

// get the what is the position we're going to played the effect at
pos = getLocalEffectPos( &modData->m_particleSystem[ newState ][ i ].locInfo, draw );
pos = getLocalEffectPos( &modData->m_particleSystem[ newState ][ i ].locInfo, draw, ClientRandomValueClass() );

//
// set position on system given any bone position provided, the bone position is
Expand Down
16 changes: 13 additions & 3 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/EMPUpdate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,24 @@ void EMPUpdate::doDisableAttack()

for (UnsignedInt e = 0 ; e < emitterCount; ++e)
{
#if RETAIL_COMPATIBLE_CRC
// TheSuperHackers @fix The particle system is now decoupled from the logic crc
// and the legacy logic random values are preserved here.
{
Coord3D offs = {0,0,0};
curVictim->getGeometryInfo().makeRandomOffsetWithinFootprint( offs, LogicRandomValueClass() );
GameLogicRandomValue(3, victimHeight);
GameLogicRandomValue(1, 100);
}
Comment on lines +307 to +315
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P1 CRC dummy calls placed outside the if (sys) null-check

In the original code all three logic-random calls (makeRandomOffsetWithinFootprint, GameLogicRandomValue(3, victimHeight), GameLogicRandomValue(1, 100)) were inside if (sys). The new RETAIL_COMPATIBLE_CRC block runs them unconditionally, before createParticleSystem is even called. If createParticleSystem returns null the original code would not have advanced the logic RNG at all, but the CRC block advances it anyway, producing a divergent RNG state and breaking the replay CRC that the block is supposed to preserve.

The same structural mistake is present in TransitionDamageFX.cpp (the getLocalEffectPos dummy call before if (pSystem)) and SpecialAbilityUpdate.cpp (the makeRandomOffsetWithinFootprint dummy call before if (sys)). All three blocks should be moved inside their respective if (sys/pSystem) guards to exactly mirror the original code path.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/EMPUpdate.cpp
Line: 307-315

Comment:
**CRC dummy calls placed outside the `if (sys)` null-check**

In the original code all three logic-random calls (`makeRandomOffsetWithinFootprint`, `GameLogicRandomValue(3, victimHeight)`, `GameLogicRandomValue(1, 100)`) were inside `if (sys)`. The new `RETAIL_COMPATIBLE_CRC` block runs them unconditionally, before `createParticleSystem` is even called. If `createParticleSystem` returns null the original code would not have advanced the logic RNG at all, but the CRC block advances it anyway, producing a divergent RNG state and breaking the replay CRC that the block is supposed to preserve.

The same structural mistake is present in `TransitionDamageFX.cpp` (the `getLocalEffectPos` dummy call before `if (pSystem)`) and `SpecialAbilityUpdate.cpp` (the `makeRandomOffsetWithinFootprint` dummy call before `if (sys)`). All three blocks should be moved inside their respective `if (sys/pSystem)` guards to exactly mirror the original code path.

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.

This is fine for as long as it is inside the if (tmp) branch.

Copy link
Copy Markdown

@Caball009 Caball009 May 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need a comment for that? Otherwise it might be tempting to combine both checks in a similar effort as #2724 I don't have a strong opinion on it either way FWIW.

#endif

ParticleSystem *sys = TheParticleSystemManager->createParticleSystem(tmp);

if (sys)
{
Coord3D offs = {0,0,0};
curVictim->getGeometryInfo().makeRandomOffsetWithinFootprint( offs );
offs.z = GameLogicRandomValue(3, victimHeight);
curVictim->getGeometryInfo().makeRandomOffsetWithinFootprint( offs, ClientRandomValueClass() );
offs.z = GameClientRandomValue(3, victimHeight);

//This puts all the sparks within a quadrahemicycloid (rectangular dome) volume
//The same shape as a four cornered camping dome tent, for those with less Greek
Expand All @@ -328,7 +338,7 @@ void EMPUpdate::doDisableAttack()
sys->attachToObject(curVictim);
sys->setPosition( &offs );
sys->setSystemLifetime(MAX(0, data->m_disabledDuration - 30));
sys->setInitialDelay(GameLogicRandomValue(1,100));
sys->setInitialDelay(GameClientRandomValue(1,100));
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/SpecialAbilityUpdate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1400,11 +1400,20 @@ void SpecialAbilityUpdate::triggerAbilityEffect()
const ParticleSystemTemplate *tmp = data->m_disableFXParticleSystem;
if (tmp)
{
#if RETAIL_COMPATIBLE_CRC
// TheSuperHackers @fix The particle system is now decoupled from the logic crc
// and the legacy logic random values are preserved here.
{
Coord3D offs = {0,0,0};
target->getGeometryInfo().makeRandomOffsetWithinFootprint( offs, LogicRandomValueClass() );
}
#endif

ParticleSystem *sys = TheParticleSystemManager->createParticleSystem(tmp);
if (sys)
{
Coord3D offs = {0,0,0};
target->getGeometryInfo().makeRandomOffsetWithinFootprint( offs );
target->getGeometryInfo().makeRandomOffsetWithinFootprint( offs, ClientRandomValueClass() );

sys->attachToObject(target);
sys->setPosition( &offs );
Expand Down
Loading