My take on the Rotate code. Is this change a good or bad programming practice?

Unity Courses Talk
, ,
#1

I’m looking for feed back on the logic behind my change to the code. On this small scale it doesn’t really matter, but I’m trying to build good habits for when I get to larger code.

Here is my reasoning behind my change. Since the left and right rotation are the same other than the direction (+/-), I created a “direction” variable. Then I pulled the actual rotation commands out of the IF statement and pasted it below the IF.

Pros:
Allows to change one calculation if it was necessary later. (Less code maintenance)
Cleaner looking code (Easier Code maintenance)

Cons:
You are technically always rotating, and calculating a rotation every frame. (More CPU usage)
Another variable (More RAM usage)

Any comments or thoughts on this matter are appreciated.
Thanks

My Rotate Code

void Rotate()
    {
        rigidBody.freezeRotation = true;
        float rotationThisFrame = rotateThrust * Time.deltaTime;
        int direction = 0;

        if      (Input.GetKey(KeyCode.A))      { direction = 1;  }
        else if (Input.GetKey(KeyCode.D))      { direction = -1; }

        transform.Rotate(direction * Vector3.forward * rotationThisFrame);
        rigidBody.freezeRotation = false;
    }   // My Rotate Method

Here is the Standard Rotate method for comparison

 void StandardRotate()
    {
        rigidBody.freezeRotation = true;
        float rotationThisFrameStandard = rotateThrust * Time.deltaTime;

        if (Input.GetKey(KeyCode.A))
        {
            transform.Rotate(Vector3.forward * rotationThisFrameStandard);
            print("A Key Pressed - Rotate Left");
        }
        else if (Input.GetKey(KeyCode.D))
        {
            transform.Rotate(-Vector3.forward * rotationThisFrameStandard);
            print("D Key Pressed - Rotate Right");
        }
        rigidBody.freezeRotation = false;
    }   //Standard Rotate Method
#2

Hi,

the idea to calculate the rotation only once is a good one. My thoughts on your solution are at first considerations on naming your variable. To name your variable according to what is is, think about that a “direction” is always a vector, therefore you are not using a direction, you are using a directionMultiplier. Naming the variable accordingly would make its meaning clear from the beginning.

Other thoughts are on the multiplier itself which has limitations (something that was not part of this course chapter). Imagine you want not only to move forward and back, but also left / right or up / down. A multiplier will not be helpful in this cases.

My idea is to use a Vector3 as direction variable and get rid of the +1/-1 multiplier. Furthermore if performance problems may occur due to the unnecessary transform.Rotate (which I am nor certain about, due to lacking Unity experience) another if statement can be used:

void Rotate()
{
	rigidBody.freezeRotation = true;

	Vector3 direction = Vector3.zero;

	if (Input.GetKey(KeyCode.A)) { direction = Vector3.forward; }
	else if (Input.GetKey(KeyCode.D)) { direction = Vector3.back; }
	else if (Input.GetKey(KeyCode.X)) { direction = Vector3.left; } // you may need more than two directions
	//... or more than three directions

	if (direction != Vector3.zero)
	{
		float rotationThisFrame = rotateThrust * Time.deltaTime;
		transform.Rotate(direction * rotationThisFrame);
	}

	rigidBody.freezeRotation = false;
}

I hope my thoughts are helpful, please keep in mind I am no native speaker, if anything in my texts sounds strange or funny.

Privacy & Terms