Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
{
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;
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
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
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!
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
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
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.
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
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

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

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.

#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
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