Search Unity

  1. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Implicit conversion between Vector2 and Vector3?

Discussion in 'Scripting' started by tigger, Jan 30, 2019.

  1. tigger

    tigger

    Joined:
    Jan 8, 2013
    Posts:
    68
    I'm somewhat new to Unity and was bitten by a feature this morning: Implicit conversion between Vector3 and Vector2.

    Why is this done / good? (instead of the compiler complaining about mismatched types)
     
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    7,345
    I can't say why Unity would do it as such, they decided to, and it's that.

    I would guess they did because Unity when originally designed was intended to be "novice-friendly" so it just implicitly does a lot of things (just wait for the UnityEngine.Object == null operator overload, it's so much fun if you attempt polymorphism).

    Big thing is that early on Unity had UnityScript which is javascript like. And that can hint at the mentality... javascript is a language that just does things and doesn't care. Well there are a lot of C# stuff they wrote that does similar stuff... namely a lot of implicit conversions and the sort.

    As someone who likes rigid statically typed languages... it's REALLY annoying.

    But... that's Unity.

    :shrug:
     
    Hurri04, Ryiah, orionsyndrome and 2 others like this.
  3. tigger

    tigger

    Joined:
    Jan 8, 2013
    Posts:
    68
    LOL. The other major headache I ran into this morning was my null tests not working and the Debugger showing values of "null" when there clearly was an object there. Right or wrong, I removed " : Object" from my class definition (no longer inheriting from Object) and the problem went away.
     
  4. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,128
    I can see why "Vector3 x = someVector2;" is an obvious thing to add, but I also get bitten by it often.

    Unity expects you to do 2D using the "real" xy plane. You can mix in 3D, with the 2D acting as if it's written on a blackboard at z=0;. If you do it that way, the implicit cast is pretty nice.

    But I often do 2D math "on the floor" (or on a table or something) in a 3D area, which is xz coords. Use a vector2 and then convert when done: Vector3 v3=new Vector3(v2,x, 0, v2.y);. The problem is I always accidentally use a pre-converted vector2 in a some 3D math. A compile error would save me. Instead it casts adding z=0 and gives junk results.

    If I do that a few more times, I may finally write a Vectorxz class, with the proper conversion.
     
    PraetorBlue and Joe-Censored like this.
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    7,345
    My feel on implicit conversion (and really it's the MSDN standard as well).

    If the conversion is lossless, implicit convert is fine. A Vector2->Vector3 is a perfectly fine conversion because you just end up with a 0 in the 'z' slot. Your x and y remain the same.

    It's just like how a int->long or int->double is fine, there's no data loss.

    But if the conversion loses information, an explicit conversion should be used. A Vector3->Vector2 would drop the z value, and therefore should be expllicit.

    It's just like how a double->int is explicit, there's the potential of losing fractional information.

    ...

    That's just my opinion though.

    Like I said, Unity decided to do it their way.
     
    Hurri04, PraetorBlue and Joe-Censored like this.
  6. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    Bumping this to say that I agree with lordofduct.

    Implicitly casting Vector2 to Vector3 is fine because it's lossless.
    Implicitly casting Vector3 to Vector2 is NOT fine because you can actually lose data. This should be explicit instead.
    Having both directions implicit also means that Vector2 + Vector3 (or reversed) is ambiguous (also happens with other operators), which results in having to use explicit casting again when it should not even be necessary since it should clearly return a Vector3.

    Also the cast from Vector3 to Vector2 should probably be defined in the Vector3 struct instead of the Vector2 struct, for consistency reasons.
    Speaking of, Unity 2017.2 (two major release cycles ago!) introduced Vector2Int and Vector3Int and there are still conversions missing. While these can be archived by writing custom extension methods "ToVector2Int" for Vector2 and "ToVector3Int" for Vector3, why not just add them to the internal implementation?
    Also to be consistent with lossless conversions allowing implicit casting, this should be used for Vector2Int to Vector3Int.

    Here's an overview:

    implicit V2 -> V3
    implicit V3 -> V2 should be explicit
    implicit V2I -> V2
    explicit V2I -> V3I should be implicit
    explicit V3I -> V2I
    implicit V3I -> V3
    V2 -> V2I missing, should be explicit
    V3 -> V3I missing, should be explicit

    Who is the Unity employee I need to slap with a large trout to have a look at this?

    P.S.: Why is the * operator defined for Vector2*Vector2, Vector2Int*Vector2Int and Vector3Int*Vector3Int but not for Vector3*Vector3?! I know there's potential ambiguity with the dot product and cross product but these have extra methods Vector3.Dot and Vector3.Cross and since the other vector types have component-wise multiplication, why not also Vector3?
     
    Last edited: May 15, 2020
    orionsyndrome likes this.
  7. tigger

    tigger

    Joined:
    Jan 8, 2013
    Posts:
    68
    Just learned Jetbrains is adding a feature to Rider that will show an "adornment" when implicit conversions occur. I'd prefer an actual warning, but the adornment is at least something.
     
  8. orionsyndrome

    orionsyndrome

    Joined:
    May 4, 2014
    Posts:
    733
    Also, the last time I checked, BoundsInt had a major bug regarding center and size misconception, if I remember correctly. It was pretty facepalmish, and we've already waited so long to have int variants in the first place. I'm not sure why such things have to be made so lousy.

    If they lack resources, they should simply release such things to the public. Making Vector3 properly is not exactly a super-secret technology that makes or breaks a company.

    Also I hate the fact that Scale (which is component-wise vector multiplication) can't be used in the chained form. Why?

    edit:
    oh this was necro'ed, I thought this was some other thread.
    but my point still stands.
     
    Last edited: Mar 24, 2020
    Hurri04 likes this.
  9. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    I've submitted a bug report to get this looked at: Case 1251716
     
  10. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    Since the bug report now remains "open" for 3 weeks, I'm just gonna post the code I submitted publicly to maybe start some discussion:
    Code (CSharp):
    1. using System;
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. [ExecuteInEditMode]
    7. public class VectorExample : MonoBehaviour
    8. {
    9.     #region Fields
    10.     [SerializeField]
    11.     private Vector2Int gridSize = new Vector2Int(16, 9);
    12.     [SerializeField]
    13.     private Vector2 scale = new Vector2(1f, 1.5f);
    14.     [SerializeField]
    15.     private Vector3 scale3d = new Vector3(1f, 1.5f, 2f);
    16.     [SerializeField]
    17.     private float depth = 5f;
    18.     [SerializeField]
    19.     private Mesh mesh = null;
    20.     [SerializeField]
    21.     private Material red = null;
    22.     [SerializeField]
    23.     private Material green = null;
    24.     [SerializeField]
    25.     private Material blue = null;
    26.     [SerializeField]
    27.     private Material yellow = null;
    28.     #endregion
    29.  
    30.     #region Properties
    31.  
    32.     #endregion
    33.  
    34.     #region Constructors
    35.  
    36.     #endregion
    37.  
    38.     #region Methods
    39.     void Update()
    40.     {
    41.         DrawExample1();
    42.         DrawExample2();
    43.         DrawExample3();
    44.         DrawExample4();
    45.     }
    46.  
    47.     private void DrawExample1()
    48.     {
    49.         Vector3 origin = transform.position;
    50.         for(int y = 0; y < gridSize.y; y++)
    51.         {
    52.             for(int x = 0; x < gridSize.x; x++)
    53.             {
    54.                 // The cast to Vector3 could be made obsolete by changing the modifier of the following
    55.                 // cast operator method in the Vector2 class from "implicit" to "explicit":
    56.                 /*
    57.                 [MethodImpl((MethodImplOptions)256)]
    58.                 public static implicit operator Vector2(Vector3 v)
    59.                 {
    60.                     return new Vector2(v.x, v.y);
    61.                 }
    62.                 */
    63.                 Graphics.DrawMesh(mesh, origin + (Vector3)(new Vector2Int(x, y) * scale), Quaternion.identity, red, 0);
    64.                 // Having this cast operator explicit removes the ambiguity for the compiler
    65.                 // when working with with mixed values:
    66.                 // Vector2 + Vector2 -> works the same as before
    67.                 // Vector2 + Vector3 -> implicitly casts Vector2 to Vector3 -> Vector3 + Vector3
    68.                 // Vector3 + Vector3 -> works the same as before
    69.                 // There is literally no downside to this. The upside is that the following line
    70.                 // would compile properly and work as expected, resulting in much cleaner code:
    71.  
    72.                 //Graphics.DrawMesh(mesh, origin + new Vector2Int(x, y) * scale, Quaternion.identity, red, 0);
    73.             }
    74.         }
    75.     }
    76.  
    77.     private void DrawExample2()
    78.     {
    79.         Vector3 origin = transform.position;
    80.         for(int y = 0; y < gridSize.y; y++)
    81.         {
    82.             for(int x = 0; x < gridSize.x; x++)
    83.             {
    84.                 // This is a slight variation of Example1:
    85.                 // Notice that the newly created Vector3 which now contains the depth variable
    86.                 // gets implicitly cast down to Vector2, resulting in data loss of the explicitly added third value:
    87.                 Graphics.DrawMesh(mesh, origin + (Vector3)(new Vector3(x, y, depth) * scale), Quaternion.identity, green, 0);
    88.                 // The proposed change in Example1 requires using an explicit cast operator
    89.                 // for this to happen (if desired), resulting in more code security:
    90.  
    91.                 //Graphics.DrawMesh(mesh, origin + (Vector2)new Vector3(x, y, depth) * scale, Quaternion.identity, green, 0);
    92.             }
    93.         }
    94.     }
    95.  
    96.     private void DrawExample3()
    97.     {
    98.         Vector3 origin = transform.position;
    99.         for(int y = 0; y < gridSize.y; y++)
    100.         {
    101.             for(int x = 0; x < gridSize.x; x++)
    102.             {
    103.                 // This is a slight variation of Example2 to fix the depth value being lost:
    104.                 Graphics.DrawMesh(mesh, origin + (Vector3)(new Vector3(x, y) * scale) + Vector3.forward * depth, Quaternion.identity, blue, 0);
    105.                 // Adding two new operator method variants with explicit types
    106.                 // into the Vector2 class allows for much simpler and cleaner code:
    107.                 /*
    108.                 public static Vector3 operator *(Vector2 a, Vector3 b)
    109.                 {
    110.                     return new Vector3(a.x * b.x, a.y * b.y, b.z);
    111.                 }
    112.                 public static Vector3 operator *(Vector3 a, Vector2 b)
    113.                 {
    114.                     return new Vector3(a.x * b.x, a.y * b.y, a.z);
    115.                 }
    116.                 */
    117.                 // This prevents the depth value from being set to 0 via multiplication:
    118.  
    119.                 //Graphics.DrawMesh(mesh, origin + new Vector3(x, y, depth) * scale, Quaternion.identity, blue, 0);
    120.             }
    121.         }
    122.     }
    123.  
    124.     private void DrawExample4()
    125.     {
    126.         Vector3 origin = transform.position;
    127.         for(int y = 0; y < gridSize.y; y++)
    128.         {
    129.             for(int x = 0; x < gridSize.x; x++)
    130.             {
    131.                 // This is a slight variation of Example3 using a Vector3 as scale:
    132.                 // This requires three lines of code because the Scale method
    133.                 // does not return the result as a new Vector3
    134.                 // but changes the internal values of the offset variable instead:
    135.                 Vector3 offset = new Vector3(x, y, depth);
    136.                 offset.Scale(scale3d);
    137.                 Graphics.DrawMesh(mesh, origin + offset, Quaternion.identity, yellow, 0);
    138.                 // Component-wise multiplication already exists for
    139.                 // the Vector2, Vector2Int and Vector3Int clases and
    140.                 // should be added to the  Vector3 class for consistency:
    141.                 /*
    142.                 public static Vector3 operator *(Vector3 a, Vector3 b)
    143.                 {
    144.                     return new Vector3(a.x * b.x, a.y * b.y, a.z * b.z);
    145.                 }
    146.                 */
    147.                 // While this operator is possibly ambigous with dot product and cross product,
    148.                 // fitting methods Vector3.Dot and Vector3.Cross already exist and can be used.
    149.                 // Adding this allows for much cleaner code:
    150.  
    151.                 //Graphics.DrawMesh(mesh, origin + new Vector3(x, y, depth) * scale3d, Quaternion.identity, yellow, 0);
    152.             }
    153.         }
    154.     }
    155.     #endregion
    156. }

    I've also found a page in the Microsoft C# Programming Guide about Casting and type conversions:
    https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/types/casting-and-type-conversions
    where it states
    This is clearly not the case with an implicit cast operator for Vector3 to Vector2.


    Also pinging @JoshPeterson since I saw that you've been discussing Vector implementations over here recently: https://forum.unity.com/threads/vector3-and-other-structs-optimization-of-operators.477338/
    Do you perhaps have any insight into this topic here as well?

    Edit:
    - fixed comment in line 67 to describe correct cast direction
    - fixed spelling error "origion" -> "origin"
     
    Last edited: Jun 21, 2020
  11. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    bumping because 2020.1.0 has been released today and the issue I submitted still was not even looked at.

    the one single "breaking" issue with the suggested change above would be that casts from Vector3 to Vector2 would require manually adding the cast operator in front at the relevant places to fix any occuring InvalidCastException compiler errors since this can not be done automatically via the [UnityUpgradable] attribute as far as I can tell.

    the biggest problem with that would probably be that package creators on the asset store would have to update their scripts this way, requiring many (if not most) packages to be updated. in cases where a package contains normal c# script files (instead of a precompiled dll) it would even be possible for users to fix this manually by clicking all relevant console errors and writing "(Vector2)" in front of the variable at the line/row number.

    but this is a change that absolutely should be done at some point and the best opportunity for this would be the first major release of a year. so I guess the next window of opportunity would be version 2021?
     
    lordofduct likes this.
  12. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,128
    It might help if you could relate it to an actual problem. What I mean is, Vector2 converting back-and-forth into Vector3 isn't a bug, since the docs say that how it's works. Violating someone's C# convention isn't even close to a bug, and hardly seems a problem to game programmers.. For the V3*V3, lack of symmetry with the existing V2*V2 also isn't a bug. Can you give an example of a use for it? That would be a feature request (but having V3*V3 an error message, as it is now, is useful since it's almost always an accident or typo. Vector3.mulBy(Vector3) might be better).
     
  13. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    The code I posted above is an SSCCE which I wrote because of an actual problem I have: I'm using Graphics.DrawMesh to draw lots of custom meshes.

    In my specific use case I draw quads which are visible within an isometric camera rect to create a custom rendering "pipeline" for a 2d tile-based side-scroller game where I do not want to instantiate hundreds of thousands of GameObjects (because that would kill all performance) or bake chunks of tiles into fewer larger meshes because the tiles need to be able to be updated on certain events (some every frame, like water moving via vertex displacement in a custom shader) which is also why I can't use the built-in Tilemap system. Drawing each tile individually and using GPU instancing on the Materials keeps it flexible and performant.

    Since I need to calculate positions based on Vector2 coordinates and depth offsets (for multiple layers) I end up needing to use Vector2 + Vector3 operations, or Vector3 * Vector3 operations to scale everything as shown above. And as I described there, the implicit cast operator of Vector3 to Vector2 makes this more complicated than it needs to be since it requires writing an explicit cast operator anyway because of the existing opposite cast operator method which is also implicit, resulting in ambiguity for the compiler (should it be Vector2 + Vector2 or Vector3 + Vector3?).

    Plus, if you do not know about the implicit V3->V2 cast then this operator can lead to unexpected results due to information loss of the z value. That's not simply bad design, that's straight out EVIL! I'd say this qualifies as a bug.

    The V3*V3 operator should be added for consistency. If I can use V2*V2 I'm then expecting to be able to use the same operator on every Vector type the exactly same way(1). Having to use a method instead of an operator leads to inconsistent code which impedes readability and maintainability since someone else reading the code with the multipication method would then expect to find a similar one for V2 as well. What's next, should we use a method to multiply two floats as well?

    Operator overloads exist to make the code shorter, simpler, snappier. All that's required is that they're implemented which I've done in the comment lines. While we're at it this should in fact be done for all all four base math operations to that we can use all combinations of Vector types added, subtracted, multiplied or divided by any other vector type. This is no witchcraft, far from it, it just needs to be done. If the Unity devs need me to I'll even do it for them! It's just writing down super basic methods.

    I'm just waiting to see a reply from a Unity dev...


    1) Someone looking to use a Dot product or Cross product should know (since he knows about their existence) that these are special operations which are not the same as component-based multiplication.
     
  14. Hurri04

    Hurri04

    Joined:
    Nov 27, 2017
    Posts:
    39
    Yesterday I got a reply to the ticket:
    I'm guessing by "protecting the stability and features of Unity" they're talking about the same problem I mentioned above.

    Unfortunately this reply does not mention whether they've read this forum thread. Seeing how this is a change which would affect many users (even if only indirectly) I'd love to see this topic being discussed more publicly (especially since the old suggestions section was removed where everyone was able to vote on certain tickets)!

    Sadly the wording "We [...] may address it sometime in the future" doesn't leave me with high hopes because of the previous sentence since this suggested improvement would always require a breaking change at some point in time.

    Because of this I've thought about a strategy to help facilitate this change:
    1. Move all Vector classes to a package which gets installed via the package manager automatically. To avoid compiler errors due to duplicate classes the internal versions need to be removed. The earliest/best possible time for this would be version 2020.2 which would then have the package installed by default. To avoid problems in earlier Unity versions the package just needs to have a minimum Unity version of 2020.2 specified. Turning this into a package should have zero affects on any existing code. One thing to consider would be the reliance of basically all Editor UI code (Inspector, EditorWindows, PropertyDrawers) on the existance of this package. For this reason it should probably be made uninstallable, the same way the package manager itself is.
    2. The giant advantage of having these Vector classes as modular package would be that basically at the same time a package version 2 could be released as preview which contains the suggested changes. Users who do not rely on third-party code (e.g. packages from the asset store with precompiled code) could then use the preview version right away. Everyone else could stay on the default version until their third-party assets have been updated, if necessary. In case any Unity Editor UI code is affected it could be fixed by the Unity devs by writing explicit V3->V2 cast operators in front where necessary (though I do not expect this to be too many places) to make the Editor work with package version 2. This would not have any downsides when using package version 1.
    3. Mark the implicit V3->V2 cast operator method as obsolete to urge third-party asset developers to not rely on it anymore via warnings in the console but instead write explicit cast operators.
    4. After enough time has passed to be able to assume that most existing libraries have been fixed remove the implicit cast operator method entirely and add the explicit one instead.
    I'm aware that point 4 might take some years but it's the only realistic solution I see right now where the suggested changes could be used in the meantime via a preview package.

    If someone sees any problems with this please let me know.
     
    Last edited: Aug 12, 2020
unityunity