GamePieces are spawned even though Coroutine (containig Instantiate()) is not starting in Update in Unity

Question

I want to limit the spawn of game pieces. PiecesOnBoard gets the number of existing GamePieces. maxSpawns is statically set to a number which should act as the maximum allowed number of GamePieces currently existing.

Now, the problem is that game pieces get spawned anyhow despite the freeSpawnSpotsAvailable condition.

Below are some corresponding code and console outputs.

void Update()
{
    if (groundPlacement.GroundSet())
    {
        CheckSpawnSpots();

        Debug.Log("FreeSpawnSpotsAvailable: " + freeSpawnSpotsAvailable + "\n" 
            + gameManager.PiecesOnGround.Length + " PiecesOnGround\n"
            + maxSpawns + " maxSpawns\n"
            + (maxSpawns - gameManager.PiecesOnGround.Length) + " free spawn spots");

        if (freeSpawnSpotsAvailable)
        {
            Debug.Log("start spawn");
            StartCoroutine(SpawnRandomGamePieces());
        }
    }
}


private IEnumerator SpawnRandomGamePieces()
{
    yield return new WaitForSeconds(Random.Range(2f, waitSpawnTime));

    GamePiece spawned = Instantiate<GamePiece>(RandomGamePiece(gamePieces), RandomPosition(), Quaternion.identity);
}


private void CheckSpawnSpots()
{
    int freeSpots = maxSpawns - gameManager.PiecesOnGround.Length;
    if (freeSpots > 0)
    {
        freeSpawnSpotsAvailable = true;
    }
    else
    {
        freeSpawnSpotsAvailable = false;
    }
}

Exemplary output when game is run (this is a later output, therefore the PiecesOnGround has a much higher number):

FreeSpawnSpotsAvailable: False
352 PiecesOnGround
10 maxSpawns
-342 free spawn spots
SpawnObjects:Update()

I know the Coroutine is not run when freeSpawnSpotsAvailable = false since the debug line Debug.Log("start spawn") is not printed in the console. But somehow game pieces get spawned anyhow (and I feel like they are every Update frame) but I don't know why since I don't Instantiate/call this Coroutine elsewhere.

But I also realize now, that even if the Coroutine is started it does not spawn the game pieces (PiecesOnGround does not change). Some console output:

FreeSpawnSpotsAvailable: True
8 PiecesOnGround
10 maxSpawns
2 free spawn spots
SpawnObjects:Update()

start spawn
SpawnObjects:Update()

timeLeftInRound: 7, RoundTimerOver: False
GameManager:Update()

FreeSpawnSpotsAvailable: True
8 PiecesOnGround
10 maxSpawns
2 free spawn spots
SpawnObjects:Update()

start spawn
SpawnObjects:Update()

I don't get where the spawns are coming from.

UPDATE:

I tried moving the conditional test into the Coroutine instead of in the Update:

void Update()
{
    if (groundPlacement.GroundSet())
    {
        CheckSpawnSpots();

        Debug.Log("FreeSpawnSpotsAvailable: " + freeSpawnSpotsAvailable + "\n"
            + gameManager.PiecesOnGround.Length + " PiecesOnGround\n"
            + maxSpawns + " maxSpawns\n"
            + (maxSpawns - gameManager.PiecesOnGround.Length) + " free spawn spots");

        StartCoroutine(SpawnRandomGamePieces());
    }
}


private IEnumerator SpawnRandomGamePieces()
{
    Debug.Log("s");
    if (freeSpawnSpotsAvailable)
    {
        yield return new WaitForSeconds(Random.Range(2f, waitSpawnTime));

        GamePiece spawned = Instantiate<GamePiece>(RandomGamePiece(gamePieces), RandomPosition(), Quaternion.identity);
        Debug.Log("insantiated at: " + spawned.gameObject.transform.position);
    }
}

Now, I get following console output:

FreeSpawnSpotsAvailable: False
300 PiecesOnGround
10 maxSpawns
-290 free spawn spots
Spawner:Update()

s
<SpawnRandomGamePieces>d__16:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr)
Spawner:Update()

insantiated at: (-0.30, -0.42, 0.84)
<SpawnRandomGamePieces>d__16:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr)

timeLeftInRound: 2, RoundTimerOver: False
GameManager:Update()

FreeSpawnSpotsAvailable: False
302 PiecesOnGround
10 maxSpawns
-292 free spawn spots
Spawner:Update()

PiecesOnGround is updated in the Update funtion of my GameManager script through following code (code extract):

    public class GameManager : MonoBehaviour
    {
       public GamePiece[] PiecesOnGround { get => piecesOnGround; set => piecesOnGround = value; }

       void Update()
       {
           PiecesOnGround = FindObjectsOfType<GamePiece>(); //update
       }
    }

Answer 1

Your if (freeSpawnSpotsAvailable) test is in the wrong place. It is true, starts a spawn, then waits for two seconds. All that time more spawns are started ([framrate] will begin every second). By the time it actually spawns it would be false, but your coroutine doens't care.

A simple workaround to fix your problem would be to move your test to the place it's actually relevant

private IEnumerator SpawnRandomGamePieces()
{
    yield return new WaitForSeconds(Random.Range(2f, waitSpawnTime));
    
    CheckSpawnSpots();
    if (freeSpawnSpotsAvailable)
        GamePiece spawned = Instantiate<GamePiece>(RandomGamePiece(gamePieces), RandomPosition(), Quaternion.identity);
}

---

But an actual fix would be to change your design. The way you are updating your freeSpawnSpotsAvailable is inneficient, and this would still run far more coroutines than are actually needed.

However, an actual solution would look something like this.

public class GameManager : MonoBehaviour
{
    private GamePiece[] peicesOnGroundCache = null;
    public GamePiece[] PiecesOnGround => peicesOnGroundCache ??= FindObjectsOfType<GamePiece>(); // return cached value if exists, otherwise get value

    // IMPORTANT to call this whenever a price is removed too. 
    // This could be a part of the OnDestory callback, or registered as they are created
    public void MarkPeicesOnGroundCacheAsDirty() => peicesOnGroundCache = null;

    // if possible, only spawn GamePeices thorugh this function, 
    // otherwise call MarkPeicesOnGroundCacheAsDirty whenever you spawn them
    public GamePeice SpawnNewGamePeice(GamePeice gamePeice, Vector3 position, Quaternion rotation 
    {
        var peice = Instantiate<GamePeice>(gamePeice, position, rotation);
        MarkPeicesOnGroundCacheAsDirty();

        return peice;
    }
}

Then in your spawner class

private Coroutine spawnCoroutine = null;

void Update()
{
    if (groundPlacement.GroundSet())
    {
        if(spawnCoroutine != null)
            return; // already spawning :)
    
        var peicesToSpawn = int freeSpots = maxSpawns - gameManager.PiecesOnGround.Length; // this is cached so fast
    
        if(peicesToSpan > 0)
            spawnCoroutine = StartCoroutine(SpawnRandomGamePieces(peicesToSpawn));
    }
}
private IEnumerator SpawnRandomGamePieces(int peicesToSpawn)
{
    yield return new WaitForSeconds(Random.Range(2f, waitSpawnTime));

    for(int i = 0; i < peicesToSpawn; i++) // spawn enough right away
    {
        var _ gameManager.SpawnNewGamePeice(RandomGamePeice(), RandomPosition(), Quaternion.Identity);      
        
        // uncomment if you want only one to spawn per frame
        //yield return null;
    }
    spawnCoroutine = null;
}

This should give you the behaviour you are looking for while being a little cleaner and better for performance.

(note the check on coroutine being null could be replaced by a bool flag if you prefer)