mono: add constants to transform and vector structs #20901
Conversation
| private static readonly Vector2 negOne = new Vector2 (-1, -1); | ||
|
|
||
| private static readonly Vector2 up = new Vector2 (0, 1); | ||
| private static readonly Vector2 down = new Vector2 (0, -1); |
There was a problem hiding this comment.
That's my fault for implementing that incorrectly in the first place, sorry 😆. I didn't know Godot used "Y is down" for 2D until recently (I only use 3D myself).
| @@ -1 +1 @@ | |||
| 7 | |||
| 9 | |||
There was a problem hiding this comment.
Seven to Nine?
It's fine as-is (after rebasing) if your other PR #20890 gets merged first.
There was a problem hiding this comment.
Yeah, that was probably a bit optimistic of me.
I've changed this back to 8 for now.
There was a problem hiding this comment.
Don't worry too much about this. I will make this automatic.
KellyThomas
force-pushed
the
mono-consts
branch
2 times, most recently
from
c04b96d
to
5c0ee95
Compare
|
CC @neikeq |
|
|
||
| // Constants | ||
| private static readonly Transform _identity = new Transform(Basis.Identity, Vector3.Zero); | ||
| private static readonly Transform _flipX = new Transform(new Basis(new Vector3(-1, 0, 0), new Vector3(0, 1, 0), new Vector3(0, 0, 1)), Vector3.Zero); |
There was a problem hiding this comment.
Only a single white space after the comma (this can be found in a other cases below).
| public static Transform Identity { get { return _identity; } } | ||
| public static Transform FlipX { get { return _flipX; } } | ||
| public static Transform FlipY { get { return _flipY; } } | ||
| public static Transform FlipZ { get { return _flipZ; } } |
There was a problem hiding this comment.
No alignment between fields and properties. Only a single white space to separate identifier from the assign token or the opening braces.
| // Constants | ||
| private static readonly Transform _identity = new Transform(Basis.Identity, Vector3.Zero); | ||
| private static readonly Transform _flipX = new Transform(new Basis(new Vector3(-1, 0, 0), new Vector3(0, 1, 0), new Vector3(0, 0, 1)), Vector3.Zero); | ||
| private static readonly Transform _flipY = new Transform(new Basis(new Vector3( 1, 0, 0), new Vector3(0, -1, 0), new Vector3(0, 0, 1)), Vector3.Zero); |
There was a problem hiding this comment.
No white space after the opening parenthesis.
|
Looks good, save for the formatting issues I mentioned. |
KellyThomas
force-pushed
the
mono-consts
branch
from
5c0ee95
to
a78d6d3
Compare
KellyThomas
force-pushed
the
mono-consts
branch
from
a78d6d3
to
815d08f
Compare
|
Thanks, I've removed all alignment beyond the line indents. It should be clean now. |
|
Thanks! |
ghost
mentioned this pull request
|
Now that Godot 3.1 has been released, we don't plan to cherry-pick new features and enhancements to the 3.0 branch, unless there is very strong support in favor of it. Removing cherrypick label for 3.0. |
What this does:
This PR brings the constants offered by
Transform,Transform2D,Vector2andVector3into line with GDScript changes in #14704For the vectors I have opted for
InfoverInfinityto match theMathfproperty, I am happy to change this if the full name is preferred.This swaps the values of
Vector2.Up/Vector2.Down, so that they match the new GDScript implementation.What this doesn't do:
I was unable add
X,Y, orZtoPlaneas it conflicts with existing instance properties.I was unable to add
BlackCircle,WhiteCircle,BlackDiamond, orWhiteDiamondtoStringExtensionsdue to lack of support for static extension properties.I haven't looked at the named colors.