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

Unity simple map generation

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

Problem

I am trying to generate a simple map like this. Everything works as I want, but I am new to programing. Is it a good way to do something like this? And if not, could someone explain to me where and why it is bad to do something like this? And how bad is it on performance?

public GameObject cube;
public GameObject cube1;
public GameObject cube2;
Vector3 here;

void Start () {
    for (int y = 0; y < 50; y++) {
        for (int x = 0; x < 10; x++) {
            here = new Vector3(x,y,0);
            int change = (int)Random.Range(0,19);
            switch (change){
            case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7: case 8: case 9: case 10:
                Instantiate(cube,here,Quaternion.identity);
                break;
            case 11: case 12: case 13: case 14: case 15: case 16:
                Instantiate(cube1,here,Quaternion.identity);
                break;
            case 17: case 18:
                Instantiate(cube2,here,Quaternion.identity);
                break;
            }
        }
    }


Running this code generates something random like this:

Instantiate takes 3 things (GameObject you want to make, coordinates where to make it, and rotation), and it makes a copy of GameObject - prefab in those coordinates.

  • Cube - Red



  • Cube1 - Green



  • Cube2 - Blue



The reason for all of this is that I want to learn to make something random likea map area of the game.

Solution

As a first step, I think this is cleaner than the switch

if (change <= 10)
{
    Instantiate(cube, here, Quaternion.identity);
}
else if (change <= 16)
{
    Instantiate(cube1, here, Quaternion.identity);
}
else
{
    Instantiate(cube2, here, Quaternion.identity);
}


But there's still repetition with the calls to Instantiate. I'd suggest creating a new method like this

private GameObject GetRandomCube()
{
    var change = (int)Random.Range(0, 19);
    if (change <= 10)
    {
        return cube;
    }

    if (change <= 16)
    {
        return cube1;
    }

    return cube2;
}


And then use it like this in your loop

Instantiate(GetRandomCube(), here, Quaternion.identity);


If here isn't being used elsewhere in the class, move its declaration inside the inner loop, or just use it in the function call. Now your method looks like this

void Start()
{
    for (var y = 0; y < 50; y++)
    {
        for (var x = 0; x < 10; x++)
        {
            Instantiate(GetRandomCube(), new Vector3(x, y, 0), Quaternion.identity);
        }
    }
}

Code Snippets

if (change <= 10)
{
    Instantiate(cube, here, Quaternion.identity);
}
else if (change <= 16)
{
    Instantiate(cube1, here, Quaternion.identity);
}
else
{
    Instantiate(cube2, here, Quaternion.identity);
}
private GameObject GetRandomCube()
{
    var change = (int)Random.Range(0, 19);
    if (change <= 10)
    {
        return cube;
    }

    if (change <= 16)
    {
        return cube1;
    }

    return cube2;
}
Instantiate(GetRandomCube(), here, Quaternion.identity);
void Start()
{
    for (var y = 0; y < 50; y++)
    {
        for (var x = 0; x < 10; x++)
        {
            Instantiate(GetRandomCube(), new Vector3(x, y, 0), Quaternion.identity);
        }
    }
}

Context

StackExchange Code Review Q#84431, answer score: 3

Revisions (0)

No revisions yet.