Search Unity

I'm a programming noob, which of these two coding styles is best?

Discussion in 'Scripting' started by SgtSargeMcFly, Sep 24, 2021.

  1. SgtSargeMcFly

    SgtSargeMcFly

    Joined:
    May 4, 2020
    Posts:
    20
    The premise for my game is that it's a sidescroller where the spherical character can be controlled in two ways - statically, with the left and right arrows, which moves the character at a fixed rate with no acceleration. The other way is to spin the character, where a lot more physics like friction and momentum get applied. On the ground, spinning won't get you far because of the friction, but if you jump and build up spin speed in the air, you can get a burst of speed when you hit the ground and go faster than you could with static movement. I'd like the character to be able to act like a buzzsaw and adhere to walls and ceilings with enough spin, but I have no idea how to approach that so it's not featured here!

    Here's my conundrum: I'm no good at programming, I came up with two drafts for the code to control this character on a simple, flat plane.

    This is the first draft, where I wasn't worried about efficiency and just wanted to get the concept running so I could visualize it and see if it's fun.

    Code (CSharp):
    1. //COUNTERCLOCKWISE
    2.         if (Input.GetKey(KeyCode.UpArrow))
    3.         {
    4.             if (GetComponent<Rigidbody2D>().IsTouching(ground)) //Set speed cap against ground
    5.             {
    6.                 if (spinAnim["spin"].speed < 0.3f)
    7.                 {
    8.                     spinAnim["spin"].speed += 0.05f;
    9.                 }
    10.             } else
    11.             {
    12.                 if (spinAnim["spin"].speed < 2.0f)
    13.                 {
    14.                     spinAnim["spin"].speed += 0.1f;
    15.                 }
    16.             }
    17.         }
    18.         else if (spinAnim["spin"].speed > 0.0f)
    19.         {
    20.             spinAnim["spin"].speed -= 0.1f;
    21.             if (spinAnim["spin"].speed < 0.0f)
    22.             {
    23.                 spinAnim["spin"].speed = 0.0f;
    24.             }
    25.         }
    26.  
    27.         //CLOCKWISE
    28.         if (Input.GetKey(KeyCode.DownArrow))
    29.         {
    30.             if (GetComponent<Rigidbody2D>().IsTouching(ground)) //Set speed cap against ground
    31.             {
    32.                 if (spinAnim["spin"].speed > -0.3f)
    33.                 {
    34.                     spinAnim["spin"].speed -= 0.05f;
    35.                 }
    36.             } else
    37.             {
    38.                 if (spinAnim["spin"].speed > -2.0f)
    39.                 {
    40.                     spinAnim["spin"].speed -= 0.1f;
    41.                 }
    42.             }
    43.         }   // Deceleration
    44.         else if (spinAnim["spin"].speed < 0.0f)
    45.         {
    46.             spinAnim["spin"].speed += 0.1f;
    47.             if (spinAnim["spin"].speed > 0.0f)
    48.             {
    49.                 spinAnim["spin"].speed = 0.0f;
    50.             }
    51.         }
    52.         Debug.Log(spinAnim["spin"].speed);
    53.  
    54.         if (GetComponent<Rigidbody2D>().IsTouching(ground))
    55.         {
    56.             finalSpeed = (100 * spinAnim["spin"].speed * -1);
    57.         }
    58.         else
    59.         {
    60.             finalSpeed = 0;
    61.         }
    62.  
    63.         // Apply all the added speed variables
    64.         rigidbody.velocity = new Vector2((100 * Input.GetAxis("Horizontal") + finalSpeed), rigidbody.velocity.y);
    It's not the entire code, but hopefully you get the idea. I knew it was ineffecient, having two very samey blocks of code just for left and right movement. I tried to streamline it more with this next iteration:

    Code (CSharp):
    1. //Check if touching the ground
    2.         if (GetComponent<Rigidbody2D>().IsTouching(ground))
    3.             spinDrag = 0.3f;
    4.         else
    5.             spinDrag = 2.0f;
    6.  
    7.  
    8.         //Set speed
    9.         spinAnim["spin"].speed += ((spinDrag / 6.0f) * spinDir);
    10.         //Set speed cap
    11.         if (Mathf.Abs(spinAnim["spin"].speed) > spinDrag)
    12.             spinAnim["spin"].speed -= ((spinDrag / 6.0f) * spinDir);
    13.  
    14.         //Deccelerate
    15.         if (Mathf.Abs(spinAnim["spin"].speed) > 0.0f)
    16.         {
    17.             //Make sure object won't start spinning in the other direction, then set the new speed
    18.             if (spinAnim["spin"].speed * (spinAnim["spin"].speed + (-0.01f * Mathf.Sign(spinAnim["spin"].speed))) >= 0)
    19.             {
    20.                 spinAnim["spin"].speed += (-0.01f * Mathf.Sign(spinAnim["spin"].speed));    // Deccelerate according to entropy
    21.                 groundSpeed = spinAnim["spin"].speed; // Use groundSpeed variable here to keep track of spin speed temporarily;
    22.                 spinAnim["spin"].speed += (-0.1f * Input.GetAxisRaw("Horizontal"));    // Deccelerate by input
    23.                 // Make sure horizontal input can't reverse the spin animation
    24.                 if (spinAnim["spin"].speed * groundSpeed < 0.0f)
    25.                     spinAnim["spin"].speed = 0.0f;
    26.                 if (Mathf.Abs(spinAnim["spin"].speed) > Mathf.Abs(groundSpeed))
    27.                     spinAnim["spin"].speed = groundSpeed;
    28.             }
    29.             else
    30.                 spinAnim["spin"].speed = 0.0f;  // If it would start spinning in the other direction, set speed to 0
    31.         }
    32.  
    33.         // Determine the final spin speed to be applied to the character, should only occur when on the ground
    34.         if (spinDrag == 0.3f)
    35.             spinSpeed = spinAnim["spin"].speed * 100 * -1;
    36.  
    37.         // Set ground speed
    38.         groundSpeed = (75 * Input.GetAxisRaw("Horizontal"));
    39.  
    40.         if (Mathf.Abs(groundSpeed) > 190)
    41.             groundSpeed = Mathf.Sign(groundSpeed) * 190;
    42.         if (Mathf.Abs(spinSpeed) > 190)
    43.             spinSpeed = Mathf.Sign(spinSpeed) * 190;
    44.  
    45.         // Set speed based on strongest force
    46.         if (Mathf.Abs(groundSpeed) > Mathf.Abs(spinSpeed))
    47.             finalSpeed = groundSpeed;
    48.         else
    49.             finalSpeed = spinSpeed;
    50.  
    51.         if (Mathf.Abs(finalSpeed) > 190)
    52.             finalSpeed = Mathf.Sign(finalSpeed) * 190;
    53.  
    54.         Debug.Log(spinSpeed);
    55.         rigidbody.velocity = new Vector2(finalSpeed, rigidbody.velocity.y);
    This code accomplishes the same purpose, with a few extras like being able to quickly diminish spin speed using static input. So, it's not the EXACT same thing, but

    Here's what I want input on the most: is the style I coded in for the second iteration actually better? Not only is it a lot harder to read and took me much longer to write, but the way it uses all of those Mathf functions has me worried about it actually being heavier on CPU than the first iteration of code. Should I be opting for simpler, easier-to-read code like the first example? Thanks for anyone who chips in, there's no need to examine the code super thoroughly or anything, I'd just like to know if the use of all these Mathf functions and the like is advisable.
     
  2. halley

    halley

    Joined:
    Aug 26, 2013
    Posts:
    2,443
    The difference between those two approaches is likely measurable but insignificant, in the long run. There will be a lot of things your game will spend more time on doing. The better code is that which is more clear for you to understand and modify as you make changes, even after a month or two without looking at the code.

    Premature Optimization is evil. Make it work. Make it work the way it should. Make it work fast. In that order.
     
  3. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,736
    First, don't repeat yourself. If you find yourself writing something complicated like
    spinAnim["spin"]
    twenty seven thousand times, do yourself a favor and assign it to a temporary variable at the start of everything.

    Second, never compare floating point values for equality, such as line 34 in the second script, and even (more subtly) the line 15 Mathf.Abs() > 0 comparision, which is effectively just asking if it is not equal to zero. Never compare for equality, but rather ask if it is close enough, either by difference or by using Mathf.Approximately().

    Finally, I'm not sure what you're even doing there, but if all you are doing is moving one value smoothly towards another, you can do that with a LOT fewer moving parts, then send the value out to your animator all at once:

    Smoothing movement between any two particular values:

    https://forum.unity.com/threads/beginner-need-help-with-smoothdamp.988959/#post-6430100

    You have currentQuantity and desiredQuantity.
    - only set desiredQuantity
    - the code always moves currentQuantity towards desiredQuantity
    - read currentQuantity for the smoothed value

    Works for floats, Vectors, Colors, Quaternions, anything continuous or lerp-able.

    The code: https://gist.github.com/kurtdekker/fb3c33ec6911a1d9bfcb23e9f62adac4

    Finally, as with ANY performance question, staring at code is useless. It's simply NOT a thing.

    DO NOT OPTIMIZE CODE RANDOMLY... always start by using the profiler:

    Window -> Analysis -> Profiler

    https://forum.unity.com/threads/is-...ng-square-roots-in-2021.1111063/#post-7148770

    Optimizing UnityEngine.UI setups:

    https://forum.unity.com/threads/how...form-data-into-an-array.1134520/#post-7289413
     
  4. SgtSargeMcFly

    SgtSargeMcFly

    Joined:
    May 4, 2020
    Posts:
    20
    Thank you guys for giving me some direction. I've probably been getting too obsessive about optimization, I just figured it's best to write the cleanest most efficient code you can right off the bat before you write a bunch of messy scripts that you can't change without messing others up.

    Code (CSharp):
    1. Finally, I'm not sure what you're even doing there, but if all you are doing is moving one value smoothly towards another, you can do that with a LOT fewer moving parts, then send the value out to your animator all at once:
    So basically you're saying for spinAnim["spin"].speed, I should set desiredQuantity to the speed cap for the current context and have currentQuantity move toward it each frame? I'm on my phone right now so I can't open the unity package file in your linked post at the moment, but I just assume that's what you're suggesting based on your post

    And thank you for actually taking the time to comb through the specifics of my code! I wrote it a day ago and even I can't remember what exactly some of these formulas are doing, so I didn't expect anyone else to understand. I appreciate it a lot!
     
  5. Ryiah

    Ryiah

    Joined:
    Oct 11, 2012
    Posts:
    21,187
    Whichever is easiest for you to work with. If someone has to work with your code they can always have their IDE format it according to the rules they prefer.