Skip to content

Conversation

@aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Aug 18, 2025

This PR is a subset of the changes in PR #109738, removing the cases of changing length() < value to length_squared() < value * value since those are arguably the least readable (that was most of the PR), but keeping zero comparisons length_squared() > 0.0f, length/length comparisons length_squared() < length_squared(), renaming d to dist, ordering variables by size, using const more, using float literals, and some fixes to type consistency with floats.

@aaronfranke aaronfranke added this to the 4.x milestone Aug 18, 2025
@aaronfranke aaronfranke requested review from a team as code owners August 18, 2025 21:58
@aaronfranke aaronfranke requested review from a team as code owners August 18, 2025 21:58
@aaronfranke aaronfranke requested review from a team as code owners August 18, 2025 21:58
@lawnjelly
Copy link
Member

lawnjelly commented Aug 19, 2025

Imo you are still trying to do too much in one PR. The objective should be to make each PR the smallest unit it can be. These are unrelated changes.

  • Changing double literals (0.0) to float literals (0.0f) is correct to do as it prevents promotion to 64 bit for 32 bit calculations, and will work fine with 64 bit, so is pretty easy to approve.
  • Changing local variables to const is more dubious, in some cases this could just be needless code churn.

As an aside (being the one who brought up the whole problem of double literal promotion problem originally, but wasn't involved in the discussion to standardize this):

Imo using integers is more clear when using whole numbers (e.g. x >= 0 rather than x ->= 0.0f), as the former works just as well and is less easy to get wrong, and easier to read. The f literal is only really necessary for fractions, 0.5f. I think half the reason contributors continue to make this mistake is because they aren't automatically using integer literals unless otherwise required.

(Obviously with some exceptions such as where you need to promote integer calculation to float.)

@aaronfranke
Copy link
Member Author

Imo using integers is more clear when using whole numbers

I find it clearer to use float literals because when I see integers it's not clear if we're working with integers or floats. As a contrived example, if I see if (a - b < 0) in my own code, I expect both a and b to be integers, but if I see if (a - b < 0.0f) then I know at least one of those is a float (because if both are integers it makes no sense to write it that way). Another example, if I see x / 2 my assumption is that this is integer division, and I would instead use x / 2.0f or x * 0.5f for floats.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants