I found that if I replaced “out selectedUnit” with “out Unit unit” I was able to eliminate the if statement because it would automatically assigning the selectedUnit for you in the following code snippit:
if(physics.Raycast(…)){
rayCastHit.transform.TryGetComponent(out selectedUnit);
}
is this okay to do?
1 Like
Technically that works, personally I find my method easier to read, first you try to get the component, if you get it then you assign the field. I find that logic easier to follow than assigning the field directly.
So that does work, however later on in the course that code is refactored so we can do more things when we change the selected unit (like fire an event), and in doing so we need to go through a function instead of just setting the field directly.
But yes you can keep that logic until you get to that refactoring.
4 Likes
I see where you are coming from; but what I have been doing after I watch a few of your videos is I delete the game and start over until I can recreate what I have learned. so readability is not my main concern; to save time I try and use as may short cuts as I can (such as using magic numbers and Not using " f " after an integral value to cast it to float (sense it doesn’t require a cast and will be implicitly converted anyways.)
It isn’t so the code readability is good for you. It’s about building good practices for people who haven’t built your code. The whole point is making their lives easier 
4 Likes
Readability should ALWAYS be your main concern.
Sure you might work solo and not care about showing your code to other developers.
But you yourself also count as “another person”. When you go back to your own code after 1 week you will not remember what all those shortcuts and magic numbers mean.
Even if you never intend to ever touch that code ever again I would highly highly encourage you to take clean code and readability seriously, it’s just a good practice all around (assuming your goal is to be a better and more capable programmer)
3 Likes
Also, if for some reason the GameObject does not have the Unit script component, your selected unit will become null, as you are using an out parameter, giving you a different result (maintaining the last selected unit). So it will also depends what you want to achieve in that scenario.
To add to what @CodeMonkey and @MrVastayan have already stated, there WILL be a time when you are either asking for a code review, or you’re giving a code review. It’s inevitable.
If you’re reading someone else’s code smell, you’re going to hate it.
Clean code is a fundamental practice. Of course, we write things to make them work, but it is a good practice to always go back and review what we’ve already done and see if it can be refactored.
Another way to put it is: If you’re starting over and over again and doing the exact same thing, then you’re really only rehearsing your mistakes.
Learning to refactor and write clean code is fundamental. It is inevitable that you will eventually be asking someone for help with your code, or someone will be asking you to read their code smell.
So many good points made by everyone.
I like your approach to the course. Trying to recreate the game on your own is a very good way to learn. There is no substitute for experience.
Now, my 2c:
You should always use the methods for its intended use. If you are not going to use the ‘if’ (as you have mentioned) then don’t use TryGetComponent but GetComponent instead. You will then be responsible for checking that the returned value is not null, and make it clear what your intentions were.
Adding an ‘f’ behind an literal takes no time to add, and there is a difference between 1.5 and 1.5f: the first is a double and the second is a float. C# cannot automatically cast this.
Lastly, shortcuts may help you get through everything quickly now, but will bite you sooner or later.
2 Likes
Hello,
Had to add a second ‘return false’ in the TryHandleUnitSelection method otherwise it gave back a null reference exception from the TryGetComponent:
bool TryHandleUnitSelection()
{
Ray ray = Camera.main.ScreenPointToRay(Input.mousePosition);
if (Physics.Raycast(ray, out RaycastHit raycastHit, float.MaxValue, _unitsLayerMask))
{
if (raycastHit.transform.TryGetComponent(out Unit unit))
{
_selectedUnit = unit;
return true;
}
return false;
}
return false;
}
Also, really enjoy your courses and have learned a lot from you. Thank you for going over even basic things in detail. Those little kernels of knowledge add up.
Where were you getting a null reference? If the Raycast hits something then raycastHit.transform should always be set
1 Like
when I was redoing my game again I found out that if had an additional collider(s), it sometimes would result in a nullReferencException . I fixed this by checking that UnitSelected was not null. So it may be a good idea to check for null before using it, or make sure you don’t make any mistake (that leaves me out.)
Every time I re-due the game I learn something new or I relearn something I forgot, catch something I was doing wrong, or reinforcing what I have learned. I don’t have a photographic/ excellent memory and I don’t learn things in exact order. It has to make sense for me to be able recreate the game. This way works for me but it may not work for you.
3 Likes
Apologize, was out of town for a bit.
I was getting a null reference from the:
‘Physics.Raycast(ray, out RaycastHit raycastHit, float.MaxValue, _unitsLayerMask)’
When I added the first ‘return false’ as shown above it cleared.
Now I can not reproduce the error. Not sure why. Sorry for the bother… weird.
Thanks man! I did try that. I tried going through the game object as a reference and checking everything for null. Made for a very frustrating night. Now I can’t reproduce it… such is life.
Sorry, for the delay to bizarrio’s post about double to float conversions . Yes if you tried to used a double when a float was required it would immediately bite you by giving you a static type checking error; this is because double is larger than float; and, using it could cause a possible loss of data so the compiler requires an explicit cast in the form of (float)1.4 or a numeric suffix of f or F. What I was talking about was implicitly using an integer in place of a float value ;which is allowed so casting it to float is not necessary. If I see an integer being used in unity I automatically assume that it is either being used as an integer or as a float due to this implicit conversion rule.