Skip to content

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

Open
Okladnoj wants to merge 9 commits into
TheSuperHackers:mainfrom
Okladnoj:okji/feat/deterministic-math-v2
Open

Okladnoj wants to merge 9 commits into
TheSuperHackers:mainfrom
Okladnoj:okji/feat/deterministic-math-v2

Conversation

@Okladnoj

@Okladnoj Okladnoj commented May 1, 2026 •

Copy link
Copy Markdown

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 was verified with SimulationMathCrc::runBenchmark — computes CRC over 10 000 iterations of sin/cos/tan/atan2/sqrt/pow across a fixed input set. That function has since been removed; the comparison now lives in Google Benchmark as BM_SimulationMathCrc and BM_SimulationMathCrcWithSystemMath.

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.

image

Note

Route game logic math through WWMath with 3-mode deterministic support

  • Expands WWMath API in wwmath.h with explicit float, double, and legacy float overloads for all math operations. Deterministic builds dispatch to single-precision GameMath functions; non-deterministic builds use precision-matched C math functions.
  • Adds USE_DETERMINISTIC_MATH and RETAIL_COMPATIBLE_CRC configuration macros in BaseDefines.h. Deterministic math enables only when GameMath is available and retail CRC compatibility is disabled. Adds GameMath as a CMake FetchContent dependency for non-VS6 builds via gamemath.cmake.
  • Replaces direct C math calls (sqrt, sin, cos, atan2, fabs, ceil, etc.) with WWMath wrappers across the entire engine: Locomotor, Weapon, PartitionManager, PhysicsUpdate, AIUpdate, pathfinding, terrain, geometry, projectiles, and dozens of other systems.
  • Introduces WWMath::Div_Safe to replace direct division in ~15 call sites (mine placement, scaffold count, health ratios, build progress, flight-path segments, turn-radius), clamping the divisor to a supplied epsilon in deterministic builds.
  • Adds -ffp-contract=off for non-VS6 compilers in compilers.cmake to prevent fused multiply-add contraction, and adds a deterministic-vs-native math benchmark via SimulationMathCrc::runBenchmark.
  • Risk: PartitionManager::calcMinRadius and calcRadiusVec now use Real intermediates instead of double outside RETAIL_COMPATIBLE_CRC builds, which may change radius results. Coord3D::length and several WWMath double overloads route through single-precision GameMath in deterministic builds, reducing precision. visualc.h no longer defines local math constants; consumers must use WWMATH_* equivalents. PolygonClass::Compute_Plane now casts squared length to float before WWMath::Sqrt, which may lose precision for large polygons.
📊 Macroscope summarized 32d3717. 188 files reviewed, 3 issues evaluated, 1 issue filtered, 2 comments posted

🗂️ Filtered Issues

GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/motchan.cpp — 0 comments posted, 1 evaluated, 1 filtered
  • line 868: filtertable is used to decode the packed motion-channel data, but this changes its generated values from WWMath::Sin(float) (which used the double-precision sin path in non-deterministic builds) to Sinf_Legacy (which uses sinf on non-x86 builds). The table is not serialized with each motion channel, so existing assets decoded by a modern non-deterministic build can reconstruct different deltas/poses, and the change also makes the decoder's quantization behavior platform/configuration dependent rather than preserving the previous table generation. [ Out of scope (triage) ]

@greptile-apps

greptile-apps Bot commented May 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High impact] Routes game math through a new deterministic math library with compile-time mode selection.

Fix the non-retail Generals rounding change and satisfy the required statement format before merging.

Findings

  1. P1 Exact integer counts round incorrectly ▶
  2. P2 Changed checks share assignment lines ▶

Summary

Routes game and shared-library math through WWMath, adds GameMath build wiring, and adds bit-result tests and per-operation benchmarks.

  • WWMath selects deterministic, native, or legacy math for each build.
  • Game logic and engine callers send their math through WWMath.
  • Selected game ratios clamp their divisor in deterministic builds.
  • Math tests compare result bits, and benchmarks compare call speeds.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Game math calls] --> B[WWMath]
  B --> C{Deterministic math enabled?}
  C -->|Yes| D[GameMath]
  C -->|No| E[CRT functions]
  B --> F[Legacy wrappers]
  F --> G{Deterministic math enabled?}
  G -->|Yes| D
  G -->|No, MSVC x86| H[Original x87 implementations]
  G -->|Other builds| E
