[Suggest] Remove `Vector3Int(Vector3 v)` constructor and create static factroy methods.

In Release 2, Vector3Int has constructor that takes Vector3 argument.

I suggest remove the Vector3Int(Vector3 v) constructor and create static factroy methods,

• Vector3Int.FloorToInt(Vector3 v)
  
• Vector3Int.RoundToInt(Vector3 v)
  
• Vector3Int.CeilToInt(Vector3 v)

I'll describe it.

Please check next test result.

Code (CSharp):

1.  
2.    [Test]
3.     public void Vector3ConstructorTest()
4.     {
5.         Assert.AreEqual(
6.             new Vector3Int(0, 0, 0),
7.             new Vector3Int(new Vector3(0, 0, 0))
8.         );
9.  
10.         Assert.AreEqual(
11.             new Vector3Int(1, 2, -1),
12.             new Vector3Int(new Vector3(1.0F, 2.0F, -1.0F))
13.         );
14.  
15.         Assert.AreEqual(
16.             new Vector3Int(1, 2, -1),
17.             new Vector3Int(new Vector3(1.2F, 2.1F, -0.7F))
18.         );
19.     }
20.  

And next code is the Vector3Int constructor implementation (via MonoDevelop assembly browser).

Code (CSharp):

1.  
2.         public Vector3Int (Vector3 v)
3.         {
4.             this.x = Mathf.FloorToInt (v.x);
5.             this.y = Mathf.FloorToInt (v.y);
6.             this.z = Mathf.FloorToInt (v.z);
7.         }
8.  

Vector3Int(Vector3 v) uses FloorToInt in its implementation.



I think that the Vector3Int(Vector3 v) is not good, because the constructor is ambiguous and easy to be misunderstood.

I think that
most users expect Vector3Int(Vector3 v) uses FloorToInt in the constructor implementation
but some users expect Vector3Int(Vector3 v) uses RoundToInt in the constructor implementation.
(Maybe, few users expect Vector3Int(Vector3 v) uses CeilToInt in the constructor implementation.)

So the Vector3Int(Vector3 v) behaviour is ambiguous and someone may make mistake with the behaviour.
(Of course, reference document will show Vector3Int(Vector3 v) behaviour exactly, but some users don't read reference document.)
(I understand the current Vector3Int(Vector3 v) constructor is useful for TileMap. But...)


So, I suggest 2 issues.

• remove Vector3Int(Vector3 v) constructor.
  
• added static factory methods
  
  • Vector3Int.FloorToInt(Vector3 v)
  • Vector3Int.RoundToInt(Vector3 v)
  • Vector3Int.CeilToInt(Vector3 v)

The static factory methods behaviour are so clear from their names. Their implementation are like next code.

Code (CSharp):

1.  
2. public static Vector3Int FloorToInt(Vector3 v)
3. {
4.     return new Vector3Int(
5.         Mathf.FloorToInt (v.x),
6.         Mathf.FloorToInt (v.y),
7.         Mathf.FloorToInt (v.z)
8.     );
9. }
10.  

Code (CSharp):

1.  
2. public static Vector3Int RoundToInt(Vector3 v)
3. {
4.     return new Vector3Int(
5.         Mathf.RoundToInt (v.x),
6.         Mathf.RoundToInt (v.y),
7.         Mathf.RoundToInt (v.z)
8.     );
9. }
10.  

Code (CSharp):

1.  
2. public static Vector3Int CeilToInt(Vector3 v)
3. {
4.     return new Vector3Int(
5.         Mathf.CeilToInt (v.x),
6.         Mathf.CeilToInt (v.y),
7.         Mathf.CeilToInt (v.z)
8.     );
9. }
10.  

And Vector2Int(Vector2 v) constructor is same.

• remove Vector2Int(Vector2 v) constructor.
• added static factory methods
  • Vector2Int.FloorToInt(Vector2 v)
  • Vector2Int.RoundToInt(Vector2 v)
  • Vector2Int.CeilToInt(Vector2 v)

 

This seems like a wise idea.

 

In Release3, Vector3Int does not have constructor with Vector3 and Vector3Int has CeilToInt, FloorToInt and RoundToInt.

Unity 2D development team members! Thank you for your adopting my suggestion!!!

By the way, I think it is breaking change.But there are no description about it in Release Note for Release3.

How about add them.


* Remove Vector3Int(Vector3) constructor
* Remove Vector2Int(Vector2) constructor
* Add Factory methods to create Vector3Int: Vector3Int.CeilToInt, Vector3Int.FloorToInt, Vector3Int.RoundToInt
* Add Factory methods to create Vector2Int: Vector2Int.CeilToInt, Vector2Int.FloorToInt, Vector2Int.RoundToInt

 

Agreed that it should be in the release notes: this was a breaking change for me. More inconvenient than the Tilemaps/TileMap syntax change, funnily enough!

 

agree