I noticed that if I hit an area not on the Mouseplane the worldMouse getPosition will return zero and the character will walk to (0,0,0) location. what I did was set the return value to Vector3.negativeInfinity if the raycast does not hit the ground plane (or Unit.) then I checked for Vector3.negativeInfinity in the Unit script Move function. Note: you cannot use equivalent operators (== or !=) to check this value you must use Equals(). I don’t think the x y z values will ever go negative so you may be able to set and check if one of them is <0 instead.
Was this already addressed, or is it something that will never happen?
It is very possible for the coordinates to become negative.
Ideally, in a case like this where the return type is a struct and cannot be null, you’d make a TrySomething function. In this case
public static bool TryGetWorldPosition(out Vector3 position)
{
var ray = Camera.main.ScreenPointToRay(InputManager.Instance.GetMousePosition());
if (Physics.Raycast(ray, out RaycastHit hit, float.MaxValue, _instance._mousePlaneLayerMask))
{
position = hit.point;
return true;
}
position = Vector3.zero;
return false;
}
Here it will return false if the mouse didn’t hit the mouse plane.
I don’t recall if this specific scenario was addressed, but it was not an issue because I believe position (0,0,0) did not translate to a valid grid position
2 Likes
I just checked it out. when you go out of bounds you get 0.0 for all three of the xyz values; but that location is also a valid location on the plane and would cause my character to move to it. And when I think about it, you are right, you can get negative number so it would probably be best just to use my first suggestion of using Vector3.negativeInfinity.
Note: I am not really sure what is going on in your code. all I did in my code was have the MouseWorld GetPosition() return Vector3.negativeInfinity if the ray cast was false. then in Unit Move( ), targetPosition would update only if it did NOT receive Vector3.negativeInfinity.
I was merely showing a method that you will find quite often. What it essentially did was to do the check before it even gets to you. You can certainly do it your way. In this case you used Vector3.negativeInfinity. You could also have used Vector3.positiveInfinity and it would have been the same. However, not all immutable types provide something like this. The function I wrote alleviates these problems from immutable types because it returns a boolean instead. If the result is false we have no position, and if it is true we do, and it is right here in the position parameter
1 Like
Yeah, you could use PositiveInfinity in place of negativeInifinity. And yes, you are right not all immutable types provide neg Infinity. but I am using a Vector3; if I used another return type that did not use + or - infinity I would do things differently. Are you trying to make the code more universal? Is that what you are trying to do?
No, I’m just showing a way to do it that is in line with the way things are generally done. You will find that this is exactly how Physics.Raycast(), and GameObject.TryGetComponent(), etc works
okay.
Since the game is with a Top Down camera I don’t think I addressed that issue specifically, just made the floor big enough to always click somewhere.
But yes to make it more robust you can refactor the function to return true or false depending on if it hit a valid position or not just like @bixarrio posted.
That approach is better than comparing with a value like negativeInfinity because it’s easier to understand. Right now you know negativeInfinity means “no hit” but in a few days/weeks/months when you go back to that code you’ll remember that’s what it meant. Whereas if you have a function that returns true or false it’s much more intuitive.
1 Like
I am basically doing it the way bixarrio described; but, GetPosition() returns a vector3 value not true or false; so I used Vector3.Infinity to represent invalid data. I guess I could add an additional bool parameter to represent invalid data, but I am not sure how much that would effect your code as you update it later on. The way I have it, it should not interferer with your code updates.
-----Would adding an extra bool value effect things that much as your code progresses?-----
This code doesn’t get touched further down the line. @bixarrio 's TryGetWorldPosition() method is identical to what I would suggest for this, and it should cause no issues down the remainder of the course.
Maybe it would be best to just show you what I did.
below is the code I original created before my first Post. I originally used Vector3.zero but later changed it to Vector3.NegativeInfininty because Vector3.zero is an actual selectable point on the layerMask Mouse plane. The code below is almost Identical to bixarrios code (except for the change from Vector3.Zero and mine returns a vector3 value instead of a bool value.)
public static Vector3 GetPosition( )
{
var ray = Camera.main.ScreenPointToRay(Input.mousePosition);
if (Physics.Raycast(ray, out RaycastHit hit, float.MaxValue, _instance._mousePlaneLayerMask))
{
return = hit.point;
}
return Vector3.NegativeInfinity; // originally (return Vector3.zero;)
}
We’re not saying that won’t work, because it absolutely will. It’s just not the ideal solution. As @CodeMonkey pointed out, it’s very easy to forget down the line what that NegativeInfnity represents.
It’s also much easier to test a bool than it is to test for a negative Vector3 (think clock cycles!). This is expecially true when you’ve technically got two if statements going on, one for the Physics.Raycast, and one to process the result of GetPosition(). If GetPosition is a bool, then determining that the result is invalid is quite literally passed down the method, where if it’s a Vector3, then it still has to be tested before short circuiting the method. I.e. a second test when the negative result has already been determined by the function that was called.
So far, we’ve actually only discussed two solutions for determining if the raycast was valid… we could also use a tuple. With a tuple, you can return multiple values in a method without declaring a specific struct or class as a return type.
public static (bool, Vector3) TryGetPosition()
{
Ray ray = Camera.main.ScreenPointToRay(Input.mousePosition);
return (Physics.Raycast(ray, out RaycastHit hit, float.MaxValue, instance.layerMask), hit.point);
}
For this, you would call the method like this:
(bool isValid, Vector3 point) = MouseWorld.TryGetPosition();
if(isValid)
{
///do stuff with point
}
This example, of course, is not a conventional one, just demonstrating that when it comes to C# and Unity, there are often many paths to the same path. Both this solution and the one @bixarrio posted don’t require testing the value of the Vector3 result in the calling function.
2 Likes
You made a good point that made me decide to change my code back to using Vector3.zero. Sure, there is a slim chance that you may hit the exact point of (0,0,0) point on the mouse plane layer mask but since that would hardly ever happen then the performance boost would out way the minor inconvenience of hitting some obscure point with nothing happening. And when you try to use the mouse again the cursor would probably move a little bit allowing you to hit some other point next to (0,0,0);
wait… my way still wont work because it sends back zero and that would mean I would still need to check for zero. Your way of using tuples was something I also tried but making that change meant that I had to make changes to all the code that uses that GetMousePosition function…oh, well maybe that is the best thing to do.
@Brian_Trotter I love tuples.
@dougwarner59 Perhaps I’m just ignorant, but I don’t understand why you don’t just use the TryGetWorldPosition function. You have gone from insisting on using your way, to deciding to use the tuple way, which - as @Brian_Trotter mentioned - is an unconventional one. Don’t get me wrong, I’m not saying any of these are ‘wrong’, I’m just trying to understand.
As I have mentioned, I love tuples. When they were first introduced, I went on a rampage and started changing everything with an out parameter to use tuples instead. But, it’s a relatively new thing in C# and it confused a lot of people. I even received a mail from someone saying they don’t know how ‘this code even builds’. The TrySomething pattern is well know and understood, and causes a lot less confusion down the line.
It’s not because I thought your code was bad; In fact, it would have solved my problem without taking a performance hit. So, you were right from the beginning; And you are also right, the code you presented is easier to read and understand compared to using Tuples; I decided to use Tuples because I wanted to get better familiar with them.
1 Like
I’m a huge fan of tuples, and if you’re comfortable, please feel free to use them. The do add a readability hit to folks who don’t understand what’s going on, but once you’re used to them, they are quite elegant. I wasn’t necessarily suggesting it as the optimization for this problem, but just showing another way to skin the cat, as it were.
FWIW, I handled this situation by returning a Vector3? (a nullable value type) that is either a valid Vector3, or it’s a null value. Then the caller can simply test the value against null to determine whether it’s a valid vector or not.
There are good arguments against returning null in general but the function signature makes it obvious that null is a potential return value, and in fact the compiler makes you deal with it, which is the whole point of a nullable type in the first place.
2 Likes
Oh wow, that’s pretty cleaver; that would have fixed things without having to make major changes.
That is an excellent solution!