I am trying to create a simple script that spawns enemies in 4 different spaces around a square. I am completely new to programming and this is the first script I made without any outside help so I really hope I can get it working. The nested for loops don’t seem to work after the first 3 iterations. Any assistance would be deeply appreciated.
using UnityEngine;
using System.Collections.Generic;
public class functioningspawner : MonoBehaviour {
public float firePause;
public float fireDelay;
public List<Vector3> fullSpaces;
public List<Vector3> emptySpaces;
public List<GameObject> enemies;
public GameObject enemy;
public int number;
void Update ()
{
if (Time.time > firePause)
{
firePause = Time.time + fireDelay;
Shuffle();
}
}
void Shuffle ()
{
emptySpaces = new List<Vector3>();
fullSpaces = new List<Vector3>();
enemies = new List<GameObject>();
emptySpaces.Clear();
fullSpaces.Clear();
enemies.Clear();
for (int l = 0; l < ((GameObject.FindGameObjectsWithTag("enemy")).Length); l++)
{
enemies.Add(GameObject.FindGameObjectsWithTag("enemy")[l]);
};
for (int i = 0; i < ((GameObject.FindGameObjectsWithTag("space")).Length); i++)
{
emptySpaces.Add((GameObject.FindGameObjectsWithTag("space"))[i].transform.position);
};
for (int i = 0; i < emptySpaces.Count; i++)
{
for (int j = 0; j < enemies.Count; j++)
{
if (enemies[j].transform.position == emptySpaces[i])
{
fullSpaces.Add(emptySpaces[i]);
emptySpaces.RemoveAt(i);
Debug.Log("the thing has triggered");
}
}
};
Debug.Log("the number of empty spaces is " + emptySpaces.Count);
Debug.Log("the number of full spaces is " + fullSpaces.Count);
number = (Random.Range(0, (emptySpaces.Count)));
Instantiate(enemy, emptySpaces[number], Quaternion.identity);
if (emptySpaces.Count == 0)
{
Debug.Log("you lost");
}
}
}
The problem will be caused by removing items during iteration.
Using an index-based for loop may work in many cases, especially if you immediately continue with the next iteration step.
However, in your case, you have the potential that the index will still be used after removing an item (i.e. when the second loop hasn’t finished). This may lead to the situation that the index is no longer valid for the collection and throws an exception.
In general, it’s a good practice to avoid removing an item during iteration unless the data structure allows to do so safely.
Some other notes:
List allows to add a range, this would be one line instead of using the combination of the for loop and the add method.
You can cache the array, avoiding unnecessary calls of Find-methods.
Find methods are pretty heavy. You shouldn’t be using them frequently as you do in your code. One way to solve it is to make them register themselves to the list under given circumstances. This basically turns around the control but may be way better.
Clearing the list does not have an effect in your scenario as you always create new ones in your shuffle method. You can create the list once and then clear it when necessary.
That was helpful, thanks. I added break to the end of the 2nd loop so it would not run again after removing something. I still have a seperate issue with the code not working after the 2nd or 3rd iteration. It will only run the for loop twice even if there are more than 2 enemies.
If you absolutely need to remove something from a list while iterating through it, assuming you are able, do the list backwards. So a for list would start at (int i = list.count; i>= 0; i–) This will keep you from hitting an index that no longer exist as you’ll be working your way backwards through the list. The reason going forward doesn’t work is when you remove something, the list will adjust. So if you remove index 2 for example, index 3 shifts into 2’s spot, and etc, which means it will no longer have the count value you expected to find.
nah, its still fine then
the indexer is at 0, you remove element zero from list. The next one you need to check is now at element zero
Decrement indexer to -1, loop ends, and indexer is incremented back to zero
