Issues with moving objects from list to list

Hello guys, I’ve got a problem here with a list and I can’t seem to figure out exactly why this is happening.

This whole script is made to spawn enemies randomly in 5 different spawners with a cost per enemy system.

The original script made Unity to crash, so this is a second try to get it to work from scratch, so right now it only generates enemies once.

As you will see, I use a few lists, one to place what enemies may spawn (List enemies), a second one to prepare the enemies (List enemiesToSpawn) and a last one to place the enemies after being spawned and before getting killed (spawnedEnemies).

It all works fine except for the last part of the script. In the coroutine “Spawn”, I try to spawn the enemies from the “enemiesToSpawn” list, remove them from this list and add them to the “spawnedEnemies” list.

It works fine sometimes, but every so often, it wouldn’t remove the enemy and add them to the other list.

I’ve been restarting the project over and over, and I’ve seen it works perfectly 8 out of 10 times, so it’s not something that happens constantly, but I’m not really sure why it happens.

Here’s the code, if anyone want to see.

using System.Collections;
using System.Collections.Generic;
using TMPro;
using UnityEngine;

public class WaveSpawner2 : MonoBehaviour
{
    [Header("Audio")]
    public AudioSource audioSource;
    public AudioClip waveClip;

    [Header("Variables")]
    public bool startGame;
    public int currentWave;
    public int waveValue;
    public int spawnIndex;
    public int waveDuration;
    private float waveTimer;
    private float spawnInterval;
    private float spawnTimer;
    private float spawnIntervalReduction = -1;
    public List<Enemy> enemies = new();
    public List<GameObject> enemiesToSpawn = new();
    public List<GameObject> spawnedEnemies = new();

    [Header("Objetos")]
    public List<GameObject> spawners;
    [SerializeField] private TMP_Text waveText;
    [SerializeField] private GameObject newWaveText;

    private void Start()
    {
        startGame = false;
        waveText.text = currentWave.ToString("");
        StartCoroutine(InitialWait());
        //InvokeRepeating("SpawnEnemies", 5, 5);
        Invoke("SpawnEnemies", 2);
    }

    IEnumerator InitialWait()
    {
        //yield return new WaitForSeconds(15f);
        yield return new WaitForSeconds(1f);
        startGame = true;
        GenerateWave();
    }

    public void GenerateWave()
    {
        PlayerPrefs.SetString("Record", waveText.text);
        waveValue = 1 + currentWave * 3;
        GenerateEnemies();
        spawnInterval = waveDuration / enemiesToSpawn.Count;
        waveTimer = waveDuration;
    }

    public void GenerateEnemies()
    {
        List<GameObject> generatedEnemies = new();
        while (waveValue > 1 && generatedEnemies.Count <= 2)
        {
            int randEnemyID = Random.Range(0, enemies.Count);
            int randEnemyCost = enemies[randEnemyID].cost;

            if (waveValue - randEnemyCost >= 0)
            {
                generatedEnemies.Add(enemies[randEnemyID].enemyPrefab);
                waveValue -= randEnemyCost;
            }
            else if (waveValue <= 0)
            {
                break;
            }
        }
        enemiesToSpawn.Clear();
        enemiesToSpawn = generatedEnemies;
    }

    public void SpawnEnemies()
    {
        Debug.Log("Spawn");
        if (startGame)
        {
            StartCoroutine(Spawn());  
        }
    }


    IEnumerator Spawn()
    {
        for (int i = enemiesToSpawn.Count - 1; i >= 0; i--)
        {
            GameObject enemy = Instantiate(enemiesToSpawn[i], spawners[Random.Range(0, spawners.Count)].transform.position, Quaternion.Euler(0, -90, 0));

            //Debug.Log($"Starting iteration. List size: {enemiesToSpawn.Count}, Current index: {i}");

            spawnedEnemies.Add(enemy);

            //Debug.Log($"Before RemoveAt: List size = {enemiesToSpawn.Count}, Removing index: {i}");

            enemiesToSpawn.RemoveAt(i);

            //Debug.Log($"After RemoveAt: List size = {enemiesToSpawn.Count}");

            yield return new WaitForSeconds(2f);
        }

        Debug.Log($"List size = {enemiesToSpawn.Count}");

    }
}

I almost forgot to add that the last Debug.Log() always says the list size = 0 whenever the for() loop is done, even if the editor shows the enemy still being in the enemiesToSpawn list and not on the spawnedEnemies list.

They do spawn tho, so the instantiate’s working.

If anyone’s got an idea as of why this wouldn’t work I’d really appreciate the help,

1 Like

Either this is the problem. or it will be. If you iterate over a list that has a chance of getting modified outside the coroutine, this code will fail eventually!

Because enemiesToSpawn.Count is used to initialize the loop once (ie 5), and other code may add more enemies to spawn which won’t get spawned (indexes 5+), or remove some enemies from the list leading to IndexOutOfRangeException (eg when clearing the list outside the coroutine).

You need to pass in a copy of the list to the coroutine, and/or you need to safeguard the other method(s) that modify the list so that they only do so when that list is empty.

Best not to use a list here because what you really want is to spawn one enemy, and then wait. Then you can get the next one. This will make the code more flexible, for instance what if the player issued a “kill everything on screen” and now you want to more quickly refill the play area with new enemies. You can’t with this approach, and you’ll struggle to modify it to account for that situation.

Honestly, this looks like it could use a rewrite WITHOUT using coroutines (or timed invokes). You really want this all in one tight update loop so that you have control over the execution and can follow the code executing line by line (you know how to use the debugger?).

All it takes is a simple float variable (or several) that holds the time when the next event should occur, and if Time.time < itsTimeToSpawn then you spawn and update the variable. This is MUCH simpler than using coroutines!

It is not necessary to clear the list since you replace it with a different list altogether.