Loading

Reviews (40) · Last reviewed commit: "test(wwmath): Add WWMath unit tests and ..." · Reviewed by Greptile

Comment thread Generals/Code/GameEngine/Source/Common/System/Trig.cpp Outdated
Comment thread Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
@Okladnoj

Okladnoj commented May 1, 2026

Copy link
Copy Markdown
Author
image 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.

@Okladnoj
Okladnoj force-pushed the okji/feat/deterministic-math-v2 branch from 854cc7b to 779f714 Compare May 1, 2026 00:49
@Skyaero42

Copy link
Copy Markdown

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

@Okladnoj Okladnoj left a comment

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.

reviwed all changes

@xezon

xezon commented May 2, 2026

Copy link
Copy Markdown

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
Okladnoj force-pushed the okji/feat/deterministic-math-v2 branch from 4b5675d to ddea128 Compare May 3, 2026 15:09
@Okladnoj

Okladnoj commented May 3, 2026

Copy link
Copy Markdown
Author

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)?

Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Include/Lib/BaseType.h Outdated
Comment thread cmake/gamemath.cmake Outdated
Comment thread Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp Outdated
Comment thread Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp Outdated
Comment thread Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp Outdated
Okladnoj added a commit to Okladnoj/GeneralsGameCode that referenced this pull request May 5, 2026


- Merge gmath.h include + USE_DETERMINISTIC_MATH into single __has_include block
- Replace all #ifdef/#if defined() with #if USE_DETERMINISTIC_MATH
- Remove TheSuperHackers @fix prefix from cmake comment
- Expand ODR abbreviation in gamemath.cmake comment
- Add blank lines after setFPMode() in benchmark
- Fix iters abbreviation in printf
- Simplify benchmark: remove replay dependency, auto-trigger at frame 400
Okladnoj added a commit to Okladnoj/GeneralsGameCode that referenced this pull request May 5, 2026


- Merge gmath.h include + USE_DETERMINISTIC_MATH into single __has_include block
- Replace all #ifdef/#if defined() with #if USE_DETERMINISTIC_MATH
- Remove TheSuperHackers @fix prefix from cmake comment
- Expand ODR abbreviation in gamemath.cmake comment
- Add blank lines after setFPMode() in benchmark
- Fix iters abbreviation in printf
- Simplify benchmark: remove replay dependency, auto-trigger at frame 400
- Rename WWMath wrappers to Function_Name convention (578 replacements, 79 files)
fbraz3 added a commit to fbraz3/GeneralsX that referenced this pull request May 7, 2026
* feat(deterministic-math): scaffold phase 4 routing

Port the first deterministic math batch derived from TheSuperHackers PR TheSuperHackers#2670 with incremental gating and attribution compliance.

- add non-MSVC anti-FMA compile flag (-ffp-contract=off)

- route trig and sqrt gateways through WWMath wrappers

- add gamemath.cmake integration scaffold with deterministic flag

- update project rule for upstream PR attribution comments

- update lessons learned and May dev diary

* fix(headless): stabilize replay simulation on macOS

- Override ParticleSystemManagerDummy::update() as no-op to prevent
  headless replay from executing the full particle update path, which
  caused EXC_BAD_ACCESS crash at ParticleSystemManager::update()+560

- Route SDL3GameEngine::createRadar() and createParticleSystemManager()
  to their Dummy counterparts when dummy=true (headless mode), matching
  upstream Win32GameEngine factory behavior

- Guard ParticleSystemManager::update() loop against stale null entries
  with early continue before sys->update() dispatch

- Skip smudge rendering path in headless via m_headless guard in
  ParticleSystemManager::update()

- Add null-file guards in RecorderClass::readNextFrame(),
  appendNextCommand(), and updatePlayback() for both Generals and ZH
  to prevent null dereference when playback file is closed mid-loop

* fix(replay-headless): harden texture creation flow

Guard D3DX8 and DX8 wrapper texture allocation paths when device or caps are unavailable in headless replay windows. Fail texture load tasks safely instead of dereferencing null state.

Also harden missing texture fallback handling and record session notes in May diary and lessons.

* fix(replay-recording): handle mixed path separators correctly when serializing map name

The loop condition checking for path separators was incomplete on Linux/macOS paths:
- realMapPathToPortableMapPath() converts platform paths to portable format
- Portable paths may contain forward slashes (Linux/macOS standard)
- Loop condition find(backslash) never matched forward-slash-only paths
- This left newMapName EMPTY when writing replay header
- Result: replays stored with corrupted map name field

Fix: Check !isEmpty() AND (find(backslash) OR find(forward slash))
- Loop correctly terminates when last token (filename) is reached
- Works with both Windows (backslash) and Unix (forward slash) separators
- Applies to both GameInfoToAsciiString() and GameInfo::setMap()

Test results:
- macos_skirmish_1v1.rep: PASS
- macos_6p_custom_map_2.rep: PASS (CRC fallback resolves map)
- macos_1v1_custom_map_1.rep: CRC mismatch (expected, data incompatible)

* fix(replay-mapcache): normalize map cache path and replay map field

Fix cross-platform replay/map issues found on macOS:\n- write/read MapCache.ini using portable path join (no literal \ filename)\n- keep replay header path handling for absolute and directory-based -replay inputs\n- add explicit replay CRC mismatch diagnostics for headless runs\n- encode/decode replay map field to preserve special characters in map names\n\nValidation:\n- macOS z_generals build completed successfully\n- replay tests: official/custom map cases load natively; incompatible replay reports frame-0 CRC mismatch

* fix(particle-emitter): null-safe strdup in copy constructor

ParticleEmitterClass copy constructor called ::_strdup() on NameString
and UserString without null checks, causing SIGSEGV when either field
was null.

Crash observed at:
  ParticleEmitterClass::Clone() -> copy ctor -> ::_strdup(nullptr)
  -> strlen(nullptr) -> SIGSEGV (KERN_INVALID_ADDRESS at 0x0)

Triggered by W3DGhostObject::snapShot() during normal gameplay.

Fix: guard strdup calls with null check before dereferencing.
Applied to both GeneralsMD and Generals variants.

* docs(replay): add headless testing reference and tech debt notes

- HEADLESS_REPLAY_TESTING.md: commands, parameters, output interpretation,
  platform notes, debug tips (GDB/lldb) for macOS and Linux
- REPLAY_MAPCACHE_TECH_DEBT.md: tracked known issues for custom map CRC
  fallback and (resolved) MapCache.ini backslash filename bug
@Okladnoj

Okladnoj commented May 8, 2026

Copy link
Copy Markdown
Author

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 added a commit to OKJID/GameClient that referenced this pull request May 8, 2026
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
Comment thread Core/Libraries/Include/Lib/BaseType.h Outdated
Comment thread Generals/Code/GameEngine/Source/Common/System/Trig.cpp Outdated
@Okladnoj

Copy link
Copy Markdown
Author

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.

Comment thread cmake/gamemath.cmake Outdated
Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h Outdated
@xezon

xezon commented May 18, 2026

Copy link
Copy Markdown

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.

@xezon xezon added Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Platform Work towards platform support, such as Linux, MacOS labels May 18, 2026
@Okladnoj

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Author

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

xezon commented May 18, 2026

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Author

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

xezon commented May 23, 2026

Copy link
Copy Markdown

The branch is already up to date

The last push in from 08 May

@xezon

This comment was marked as resolved.

@macroscopeapp

This comment was marked as spam.

@xezon

This comment was marked as resolved.

Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h
@Okladnoj

This comment was marked as resolved.

@xezon

This comment was marked as resolved.

Comment thread Core/Libraries/Include/Lib/BaseFunctions.h
@fbraz3

fbraz3 commented Oct 4, 2026

Copy link
Copy Markdown

Hey guys, I wanted to share an interesting finding from the community that might be very relevant to this PR and the overall direction of cross-platform determinism in the engine.

With just -ffp-contract=off, native system math already achieves 100% bit-exact parity between modern x86 and ARM64, while running more than 2x faster than fdlibm.

It's an amazing work you guys are doing in this PR, but I'm wondering if introducing GameMath/fdlibm is truly necessary (or even desirable) to achieve cross-platform determinism.

I'm sharing below a technical case study based on @MYSOREZ 's Android port, and how full cross-play was achieved there.

(Note: The technical breakdown below was compiled with the assistance of an AI tool to summarize the findings clearly)


Real-world Case Study: Android ARM64 ↔ Windows PC Cross-Play

In the recent releases of the Android port (MYSOREZ/GeneralsZH-Android-Port, v1.4.0), the author successfully achieved live cross-play against the official Windows PC client (GeneralsOnline) and bit-exact CRC verification over huge replays (e.g., Global_War2.rep, 194,600 frames / 54 minutes across all factions and superweapons, matching on all 1,946 checkpoints).

Crucially: the Android port achieved this entirely with native system math, without GameMath or fdlibm.

In fact, the author initially pursued the theory that cross-architecture desync was an arithmetic/FPU divergence issue between ARM and x86. However, testing the 12 transcendental simulation expressions across ARM64 and x86_64 proved that single-precision IEEE 754 math produces bit-identical results as long as -ffp-contract=off is active. Attempting to route math through GameMath / fdlibm actually caused desyncs against the Windows client, because the Windows PC executable itself runs on native MSVC CRT math.

What actually caused the desyncs?

When investigated frame-by-frame against the PC client, every single desync was tracked down to codebase diffs, runtime shims, or state leaks rather than transcendental math:

  1. FMA Contraction: -ffp-contract=off is mandatory. Without it, Clang on ARM emits fused multiply-add instructions with infinite intermediate precision, which breaks parity with x86.
  2. Asymmetric RNG Consumption:
    • Code paths drawing from Logic RNG instead of Client RNG for cosmetic effects (e.g., EMPUpdate drew particle vertical offsets from Logic RNG, burning 15+ random draws per frame on disabled units).
    • Early returns before FX/OCL logic in death behaviors (e.g., InstantDeathBehavior::onDie).
  3. Enum and Key Shifting:
    • Registering new GUI functions early in the lexicon shifted the integer keys in UpgradeStore across the network.
    • Inserting new values (like KINDOF_AIRFIELD or DAMAGE_FLESHY_SNIPER) mid-enum shifted mask bit positions.
  4. C++ Runtime Shims (libc++ vs MSVC CRT):
    • Dynamic bone names (like "FIREPOINT0" + itoa(n)) using custom itoa shims based on std::stringbuf::pubsetbuf() (which is a no-op in LLVM libc++), leaving uninitialized garbage on the stack that was copied into object matrices and hashed into the CRC.
  5. FPU Environment State Leaks:
    • Audio backends (OpenAL, MiniAudio) and OS threads often enable Flush-To-Zero (FTZ) or Denormals-Are-Zero (DAZ) for DSP performance. If this state leaks into GameLogic::update(), simulation math diverges. Scoping FPU state at logic boundaries is critical.
  6. NaN / Inf Integer Casting:
    • On ARM64, (int)NaN evaluates to 0, while on x86 it evaluates to INT_MIN. Guarding divisions by zero (Div_FixNaN / health ratio checks) prevents silent branch divergence.
  7. Replay Checksum Frame 100 Tag:
    • Recent PC releases stopped appending the MARKER:OfficialLogicCRCRevision tag to the end of the frame logic CRC, which caused frame 100 SyncCrash even when the entire simulation was 100% in sync.

Insight on the "Open Question"

Regarding:

"Replay checks pass both with and without USE_DETERMINISTIC_MATH, even though golden replays were recorded with an x87 build..."

