source-sdk-2013 icon indicating copy to clipboard operation
source-sdk-2013 copied to clipboard

Incorrect usage of FLT_MIN

Open Kefta opened this issue 2 years ago • 9 comments

Reposting from https://github.com/ValveSoftware/source-sdk-2013/issues/518 since the creator is banned. These usages from FLT_MIN should be replaced with std::numeric_limits<float>::lowest(), and FLT_MAX with std::numeric_limit<float>::max() for consistency though these are standard to be equivalent. Here are all the incorrect usages in the public repo: https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/sp/src/public/builddisp.cpp#L2779 https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/sp/src/utils/vrad/vrad_dispcoll.cpp#L868 https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/sp/src/utils/vrad/vrad_dispcoll.cpp#L1017 https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/sp/src/game/client/c_effects.cpp#L1678

This has been (incorrectly but passably) fixed in Garry's Mod as of Aug 6: https://commits.facepunch.com/388626

Kefta avatar Aug 06 '21 15:08 Kefta

Yes, need to punish the developers who made this mistake!

Paradise-floor avatar Aug 07 '21 19:08 Paradise-floor

I think Valve has no active developers. Or no money for fixing errors)

punishman avatar Aug 09 '21 09:08 punishman

Replying to https://github.com/ValveSoftware/source-sdk-2013/issues/519#issue-962826784

Hang on, are there also Source SDK MP analogues for these, or were these only found in the Singleplayer branch?

SC1040-TS2 avatar Aug 09 '21 10:08 SC1040-TS2

@SC1040-TS2 They are also in the MP branch.

Voltstro avatar Aug 09 '21 10:08 Voltstro

The code for these portions is identical between MP and SP.

Kefta avatar Aug 09 '21 12:08 Kefta

Understood. Just clarifying that given MP entries were not posted.

On a different relevant note, I noticed that the vecMin values are specified as FLT_MAX, while the vecMax values are specified as FLT_MIN. Is this correct, or also an incorrect usage that must be swapped to fix?

SC1040-TS2 avatar Aug 09 '21 16:08 SC1040-TS2

It is a correct usage - they are used in a loop to pick the min/max values from a range, so everything less than FLT_MAX/greater than std::numeric_limits::lowest() respectively should be considered. If they were swapped, the checks would never pass and thus the vectors would always keep their extreme values.

Kefta avatar Aug 09 '21 16:08 Kefta

It is a correct usage - they are used in a loop to pick the min/max values from a range, so everything less than FLT_MAX/greater than std::numeric_limits::lowest() respectively should be considered. If they were swapped, the checks would never pass and thus the vectors would always keep their extreme values.

Understood. Just making sure that was the case, for my own understanding's sake.

SC1040-TS2 avatar Aug 09 '21 18:08 SC1040-TS2

I think it will not be correct usage. Need init values with first value, and FLT_MIN/FLT_MAX not be used????

punishman avatar Aug 10 '21 05:08 punishman