This function in UnrealMathUtility.h seems to be weird or is it just me ?
/** Wraps X to be between Min and Max, inclusive */
template< class T >
static FORCEINLINE T Wrap(const T X, const T Min, const T Max)
{
T Size = Max - Min;
T EndVal = X;
while (EndVal < Min)
{
EndVal += Size;
}
while (EndVal > Max)
{
EndVal -= Size;
}
return EndVal;
}
Considering Min = 0, Max = 5 :
for X = 5 it returns 5
for X = 6 it returns 1
for X =0 it returns 0
for X = -1 it returns 4
Isn’t it supposed to be X=6 => 0, x=-1 => 5 ?
Or maybe make the max exclusive and for X=5 => 0, X=6 => 1 ?
I just came across this and seems like a bug to me.
Size should be Max - Min + 1 and if Min == Max then just return X. That way it will have the desired wrap around behavior with Min and Max included. Right now it is doing [Min,Max)
Given how they chose to implement this, you are correct. But the current function added if Min == Max, return Max. I think that is better than returning X so you still get the value in range.
My take on why neither was merged is that the function is intended to be used with floating points (where Min != Max) rather than integral types. So rather than changing the existing function that may very well work as intended, it would probably be better to add another one for integral types and choose the appropriate one via enable_if or similar pattern.
With floating point the + 1 suddenly doesn’t make sense anymore. Taking the OP’s example:
Considering Min = 0.0f, Max = 5.0f :
for X = 5.0f it returns 5.0f
for X = 6.0f it returns 1.0f
for X = 5.1f it returns 0.1f (would be -0.9f with + 1 to size)
for X = 6.1f it returns 1.1f (would be 0.1f with + 1 to size which is clearly wrong)
Great point. Didn’t even think about that. My initial thought was maybe the intention was to use saturation semantics rather than modular arithmetic for integers. Didn’t even think about floating points. That makes much more sense.