This aligns with what was observed above: the simulation's standard IEEE 754 float logic with -ffp-contract=off already matches retail/PC logic without needing software float emulation.

Food for Thought

Given that:

  1. Native system math already produces identical CRCs between modern MSVC and Clang ARM64 with -ffp-contract=off (as shown in your benchmark: E8B6385A),
  2. Real-world Android ↔ PC cross-play is already working in production using native math,
  3. GameMath / fdlibm adds an external dependency, ~2x CPU overhead in math routines, and actually breaks compatibility with existing Windows/retail clients that use native CRT math,

Is bringing in GameMath as a mandatory dependency the best path forward, or would enforcing -ffp-contract=off alongside targeted guards (FPU guard, NaN-cast guards, and strict RNG/enum parity) provide a faster, simpler, and more compatible solution?

@MYSOREZ

MYSOREZ commented Oct 4, 2026

Copy link
Copy Markdown

Thanks for the mention — one correction to the case study. The Android port did not use the platform libm as-is: bionic's sinf/cosf differed from the VC6 client's in ~1.2% of 62,801 sampled angles, because VC6's CRT evaluates them in double on x87. We now evaluate in double and narrow once ((float)sin((double)x)), which matched the client on every sampled angle. Several other divergences were not math at all (itoa under libc++, #if arms, upstream gameplay fixes the client lacks). So our result shows that matching the VC6 client's behaviour is enough for cross-play with it — not that a deterministic library is unnecessary for parity between new builds. Details: https://github.com/MYSOREZ/GeneralsZH-Android-Port/blob/claude/network-diagnostics/docs/WORKDIR/lessons/LESSON-cross-play-desync-method.md


Generated by Claude Code

@Okladnoj

Okladnoj commented Oct 4, 2026 •

Copy link
Copy Markdown
Author

Thanks @fbraz3 and @MYSOREZ — valuable data.

The goal of this PR is to encapsulate all CRC-affecting math in one layer (WWMath) — not to replace native math with GameMath, but to define one point where it's decided which math feeds game logic and the CRC.

The desync sources you list are handled through that same layer in wwmath.h — FPU mode (setFPMode()), NaN/Inf casts (Div_Safe with the epsilon clamp), and evaluation order.

Three modes: native/CRT (what you describe — -ffp-contract=off + native math, E8B6385A), deterministic/GameMath (opt-in, only when RETAIL_COMPATIBLE_CRC is off), and VC6 x87 (retail, B7B83850).

A retail build does not go through GameMath — BaseDefines.h disables USE_DETERMINISTIC_MATH under RETAIL_COMPATIBLE_CRC.

@xezon

xezon commented Oct 4, 2026

Copy link
Copy Markdown

I think what they try to say is that GameMath is unnecessary or even counterproductive. But is this really true? This implies that all CPU architectures and system math libraries have the exact implementations and produce the same results.

This sounds unlikely to be plausible.

@fbraz3

fbraz3 commented Oct 4, 2026

Copy link
Copy Markdown

I think what they try to say is that GameMath is unnecessary or even counterproductive. But is this really true? This implies that all CPU architectures and system math libraries have the exact implementations and produce the same results.

This sounds unlikely to be plausible.

The idea is just bring it to discussion, there are no silver bullet in IT world =)

@Okladnoj

Okladnoj commented Oct 4, 2026

Copy link
Copy Markdown
Author

@xezon exactly — and this is what my tests show.

FMA contraction is off on both sides, but by different means: -ffp-contract=off on macOS, /fp:precise on Windows (MSVC has no separate flag; /fp:precise disables contraction by default).

  • mac-arm64-clang vs win-x64-precise — 0 differing lines
  • mac-arm64-clang vs win-x64-strict — 0 differing lines
  • mac-arm64-clang vs win-x86-precise-PC24 — 338 differing lines
  • mac-arm64-clang vs win-x86-strict-PC24 — 338 differing lines
  • mac-arm64-clang vs win-x86-precise-PC53 — 22 differing lines
  • mac-arm64-clang vs win-x86-strict-PC53 — 22 differing lines

