Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Script Optimization

Discussion in 'Scripting' started by naughti_buoy, Jul 23, 2015.

  1. naughti_buoy

    naughti_buoy

    Joined:
    Mar 15, 2015
    Posts:
    4
    I am very new to Unity, and was wondering how I can shorten the length of this C# script. It seems to me that it is very inefficient, and I would like to shorten it. The code automatically adjusts the scale and position of walls depending on the scale of the floor.

    Code (CSharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class AutoWallScaling : MonoBehaviour {
    5.  
    6.     public float Wall_Height;
    7.  
    8.     public GameObject Floor;
    9.     public GameObject North_Wall;
    10.     public GameObject East_Wall;
    11.     public GameObject South_Wall;
    12.     public GameObject West_Wall;
    13.  
    14.     private Vector3 North_Wall_Position;
    15.     private Vector3 East_Wall_Position;
    16.     private Vector3 South_Wall_Position;
    17.     private Vector3 West_Wall_Position;
    18.     private Vector3 North_Wall_Scale;
    19.     private Vector3 East_Wall_Scale;
    20.     private Vector3 South_Wall_Scale;
    21.     private Vector3 West_Wall_Scale;
    22.     private Vector3 Floor_Scale;
    23.  
    24.     private float Floor_Scale_x;
    25.     private float Floor_Scale_y;
    26.  
    27.     private float Wall_Position_y;
    28.  
    29.     private float North_Wall_Position_Change;
    30.     private float East_Wall_Position_Change;
    31.     private float South_Wall_Position_Change;
    32.     private float West_Wall_Position_Change;
    33.  
    34.     // Use this for initialization
    35.     void Start () {
    36.         Floor_Scale = Floor.transform.localScale;
    37.  
    38.         Floor_Scale_x = Floor_Scale[0];
    39.         Floor_Scale_y = Floor_Scale [1];
    40.  
    41.         Wall_Position_y = Wall_Height / 2;
    42.  
    43.         North_Wall_Position_Change = Floor_Scale_x / 2;
    44.         East_Wall_Position_Change = -(Floor_Scale_y / 2);
    45.         South_Wall_Position_Change = -(Floor_Scale_x / 2);
    46.         West_Wall_Position_Change = Floor_Scale_y / 2;
    47.  
    48.         North_Wall_Position = new Vector3 (North_Wall_Position_Change, Wall_Position_y, 0);
    49.         East_Wall_Position = new Vector3 (0, Wall_Position_y, East_Wall_Position_Change);
    50.         South_Wall_Position = new Vector3 (South_Wall_Position_Change, Wall_Position_y, 0);
    51.         West_Wall_Position = new Vector3 (0, Wall_Position_y, West_Wall_Position_Change);
    52.  
    53.         North_Wall_Scale = new Vector3(Floor_Scale_y, Wall_Height, 0);
    54.         East_Wall_Scale = new Vector3(Floor_Scale_x, Wall_Height,0);
    55.         South_Wall_Scale = new Vector3(Floor_Scale_y, Wall_Height, 0);
    56.         West_Wall_Scale = new Vector3(Floor_Scale_x, Wall_Height, 0);
    57.  
    58.         North_Wall.transform.position = North_Wall_Position;
    59.         East_Wall.transform.position = East_Wall_Position;
    60.         South_Wall.transform.position = South_Wall_Position;
    61.         West_Wall.transform.position = West_Wall_Position;
    62.  
    63.         North_Wall.transform.localScale = North_Wall_Scale;
    64.         East_Wall.transform.localScale = East_Wall_Scale;
    65.         South_Wall.transform.localScale = South_Wall_Scale;
    66.         West_Wall.transform.localScale = West_Wall_Scale;
    67.     }
    68.  
    69.     // Update is called once per frame
    70.     void Update () {
    71.  
    72.     }
    73. }
     
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    Why are all those variables class level fields?

    Are they ever used outside of the Start function? If not, don't make them class level fields.

    If this is ALL the script does, the only class level fields you need are the public ones. The others can be in the Start function, where they're only ever used.

    Oh, and remove the Update function if no code goes in it. Otherwise it's getting called every Update... though small, it still has some cost if it exists.
     
    naughti_buoy likes this.
  3. DonLoquacious

    DonLoquacious

    Joined:
    Feb 24, 2013
    Posts:
    1,667
    You don't need to have separate variables for "position" if you're simply using it to make redundant copies of the transform on the GameObject- same for "scale" and "rotation".

    The "change" variables are perfect examples of somewhere you might use a property instead, if the relationship will always be the same:
    Code (csharp):
    1. public float NorthWall_PositionChange
    2. {
    3.     get{ return (Floor.transform.localScale[0].x / 2); }
    4. }
    ... for example. That way the value itself doesn't need to be stored.
     
    Last edited: Jul 23, 2015
    naughti_buoy likes this.
  4. naughti_buoy

    naughti_buoy

    Joined:
    Mar 15, 2015
    Posts:
    4
    Here is the updated code. Are there any more possible optimizations?

    Code (CSharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class AutoWallScaling : MonoBehaviour {
    5.  
    6.     public float Wall_Height;
    7.  
    8.     public GameObject Floor;
    9.     public GameObject North_Wall;
    10.     public GameObject East_Wall;
    11.     public GameObject South_Wall;
    12.     public GameObject West_Wall;
    13.  
    14.     void Start () {
    15.         North_Wall.transform.position = new Vector3 ((Floor.transform.localScale[0] / 2), (Wall_Height / 2), 0);
    16.         East_Wall.transform.position = new Vector3 (0, (Wall_Height / 2), -(Floor.transform.localScale[1] / 2));
    17.         South_Wall.transform.position = new Vector3 (-(Floor.transform.localScale[0] / 2), (Wall_Height / 2), 0);
    18.         West_Wall.transform.position = new Vector3 (0, (Wall_Height / 2), (Floor.transform.localScale[1] / 2));
    19.  
    20.         North_Wall.transform.localScale = new Vector3 (Floor.transform.localScale[1], Wall_Height, 0);
    21.         East_Wall.transform.localScale = new Vector3 (Floor.transform.localScale[0], Wall_Height, 0);
    22.         South_Wall.transform.localScale = new Vector3 (Floor.transform.localScale[1], Wall_Height, 0);
    23.         West_Wall.transform.localScale = new Vector3 (Floor.transform.localScale[0], Wall_Height, 0);
    24.     }
    25. }
     
  5. manpower13

    manpower13

    Joined:
    Dec 22, 2013
    Posts:
    140
    Hi,

    I think you already did a great job on improving the script. Something I would add is the following (the speed difference won't be that much, but since you ask for more optimalization ;)):

    Code (CSharp):
    1. using UnityEngine;
    2. using System.Collections;
    3. public class AutoWallScaling : MonoBehaviour {
    4.     public float Wall_Height;
    5.     public GameObject Floor;
    6.     public GameObject North_Wall;
    7.     public GameObject East_Wall;
    8.     public GameObject South_Wall;
    9.     public GameObject West_Wall;
    10.     void Start () {
    11. Vector3 LocalScale0 = Floor.transform.localScale[0];
    12. Vector3 LocalScale1 = Floor.transform.localScale[1];
    13.         North_Wall.transform.position = new Vector3 ((LocalScale0/ 2), (Wall_Height / 2), 0);
    14.         East_Wall.transform.position = new Vector3 (0, (Wall_Height / 2), -(LocalScale1/ 2));
    15.         South_Wall.transform.position = new Vector3 (-(LocalScale0 / 2), (Wall_Height / 2), 0);
    16.         West_Wall.transform.position = new Vector3 (0, (Wall_Height / 2), (LocalScale1 / 2));
    17.         North_Wall.transform.localScale = new Vector3 (LocalScale1, Wall_Height, 0);
    18.         East_Wall.transform.localScale = new Vector3 (LocalScale0, Wall_Height, 0);
    19.         South_Wall.transform.localScale = new Vector3 (LocalScale1, Wall_Height, 0);
    20.         West_Wall.transform.localScale = new Vector3 (LocalScale0, Wall_Height, 0);
    21.     }
    22. }
    I typed this on my Ipad so there might be some mistakes with uppercase or lowercase. I think you cannot optimize your script more :p.

    Good luck,
    Sincerely,
    Floris Weers, WeersProductions
     
    naughti_buoy likes this.
  6. naughti_buoy

    naughti_buoy

    Joined:
    Mar 15, 2015
    Posts:
    4
    Thanks everyone on the feedback. Floris Weers, could you please explain to me why assigning a variable to the floor scale makes the code more efficient? I'm just curious why it does.
     
  7. DonLoquacious

    DonLoquacious

    Joined:
    Feb 24, 2013
    Posts:
    1,667
    He only assigned it locally, so the reference dies when the function is finished. It wasn't really about code efficiency as much as reading efficiency- it cut a ton of text out of the Start function. Readability is arguably more important than minor optimizations. It's the same reason we use "using" statements to include other namespaces- because writing it out all of the time is exhausting and hard to read.
     
    naughti_buoy likes this.
  8. WaylandGod

    WaylandGod

    Joined:
    Jul 23, 2015
    Posts:
    7
    Actually, the code above can be organized in a different way.
    Just define a class named WallItem for example, this class contains:
    wall height, floor and Current wall, this is a unit. Then wen can encapsulate several wall with their own adjust rule and logic.
     
    naughti_buoy likes this.
  9. DonLoquacious

    DonLoquacious

    Joined:
    Feb 24, 2013
    Posts:
    1,667
    Yes, it would be more easily scaleable that way as you can use, for instance, a "list of western walls", "list of northern walls" etc... and even maybe encapsulate THEM into new classes too, that have an organizational logic on entire sections rather than individual walls. That said, this is no longer a discussion on efficiency but more on scalability, and that discussion is endless.
     
    naughti_buoy likes this.
  10. naughti_buoy

    naughti_buoy

    Joined:
    Mar 15, 2015
    Posts:
    4
    Thanks everyone on the large amounts of feedback.