cyo
cyo

Reputation: 77

Is this an effective way to compare the name of a gameobject with the text of an InputField, in order to turn the gameobject on/off?

I am trying to make a simple search and reveal for an inventory, wondering if there is a better way to do this? This is attached to the parent Gameobject and works with an InputField to compare the text in the field and the name of the Gameobjects, in order to show and reveal them.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
using System.Linq;

public class findFrame : MonoBehaviour
{

    public InputField searchText;

    void Find()
    {
        var found = new List<GameObject>(GameObject.FindGameObjectsWithTag("Button")).Where(g => g.transform.IsChildOf(this.transform));

        foreach (GameObject g in found)
        {
            Debug.Log("hello");

            if (g.transform.name != searchText.text)
            {
                gameObject.SetActive(false);
            }
            else
            {
                gameObject.SetActive(true);
            }
        }
    }


        void Update()
        { 
            Find();
        }
    }

Upvotes: 1

Views: 3421

Answers (2)

derHugo
derHugo

Reputation: 90833

Short Answer: NO!

  • In general as already mentioned: Do not do it in Update => every frame! (Btw also not in FixedUpdate which runs less frequent but still adds completely unnecessary calls)

    Rather make your code event driven. In your case you could rather add a listener to InputField.onValueChanged which is only called when the value did actually change which reduces your method cals to the necessary.

  • Then I would rather at the beginning store all objects once in a HashSet (or depending on your needs maybe even a Dictionary in case you want to be able to access a button by its name) which is way more efficient then retrieving the list over and over again each time the method gets called.

  • Also using FindObjectsWithTag is expensive and then filtering out using isChildOf is even more expensive and unnecessary - afaik it basically bubbles up the transform.parent recursively until it either finds the given value or null. Transform implements an IEnumerable returning all child Transforms so that you can simply iterate through all child objects using a foreach. So rather go the other way round and iterate only the children and check if they have the tag using CompareTag.

    This depends on your setup ofcourse since isChildOf also returns true if the given object is a nested (deep) child. In case your buttons actually have specific component attached - like I would e.g. guess in this case a Button - you can overcome this by using GetComponentsInChildren<Button>() and iterate on this instead.

  • Finally for comparing strings there are way more efficient and more importantly more secure ways then simply using ==(see C# difference between == and Equals())

    You could either search for an exact match using string.Eqials

    var isMatch = name.Equals(searchString, COMPARISONTYPE);
    

    Or - what I would prefer - rather find all partial matches by either simply using string.Contains

    var isMatch = name.Contains(searchString);
    

    or in case you need it more customized use string.Compare

    var isMatch = string.Compare(name, searchString, COMPARISONTYPE)
    

    Where for the optional (but recommended) parameter COMPARISONTYPE you can choose one of the available StringComparison values in order to e.g. match case insensitive etc.

So I would use something like

public class findFrame : MonoBehaviour
{ 
    public InputField searchText;

    // A HashSet is basically a list but makes sure every element is unique
    // I would make this rather a Dictionary<string, GameObject> in case
    // you also plan to access a specific button by name
    // otherwise a HashSet does it
    private HashSet<GameObject> childButtons = new HashSet<GameObject>();

    public void Awake()
    {
        //Adds a listener to the input field and invokes a method when the value changes.
        searchText.onValueChanged.AddListener(OnValueChanged);

        // Run through all children GameObjects of this GameObject and check the tag
        foreach(Transform child in transform)
        // or as said if possible/necessary for nested children you could use
        //foreach(var child in GetComponentsInChildren<Button>())
        {
            // check if this child has the correct tag
            if(!child.CompareTag("Button")) continue;

            // If the tag matches add it to the HashSet
            childButtons.Add(child.gameObject);
        }
    }

    // Invoked when the value of the text field changes.
    public void OnValueChanged()
    {
        Debug.Log("Value Changed");

        // Get the search value
        var searchString = searchTexxt.text;

        // Simply run through the HashSet and set the matching object(s) active, the others inactive
        foreach(var button in childButtons)
        {
            // Skip null entries
            if(!button) continue;

            var name = button.name;

            // Here as said you can either check for an exact match
            //var isMatch = name.Equals(searchString, COMPARISONTYPE);
            // Or you could also check for partial matches - I would prefer this
            // (required if you want to see objects also while the user didn't type out the full name yet)
            // either by simply using Contains
            //var isMatch = name.Contains(searchString);
            // or if you need it more customized 
            var isMatch = string.Compare(name, searchString, COMPARISONTYPE) == 0;

            button.SetActive(isMatch);

            if(isMatch) Debug.Log($"Found match for {searchString} in button ${name} ");
        }
    }

    // When creating the buttons on runtime call this to also add them to the HashSet
    public void AddButton(GameObject button)
    {
        if(childButtons.Contains(button)) return;

        childButtons.Add(button);
    }
}
  • If you create the buttons on runtime via a script, well, then make sure you add them to the HashSet everytime you instantiate one.

    // In another script you need the reference to the `findFrame` component
    // Drag it in here via the Inspector
    [SerializeField] findFrame findFrameReference;
    
    // As fallback find it on runtime (in case there is only one in the scene
    private void Awake ()
    {
        if(! findFrameReference) findFrameReference = FindObjectOfType<findFrame>();
    }
    

    Then wherever you create the new buttons

    var button = Instantiate(buttonPrefab, ...);
    findFrameReference.AddButton(button.gameObject);
    

Upvotes: 1

Jonathan Alfaro
Jonathan Alfaro

Reputation: 4376

In Unity the Update function runs once every single frame!

So doing things like string comparisons in here might not be the best idea.

You might want to take user input in the Update and then do some calculations in the FixedUpdate which executes at a fixed frame rate.

That aside... In Unity there is a better way to do what you are doing by using CompareTag method.

This method gets compiled in a "special" way by the Unity compiler in order to avoid string allocations etc.. Basically is optimized for performance.

Here are some links that might help you clear the waters:

Performance of CompareTag

In this one do a search for CompareTag in your browser to see exactly your situation: Garbage Collection and other Optimizations

From Unity Forum

From JetBrains

Finally if you really need to compare two strings... The fastest way would most likely be like this:

string.CompareOrdinal(myString1, myString2) == 0 //this means equals


string.CompareOrdinal(myString1, myString2) != 0 //this means not equals

Upvotes: 0

Related Questions