ecsharp icon indicating copy to clipboard operation
ecsharp copied to clipboard

FPL Abs bug

Open ZhangHuan0407 opened this issue 1 year ago • 3 comments

PL16 value = FPL16.MinValue; FPL16 absValue = value.Abs(); Console.WriteLine($"{value}, {absValue}"); // -140737488355328, -140737488355328 value = new FPL16(-1); absValue = value.Abs(); Console.WriteLine($"{value}, {absValue}"); // -1, 1

long testValue = long.MinValue; Console.WriteLine($"{long.MinValue}, {-testValue}"); // -9223372036854775808, -9223372036854775808 try { Math.Abs(testValue); } catch (Exception ex) { // System.OverflowException: Negating the minimum value of a twos complement number is invalid. Console.WriteLine(ex.ToString()); }

ZhangHuan0407 avatar May 16 '24 09:05 ZhangHuan0407

I think the result of FPL16.Abs cannot be negative. Version: Nuget 30.1.1

// Decompile the code public FPL16 Abs() { return Prescaled((N >= 0) ? N : (-N)); }

// Modified public FPI16 Abs() { Int32 valueN = N; if (valueN == MinValue.N) valueN = MaxValue.N; return Prescaled(valueN >= 0 ? valueN : -valueN); }

// dotnet source code https://referencesource.microsoft.com/#mscorlib/system/random.cs,61 They changed MinValue to MaxValue where they didn't expect an error.

System.Math.Abs throw exception, if value equal long.MinValue.

Console.WriteLine(Math.Abs(float.MinValue)); // 3.4028235E+38

Are fixed point and decimal more similar?

ZhangHuan0407 avatar May 16 '24 09:05 ZhangHuan0407

Hi, sorry for not replying to this.

I suggest adding an extension method for this. I definitely don't want to throw an exception like Math.Abs(int.MinValue) (it's very surprising behavior) so if I were to fix it, I'd have MinValue flip to MaxValue. But I don't even want the performance cost of checking for MinValue either.

qwertie avatar Jun 30 '24 23:06 qwertie

I'm currently clone and modified the repository source code. As described in the license, since I use esharp, I need to publish the modified ecsharp source code when I release the product. (At least 3 months before the product will be released)

I also didn't test the performance... I'm using Unity's IL2CPP, and I'm guessing that a simple comparison of values won't be able to test the performance difference.

	public FPL16 Abs()
	{
		Int64 valueN = N;
		if (valueN == MinValue.N)
			valueN = MaxValue.N;
		return Prescaled(valueN >= 0 ? valueN : -valueN);
	}

ZhangHuan0407 avatar Jul 01 '24 15:07 ZhangHuan0407