Fix #1769 and introduce an optional argument to specify a different r… #1964
Conversation
|
I removed the new constructors because we already have conversion operators, so it's not needed. If someone wants to convert a Vector3 vec3 = new Vector3(1.0f, 1.0f, 1.0f);
Int3 int3 = (Int3)vec3;
vec3 = (Vector3)int3; |
Kryptos-FR
force-pushed
the
feature/int3
branch
from
ba773de
to
db8aeec
Compare
…ferent rounding strategy if needed.
Kryptos-FR
force-pushed
the
feature/int3
branch
from
db8aeec
to
f6c616f
Compare
| public static void Round(ref Vector3 value, out Int3 result, MidpointRounding rounding = default) | ||
| { | ||
| result.X = (int)Math.Round(value.X, rounding); | ||
| result.Y = (int)Math.Round(value.Y, rounding); | ||
| result.Z = (int)Math.Round(value.Z, rounding); | ||
| } |
There was a problem hiding this comment.
Replace ref Vector3 value with in Vector3 value
MathF has an overload for midpoint rounding see https://learn.microsoft.com/en-us/dotnet/api/system.mathf.round?view=net-7.0
There was a problem hiding this comment.
Replace ref Vector3 value with in Vector3 value
Those should be wider changes throughout the whole codebase. A lot of methods don't use the in keyword. For me that's outside this PR which is mostly a build fix.
MathF has an overload for midpoint rounding
Should I use MathF for the other places in that file (e.g. Sqrt and Pow)?
There was a problem hiding this comment.
For me that's outside this PR
Fixing it later would be a breaking change, we can't have two methods whose signature only differ in whether they use the ref or in keyword afair.
A lot of methods don't use the in keyword.
I took care of a bunch of them a while ago but not all of them, no. So there's a bit of a mix right now.
Should I use MathF for the other places in that file (e.g. Sqrt and Pow)?
Yes please, as long as you touched those lines in this PR, we'll leave the rest of the file for another PR.
There was a problem hiding this comment.
For Round I will use MathF.
However for Length() and LengthUntruncated() I think we should keep using Math. The rationale is that it is doing a square root of a multiplication of int. An int * int fits in a long and the equivalent floating point type is double. So to avoid overflows, we should only convert the result to float at the end. Although, it is probably already overflowing since we don't convert to long in the first place... 🤔
Ok, that's again a subject for another PR. For consistency I will also use MathF then.
|
Thanks ! |
…ounding strategy if needed.
PR Details
Types of changes
Checklist