[GEN][ZH] Separate ParticleUplinkCannonUpdate's damage radius calculation from LaserUpdate's client update (2)#1061
Merged
helmutbuhler merged 4 commits intoTheSuperHackers:mainfrom Jun 17, 2025
Conversation
…plinkCannonUpdate from the Orbit To Ground Laser drawable (#976)" (#1050)" This reverts commit ec2c22e.
dbc9228 to
e1308f0
Compare
|
Are we absolutely sure that WWMath::Inv_Sqrt always yields the exact same result as before its changes? The PRs say it was tested against golden replay, but maybe it should have tested against more replays, because it is such a basic math function. |
Author
I tested it with 1700 replays, and there is no mismatch. But that's not a 100% guarantee. |
xezon
reviewed
Jun 15, 2025
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp
Outdated
Show resolved
Hide resolved
xezon
approved these changes
Jun 17, 2025
fbraz3
pushed a commit
to fbraz3/GeneralsX
that referenced
this pull request
Nov 10, 2025
…tion from LaserUpdate's client update (2) (TheSuperHackers#1061)
1 task
fbraz3
pushed a commit
to fbraz3/GeneralsX
that referenced
this pull request
Feb 23, 2026
…tion from LaserUpdate's client update (2) (TheSuperHackers#1061)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds the recently reverted Decoupling of Game Logic for the Particle SW again and fixes the incompatibility.
I found the source of the incompatibility, but it's a bit tricky:
For context: Logic separation of the SW was introduced in #874 and #976. Then a mismatch in a replay was discovered and fixed for that replay in #1012. That PR changed this:
if( orbitalBirthFrame <= now && now < orbitalDeathFrame )(Version A)into
const Bool isFiring = m_laserStatus != LASERSTATUS_NONE && m_laserStatus != LASERSTATUS_DEAD; if ( isFiring )(Version B)Around the same time,
WWMath::Inv_Sqrtwas slightly changed in PRs #1009 and #1013.It changed:
WWINLINE __declspec(naked) float __fastcall WWMath::Inv_Sqrt(float a)(Version X)into
WWINLINE float WWMath::Inv_Sqrt(float a)(Version Y)(The implementation was also slightly changed)
Now, which versions are mismatching for that replay in #1012?
Here is a table:
AX: MM
AY: No MM
BX: No MM
BY: No MM
This is very surprising because it seems like changing that math function fixes a mismatch for the Particle SW. It's also surprising that the apparent change of behavior in Version Y causes no mismatches (I checked with all my replays).
Later it was discovered that Version B causes another replay to mismatch, #976 and #1012 was then reverted in #1050.
To make it compatible again, we can simply go to Version AY, which causes no MM, but that feels very brittle because that fix depends on Version Y (which is different from retail). So I did some more testing and found out that changing:
if( orbitalBirthFrame <= now && now < orbitalDeathFrame )(Version A)into
const Bool isFiring = orbitalBirthFrame <= now && now < orbitalDeathFrame; if ( isFiring )(Version C)fixes the incompatibility, even with Version X.
This way, changing A to C fixes the incompatibility, and keeping Version Y also hopefully doesn't hurt. That's what th 8000 is PR does.
As to what causes this crazy behavior: Well,
ParticleUplinkCannonUpdate::update()callsVector2::Normalize(), which callsWWMath::Inv_Sqrt. #976 simplifiedParticleUplinkCannonUpdate::update()a bit and probably caused inlining to be different, causing different floating-point-math. My theory is that Version B and C restore the old inlining by introducing an extra variable. And Version Y fixes it by removing__declspec(naked)I still feel a bit uneasy about Version Y. It's quite possible that it introduces an incompatibility elsewhere, and we simply haven't come across a replay that exposes it. After this PR we can decide to revert to Version X.