Skip to content

Make the RAND_MAX to float conversions explicit - #72

Open
frewsxcv wants to merge 1 commit into
carbonengine:mainfrom
frewsxcv:rand-max-float-conversion
Open

Make the RAND_MAX to float conversions explicit#72
frewsxcv wants to merge 1 commit into
carbonengine:mainfrom
frewsxcv:rand-max-float-conversion

Conversation

@frewsxcv

@frewsxcv frewsxcv commented Aug 26, 2026

Copy link
Copy Markdown

Summary

RAND_MAX is 32767 on Windows and converts to float exactly, but it is
INT_MAX everywhere else, where 2147483647 has no exact float
representation. Clang raises -Wimplicit-const-int-float-conversion and the
build fails under -Werror, which is what currently stops trinity
compiling on anything but Windows. This makes the conversion explicit at all
32 sites.

AI assistance disclosure

Substantial. Claude (via Claude Code) found the failures by building
trinity_metal on macOS, applied the edits, and wrote the commit message and
this description. Every changed line was reviewed and the 30 -> 1 error count
is from real builds, not an estimate. Not verified: no Windows build, and no
runtime testing (there is no behaviour change to test).

Type of change

  • Bug fix
  • Refactor / cleanup (no behaviour change)
  • New feature
  • Documentation
  • Build, CI, or tooling
  • Breaking change (public API or ABI)
  • Other (describe below)

Both apply: it fixes a build failure, and the emitted code is unchanged.

Linked issue (optional)

None.

What changed

  • Made RAND_MAX's conversion to float explicit at 32 sites across 11
    files, all the same idiom:
    ( (float)rand() / RAND_MAX ) -> ( (float)rand() / float( RAND_MAX ) )
  • Two sites in ProcessLifetime.cpp (lines 217 and 405) have RAND_MAX as
    the numerator rather than the divisor. They divide by a float, so it is
    the same conversion, fixed the same way.
  • No logic touched. The conversion was already happening implicitly; it is
    now written out, which is what silences the diagnostic. Values are
    bit-identical on every platform, Windows included.

Testing

cmake --build .cmake-build-arm64-osx-debug --target trinity_metal -j8 -- -k

trinity_metal on arm64-osx-debug: 30 errors before, 1 after. The
remaining error is variable 'mesh' set but not used in
EveChildMesh.cpp:886, an unrelated diagnostic left alone here. With that
one also fixed locally, the target links cleanly.

No tests added: trinity has no test target or test sources, so there is
nothing to extend. Per CONTRIBUTING I would have run the component's tests
locally, but none exist for this component.

This is a compile-time diagnostic fix with no runtime behaviour change, so
there is nothing to exercise at runtime beyond the build itself.

Platforms tested

  • Windows
  • macOS
  • Not applicable

Not built on Windows. That is the platform most likely to regress from a
careless edit here, so it is worth a maintainer confirming. The change should
be a no-op there: RAND_MAX is 32767, the conversion was already implicit,
and making it explicit does not alter the value.

Screenshots / captures

Not applicable.

Checklist

  • I've read CONTRIBUTING.md.
  • My commits follow the commit-message style described there.
  • I've added or updated tests where it made sense.
  • I've updated docs / inline API comments for any behaviour change.
  • My CLA / ICLA is signed (the bot will let you know if it isn't).

On the unticked boxes: no tests were added because trinity has no test
infrastructure, and no docs were updated because there is no behaviour change
to document. Neither is being skipped silently. The CLA box is left for the
submitter to confirm.

Commit checked against the seven rules: 47-character subject, imperative
mood, no trailing period, body wrapped at 72.

RAND_MAX is 32767 on Windows and converts to float exactly, so this is
invisible on our primary target. Everywhere else it is INT_MAX, and
2147483647 has no exact float representation, so clang raises
-Wimplicit-const-int-float-conversion and the build fails under
-Werror. This is what stops trinity compiling anywhere but Windows.

32 conversions across 11 files, all the same idiom:

    before:  ( (float)rand() / RAND_MAX )
    after:   ( (float)rand() / float( RAND_MAX ) )

The value is unchanged on every platform. The conversion was already
happening; it is now written out rather than left implicit, which is
what silences the diagnostic.

Two sites in ProcessLifetime.cpp have RAND_MAX as the numerator rather
than the divisor. They divide by a float, so it is the same conversion
and they are fixed the same way.

This takes an arm64-osx-debug build of trinity_metal from 30 errors to
1. The remaining error is an unused variable in EveChildMesh.cpp, an
unrelated diagnostic that is deliberately left alone here.

No behaviour change on Windows, and no new build configuration is
needed to review it: the diff is one token per line, no logic touched.

Claude-Session: https://claude.ai/code/session_01NSpJ5h8fGynb2qyHa4NE7Z
Copilot AI lite review requested due to automatic review settings August 26, 2026 03:08

Copilot AI 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.

Pull request overview

This PR fixes non-Windows (notably macOS/Clang) build failures caused by -Wimplicit-const-int-float-conversion when using RAND_MAX in floating-point expressions, by making the conversion to float explicit at each affected call site.

Changes:

  • Replaced implicit RAND_MAX-to-float conversions with float( RAND_MAX ) in random-normalization expressions across the codebase.
  • Updated both divisor-form (rand() / RAND_MAX) and numerator-form occurrences in ProcessLifetime.cpp to make the conversion explicit while preserving behavior.
  • Kept logic unchanged (mechanical cast insertion) to silence Clang warnings under -Werror.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
trinity/Eve/Volume/EveSphereVolume.cpp Makes RAND_MAX float conversion explicit in point generation randomization.
trinity/Eve/Volume/EveEllipsoidVolume.cpp Makes RAND_MAX float conversion explicit in ellipsoid point distribution math.
trinity/Eve/SpaceObject/Utils/EveDistributionMethods/DistributionSpawners/EveDistributionSpawnerTriggerSnake.cpp Makes RAND_MAX float conversion explicit in trigger timing interpolation.
trinity/Eve/SpaceObject/Children/SmartLightSets/attributeModifiers/EveSmartLightAttributeModifierExpressionBucket.cpp Makes RAND_MAX float conversion explicit in expression-bucket random helpers/constant.
trinity/Eve/SpaceObject/Children/EveChildParticleSphere.cpp Makes RAND_MAX float conversion explicit in particle random helper lambdas.
trinity/Eve/SpaceObject/Children/Behaviors/ProcessLifetime.cpp Makes RAND_MAX float conversion explicit in lifetime behavior random offset/spawn computations.
trinity/Eve/Renderable/Stretch/EveStretch2.cpp Makes RAND_MAX float conversion explicit in effect random seed initialization/use.
trinity/Curves/Tr2ScalarExprKeyCurve.cpp Makes RAND_MAX float conversion explicit in scalar expression random function.
trinity/Curves/Tr2CurveVector3Expression.cpp Makes RAND_MAX float conversion explicit in vector3 expression random helpers/constants.
trinity/Curves/Tr2CurveScalarExpression.cpp Makes RAND_MAX float conversion explicit in scalar expression random helpers/constants.
trinity/Curves/Tr2CurveEulerRotationExpression.cpp Makes RAND_MAX float conversion explicit in euler rotation expression random helpers/constants.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants