patterncsharpMinor
Generating 3D voxel terrain with Perlin Noise
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.
```
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
- 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:
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
This is comment is not necessary:
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
Usually, everything that your class needs to function properly should be an argument in its constructor. This may not be possible with a Unity
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.