HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Obstacle-spawning and movement scripts

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptsmovementandobstaclespawning

Problem

I have these scripts I made that work absolutely perfectly as I need them to. Is there any improvements that can be done to these scripts and make them more efficient for Mobile Platforms? Or even just combine them into one script?

These 4 scripts are just about identical except that each one controls a different obstacle and I have it that way so I can have more control over each individual obstacle. Does that make sense? (For instance, I want one obstacle to start spawning 10 seconds into the game and another to spawn RIGHT when the game starts).

Obstacle 1:

```
using UnityEngine;
using System.Collections;

[System.Serializable] // Serialize this so it looks neater in the inspector

public class Obstacle1 // Hammer Obstacle
{

public GameObject hammer; // The first obstacle gameobject. This is attached in the inspector.
public Vector3 spawnHPosValues; // Where the first obstacle will be spawned at on the X,Y,Z plane.
public int hCount; // This is the count of the first obstacle in a given wave. //
public float hSpawnWait; // Time in seconds between next wave of obstacle 1.
public float hStartGameWait; // Time in seconds between when the game starts and when the first obstacle start spawning.
public float hWaveSpawnWait; // Time in seconds between waves when the next wave of obstacle 1 will spawn.
}

public class SpawnHammers : MonoBehaviour {

public Obstacle1 obstacle1; // Reference to the Obstacle 1 class //

void Start () {

StartCoroutine (SpawnHammer ()); // at the beginning of the game, start to function SpawnHammer() //

}

IEnumerator SpawnHammer () {

yield return new WaitForSeconds(obstacle1.hStartGameWait);
while (true)
{

for (int i = 0; i < obstacle1.hCount; i++) {

Vector3 spawnPosition = new Vector3 (Random.Range(-obstacle1.spawnHPosValues.x, obstacle1.spawnHPosValues.x),
obstacle1.spawnHPosValue

Solution

-
Bracket alignment. For some reason, all obstacle classes and while-loops are written with vertically aligned bracket, while the MonoBehaviours are egyptians.


[Coding style] create a consistent look to the code, so that readers can focus on content, not layout. - C# Coding Convention

And, the commonly agreed convention for this one is vertically aligned,

-
Naming.

-
Do not abbreviate the variable name... Go for the full name, with exception of some short lambda functions', event handlers' parameters and for-loops' variables.

At first, I thought they were some kind of hungarian notation, and had trouble figuring out what kind of handles are they? It was until I saw the next obstacle class that I realize it was h for Hammer!

public class Obstacle1 // Hammer Obstacle
{
    public GameObject hammer; // The first obstacle gameobject. This is attached in the inspector.
    public Vector3 spawnHPosValues; // Where the first obstacle will be spawned at on the X,Y,Z plane.
    public int hCount; // This is the count of the first obstacle in a given wave. //
    public float hSpawnWait; // Time in seconds between next wave of obstacle 1.
    public float hStartGameWait; // Time in seconds between when the game starts and when the first obstacle start spawning.
    public float hWaveSpawnWait; // Time in seconds between waves when the next wave of obstacle 1 will spawn.
}


So, do use the full name, like spawnHammerPosition, hammerCount and so on... It does irk a bit, but I will refactor this with other Obstacles later.

-
In this bit of code

var spawnRotation = Quaternion.identity; // Spawn with NO rotation
Instantiate(hammer.GameObject, spawnPosition, spawnRotation); // Bring the object into the scene


Instead of spawnRotation, you could have gone with noRotation and gotten rid of the comment.

var noRotation = Quaternion.identity;
Instantiate(hammer.GameObject, spawnPosition, noRotation); // Bring the object into the scene


Or, if you are proficient enough, just inline it.

-
Normally, you should use PascalCase for public menbers, and not camelCase. However, Unity3D seems favor the latter. Therefore, it is up to you to decide. (Quaternion.identity twiches eye)

  • When your method name is already SpawnObstacles, you don't also have to name the local variable spawnPosition, just position should suffice.



-
Class name should never be plurial, and never a verb or verb+complement.


This distinguishes type names from methods, which are named with verb phrases. - Names of Classes, Structs, and Interfaces

SpawnHammers -> HammerSpawner

-
I don't understand, why you have 4 different classes with the same properties and copy-pasta comments, and, most importantly, shareing the same role of obstacle.

Combine them into a single class, subclass if you had to keep the class name for distinction :

[Serializable]
public abstract class Obstacle
{
    public GameObject gameObject;
    public Vector3 spawnPosition;
    public int spawnCount;
    public float spawnWait;
    public float startGameWait;
    public float waveSpawnWait;
}

public class Hammer : Obstacle { }
public class RoadBarrier : Obstacle { }
public class Cone : Obstacle { }
public class Barrel : Obstacle { }


-
Try keep the line width not too long and don't align the parameters :

// compare these two :
Vector3 spawnPosition = new Vector3(Random.Range(-cone.spawnCPosValues.x, cone.spawnCPosValues.x),
                                    cone.spawnCPosValues.y,
                                    cone.spawnCPosValues.z);
Vector3 spawnPosition = new Vector3(
    Random.Range(-cone.spawnCPosValues.x, cone.spawnCPosValues.x),
    cone.spawnCPosValues.y,
    cone.spawnCPosValues.z);


-
As you have already noticed, all 4 spawners' implementation are quite repetitive, namely the waiting part. However, each obstacle has its own spawn rotation and position. So, we will have to extract the former while isolating the latter :

public abstract class ObstacleSpawnerBase : MonoBehaviour
{
    public Obstacle obstacle;

    void Start()
    {
        StartCoroutine(SpawnObstacles());
    }

    // the repeated pattern in SpawnXXXXs has been extracted to here
    IEnumerator SpawnObstacles()
    {
        yield return new WaitForSeconds(obstacle.startGameWait);

        while (true)
        {
            foreach (var obstacle in SpawnObstaclesImpl())
                yield return new WaitForSeconds(obstacle.spawnWait);

            yield return new WaitForSeconds(obstacle.waveSpawnWait);
        }
    }

    // with the actual implementation detail separated for customization
    protected abstract IEnumerable SpawnObstaclesImpl();
}


And the concrete obstacle spawner classes :

```
public class HammerSpawner : ObstacleSpawnerBase
{
protected override IEnumerable SpawnObstaclesImpl()
{
for (int i = 0; i < obstacle.spawnCount; i++)
{
// r

Code Snippets

public class Obstacle1 // Hammer Obstacle
{
    public GameObject hammer; // The first obstacle gameobject. This is attached in the inspector.
    public Vector3 spawnHPosValues; // Where the first obstacle will be spawned at on the X,Y,Z plane.
    public int hCount; // This is the count of the first obstacle in a given wave. //
    public float hSpawnWait; // Time in seconds between next wave of obstacle 1.
    public float hStartGameWait; // Time in seconds between when the game starts and when the first obstacle start spawning.
    public float hWaveSpawnWait; // Time in seconds between waves when the next wave of obstacle 1 will spawn.
}
var spawnRotation = Quaternion.identity; // Spawn with NO rotation
Instantiate(hammer.GameObject, spawnPosition, spawnRotation); // Bring the object into the scene
var noRotation = Quaternion.identity;
Instantiate(hammer.GameObject, spawnPosition, noRotation); // Bring the object into the scene
[Serializable]
public abstract class Obstacle
{
    public GameObject gameObject;
    public Vector3 spawnPosition;
    public int spawnCount;
    public float spawnWait;
    public float startGameWait;
    public float waveSpawnWait;
}

public class Hammer : Obstacle { }
public class RoadBarrier : Obstacle { }
public class Cone : Obstacle { }
public class Barrel : Obstacle { }
// compare these two :
Vector3 spawnPosition = new Vector3(Random.Range(-cone.spawnCPosValues.x, cone.spawnCPosValues.x),
                                    cone.spawnCPosValues.y,
                                    cone.spawnCPosValues.z);
Vector3 spawnPosition = new Vector3(
    Random.Range(-cone.spawnCPosValues.x, cone.spawnCPosValues.x),
    cone.spawnCPosValues.y,
    cone.spawnCPosValues.z);

Context

StackExchange Code Review Q#128386, answer score: 3

Revisions (0)

No revisions yet.