Refactor wwmath and fix issues in weapon and EulerAnglesClass - #7
Open
Okladnoj wants to merge 7 commits into
Open
Refactor wwmath and fix issues in weapon and EulerAnglesClass#7Okladnoj wants to merge 7 commits into
Okladnoj wants to merge 7 commits into
Conversation
Okladnoj
marked this pull request as ready for review
August 3, 2026 09:56
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Okladnoj
force-pushed
the
okji/feat/deterministic-math-v2.2.4
branch
from
August 3, 2026 17:18
2a23091 to
4a3002d
Compare
Okladnoj
changed the base branch from
okji/feat/deterministic-math-v2.2.3
to
okji/feat/deterministic-math-v2
August 3, 2026 17:20
|
Looking good :)
Nits: |
OmniBlade
reviewed
Aug 10, 2026
|
|
||
| // length of projection onto XY plane | ||
| float len2 = (float)WWMath::Sqrt_Legacy(dx*dx + dy*dy); | ||
| float len2 = (float)WWMath::Sqrt(dx*dx + dy*dy); |
There was a problem hiding this comment.
Is the cast to float actually needed here? All the math is done at float precision so it should call the float overload of Sqrt.
Okladnoj
force-pushed
the
okji/feat/deterministic-math-v2.2.4
branch
from
August 11, 2026 20:19
4a3002d to
69058b0
Compare
Okladnoj
added a commit
to OKJID/GameClient
that referenced
this pull request
Aug 12, 2026
Port the math changes reviewed in Okladnoj#7 and the follow-up branch: - drop WWMath::Sqr and route the 12 former call sites in PartitionManager, POWTruckAIUpdate and BuildAssistant back through WWMath::Pow(expr, 2); - compute WWMath::Pow through gm_powf under deterministic math, since gm_pow runs on x87 on the 32-bit Windows build and diverges from ARM64, while fdlibm returns x*x for y == 2 so the result is unchanged; - drop the redundant float casts in matrix3d.cpp and vector2.h; - restore Point.h to its upstream form, it is rendering-only and needs no deterministic math; - guard the SpectreGunship gattling targeting logic behind RETAIL_COMPATIBLE_CRC so a missing particle template no longer skips it in headless replay playback, which was desyncing win against mac. Verified by replaying the reference file on both platforms: CRC matches.
…wrappers These five _Legacy wrappers are bit-identical to their non-Legacy siblings in every math mode (verified exhaustively and independently), so route all callers to the siblings and drop the wrappers. Force float binding where a call site passed a double literal or double expression, to keep the original single-precision behaviour and avoid selecting the true-double overload. EulerAnglesClass::From_Matrix switches sy/cy to float as part of this: Matrix3D elements are float, and passing double sy/cy to WWMath::Atan2 produced mixed (double, float) argument lists that matched neither the float nor the double overload once the wrappers were gone, causing error C2666. Kept: Fabsf/Sinf/Cosf/Inv_Sqrt _Legacy, which differ under retail (x87 asm, bit twiddle, NaN handling) and must stay to preserve RETAIL_COMPATIBLE_CRC.
…loat TheSuperHackers @refactor Okladnoj Sqrt(int) collapsed to the same numeric result as Sqrt(float): in DET it already truncated the argument to float (gm_sqrtf((float)x)); in RETAIL it returned (float)sqrt((double)x). After removal an int argument would be ambiguous between Sqrt(float)/Sqrt(double), so the pathfinding call sites in AIPathfind (Int dx,dy cell deltas) get an explicit (float) cast. This preserves CRC bit-for-bit in both modes: DET is identical, RETAIL is identical because the int magnitudes (dx*dx+dy*dy of cell coordinates) are far below 2^24 and thus exactly representable in float.
TheSuperHackers @refactor Okladnoj Finish the constant-only WWMath conversion in W3DMouse::draw() (WWMATH_HALF_PI) by also routing the scroll-cursor angle through WWMath::Atan2 instead of the raw libm atan2, per review consensus (either all operations through WWMath or revert the constant). Cursor rendering is not part of the simulation CRC.
…c branch TheSuperHackers @refactor Okladnoj Finish the Sqrt consolidation per review: the non-deterministic branch of Sqrt(float) now calls sqrtf(x) directly instead of (float)Sqrt((double)x), dropping the float->double->float round-trip left over from the old Sqrt_Legacy removal. CRC is preserved bit-for-bit: sqrtf(x) == (float)sqrt((double)x) == x87 fsqrt across all 2^31 non-negative float encodings (VC6, MSVC x86/x64; 0 divergences), and double rounding is harmless for sqrt. The deterministic branch (gm_sqrtf) is unchanged.
…Math::Pow Sqr was introduced only to avoid gm_pow, but it is not a wrapper around an existing engine function: retail squares via pow(x, 2), so a dedicated Sqr adds a new API where routing the original call through WWMath::Pow is enough. Restore WWMath::Pow(expr, 2) at every former Sqr call site in PartitionManager, POWTruckAIUpdate and BuildAssistant across both engines, and remove Sqr. Pow itself now squares through gm_powf under deterministic math: gm_pow runs on x87 under _PC_24 on the 32-bit Windows build and diverges from macOS ARM64, while gm_powf is cross-platform bit-identical. The retail path keeps pow(double) and is unaffected. Existing x*x expressions and the sqr() helper in BaseType.h are left untouched.
Routing TPoint2D/TPoint3D Length and Normalize through WWMath::Sqrt is what forced wwmath.h into Point.h in the first place, and that include is what the qualified-path build fix was patching. Point.h is consumed only by rendering code (metalmap, dazzle), never by the simulation, so it needs no deterministic math. Restore the original sqrt calls and drop the include along with them.
Okladnoj
force-pushed
the
okji/feat/deterministic-math-v2.2.4
branch
from
August 12, 2026 17:23
86b1e83 to
dec57e5
Compare
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.
Math Refactoring
This PR contains only the math refactoring recommended by @bobtista and @Caball009 in their reviews — 7 commits, 23 files, all under
WWMathand its call sites. Scope is closed; further math work will come as separate PRs.The non-math fixes that used to be bundled here were split out into their own upstream PRs and are no longer part of this branch:
DieOnDetonateCRC loggingThis PR is a sub-part of the fundamental milestone tracked in TheSuperHackers#2670, where the final deterministic math implementation is waiting for its time.
To verify
RETAIL=1compatibility, I use my own workflow that runs retail replays—the golden set—in VC6 mode.To verify deterministic math support, I use the
1x1x2x2x2_mac_win_3.repreplay (118,800 frames), running it simultaneously on macOS and Windows.1x1x2x2x2_mac_win_3.rep.zip