Search Unity

How do I know when to use a local variable?

Discussion in 'Editor & General Support' started by MinhocaNice, May 22, 2020.

  1. MinhocaNice

    MinhocaNice

    Joined:
    May 3, 2020
    Posts:
    249
    I am working on a game using C# and I wanted to know in which situations creating a local variable to store temporary data (GetComponent, Physics.Raycasts, gamObject.transform, etc) is a good idea, and which situations it isn't. I searched online and found conflicting answers, some going as far as saying that you should do it every single time and others saying only very specific situations need that. Example: https://answers.unity.com/questions/452946/which-unity-functions-are-generally-slowbad-for-pe.html
    In this example, a guy says that this script:
    Code (CSharp):
    1.  
    2.  //This is bad as it create an unique block each time you call FindWithTag.
    3. Destroy(GameObject.FindWithTag("ToBeDestroyed_A");
    4. Destroy(GameObject.FindWithTag("ToBeDestroyed_B");
    5. Destroy(GameObject.FindWithTag("ToBeDestroyed_C");
    6.  
    Is worse than this script:
    Code (CSharp):
    1.  
    2. //This is better since it only create an unique block for Target once and reuse the same block afterward when necessary.
    3. GameObject Target = GameObject.FindWithTag("ToBeDestroyed_A");
    4. Destroy(Target);
    5. Target = GameObject.FindWithTag("ToBeDestroyed_B");
    6. Destroy(Target);
    7. Target = GameObject.FindWithTag("ToBeDestroyed_C");
    8. Destroy(Target);
    9.  
    Even though the second ones looks kind of redundant (to me at least). And some other people proceeded to disagree with this answer. I got confused with these discussions, and I wanted to know if this is really a hard situation to have a good answer with or are those people simply saying crazy stuff.

    I am going to share my situation here, but I would really like a global explanation that I can apply every time I get in this dilema. I am making a FPS game, and I just added a target system based on a tutorial from Brackeys, so there is a target script, and a gun script, and the gun script references the target script in a local variable in his code. Here is the code inside the gun script (when the gun fire function):

    Code (CSharp):
    1.  
    2.     void Fire ()
    3.     {
    4.         MuzzleFlash.Play();
    5.         if (AimingAtObject) //Checks if there is someting on front, otherwise there is no need to do anything//
    6.         {
    7.          
    8.             Debug.Log (Trajectory.transform.name + " was hit by a bullet;");
    9.             if (Trajectory.transform.CompareTag ("Shootable"))
    10.             {
    11.                 Target target = Trajectory.transform.GetComponent<Target> (); //Is this local variable necessary?//
    12.                 if (target != null)
    13.                 {
    14.                     target.TakeDamage (Damage);
    15.                 }
    16.             }
    17.             GameObject Impact = Instantiate (ImpactEffect, Trajectory.point, Quaternion.LookRotation (Trajectory.normal));
    18.             Destroy (Impact, 2f);
    19.         }
    20.     }
    21.  
    I was wondering if it is really necessary to reference the Target script in the target variable?
    I also made a code myself to raycast all the time in Update (), so the crosshair follows where the gun is pointed to instead of just the center of the screen. I used rectTransform for that, but didn't make any local variable, I was wondering if it would be a good idea to do so, or is it unnecessary?
    Code (CSharp):
    1.  
    2.     void Update ()
    3.     {
    4.         if (Input.GetButtonDown ("Fire1"))
    5.         {
    6.             Fire();
    7.         }
    8.          
    9.         if (Physics.Raycast (Gun.position, FPCamera.transform.forward, out Trajectory, Range))
    10.         {
    11.             AimingAtObject = true;
    12.             Vector2 ScreenPos = FPCamera.WorldToScreenPoint (Trajectory.point);
    13.             DynamicCrosshair.rectTransform.position = ScreenPos; //Would a local variable here be good?//
    14.         }
    15.         else
    16.         {
    17.             AimingAtObject = false;
    18.             DynamicCrosshair.rectTransform.position = CenterCrosshair.rectTransform.position; //How about here?//
    19.         }
    20.     }
    21.  
     
    Last edited: May 22, 2020
  2. PraetorBlue

    PraetorBlue

    Joined:
    Dec 13, 2012
    Posts:
    7,914
    First, write whatever is most readable and maintainable. Then optimize later if you run into performance issues.

    Btw I have no idea what that first example is about with the FindObjectByTag, but I'm pretty sure they both will compile to the exact same IL code.
     
    Joe-Censored and MinhocaNice like this.
  3. MinhocaNice

    MinhocaNice

    Joined:
    May 3, 2020
    Posts:
    249
    Thanks for the (quick) reply, well I heard around it is better to do the best option right away than have to keep tweaking later, so that's why i am worried about performance issues, even when I didn't have none yet (the game is yet too simple to have any impacts). Also, I don't know how the game will run on slower computers than mine, so just tweaking when it starts running slow on my PC gives ground for lesser performance issues that don't affect me to stick around and affect people who play the game with slower machines.

    About the first example I mentioned, I don't want to sound rude, but it sounds weird to me that you think 6 lines of code with 3 of them being practically useless as you said are easier to read than just 3 lines that explicitly do said action. I think even if I showed the first example to my brother who never coded in his life he would understand what those do, while the latter is quite weird and hard to capture what is going on if you read quickly. But I guess it's just preference as you said.

    Also, if you could check the link for that example, would you know why some people seemed to disagree with such argument that the latter example is better? You spoke like it was such an easy thing to know which is better than which, yet in the link I see a FullOnGiantText type discussion of why that is wrong (even though it seems they are the ones who are wrong).
     
  4. MinhocaNice

    MinhocaNice

    Joined:
    May 3, 2020
    Posts:
    249
    Read the link I mentioned, it's used by someone to explain why should you always use local variables. Some people didn't agree with that, so I placed the example here to see what you guys think.
     
  5. PraetorBlue

    PraetorBlue

    Joined:
    Dec 13, 2012
    Posts:
    7,914
    Hey sorry about the confusion. The first time I read your example I had thought that it was this
    Code (CSharp):
    1.  //This is bad as it create an unique block each time you call FindWithTag.
    2. Destroy(GameObject.FindWithTag("ToBeDestroyed_A");
    3. Destroy(GameObject.FindWithTag("ToBeDestroyed_A");
    4. Destroy(GameObject.FindWithTag("ToBeDestroyed_A");
    VS this
    Code (CSharp):
    1.  //This is bad as it create an unique block each time you call FindWithTag.
    2. GameObject Target = GameObject.FindWithTag("ToBeDestroyed_A");
    3. Destroy(Target);
    4. Destroy(Target);
    5. Destroy(Target);
    Now I know this example makes no sense as well (why would you destroy the same object 3 times?) but let's assume maybe it was a different method, maybe one that moved the target 1 unit forward or something? In this example it would be faster to use a local variable than to call FindWithTag 3 times.

    So it was that misunderstanding which led me to originally create my post agreeing with the use of the local variable there and then edit it later when I reread what the example actually was.

    I looked at your link, and that weird example doesn't actually have a single person agreeing with it, because it doesn't make any sense at all. The "unique block" thing he is talking about is nonsense int hat context. Sorry for the confusion.
     
    MinhocaNice likes this.
  6. MinhocaNice

    MinhocaNice

    Joined:
    May 3, 2020
    Posts:
    249
    Alright, so in the first example there is no need for local variables right?

    But how about in my two next examples?
    Is it better to set
    DynamicCrosshair.rectTransform.position
    as a local variable before setting it equals to ScreenPos? In fact, is it necessary to set
    FPCamera.WorldToScreenPoint (Trajectory.point)
    as ScreenPos in the first place? Or could I just do this?
    DynamicCrosshair.rectTransform.position = FPCamera.WorldToScreenPoint (Trajectory.point);


    BTW, I am talking about this one:


    Code (CSharp):
    1.  
    2. void Update ()
    3.     {
    4.         if (Input.GetButtonDown ("Fire1"))
    5.         {
    6.             Fire();
    7.         }
    8.        
    9.         if (Physics.Raycast (Gun.position, FPCamera.transform.forward, out Trajectory, Range))
    10.         {
    11.             AimingAtObject = true;
    12.             Vector2 ScreenPos = FPCamera.WorldToScreenPoint (Trajectory.point);
    13.             DynamicCrosshair.rectTransform.position = ScreenPos; //Would a local variable here be good?//
    14.         }
    15.         else
    16.         {
    17.             AimingAtObject = false;
    18.             DynamicCrosshair.rectTransform.position = CenterCrosshair.rectTransform.position; //How about here?//
    19.         }
    20. }
    21.