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

Generating 3D voxel terrain with Perlin Noise

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

Problem

I've created this simple 3D voxel terrain generator using Perlin Noise. I have a few concerns about it though.

  • Is there any way to make it more performant? It currently takes ~1.5 seconds to generate a 200x200 map.



  • Am I splitting up the different "jobs" of the generator correctly?



  • Is my general code style good?



```
using UnityEngine;

///
/// Terrain generator.
///
public class TerrainGenerator : MonoBehaviour
{
public GameObject cube;
public int worldWidthX;
public int worldWidthZ;

public float lowerNoiseScaleValue;
public float upperNoiseScaleValue;

public float lowerNoiseScaleModifierValue;
public float upperNoiseScaleModifierValue;

private float noiseScale;
private float noiseScaleModifier;

///
/// Generates the cubes as children of a GameObject.
///
public void InitalizeCubes()
{
for(float x = 0; x
/// Sets the color of the cube based on the cube's height.
///
/// Cube.
/// Cube height.
public void SetCubeHeightColor(Transform cube, float cubeHeight)
{
cube.renderer.material.color = new Color(cubeHeight / 5, cubeHeight, cubeHeight / 5);
}

///
/// Applies a height value to the transform of a cube.
///
/// Cube.
/// Cube height.
public void ApplyHeightToCube(Transform cube, float cubeHeight)
{
int newHeight = Mathf.RoundToInt(cubeHeight * this.noiseScaleModifier);
Vector3 newPosition = new Vector3(cube.transform.position.x, newHeight, cube.transform.position.z);
cube.transform.position = newPosition;
}

///
/// Updates the positions of the cubes by giving
/// each cube a specific perlin noise value,
/// coloring them, and finally, applying the height
/// values to the transform.position's of the cubes.
///
public void GenerateCubes()
{
foreach(Transform cube in transform)
{
float noiseScaleAddition = Random.Range(this.noiseSc

Solution

I don't know much about Unity, but one of the Unity operations must be the bottleneck here if it takes 1.5 seconds to process 40k cubes. I'd focus on reducing the number of Unity operations you're doing (manipulating the game objects). For example, you're first placing all the cubes at y=0, then moving them later. Instead, try generating the Perlin Noise first into an array, and then place the cubes at the correct height on the Instantiate call.

Also consider this line:

cube.renderer.material.color = new Color(cubeHeight / 5, cubeHeight, cubeHeight / 5);


You have 40k cubes but only about 20 colors. Is there some way of sharing materials (or even renderers) between the cubes? You may be asking Unity to manage many copies of identical materials. Look into other ways to share resources; is there some way to only create a few cubes with different colors, and render one entire height-layer with copies of it?

As for general coding style:

You have public GameObject cube; as an instance variable, but you also use the word cube all over the place for local variables. Is there a better name you could give this, like templateCube?

This is comment is not necessary:

/// 
/// Sets the color of the cube based on the cube's height.
/// 
/// Cube.
/// Cube height.
public void SetCubeHeightColor(Transform cube, float cubeHeight)


This takes up a lot of lines to give me the same information as your method signature ... although when my eyes see "SetCubeHeight" I assume you're setting the cube's height, but you're actually setting the color. Consider SetCubeColorByHeight instead.

Usually, everything that your class needs to function properly should be an argument in its constructor. This may not be possible with a Unity MonoBehavior, but I thought I'd throw it out there.

Code Snippets

cube.renderer.material.color = new Color(cubeHeight / 5, cubeHeight, cubeHeight / 5);
/// <summary>
/// Sets the color of the cube based on the cube's height.
/// </summary>
/// <param name="cube">Cube.</param>
/// <param name="cubeHeight">Cube height.</param>
public void SetCubeHeightColor(Transform cube, float cubeHeight)

Context

StackExchange Code Review Q#99307, answer score: 4

Revisions (0)

No revisions yet.