Is there a cleaner way to write this simple script?

Hello!

Im new to the code game and I think there might be a simpler way to write this toggle script. As I am learning im always looking for improved ways to write code. Any tips ?

public class BaseRotation : MonoBehaviour
{

    [SerializeField] float rotationSpeed = 1f;
    [SerializeField] bool isRotating = false;

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.R))
        {
            isRotating = !isRotating;
        }
        if (isRotating == true)
        {
            StartRotation();
        }
    }

    void StartRotation()
    {
        transform.Rotate(Vector3.up * rotationSpeed * Time.deltaTime);
    }
}

Well, I would mostly focus on accurate naming of things, because StartRotation() doesn’t start the rotation, it’s called every frame - it just rotates it.

You also don’t need " == true" in the if statement.

Beyond that, it really depends on how you expect to expand this script in the future. As one example, are you planning on adding multiple speeds or directions? It might be better to replace isRotating with a float speedMultiplier, and use 1 for rotating right, -1 for rotating left, 0 for stopped, etc.

2 Likes

No, you can only condense it further to

void Update() {
  if(Input.GetKeyDown(KeyCode.R))
    if(isRotating = !isRotating) StartRotation();
}

But this only makes it less readable, and employs a bad practice in general.

(This works because assignments evaluate from right to left, and return the last assigned value. We typically don’t use this value and assignments are usually used as statements, but this expression is completely valid. The bad practice is using = inside the if clause, because this can confuse you or someone else into seeing an equality test which it is not.)

The most compact/legible hybrid would be

void Update() {
  if(isKeyDown(KeyCode.R)) isRotating = !isRotating;
  if(isRotating) StartRotation(); // but I guess you want to do this only if a key was pressed??
}

//

bool isKeyDown(KeyCode key) => Input.GetKeyDown(key);

However if you’re planning to expand the method, none of this makes sense, and your original code is the best choice. I would only get rid of == true like StarManta said.

To get more insight about this @goobtasticgaming , I recommend you spend time just trying out random cool tutorials done by different people, which will gradually give you insights into all the different ways of doing things, and from that you can start to get a feel for the tradeoffs that Manta implies above.

Here’s a slew of great random tutes to start from:

Imphenzia / imphenzia - super-basic Unity tutorial:

https://www.youtube.com/watch?v=pwZpJzpE2lQ

Jason Weimann:

https://www.youtube.com/watch?v=OR0e-1UBEOU

Brackeys super-basic Unity Tutorial series:

https://www.youtube.com/watch?v=IlKaB1etrik

Sebastian Lague Intro to Game Development with Unity and C#:

ALSO, to maximize your time spent on tutorials, keep this in mind:

How to do tutorials properly:

Tutorials are a GREAT idea. Tutorials should be used this way:

Step 1. Follow the tutorial and do every single step of the tutorial 100% precisely the way it is shown. Even the slightest deviation (even a single character!) generally ends in disaster. That’s how software engineering works. Every single letter must be spelled, capitalized, punctuated and spaced (or not spaced) properly. Fortunately this is the easiest part to get right. Be a robot. Don’t make any mistakes. BE PERFECT IN EVERYTHING YOU DO HERE.

If you get any errors, learn how to read the error code and fix it. Google is your friend here. Do NOT continue until you fix the error. The error will probably be somewhere near the parenthesis numbers (line and character position) in the file. It is almost CERTAINLY your typo causing the error, so look again and fix it.

Step 2. Go back and work through every part of the tutorial again, and this time explain it to your doggie. See how I am doing that in my avatar picture? If you have no dog, explain it to your house plant. If you are unable to explain any part of it, STOP. DO NOT PROCEED. Now go learn how that part works. Read the documentation on the functions involved. Go back to the tutorial and try to figure out WHY they did that. This is the part that takes a LOT of time when you are new. It might take days or weeks to work through a single 5-minute tutorial. Stick with it. You will learn.

Step 2 is the part everybody seems to miss. Without Step 2 you are simply a code-typing monkey and outside of the specific tutorial you did, you will be completely lost.

Of course, all this presupposes no errors in the tutorial. For certain tutorial makers (like Unity, Brackeys, Imphenzia, Sebastian Lague) this is usually the case. For some other less-well-known content creators, this is less true. Read the comments on the video: did anyone have issues like you did? If there’s an error, you will NEVER be the first guy to find it.

Beyond that, Step 3, 4, 5 and 6 become easy because you already understand!

1 Like

Not only that, your first example does not really do the same as the original code. Your code would only rotate the object once when the key is pressed. A “cryptic” solution that actually works would be

    private void Update()
    {
        if (isRotating ^= Input.GetKeyDown(KeyCode.A))
            transform.Rotate(0f, rotationSpeed * Time.deltaTime, 0f);
    }

This does work since we use the xor operator to toggle the isRotating boolean value inplace. “Xoring” any boolean value with false will keep the value as it is, Xoring with true will toggle it. Though as already mentioned, those are all negative examples how you should not write code. It’s cryptic and hard to reason about. Shorter code is not always better code. Shorter code also does not need to be more efficient. Sometimes it’s actually the opposite (for example Linq allows to write neat one-liners which are almost always less efficient than a manual implementation).

I would recommend sticking to what StarManta said.

1 Like

Yeah right, I didn’t pay too much attention, because I don’t really expect anyone to do this.
I could’ve done ^= inside if for once in life, oh well. :slight_smile:

If you like writing code like that you may apply to the IOCCC but better not make games ^^.

1 Like