Object spawning script Unity

Question

I'm trying to make a random object spawning script in Unity.

Bellow is the code, any suggestions for improvement / changes? I'm new to both Unity and C#.

// Game objects and Transforms
public Transform playerTransform;
public Transform[] obstaclePrefab;

// spawn params
public float minYDicstane = 6.0f;
public float maxYDistance = 11.0f;
private float boxPositionY;

public float minXDistance = 0.0f;
public float maxXDistance = 3.0f;
private float boxPositionX;

private float minSpawnTime;
private float maxSpawnTime;
private float spawnTime;
private float timeCounter;

// Distances
private float playerDistance;
private float boxDistance;
private float ySpread;

private void Start() 
{
    // Count sequence
    minSpawnTime = 3.0f;
    maxSpawnTime = 8.0f;

    // Count timer
    timeCounter = 0;
    spawnTime = Random.Range(minSpawnTime, maxSpawnTime);

}

private void Update() 
{
    // Count the random spawn time
    timeCounter += Time.deltaTime;
    Debug.Log ("Spawn Time: " + spawnTime + " spawnCount: " + timeCounter);

    if(timeCounter >= spawnTime) 
    {
        // boxDistanceFromPlayer
        playerDistance = playerTransform.position.y;

        // Box spawn distance
        // X position
        boxPositionX = Random.Range(minXDistance, maxXDistance);
        boxPositionX = (boxPositionX-1)*2.0f;

        // Y Position
        boxPositionY = playerDistance + Random.Range(minYDicstane, maxYDistance);

        // Select box color
        int boxColor = Random.Range (0, 4);

        // Let the boxes awake!!!
        Instantiate (obstaclePrefab [boxColor], new Vector2 (boxPositionX, boxPositionY), Quaternion.identity);

        // Make new random spawn time
        spawnTime = Random.Range (minSpawnTime, maxSpawnTime);
        timeCounter = 0;
    }

}

Code after changes, using coroutines

// Game objects and Transforms
public Transform playerTransform;
public GameObject[] obstaclePrefab;

// spawn params
public float minYDicstane = 6.0f;
public float maxYDistance = 11.0f;

public float minXDistance = 0.0f;
public float maxXDistance = 3.0f;

public float minSpawnTime = 2.0f;
public float maxSpawnTime = 5.0f;

public float spawnTime = 4.0f;



IEnumerator SpawnBoxes() 
{
    while (true) 
    {
        float boxPositionY;
        float boxPositionX;

        //Distances
        float playerDistance;

        // Player position
        playerDistance = playerTransform.position.y;

        // Box position
        boxPositionX = Random.Range(minXDistance, maxXDistance);
        boxPositionY = playerDistance + Random.Range(minYDicstane, maxYDistance);

        // Select box
        GameObject box = obstaclePrefab[Random.Range(0, obstaclePrefab.Length - 1)];

        // Instantiate box
        Instantiate (box, new Vector2 (boxPositionX, boxPositionY), Quaternion.identity);

        // Coroutine random amount of time
        yield return new WaitForSeconds(Random.Range(minSpawnTime, maxSpawnTime));
    } 
}

private void Start() 
{
    StartCoroutine(SpawnBoxes());
}

Answer 1

public Transform[] obstaclePrefab;

That's wrong. Prefabs are of type GameObject, not Transform.

---

private float minSpawnTime;
private float maxSpawnTime;

Why did you make these private? They should have been public with default values - just as all the others.

private float boxPositionY;
private float boxPositionX;
// Distances
private float playerDistance;
private float boxDistance;
private float ySpread;

These shouldn't even been private, but local to the Update() method, as their values are never reused.

---

// Select box color
int boxColor = Random.Range(0, 4);

This is very likely to break. Better read the array length instead of hard coding it:

// Select box color
int boxColor = Random.Range(0, obstaclePrefab.Length - 1);

Or just get rid of boxColor all together:

// Select box
GameObject box = obstaclePrefab[Random.Range(0, obstaclePrefab.Length - 1)];

---

// Count the random spawn time
timeCounter += Time.deltaTime;
if(timeCounter >= spawnTime) 
{
    // Make new random spawn time
    spawnTime = Random.Range (minSpawnTime, maxSpawnTime);
    timeCounter = 0;
}

That's one way to do it. The cleaner method would have been to handle this in a Coroutine and then use WaitForSeconds. Right now, your code runs every single frame, without actually doing anything useful.

---

Debug.Log ("Spawn Time: " + spawnTime + " spawnCount: " + timeCounter);

Be careful when you log. Logging when something spawns? OK. But spamming a log entry every single frame? Waste of resources.

---

boxPositionX = (boxPositionX-1)*2.0f;

What is this line supposed to do? That should have been directly computed into minXDistance and maxXDistance, so this line is obsolete.

---

If this is actually supposed to be C#, the whole script you posted should actually have wrapped in a regular C# class:

using UnityEngine;
using System.Collections;

public class ScriptName : MonoBehaviour {
    // <---- Your stuff goes here
}

Did you just omit this when posting your code here, or did you actually write your scripts without it?