Some changes made to the CameraController

Unity Courses Show
,
#1

This are some changes I made to the CameraController script, in my opinion they make the code more readable and organized.

using UnityEngine;

public class CameraController : MonoBehaviour
{
    private Transform thisTransform;
    
    private void Awake()
    {
        thisTransform = transform;
    }

    private void Update()
    {
        CameraMovement();
        CameraRotation();
    }

    private void CameraMovement()
    {
        Vector3 inputMoveDir = Vector3.zero;
        
        if (Input.GetKey(KeyCode.W))
            inputMoveDir += Vector3.forward;
        
        if (Input.GetKey(KeyCode.S))
            inputMoveDir += Vector3.back;
        
        if (Input.GetKey(KeyCode.A))
            inputMoveDir += Vector3.left;
        
        if (Input.GetKey(KeyCode.D))
            inputMoveDir += Vector3.right;
        
        Vector3 moveVector = thisTransform.forward * inputMoveDir.z + thisTransform.right * inputMoveDir.x;
        
        const float moveSpeed = 10f;
        thisTransform.position += moveVector * (moveSpeed * Time.deltaTime);
    }

    private void CameraRotation()
    {
        Vector3 rotationVector = Vector3.zero;
        
        if (Input.GetKey(KeyCode.Q))
            rotationVector += Vector3.up;
        
        if (Input.GetKey(KeyCode.E))
            rotationVector += Vector3.down;
        
        const float rotationSpeed = 100f;
        thisTransform.eulerAngles += rotationVector * (rotationSpeed * Time.deltaTime);
    }
}

The main changes are:

  • Removed the {} on the ifs because they have only one line inside (I know this is very controversial, and deppends on personal preference)
  • Separated the camera movement and camera rotation in two different private methods, so it is more clear what each part of this code does.
  • Saved the transform reference in a private variable. Since its going to get accesed a lot, I think this is a good optimizacion to not look for the same reference lots of times on Update.
  • Reordered this multiplication to make it (as my IDE recommends) more efficent. moveVector * (moveSpeed * Time.deltaTime). The result is exactly the same, as multiplying vector * number or number * vector is commutative. The explanation for this one is that it is easier for the computer to calculate vector * (number) than vector * number, and then resulting vector * number again.

Hope you like my modifications to this code, if there is something you do not agree or think I am wrong dont doubt in telling me.

Thanks!

2 Likes
#2

Yes the compiler does not care if the ifs have a code block {} however it really is good practice to always use them.
Without a code block you can very easily accidentally cause tons of weird bugs, if you forget to add a line or add multiple lines you can easily break everything without understanding why.

At least if you really don’t want to use them then make the if statement inline

if (Input.GetKey(KeyCode.W)) inputMoveDir += Vector3.forward;

That way at least it’s more clear that there’s only one statement inside that if

Transform access is already optimized so you don’t need to cache it.

Yup your IDE is correct, multiplying two floats and then multiplying the result with a Vector is indeed faster than Vector * float * float.
Although do be careful not to get caught up with trying to do needless optimizations, the difference in those two is in the order of microseconds.

Great job on trying to do code improvements on your own, that’s the best way to learn!

3 Likes
#3

Thanks for the feedback!

I will not be doing unnecesary or excesive optimizations when I work in my “real” projects.

But as you said, I am trying this stuff just to learn.

Thanks!

1 Like
#4

I like using the directional vectors instead of some +1/-1 on some axis that you have to map to which direction it points to when you want to follow what the code does.

I should apply this to my own project… :slight_smile:

Privacy & Terms