feat(math): Route game logic math through WWMath with 3-mode deterministic support

[Okladnoj]

Merge by rebase

Rework of #2602, incorporating review feedback:

• GameMath via FetchContent (per @stephanmeesters, @OmniBlade recommendation)
• Trig.cpp preserved, redirected to WWMath instead of deleted (per @xezon request for standalone change)
• 3 math modes: VC6 (x87 inline asm), CRT (standard library), GameMath deterministic (per @Mauller recommendation)
• USE_DETERMINISTIC_MATH defaults on for non-VC6. BaseDefines.h turns it off automatically when gmath.h is not available or when RETAIL_COMPATIBLE_CRC is set, so a build without GameMath falls back to the CRT path rather than failing
• GameMath keeps its own intrinsics default — the earlier GM_ENABLE_INTRINSICS=OFF override was dropped after a Windows replay run showed byte-identical CRC logs with intrinsics on and off
• Linear history on top of current main, no merge commits

Open question: Replay checks pass both with and without USE_DETERMINISTIC_MATH, even though golden replays were recorded with an x87 build. The replays may not contain MSG_LOGIC_CRC messages, meaning the check only validates absence of crashes rather than game state CRC parity. If anyone has insight on this — please share.

Testing results

Cross-platform deterministic math parity verified with SimulationMathCrc::runBenchmark — computes CRC over 10 000 iterations of sin/cos/tan/atan2/sqrt/pow across a fixed input set.

System	Compiler	Math Library	CRC	Perf (10 000 iters)
Win32 x86	MSVC (modern)	fdlibm (deterministic)	🟩 76B53840	~6 ms
macOS ARM64	Apple Clang	fdlibm (deterministic)	🟩 76B53840	~11 ms
Win32 x86	MSVC (modern)	system math (native)	🟦 E8B6385A	~3 ms
macOS ARM64	Apple Clang	system math (native)	🟦 E8B6385A	~5 ms
Win32 x86	VC6 (legacy)	x87 CRT (no fdlibm)	🟧 B7B83850	~17 ms
Win32 x86	VC6 (legacy)	system math (native)	🟥 8BB5B841	~5 ms

• 🟩 cross-platform deterministic parity achieved (Win32 modern = macOS ARM64)
• 🟦 native system math match (Win32 modern = macOS ARM64)
• 🟧 VC6 deterministic (x87 CRT, separate group — fdlibm not supported)
• 🟥 VC6 native (x87 CRT, separate group)

Key fix: -ffp-contract=off in cmake/compilers.cmake — prevents Clang from emitting FMA instructions (fmadd) that skip intermediate rounding, breaking bit-exact parity with MSVC's /fp:precise default.

[greptile-apps]

Greptile Summary

This PR routes all game-simulation math through a new WWMath abstraction layer that supports three modes: VC6 x87 inline-asm (legacy), CRT (platform-native), and GameMath/fdlibm (cross-platform deterministic). GameMath is fetched via FetchContent at a pinned commit and is wired behind USE_DETERMINISTIC_MATH, which is disabled by default while RETAIL_COMPATIBLE_CRC=1.

• wwmath.h gains a complete set of float/double overloads (Sinf, Cosf, Sqrtf, Div_Safe, …) plus _Legacy x87-asm variants; every overload dispatches to gm_* or CRT under a single #if USE_DETERMINISTIC_MATH guard.
• BaseDefines.h (new) centralises RETAIL_COMPATIBLE_CRC and USE_DETERMINISTIC_MATH defaults, using __has_include("gmath.h") to gate the deterministic path so builds without GameMath automatically fall back to CRT.
• -ffp-contract=off is added for non-MSVC compilers to suppress FMA contraction and achieve bit-exact parity with MSVC's /fp:precise; cross-platform CRC equality is validated in the PR's benchmark table.

Confidence Score: 5/5

• The change is safe to merge. Deterministic math is off by default (gated behind RETAIL_COMPATIBLE_CRC=0), so existing retail-compatible builds are completely unaffected. The new WWMath overloads, lookup-table seeding, and -ffp-contract=off flag have all been validated against cross-platform CRC runs documented in the PR.
• All changed paths either fall back transparently to the original CRT calls (when RETAIL_COMPATIBLE_CRC=1, the default) or have been cross-platform CRC-verified when deterministic math is enabled. The Trig.cpp inconsistency (direct gm_sqrtf vs WWMath::Sqrtf) is a maintenance concern but produces identical output.
• No files require special attention. The only notable inconsistency is in Generals/ and GeneralsMD/ Trig.cpp where Sqrt directly calls gm_sqrtf instead of routing through WWMath::Sqrtf like the other functions.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WWMath/wwmath.h	Major expansion: adds full set of WWMath::*f / WWMath::* overloads with #if USE_DETERMINISTIC_MATH guards routing to gm_* (GameMath/fdlibm) or CRT. Includes Div_Safe, Normalize_Angle, Lerp, Inverse_Lerp, and _Legacy x87-asm variants. Logic is sound; the float-to-double narrowing in double overloads is documented and intentional per resolved thread.
Core/Libraries/Include/Lib/BaseDefines.h	New header centralising RETAIL_COMPATIBLE_CRC and USE_DETERMINISTIC_MATH defaults. Uses __has_include("gmath.h") to set HAS_GAMEMATH before the #undef guard, which correctly falls back to no-op on pre-C++17 and VC6 compilers. Logic is correct.
Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp	Splits appendSimulationMathCrc into deterministic (WWMath wrappers) and native (double-precision CRT) variants; adds runBenchmark and runBenchmarkOnFrame. The benchmark is disabled by default via RUN_MATH_BENCHMARK_REPLAY400_FLAG=0. The calculate() path always calls the deterministic variant (which falls back to CRT when GameMath is absent).
cmake/gamemath.cmake	Fetches GameMath via FetchContent at a pinned SHA and adds its include directory globally with include_directories so __has_include("gmath.h") resolves for all targets. Skipped for VS6 builds via the interface library stub.
cmake/compilers.cmake	Adds -ffp-contract=off for non-MSVC compilers to disable FMA contraction, which was the key fix for bit-exact cross-platform parity with MSVC's /fp:precise default. Intentionally not applied to MSVC (already /fp:precise by default).
Generals/Code/GameEngine/Source/Common/System/Trig.cpp	Replaces direct sinf/cosf/tanf/acosf/asinf CRT calls with WWMath::*f() wrappers. All functions except Sqrt(Real x) delegate uniformly through WWMath; Sqrt directly calls gm_sqrtf (breaking the pattern and requiring an extra #include "gmath.h" block).
Core/Libraries/Source/WWVegas/WWMath/wwmath.cpp	Lookup tables (_FastAcosTable, _FastAsinTable, _FastSinTable) are now populated via WWMath::Acosf/Asinf/Sinf when RETAIL_COMPATIBLE_CRC=0, ensuring they are seeded with the same deterministic functions used elsewhere. Guarded correctly with #if RETAIL_COMPATIBLE_CRC.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Adds the #if RUN_MATH_BENCHMARK_REPLAY400_FLAG call to SimulationMathCrc::runBenchmarkOnFrame in the update loop. The flag defaults to 0, so this path is compiled in but never active in normal builds.
Core/Libraries/Include/Lib/BaseType.h	Adds #include "BaseDefines.h" at the top (so all translation units that include BaseType.h see RETAIL_COMPATIBLE_CRC / USE_DETERMINISTIC_MATH before any conditional code). Enables the REAL_TO_INT_CEIL/FLOOR fast-path only when both RTS_GENERALS and RETAIL_COMPATIBLE_CRC hold, and replaces raw sqrt() calls in Coord2D::length() with Sqrt().

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Call site: Sin / Sqrt / Atan2 / …] --> B[WWMath::Sinf / Sqrtf / Atan2f]
    B --> C{USE_DETERMINISTIC_MATH?}
    C -- Yes --> D[gm_sinf / gm_sqrtf / gm_atan2f\nGameMath / fdlibm]
    C -- No --> E{VC6 _Legacy path?}
    E -- Yes --> F[x87 inline asm\nfsin / fsqrt]
    E -- No --> G[CRT: sinf / sqrtf / atan2f]

    H[BaseDefines.h] --> I{__has_include gmath.h?}
    I -- No --> J[HAS_GAMEMATH undefined\n→ USE_DETERMINISTIC_MATH undef]
    I -- Yes --> K{RETAIL_COMPATIBLE_CRC?}
    K -- 1 default --> J
    K -- 0 --> L[USE_DETERMINISTIC_MATH active]
    L --> C

    M[cmake/gamemath.cmake\nFetchContent GameMath SHA pinned] --> N[gmath.h on include path]
    N --> I

Loading

_{Reviews (21): Last reviewed commit: "refactor(neutronmissile): Route debug bl..." | Re-trigger Greptile}

[Okladnoj]

Here is what replay playback looks like at the moment.

I’m testing this on a separate branch:
https://github.com/Okladnoj/GeneralsGameCode/okji/test/deterministic-math-v2

I slightly adjusted the CI there so I can run Win32 and get access to the game resources.

[Skyaero42]

You did not review the changes you made with AI. It has issues that you should fix before asking it to be reviewed.

[Okladnoj]

reviwed all changes

[xezon]

This change does too many things. It is better to first consolidate trig and wwmath and maybe other sources of math, before going into gamemath territory.

[Okladnoj]

This change does too many things. It is better to first consolidate trig and wwmath and maybe other sources of math, before going into gamemath territory.

@xezon Hey! I understand your point, but the reason I didn't fully consolidate trig and wwmath in this PR is exactly to avoid doing too many things at once.

As we saw in PR #2602, fully removing trig.h and replacing it with WWMath across the codebase touches over 120 files. Mixing a massive 120+ file architectural refactoring with a core feature addition (GameMath) made the previous PR extremely difficult to review and broke compilation for some standalone utilities, because trig.h is used outside of just game math.

That's exactly why I chose this "routing" approach for this PR. By keeping the trig.h interface intact and just routing its internal implementation to WWMath, we achieve the deterministic math goals with a much smaller and safer footprint.

Perhaps the best option would be to test this PR first, and if everything is fine — merge it. And only after that, we can focus on a second PR dedicated purely to the architectural cleanup (removing trig.h across all 120+ files)?

[Okladnoj]

Hi @xezon! I have addressed all your review feedback points and updated the PR.

CI Status:
The CI is completely green. I ran the benchmarks on both Win32 and VC6 with the latest changes, and the CRC results perfectly match our previous deterministic baselines (76B53840 for deterministic, E8B6385A for native).

To save you from hunting through all the comment threads, here is a consolidated list of the answers and solutions to your review points:

• Function_Name convention / Naming inconsistencies
  Fixed. Renamed all math wrappers to use the _Origin and _Trig convention. The _Trig suffix also cleanly resolves conflicts with legacy EA names (e.g., ACos_Trig vs Acos).
• Move #define next to #include gmath
  Fixed.
• Redundant VC6 guard
  Fixed — removed the outer #if !(defined(_MSC_VER)...) guard, kept only __has_include. VC6 doesn't support __has_include, so the block is naturally skipped.
• "origin" terminology
  "Origin" means the original EA code called bare CRT functions (sqrt, acos, sinf...). The suffix explicitly marks which exact CRT function was used originally. These are not just type variants — they are different precision math paths.
• Missing gm math variants / CeilfOrigin identical to Ceil
Ceil(float) and Floor(float) are original EA code used only in rendering (visrasterizer.cpp). Determinism isn't needed there. However, CeilfOrigin(float) is a game logic wrapper that routes to gm_ceilf. Therefore, they are not identical.
• C++ overloads instead of f suffix
  Overloads are dangerous here. GameMath only provides float functions (the double version always narrows). With overloads, the compiler silently picks the version by argument type and could inadvertently change the precision path. Explicit names protect against this.
• No @fix prefix in CMake / What is ODR? / Line breaks / iters typo
  Fixed.
• Benchmark in GameLogic::update()
  Moved the auto-benchmark out of the replay loop. It is now a simple compile-time flag. (Did not prepare an ImGui stub since ImGui does not exist in the project).
• VS6 exclusion necessary in CMake?
  Yes, it is necessary. VC6 doesn't support <stdint.h> and long long required by GameMath. Removing the exclusion will break the build.
• Sqrt(double) intentional in BaseType.h?
  Yes, intentional. Coord3D::length() is used in game logic and participates in CRC — it must be strictly deterministic.

[Okladnoj]

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

However, there are a couple of critical architectural points concerning the preservation of old replays (suffixes) and determinism (Trig.cpp) that I want to clarify before pushing changes.

1. C++ Overloads vs Explicit types (why suffixes are needed)

I want to explain why I had to come to an explicit separation of functions via suffixes instead of using C++ overloads. This is tied to the necessity of preserving 100% backwards compatibility for old builds (VC6 Retail Compatibility).

I introduced 3 types of functions because they reflect 3 completely different mathematical paths (math paths) in the original EA engine. Our codebase serves three build modes at once (VC6, Win32, and Deterministic), and if we don't strictly fix the paths, we will lose Retail compatibility on old compilers:

1. Without suffix (WWMath::Cos): This is the original Westwood Math implementation. In the original game on VC6/Win32, it compiles into inline x87 asm (fcos).
2. With _Trig suffix (WWMath::Cos_Trig): This is a replacement for the global Cos() function from Trig.cpp. In the original game on VC6, it called the CRT function cosf() (not fcos!). The difference in the lowest bits between fcos and cosf() is critical: if I merge them into a single function without a suffix, the retail build will start calling fcos instead of cosf(), and the original logic will break.
3. With _Origin and f_Origin suffixes (ACos_Origin vs ACosf_Origin): These replace direct system calls to acos(double) and acosf(float) in GameLogic. The deterministic library GameMath provides only float versions. My double version is forced to do a narrowing cast: (double)gm_acosf((float)x).
   The original EA code often passed variables of type float into system functions expecting double (e.g., acos()), relying on automatic type promotion by the compiler.
   If I switch to C++ overloads (just ACos), then when passing a float, the compiler will automatically pick the float overload. This will change the original math path (instead of calling the double version with narrowing, it will call the pure float version).

Explicit suffixes strictly lock the original execution path. They guarantee that the exact function intended in the original game is called, avoiding unpredictable compiler behavior during overload resolution.

Examples (The mechanics of overload conflicts)

Here is, with examples, how the overload mechanism breaks the original branches when compiling under VC6:

Example A: Conflicting identical signatures (_Trig)
In the original game, we had two different math paths that took the exact same type (float), but executed different instructions:

1. The original WWMath::Cos(float) → compiled into fcos (inline asm).
2. The original Trig::Cos(float) → compiled into cosf() (CRT).

C++ overloads only work with different argument types. How is the compiler supposed to know which of the two Cos(1.0f) calls should go to assembler, and which should go to the system CRT, if their signatures are absolutely identical? It can't.
If we remove _Trig and leave only WWMath::Cos(float), then in the VC6 build, all code from the former Trig.cpp will start invoking the fcos assembler instead of the original cosf(). The math is broken.

Example B: Path substitution via typing (_Origin)
On the calling code side in GameLogic, EA often wrote like this:

float myVal = 0.5f;
float result = acos(myVal); // In the original, this is a call to <math.h> double acos(double)

Since acos in C accepted a double, the compiler did an implicit cast: float -> double -> acos(double) -> float.

What happens if we introduce the overloads WWMath::ACos(float) and WWMath::ACos(double)?
The call to WWMath::ACos(myVal) will see the float type. The C++ overload mechanism will directly call the float overload, completely ignoring the original path with promotion to double. The VC6 logic is broken! The explicit suffix ACos_Origin(double) takes away the compiler's right to choose and strictly forces the original math path.

2. Sqrt(double) in BaseType.h:391

And (Real)sqrt( x*x + y*y + z*z ); was calling double sqrt(double) ?

Yes, in the original game it fell back to the system CRT double sqrt(double). But the problem is that Coord3D::length() is actively used in game logic (it participates in physics and CRC calculations). If I leave the system double, we will have discrepancies between Mac, Win32, and VC6. I have to forcibly cast it to deterministic float (at the cost of precision loss) to guarantee cross-platform sync.

3. "Trampolines" in Trig.cpp

What is the point of moving the function body to WWMath... No trampoline to WWMath.

The fact is that I was acting exactly according to your original task from the previous PR (#2602).
You wrote then: "Generally it is a bad sign if simplifying code would break something. If so, it needs to be fixed", and asked me to physically delete the old Trig.cpp files, migrating everything to WWMath.

I did exactly that. But stephanmeesters discovered that completely deleting trig.h breaks the VC6 / Win32 compilation (over 120 files are affected due to implicit includes).
To save the VC6 build, I had to restore the old Trig.h interface.

But I moved the implementation itself to wwmath.h to fulfill your requirement for math consolidation. If I write #if USE_DETERMINISTIC_MATH directly inside Trig.cpp, I will have to do it twice (since there are two Trig.cpp files in the engine — in Generals and GeneralsMD).
The trampoline is a transitional compromise that allowed us to not break the VC6 build and to gather the deterministic logic strictly in one place, as we planned. In the second phase, when there is already a working system with deterministic math in the main branch, we can start looking for the best way to delete trig.h and fully rely on wwmath.

4. Duplicates (Ceil / Floor)

Regarding Ceil and Floor — here I completely agree with you.
Since these functions (along with their original EA versions) are used exclusively in rendering (e.g., in visrasterizer.cpp) and do not participate in CRC calculations for network play, wrapping them in WWMath makes no sense.
I will completely remove these wrappers from wwmath.h and write direct calls to std::ceil / std::floor right at their call sites in the render code.

[xezon]

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

However, there are a couple of critical architectural points concerning the preservation of old replays (suffixes) and determinism (Trig.cpp) that I want to clarify before pushing changes.

It is a bit tough to fight through this much AI generated text. Please push the last state of the code and then I can take a look at it in Visual Studio and try to polish it up if it needs polishing. I expect this is faster than chatting about where to go with this. Generally, try to not trust the AI generated code too much. It generates code that is for machines, not humans.

[Okladnoj]

It is a bit tough to fight through this much AI generated text. Please push the last state of the code and then I can take a look at it in Visual Studio and try to polish it up if it needs polishing. I expect this is faster than chatting about where to go with this. Generally, try to not trust the AI generated code too much. It generates code that is for machines, not humans.

I wrote every point personally — I only asked AI to format it properly, fix spelling, and translate it into English, exactly like I’m asking now, because my English is not very strong.

I personally worked through every point of that long text, so it would be better to read it carefully and understand the reasoning behind it — there is nothing unnecessary there.

The main point is that suffixes like _Trig and _Origin are physically necessary for us, because overloading cannot handle this task properly.

In the original project, before deterministic math was introduced, there were places with mixed math inside the game logic that affects the CRC. When USE_DETERMINISTIC_MATH is disabled, we need to support the old CRC calculation system, which means we need simultaneous _Trig and _Origin implementations.

If we could simply remove USE_DETERMINISTIC_MATH from the project, there would not be such a large-scale transformation and interweaving of math functions. But in the old mode, we support not only Win32, but also VC6 with its own assembly functions.

[Okladnoj]

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

@xezon
In short, I don’t think it can be explained much shorter or simpler than in that message.

The project’s math was not always written with a clean and transparent architecture — or at least not all parts of it were. Maybe this was even done intentionally to make it harder to reverse-engineer the CRC logic.

At the moment, all workflows build successfully, and all replays also play successfully both with deterministic math enabled and disabled.

Above, I sent a screenshot of your job, plus one additional replay run that I configured specifically to verify Win32.

[xezon]

Ok fair comments. I was under the impression I was chatting with AI generated text because of all the polished formatting. Can you push the latest state to the branch that you have now? I would like to take a look at it in Visual Studio next.

Btw, Replay Check is currently broken. We need to wait until after that is fixed.

[Okladnoj]

Ok fair comments. I was under the impression I was chatting with AI generated text because of all the polished formatting. Can you push the latest state to the branch that you have now? I would like to take a look at it in Visual Studio next.

Btw, Replay Check is currently broken. We need to wait until after that is fixed.

The branch is already up to date — I haven't made any changes since the last push, I was waiting for your feedback. Feel free to take the current branch and work on it in VS. If you need my help — push your changes and I'll pick up from there.

Regarding the broken Replay Check — the CI runner has no way to obtain the game data. I solved this by extracting a minimal set of files from the Steam distribution (no textures, audio, or GUI — just enough for replay verification), uploaded them as a release to a private repository (Okladnoj/generals-gamedata), and connected it to the workflow via a PAT secret (GAMEDATA_PAT). The CI downloads the data using gh release download, verifies SHA256, and uses it for replay check. You can see the configuration on the test branch: okji/test/deterministic-math-v2 — file .github/workflows/check-replays.yml. Feel free to adopt this approach — or give me access to your organization, and I'll create a similar private repo with the data and wire it up to your CI.

[xezon]

The branch is already up to date

The last push in from 08 May

[Okladnoj]

Hi! @xezon

My main concern is that regrouping the finished solution by function family across 188 files again risks breaking what has been verified with dozens of network replays, and losing a stable state. I raised this on 05.09 (comment), and as I understood it, we agreed to only make each commit build on its own (comment) — all 16 do now. I'm happy to do small moves between commits that leave the final code unchanged, and to add explanatory comments. I strongly recommend testing retail compatibility on the last commit, since that is the state that was actually tested.

[xezon]

The problem is when we commit the individual commits to main branch and an earlier commit knowingly breaks retail compatibility and stays broken for 10 commits or so, then it there is a gap of 10 broken commits and it is not easy to narrow which one of them intruduced a real issue.

I would expect if you ask an LLM to rearrange commits and edits it can do that without error.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e9889cb9-981c-4829-a3f6-72cd024944fe

📥 Commits

Reviewing files that changed from the base of the PR and between c992309 and f716706.

📒 Files selected for processing (1)

• GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

Walkthrough

The pull request integrates GameMath, expands WWMath, routes math operations through shared wrappers across engine variants, and adds optional CRC benchmarking for deterministic and native calculations.

Changes

Deterministic math integration

Layer / File(s)	Summary
Build and configuration CMakeLists.txt, cmake/*, Core/CMakeLists.txt, Core/Libraries/...	Non-VC6 builds fetch and link GameMath. Non-MSVC builds disable floating-point contraction. Shared compatibility and deterministic-math guards move into BaseDefines.h.
WWMath API and implementations Core/Libraries/Include/Lib/*, Core/Libraries/Source/WWVegas/WWMath/*, Generals*/Code/GameEngine/Source/Common/System/Trig.cpp	WWMath gains deterministic-aware overloads, legacy float functions, conversion helpers, angle normalization, safe division, and square-root overloads. Trigonometric implementations delegate through WWMath.
Engine call-site migration Core/*, Generals/Code/*, GeneralsMD/Code/*	Math calls use WWMath wrappers across AI, geometry, physics, rendering, collision, locomotion, weapons, and object updates. Several divisions use Div_Safe with explicit fallbacks.
Bezier and diagnostic support Core/GameEngine/Include/Common/BezierSegment.h, Core/GameEngine/Source/Common/Bezier/*, Core/GameEngine/Source/Common/Diagnostic/*, GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Bezier vector operations use deterministic wrappers. SimulationMathCrc compares deterministic and native calculations and can run a benchmark at replay frame 400.

Priority: ➖ Normal

Estimated code review effort: 5 (Critical) | ~90 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant CMake
  participant GameMath
  participant WWMath
  participant GameLogic
  CMake->>GameMath: Fetch pinned dependency
  GameMath->>WWMath: Provide deterministic math headers
  GameLogic->>WWMath: Call shared math wrappers
  WWMath-->>GameLogic: Return deterministic or native results

Loading

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: routing game logic math through WWMath with three deterministic math modes.
Description check	✅ Passed	The description is directly related to the changeset and explains the GameMath integration, math modes, deterministic configuration, compiler settings, and test results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[Okladnoj]

@xezon Rearranged. Commit 4 (836e59bc0f) now also moves all existing callers of the functions it changes to the _Legacy and float variants, and the restored upstream logic is folded into commit 10 (56e1a3100a), where it was dropped. The two WW3D2 commits became empty and are gone, so the series is 13 commits. The final code is unchanged apart from the two comments from the gm_atan2f thread.

Replays ran on every commit in my fork, with only a CI commit on top that swaps the game data source. On all 13 commits vc6+t+e and vc6-releaselog+t+e pass 10 of 10 replays with no CRC mismatch; the win32+t+e control mismatches on all 10.

commit	run
1 4f481a9f77	35004187542
2 ead7d7e29d	35004190223
3 7c877822dc	35004192547
4 836e59bc0f	35004194989
5 ffd596cb85	35004197288
6 506de26960	35004199829
7 b20dbcc3f3	35004202684
8 19e67c3e36	35004204817
9 2b1bd82461	35004206925
10 56e1a3100a	35004209889
11 e5ffdd3ab4	35004213607
12 37aed92d2e	35004217124
13 134c53504e	35004220462

[coderabbitai]

Actionable comments posted: 10

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b0ff1d9c-4096-4f05-8c62-e681147dc5fc

📥 Commits

Reviewing files that changed from the base of the PR and between 288a3ea and 134c535.

📒 Files selected for processing (188)

• CMakeLists.txt
• Core/CMakeLists.txt
• Core/GameEngine/Include/Common/BezierSegment.h
• Core/GameEngine/Include/Common/Diagnostic/SimulationMathCrc.h
• Core/GameEngine/Include/Common/GameDefines.h
• Core/GameEngine/Source/Common/Bezier/BezFwdIterator.cpp
• Core/GameEngine/Source/Common/Bezier/BezierSegment.cpp
• Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp
• Core/GameEngine/Source/Common/INI/INI.cpp
• Core/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
• Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/CameraShakeSystem.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DTankDraw.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DTankTruckDraw.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DMouse.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DProfilerFrameCapture.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTreeBuffer.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
• Core/Libraries/Include/Lib/BaseDefines.h
• Core/Libraries/Include/Lib/BaseType.h
• Core/Libraries/Include/Lib/trig.h
• Core/Libraries/Source/WWVegas/WW3D2/animatedsoundmgr.cpp
• Core/Libraries/Source/WWVegas/WW3D2/colorspace.h
• Core/Libraries/Source/WWVegas/WW3D2/coltest.cpp
• Core/Libraries/Source/WWVegas/WW3D2/hanim.cpp
• Core/Libraries/Source/WWVegas/WW3D2/htree.cpp
• Core/Libraries/Source/WWVegas/WW3D2/inttest.h
• Core/Libraries/Source/WWVegas/WW3D2/metalmap.cpp
• Core/Libraries/Source/WWVegas/WW3D2/ringobj.cpp
• Core/Libraries/Source/WWVegas/WW3D2/segline.cpp
• Core/Libraries/Source/WWVegas/WW3D2/shattersystem.cpp
• Core/Libraries/Source/WWVegas/WW3D2/streak.cpp
• Core/Libraries/Source/WWVegas/WW3D2/texproject.cpp
• Core/Libraries/Source/WWVegas/WW3D2/visrasterizer.cpp
• Core/Libraries/Source/WWVegas/WWLib/WWDefines.h
• Core/Libraries/Source/WWVegas/WWLib/visualc.h
• Core/Libraries/Source/WWVegas/WWMath/CMakeLists.txt
• Core/Libraries/Source/WWVegas/WWMath/aabox.h
• Core/Libraries/Source/WWVegas/WWMath/colmathaabox.cpp
• Core/Libraries/Source/WWVegas/WWMath/colmathaabox.h
• Core/Libraries/Source/WWVegas/WWMath/colmathaabtri.cpp
• Core/Libraries/Source/WWVegas/WWMath/colmathobbobb.cpp
• Core/Libraries/Source/WWVegas/WWMath/colmathobbox.cpp
• Core/Libraries/Source/WWVegas/WWMath/colmathobbtri.cpp
• Core/Libraries/Source/WWVegas/WWMath/colmathsphere.cpp
• Core/Libraries/Source/WWVegas/WWMath/euler.cpp
• Core/Libraries/Source/WWVegas/WWMath/lookuptable.h
• Core/Libraries/Source/WWVegas/WWMath/matrix3.cpp
• Core/Libraries/Source/WWVegas/WWMath/matrix3.h
• Core/Libraries/Source/WWVegas/WWMath/matrix3d.cpp
• Core/Libraries/Source/WWVegas/WWMath/matrix3d.h
• Core/Libraries/Source/WWVegas/WWMath/matrix4.h
• Core/Libraries/Source/WWVegas/WWMath/obbox.cpp
• Core/Libraries/Source/WWVegas/WWMath/obbox.h
• Core/Libraries/Source/WWVegas/WWMath/quat.cpp
• Core/Libraries/Source/WWVegas/WWMath/quat.h
• Core/Libraries/Source/WWVegas/WWMath/sphere.h
• Core/Libraries/Source/WWVegas/WWMath/tri.cpp
• Core/Libraries/Source/WWVegas/WWMath/v3_rnd.cpp
• Core/Libraries/Source/WWVegas/WWMath/vector2.h
• Core/Libraries/Source/WWVegas/WWMath/vector3.h
• Core/Libraries/Source/WWVegas/WWMath/vector4.h
• Core/Libraries/Source/WWVegas/WWMath/vehiclecurve.cpp
• Core/Libraries/Source/WWVegas/WWMath/wwmath.cpp
• Core/Libraries/Source/WWVegas/WWMath/wwmath.h
• Core/Tools/W3DView/RingSizePropPage.cpp
• Core/Tools/W3DView/SphereSizePropPage.cpp
• Generals/Code/GameEngine/Source/Common/RTS/Player.cpp
• Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp
• Generals/Code/GameEngine/Source/Common/System/Geometry.cpp
• Generals/Code/GameEngine/Source/Common/System/Trig.cpp
• Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/AI.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/AIGroup.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
• Generals/Code/GameEngine/Source/GameLogic/AI/TurretAI.cpp
• Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
• Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/BridgeBehavior.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/DumbProjectileBehavior.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/GenerateMinefieldBehavior.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/MinefieldBehavior.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/SlowDeathBehavior.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Body/ActiveBody.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Contain/ParachuteContain.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Locomotor.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DeliverPayloadAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/POWTruckAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/CleanupHazardUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/CommandButtonHuntUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/DockUpdate/SupplyWarehouseDockUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/DynamicShroudClearingRangeUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/FloatUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/NeutronMissileSlowDeathUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/NeutronMissileUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/PointDefenseLaserUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/SlavedUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/StealthUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/TensileFormationUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Update/ToppleUpdate.cpp
• Generals/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp
• Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
• Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DVolumetricShadow.cpp
• Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/camera.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/mapper.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/motchan.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp
• Generals/Code/Tools/WorldBuilder/src/GlobalLightOptions.cpp
• GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/Geometry.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/Trig.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AI.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIGroup.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/AI/TurretAI.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/BridgeBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/DumbProjectileBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/GenerateMinefieldBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/MinefieldBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/SlowDeathBehavior.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Body/ActiveBody.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/ParachuteContain.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Locomotor.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/ChinookAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DeliverPayloadAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/JetAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/MissileAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/POWTruckAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/CleanupHazardUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/CommandButtonHuntUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/DockUpdate/SupplyWarehouseDockUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/DynamicShroudClearingRangeUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/FloatUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/NeutronMissileSlowDeathUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/NeutronMissileUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PointDefenseLaserUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/SlavedUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/SpectreGunshipDeploymentUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/StealthUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/TensileFormationUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ToppleUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
• GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DVolumetricShadow.cpp
• GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/camera.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/lightenvironment.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/linegrp.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/mapper.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/motchan.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/GlobalLightOptions.cpp
• cmake/compilers.cmake
• cmake/config-retail.cmake
• cmake/gamemath.cmake

💤 Files with no reviewable changes (1)

• Core/Libraries/Source/WWVegas/WWLib/visualc.h

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[xezon]

Next wave. I am not of fan of the Div_Safe usage. Is protects from division by zero, but it does so in an illogical way.

[Caball009]

nit: use 'unfortunately' if you want to express disappointment instead of a smiley.

[xezon]

I placed it :)

Seemed fitting :(

[Caball009]

#862 (comment) :)

[Caball009]

This comment should be amended. The issue that you're seeing is that the value gets truncated from 80 bit to 32 bit for the Sqrt version. Whether Coord3D::length is inlined doesn't appear relevant per se.

FYI, this is what VC6 does for the original (Real)sqrt( x*x + y*y + z*z ):

1. Load x coordinate twice and multiply, keep result in x87 register.
2. Repeat for y and z coordinates.
3. Add the 3 results.
4. Clean the registers and keep the final result in the first register (st0).
5. Call fsqrt (and return function if not inlined)
6. Use returned value.

Contrast that to what VC6 does for Sqrt( x*x + y*y + z*z ):

1. Load x coordinate twice and multiply, keep result in x87 register.
2. Repeat for y and z coordinates.
3. Add the 3 results.
4. Clean the registers and store the final result on the stack.
5. Call Sqrt and load value from stack to first x87 register (st0).
6. Call fsqrt (and return function(s) if not inlined)
7. Use returned value.

The original uses the full 80 bit value, the new version uses just 32 bits.

[Caball009]

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/TurretAI.cpp

Line 1132 in 039bd17

if( v.length() > 0 )

If you make it so that the above call uses the original sqrt version, then all other call sites can use the new Sqrt version.

Not sure if this is guaranteed to work for all replays and will keep working with future code changes, but it passes golden replay 1. GR1 would otherwise mismatch at frame 102635.