Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FPL Abs bug #144

Open
ZhangHuan0407 opened this issue May 16, 2024 · 3 comments
Open

FPL Abs bug #144

ZhangHuan0407 opened this issue May 16, 2024 · 3 comments

Comments

@ZhangHuan0407
Copy link

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
Copy link
Author

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?

@qwertie
Copy link
Owner

qwertie commented Jun 30, 2024

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.

@ZhangHuan0407
Copy link
Author

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);
	}

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

No branches or pull requests

2 participants