[xezon]

• Merge after bugfix(baseheightmap): Restore Generals retail compatibility from floor math changes in BaseHeightMapRenderObjClass::getHeightMapHeight() #3644

This change gets rid of fast_float_trunc, fast_float_floor, fast_float_ceil except for retail compatible Zero Hour. Reason: The function results are fundamental flawed and are best abandoned. See explanations below.

AI Summary

fast_float_floor and fast_float_ceil subtract or add almost 1 before truncating. That only gives the right result if the FPU rounds toward zero. The game sets the FPU to round to nearest (setFPMode), so:

• fast_float_floor returns n-1 for negative whole numbers above -2^24, for example -2 returns -3.
• fast_float_ceil returns n+1 for positive whole numbers below 2^24, for example 2 returns 3.
• Values closer to zero than about 3e-8 return 0 instead of -1 or 1.
• fast_float_trunc loses bits that are not fractional for magnitudes of 2^32 and larger, and turns NaN into infinity.

REAL_TO_INT_FLOOR and REAL_TO_INT_CEIL are used at about 270 places. They carried these errors into every configuration except retail compatible Generals.

Changes

• Retail compatible Zero Hour keeps the fast functions, because its replays depend on their results. The functions are now only defined in that configuration, so a new use elsewhere fails to compile. Each function now has a comment that documents its errors.
• Every other configuration uses gm_floorf/gm_ceilf with deterministic math, or floorf/ceilf without it. Retail compatible Generals keeps floorf/ceilf as before.
• gmath.h is now included and detected with angle brackets, because it is found through the include path and not next to the including file.

Retail compatible builds of both games keep their results. Non retail builds of both games now floor and ceil correctly, so REAL_TO_INT_* results change for whole numbers and for tiny values.

Depends on #3644, which removes the FAST_REAL_* macros and the last direct use of fast_float_floor outside retail compatible Zero Hour.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The gmath.h availability check and conditional includes now use angle-bracket lookup. Float truncation, floor, and ceil helpers are limited to retail-compatible Zero Hour builds. The rounding macros select deterministic or standard math functions in other builds.

Changes

Math rounding configuration

Layer / File(s)	Summary
Configure rounding helpers Core/Libraries/Source/Lib/Include/Lib/BaseDefines.h, Core/Libraries/Source/Lib/Include/Lib/BaseFunctions.h, Core/Libraries/Source/Lib/Trig.cpp, Core/Libraries/Source/WWVegas/WWMath/wwmath.h	The gmath.h availability check and conditional includes use angle-bracket lookup. Custom truncation, floor, and ceil helpers apply only when RTS_ZEROHOUR and RETAIL_COMPATIBLE_CRC are enabled. Other builds use deterministic math functions when enabled, or standard math functions otherwise.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

• TheSuperHackers/GeneralsGameCode#3644: It changes floor-function selection for retail-compatible Zero Hour builds. This PR extends the build-specific rounding selection to truncation, floor, and ceil.
• TheSuperHackers/GeneralsGameCode#3532: It changes the implementation of the same float conversion helpers whose use this PR conditions on the build configuration.

Suggested reviewers: okladnoj

Merge Risk: Minimal · up to 7f1e7

The rounding change preserves the checked retail Generals behavior. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title accurately describes the main change: replacing the fast float helpers outside retail-compatible Zero Hour. It is long, but it is clear and specific.
Description check		The description explains the helper-function errors, the configuration-specific replacements, and the replay-compatibility reason for retaining the fast functions in retail-compatible Zero Hour.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a39aea70-1e70-4fbd-9450-cb3c818ecbf9

Commits

Reviewing files that changed from the base of the PR and between f1f5986 and 7f1e78d.

Files selected for processing: 2

• Core/Libraries/Source/Lib/Include/Lib/BaseDefines.h
• Core/Libraries/Source/Lib/Include/Lib/BaseFunctions.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[greptile-apps]

Confidence Score: 5/5

[High impact] The PR appears safe to merge; no blocking issues were found.

Summary

Replaces the flawed rounding helpers outside retail-compatible Zero Hour.

• Game calculations now use proper floor and ceiling results outside retail-compatible Zero Hour.

No actionable issues were found. The review used the full PR changes because the previous-review merge bases differed. No build or runtime tests were run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[REAL_TO_INT_CEIL / REAL_TO_INT_FLOOR] --> B{Retail-compatible Zero Hour?}
  B -->|Yes| C[Existing fast helpers]
  B -->|No| D{Deterministic math enabled?}
  D -->|Yes| E[gm_ceilf / gm_floorf]
  D -->|No| F[ceilf / floorf]
  C --> G[fast_float2long_round]
  E --> G
  F --> G

Loading

_{Reviews (2) · Last reviewed commit: "fix: Replace fast_float_trunc, fast_floa..." · Reviewed by Greptile}

[bobtista]

More progress towards standard / deterministic :) -1 no longer turns into -2, which is nice, and retail compat is untouched. I had my agents double check it too, it's good to go once 3644 merges

[Caball009]

Looks good to me.

Needs #3644 to be merged first, and a rebase after that.