On modern x86/ARM64 native math gives bit-exact parity.
On 32-bit x86 (x87) there is no parity:

  • _PC_53: 22 differing lines
  • _PC_24: 338 differing lines

Tests: https://github.com/Okladnoj/GeneralsGameCode/tree/okji/test/deterministic-math-v2.2.8/tests

@OmniBlade

Copy link
Copy Markdown

The reason round tripping through the double version and back to float works is because you are basically doing the calculation at a higher precision than you need the result at and then throwing away the excess. As long as the rounding mode remains the same and the double CRT functions rate of error is less than the precision of float it will hold.

What this is not doing is getting the transcendental functions to return the same value between platforms and there is no way to guarantee this will work across all platforms and C runtimes.

Maintaining compatibility with the VC6 clients from all builds long terms isn't a goal either as it places a lot of constraints on what improvements can be made to the engine.

Finally, performance with GameMath is close to the CRT performance now with the use of the sqrt intrinsics.

Comment thread Core/Libraries/Source/WWVegas/WWMath/wwmath.h
@xezon

This comment was marked as resolved.

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close :-)

Comment thread Core/Libraries/Source/WWVegas/WWLib/visualc.h Outdated
@Okladnoj

This comment was marked as resolved.

Comment thread Tests/Google/Core/Libraries/WWVegas/WWMath/WWMathTestCase.h Outdated
Comment thread Tests/Google/Core/Libraries/WWVegas/WWMath/WWMathTestCase.h Outdated

#include "WWMathTestCase.h"

static const std::map<std::string, WWMathDump> wwmathDumps = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like wwmathDumps needs to be kept in sync with wwmathCalls. This is probably no problem for the AI to handle, but there is no compiler guarantee and the human can easily break the order by fat fingering it. It would be better to redesign this implementation.

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.

Reworked in commit 9 (abe1a94):

  • Each key is now written once, in WWMathKeys.h, and both tables refer to it as WWMathKey::Sin_0_5f. A typo in a key no longer compiles.
  • The tables are merged by key, so the order of rows does not matter.
  • WWMathTables.KeysAreUnique reports every duplicate key in either table. A call without a dump, or a dump without a call, fails its own test.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the DET column not be sufficient? Why do we want to test the win32 CRT output? Right now this locks the test to only succeed on a certain platform?

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.

CI builds the tests only in the win32* presets, and none of them sets RTS_BUILD_OPTION_RETAIL_COMPATIBLE_GAME=OFF, so all of them use CRT math. With only the DET column the test would always be skipped in CI. A build configuration without a column skips the test rather than fails it. Once the CRT path goes away, its column goes with it: one column in WWMathDumps.h and one entry in WWMathTestCase.h.

@stephanmeesters stephanmeesters Oct 12, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to test the win32 CRT output

So the only reason is not to skip the test?

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.

@stephanmeesters

So the only reason is not to skip the test

No. The CRT column guards the retail-compatible branch of WWMath. That branch has its own code (x87 assembly in Sinf_Legacy and Inv_Sqrt_Legacy, no epsilon guard in Div_Safe), and VC6 compiles the same code. VC6 does not build the tests, so this column is where a change to that branch shows up.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, this is just to test those legacy branches and not the win32 CRT. Perhaps clarify somewhere?

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.

Done, thanks for the improvement.

…ns (TheSuperHackers#2670)

Introduces the gamemath.cmake module and wires HAS_GAMEMATH /
USE_DETERMINISTIC_MATH through the compiler configuration.
…ATH switches (TheSuperHackers#2670)

Moves RETAIL_COMPATIBLE_CRC into BaseDefines.h so that WWMath can see it
without depending on GameDefines.h, and adds USE_DETERMINISTIC_MATH which
is disabled automatically when retail CRC compatibility is required.
BaseFunctions.h and Coord2D.h include BaseDefines.h, which enables the
RETAIL_COMPATIBLE_CRC condition of REAL_TO_INT_CEIL, REAL_TO_INT_FLOOR and
Coord2D::toAngle.
config-retail.cmake collects the RETAIL_COMPATIBLE_ defines of
BaseDefines.h as well, so RTS_BUILD_OPTION_RETAIL_COMPATIBLE_GAME keeps
controlling RETAIL_COMPATIBLE_CRC.
…ers#2670)

Moves the WWMath declarations and definitions into the layout used by
the deterministic math entry points, so that the next commit only adds
and changes functions in place. Merges the per-platform duplicate
definitions into one body with the platform branches inside, moves the
inline bodies of Fabs, Atan, Atan2, Ceil and Floor out of the class and
drops the section banners.

No functional change.
Adds the WWMath wrappers that dispatch between the deterministic gamemath
implementation and the platform libm, plus the _Legacy variants used by
rendering code that must stay outside the simulation. Adds WWMath::Div_Safe,
which clamps the divisor to an epsilon under deterministic math.

Routes the Trig.cpp functions through WWMath, adds Sqrt(Real) and
Sqrt(double) there and drops the unused REGENERATE_TRIG_TABLES generator.
Coord3D::length() keeps the original sqrt call under RETAIL_COMPATIBLE_CRC.

Existing callers of the changed functions move to the _Legacy and float
variants in this commit, so retail behaviour holds between commits.
…rappers (TheSuperHackers#2670)

Replaces the libm calls inside the WWMath library itself (matrices,
quaternions, vectors, spheres and vehicle curves) with the WWMath wrappers.
…Hackers#2670)

Replaces the remaining direct libm calls in the shared engine code and in the
game simulation of both Generals and Zero Hour with the WWMath wrappers, so
that the simulation uses the deterministic implementation when it is enabled.
…heSuperHackers#2670)

Adds BezierMath with D3DXVec4Transform and D3DXVec4Dot. Under
USE_DETERMINISTIC_MATH they are computed in plain C++ with a fixed order of
additions; otherwise they call D3DX as before.
Removes the M_* math constants from visualc.h and switches their remaining
uses in W3DMouse.cpp and W3DView.cpp to the WWMATH_* constants. Without
them visualc.h is empty, so it is removed together with its include in
always.h.
GoogleTestMain.cpp
GameEngine/Common/AsciiStringTest.cpp
Libraries/Compression/CompressionManagerTest.cpp
Libraries/WWVegas/WWMath/WWMathTest.cpp

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better also list the header files here.

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.

Done.


inline std::string toBitsString(long value)
{
return std::to_string(value);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one now is not longer hex. Is that intended?

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.

Yes. It was decimal from the start (%ld): Float_To_Long returns an integer, and long is 4 bytes on Windows but 8 on Linux and macOS.


namespace WWMathKey
{
constexpr const char *Sin_0_5f = "Sin(0.5f)";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using const char* as a key, can we use enum values starting at 0 as keys? This should have the benefit that lookups are array indexes and not string searches.

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.

Done. The keys are an enum, both tables are indexed by it, and a static_assert checks that each table has one row per key, in key order.

EXPECT_EQ(computed, mathCase.dump);
}

INSTANTIATE_TEST_SUITE_P(WWMath, WWMathBits, ::testing::ValuesIn(mergeCallsWithDumps()), testNameOf);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way the test is setup here is difficult to intuitively understand. I do not understand it. It's fine if it works, but maybe we can simplify it somehow and add some comments that explain the flow.

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.

Simplified: each key is one test that runs its row in wwmathCalls and compares it with its row in wwmathDumps. The flow is described at the top of WWMathTest.cpp.

)

Adds a unit test that checks the bits of each WWMath function with a
deterministic implementation on fixed inputs, so a change in the math shows
up from one build to the next. Deterministic math has one set of reference
bits for every platform, the retail-compatible math one set for win32, where
its branch is built with the tests; the test is skipped where a build
configuration has none.

A Google Benchmark times each WWMath function next to the CRT function it
replaces.
Comment thread Core/Libraries/Include/Lib/BaseFunctions.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker Platform Work towards platform support, such as Linux, MacOS ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants