Box2n and Box3n cleanup #920
Conversation
| @@ -208,7 +189,7 @@ public override int GetHashCode() | |||
| /// <inheritdoc/> | |||
| public override string ToString() | |||
| { | |||
| return string.Format("({0}{4} {1}) - ({2}{4} {3})", Left, Top, Right, Bottom, ListSeparator); | |||
| return string.Format("({0}{4} {1}) - ({2}{4} {3})", Min.X, Min.Y, Max.X, Max.Y, ListSeparator); | |||
There was a problem hiding this comment.
I'd probably display it as position/size as that's a lot easier to reason about.
There was a problem hiding this comment.
This does go back to the, do we want min/max, throughout or do we want position/size. I think it makes more sense to have min/max, as this is how the rest of the box2 is structured by now
There was a problem hiding this comment.
Btw this is still relevant, however the structure has changed from using String.Format to using a string like this $""
| else | ||
| { | ||
| _min.X = value.X; | ||
| } |
There was a problem hiding this comment.
Suggest we use clamping instead of if/else for these.
There was a problem hiding this comment.
I don't exactly get what you mean by that. And clamping would have the same branch count...
There was a problem hiding this comment.
I tried making them work with clamping but i couldn't any suggestions on how to do this?
…to frederikja163-BoxCleanup
Frederikja163 box cleanup
| ? point.Y >= Top != point.Y > Bottom | ||
| : point.Y > Top != point.Y >= Bottom; | ||
| var newDistMax = (anchor - _max) * scale; | ||
| _max = new Vector2( |
There was a problem hiding this comment.
is that really stylecop conform?
|
Note that this hasn't been significantly reviewed, but merging as it fixes broken master. |
Purpose of this PR
Testing status
Comments