-
Notifications
You must be signed in to change notification settings - Fork 202
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.
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
Changes from all commits
f355793
edfee4a
bbdce66
bbe06dd
e2bd719
7ce6de8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 side effects on the logic random seed values are preserved for retail compatibility. | ||
| { | ||
| Coord3D offs = {0,0,0}; | ||
| curVictim->getGeometryInfo().makeRandomOffsetWithinFootprint( offs, LogicRandomValueClass() ); | ||
| GameLogicRandomValue(3, victimHeight); | ||
| GameLogicRandomValue(1, 100); | ||
| } | ||
|
Comment on lines
+307
to
+315
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the original code all three logic-random calls ( The same structural mistake is present in Prompt To Fix With AIThis 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine for as long as it is inside the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think a comment is needed. Moving logic out of conditions will always the carry risk of logic mismatch and this one is not special. |
||
| #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 | ||
|
|
@@ -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)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.