koobas.hobune.stream
Help with making script efficient
Hi everyone! I have this script, which essentially works, but I feel that it could be optimized to work better, or probably shorten the code and get the same result. I have a grid of 16 circles which get destroyed destroyed at different random times. I want to respawn them in the exact position as the one which was destroyed after two seconds. This is the code that I use to do this :
public List<GameObject> circles = new List<GameObject>();
public GameObject circle;
void Update()
{
if (circles1.Count < 16)
{
foreach (GameObject a in circles)
{
circles1.Add(a.transform.position);
}
}
foreach (GameObject b in circles)
{
if (b.gameObject == null && on == false)
{
StartCoroutine(Spawn());
on = true;
}
}
IEnumerator Spawn()
{
yield return new WaitForSeconds(2f);
if (circles[0] == null)
{
circles[0] = (GameObject)Instantiate(circle, circles1[0], Quaternion.identity);
circles[0].transform.parent = gameObject.transform;
}
if (circles[1] == null)
{
circles[1] = (GameObject)Instantiate(circle, circles1[1], Quaternion.identity);
circles[1].transform.parent = gameObject.transform;
}
if (circles[2] == null)
{
circles[2] = (GameObject)Instantiate(circle, circles1[2], Quaternion.identity);
circles[2].transform.parent = gameObject.transform;
}
if (circles[3] == null)
{
circles[3] = (GameObject)Instantiate(circle, circles1[3], Quaternion.identity);
circles[3].transform.parent = gameObject.transform;
}
if (circles[4] == null)
{
circles[4] = (GameObject)Instantiate(circle, circles1[4], Quaternion.identity);
circles[4].transform.parent = gameObject.transform;
}
if (circles[5] == null)
{
circles[5] = (GameObject)Instantiate(circle, circles1[5], Quaternion.identity);
circles[5].transform.parent = gameObject.transform;
}
if (circles[6] == null)
{
circles[6] = (GameObject)Instantiate(circle, circles1[6], Quaternion.identity);
circles[6].transform.parent = gameObject.transform;
}
if (circles[7] == null)
{
circles[7] = (GameObject)Instantiate(circle, circles1[7], Quaternion.identity);
circles[7].transform.parent = gameObject.transform;
}
if (circles[8] == null)
{
circles[8] = (GameObject)Instantiate(circle, circles1[8], Quaternion.identity);
circles[8].transform.parent = gameObject.transform;
}
if (circles[9] == null)
{
circles[9] = (GameObject)Instantiate(circle, circles1[9], Quaternion.identity);
circles[9].transform.parent = gameObject.transform;
}
if (circles[10] == null)
{
circles[10] = (GameObject)Instantiate(circle, circles1[10], Quaternion.identity);
circles[10].transform.parent = gameObject.transform;
}
if (circles[11] == null)
{
circles[11] = (GameObject)Instantiate(circle, circles1[11], Quaternion.identity);
circles[11].transform.parent = gameObject.transform;
}
if (circles[12] == null)
{
circles[12] = (GameObject)Instantiate(circle, circles1[12], Quaternion.identity);
circles[12].transform.parent = gameObject.transform;
}
if (circles[13] == null)
{
circles[13] = (GameObject)Instantiate(circle, circles1[13], Quaternion.identity);
circles[13].transform.parent = gameObject.transform;
}
if (circles[14] == null)
{
circles[14] = (GameObject)Instantiate(circle, circles1[14], Quaternion.identity);
circles[14].transform.parent = gameObject.transform;
}
if (circles[15] == null)
{
circles[15] = (GameObject)Instantiate(circle, circles1[15], Quaternion.identity);
circles[15].transform.parent = gameObject.transform;
}
}
}
Does anyone have a more optimized way of doing this? Any help would be highly appreciated!
circles1 is a list of Vector3 positions
public List circles1 = new List();
There are 3 optimisation I can see. 1st, don't destroy them, deactivate and reactivate them, you'll save on garbage collection memory allocation. 2nd, make this a loop, like it has already been suggested. It'll be easier to manage. 3rd, you currently trigger the coroutine for every object, only to check every object again. So, the cost will be exponential. 10 objects = 100 checks, 100 objects = 10000 checks.
So, here's how it can be done if you want to keep it as a main manager script :
private List<GameObject> _targets;
public GameObject reference;
public List<Vector3> positions;
private bool on;
private float _delay = 2f;
void Awake ()
{
}
void Start ()
{
_targets = new List<GameObject>(); // this has to be initialised in Start, not in the inspector
// initial instanciation
for (int i = 0 ; i < positions.Count ; i++)
{
GameObject g = (GameObject) Instantiate (reference, positions[i], Quaternion.identity, transform); // you can pass this object's transform to the instanciate method to parent the instanciated object to it
_targets.Add (g);
}
}
void Update()
{
for (int i = 0 ; i < positions.Count ; i++)
{
positions[i] = _targets[i].transform.position;
}
foreach (GameObject b in _targets)
{
if (!b.activeSelf && on == false) // checking for active state instead of the object being null
{
StartCoroutine (ReSpawn (b)); // passing the object to the coroutine
on = true;
}
}
}
IEnumerator ReSpawn (GameObject target) // coroutine takes the object for parameter
{
yield return new WaitForSeconds (_delay);
target.SetActive (true); // simply reactivate the object
}
(Sorry for typos and such) You can use your if statements in a foreach loop similar to what you did before, although i guess it only looks prettier than a list of 'if statements' and isn't more optimized.
IEnumerator Spawn()
{
yield return new WaitForSeconds(2f);
foreach(Gameobject circleOb in circles)
{
if (circleOb == null)
{
circleOb = (GameObject)Instantiate(circle, circleOb, Quaternion.Identity);
//dont need to say gameObject.transform, can just say transform
circleOb.transform.parent = transform;
}
}
Another thing you probably want to look into, Object Pooling, as instantiate and destroy have a lot of overhead, Unity has a great tutorial for this here: https://unity3d.com/learn/tutorials/topics/scripting/object-pooling
int i;
i=circles.Length;
while(i>0){
i--;
if (circles[i] == null)
{
circles[i] = (GameObject)Instantiate(circle, circles1[i], Quaternion.identity);
circles[i].transform.parent = gameObject.transform;
}
}
