Advice on Making Function Calls "Efficient"

Hi everyone!


Just as a foreward: all of this code I’m using seems to be working fine, I’m asking more about whether it’s good/efficient WRT C# and the way Unity works.


Now, I know not to clog up functions that run all the time like Update() with costly methods like GetComponent, but I wanted some second eyes on what I’m generally using to call functions between scripts, because they do sometimes use methods like that, as well as iterating through children of GameObjects.


For example, this piece of code attached to my player:


private void ChangeHatIndex(int index)
    {
        selectedHatIndex = index;
        if (selectedHatIndex >= hats.Count)
        {
            selectedHatIndex = 0;
        }

        for (
            int i = 0; i < hats.Count; i++
        )
        {
            if (i == selectedHatIndex)
            {
                hats*.gameObject.SetActive(true);*

string hatName = hats*.gameObject.name;*

if (hatName == “The Don”)
{
if (TheDonOn == false)
{
TheDonOn = true;
References.canvas.ShowTheDon();
}
}

if (hatName != “The Don”)
{
TheDonOn = false;
References.canvas.HideTheDon();
}
}
}
}
----------
Changes a bool that keeps track of if the hat is equipped, then calls up either of these methods from a script for my canvas when a change is made to the player’s hat (hat equipped, equipped hat switched, etc.).
----------
public void ShowTheDon()
{
foreach (Behaviour childComponent in TheDon.GetComponentsInChildren())
{
childComponent.enabled = true;
}
}

public void HideTheDon()
{
foreach (Behaviour childComponent in TheDon.GetComponentsInChildren())
{
childComponent.enabled = false;
}
}
----------
Which enables/disables the UI element for the hat, by iterating through the child components OF the hat image.
----------
Is there any glaring problems with how this works, or things to watch out for if I wanted to add more calls, to different functions into code like this? Is nesting a bunch of called Functions slow, or liable to bite me in the ass later if I keep adding to it?
----------
Maybe I’m just worrying about nothing, but I really wanna try and get the basics of this stuff down. Thanks in advance!

Performance isn’t really one of the issues I’m seeing here, but there are ways you could clean up/simplify your code that might help you expand this hat system. One that jumps out at me is that you can consolidate these two methods into one:


     public void ShowTheDon()
     {
         foreach (Behaviour childComponent in TheDon.GetComponentsInChildren<Behaviour>())
         {
             childComponent.enabled = true;
         }
     }
 
     public void HideTheDon()
     {
         foreach (Behaviour childComponent in TheDon.GetComponentsInChildren<Behaviour>())
         {
             childComponent.enabled = false;
         }
     }

To:


        public void SetComponentsEnabled(GameObject parent, bool value)
        {
            foreach (Behaviour childComponent in parent.GetComponentsInChildren<Behaviour>())
            {
                childComponent.enabled = value;
            }
        }

That way, if you add more hats that do the same thing (activate/deactivate UI elements), you could do something like this with your hat manager class:


public class HatManager : MonoBehaviour
        {
            private int selectedHatIndex;
            private List<Hat> hats;

            // Add dictionary to store UI parent objects per hat
            private Dictionary<string, GameObject> hatUiParents = new Dictionary<string, GameObject>();

            private void Awake()
            {
                // Add the Don UI parent to dictionary for later
                // reference.
                hatUiParents.Add("The Don", TheDon);
            }

            private void ChangeHatIndex(int index)
            {
                selectedHatIndex = index;
                if (selectedHatIndex >= hats.Count)
                {
                    selectedHatIndex = 0;
                }

                for (int i = 0; i < hats.Count; i++)
                {
                    string hatName = hats*.gameObject.name;*

// Retrieve ui parent for hat, if one exists
hatUiParents.TryGetValue(hatName, out GameObject uiParent);

// Check if hat is selected hat
bool isSelectedHat = (i == selectedHatIndex);

// Set active if selected, inactive if not
hats*.gameObject.SetActive(isSelectedHat);*

// If ui parent is not null (meaning it is defined in the hatUiParents
// dictionary, then set its components to enabled or disabled depending
// on whether this hat is selected or not
if (uiParent)
References.canvas.SetComponentsEnabled(uiParent, isSelectedHat);
}
}
}
----------
Hopefully this gives you some ideas on how you can tidy up your code and make it more extensible. :slight_smile: Cheers!