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

Terrain Generator - KhanAcademy

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

Problem

I'm new to JavaScript, and I've been learning off Khanacademy. I wrote the following basic terrain generator and I'm wondering if it could use any improvements. Here's the code:

var res = 7;

var area = [];

var camP = [random(500,1000), random(500,1000)];

var setArea = function()
{
    area = [];
    for (var y = -200; y < 200; y+=res)
    {
        var row = [];
        for (var x = -200; x < 200; x+=res)
        {
            row.push(noise((x+camP[0])/200, (y+camP[1])/200));
        }
        area.push(row);
        row = [];
    }
};

var drawPix = function(v, x, y){
    if (v < 0.49)
    {
        fill(random(30,40), random(100,128), random(20,30));
    } 
    else 
    {
        fill(random(0,10), random(40,50), random(245,255));
    }
    rect(x*res,y*res,res,res);
};

var drawArea = function()
{
    background(255, 255, 255);
    noStroke();
    for (var y in area)
    {
        for (var x in area[y])
        {
            drawPix(area[y][x],x,y);
        }
    }
};

setArea();
drawArea();


Here's an example of output:

Solution

It's nicely done! One thing I'd pick on though is that your code is full of magic numbers:

var camP = [random(500,1000), random(500,1000)];
// ...
for (var y = -200; y < 200; y+=res)
// ...
for (var x = -200; x < 200; x+=res)
// ...
row.push(noise((x+camP[0])/200, (y+camP[1])/200));
// ...
if (v < 0.49)
// ...
fill(random(30,40), random(100,128), random(20,30));
// ...
fill(random(0,10), random(40,50), random(245,255));


Put these numbers into variables, defined at the top.
By doing so you will effectively give them names,
which could explain what they are.

This is especially recommended for values that you use repeatedly, for example 200.

And when a value depends on another (derived from another),
that relationship can be expressed in the definition.
For example, the difference in the parameter pairs in random(0,10), random(40,50), random(245,255) is suspiciously 10, probably for a reason, which would become clear in the variable definition.
I've no idea how fill works, so these names are probably completely inappropriate, but to illustrate what I mean:

var FILL_RGB_RANGE = 10;
var FILL_R = 0;
var FILL_G = 40;
var FILL_B = 245;

fill(
  random(FILL_R, FILL_R + FILL_RGB_RANGE),
  random(FILL_G, FILL_G + FILL_RGB_RANGE),
  random(FILL_B, FILL_B + FILL_RGB_RANGE)
);

Code Snippets

var camP = [random(500,1000), random(500,1000)];
// ...
for (var y = -200; y < 200; y+=res)
// ...
for (var x = -200; x < 200; x+=res)
// ...
row.push(noise((x+camP[0])/200, (y+camP[1])/200));
// ...
if (v < 0.49)
// ...
fill(random(30,40), random(100,128), random(20,30));
// ...
fill(random(0,10), random(40,50), random(245,255));
var FILL_RGB_RANGE = 10;
var FILL_R = 0;
var FILL_G = 40;
var FILL_B = 245;

fill(
  random(FILL_R, FILL_R + FILL_RGB_RANGE),
  random(FILL_G, FILL_G + FILL_RGB_RANGE),
  random(FILL_B, FILL_B + FILL_RGB_RANGE)
);

Context

StackExchange Code Review Q#63202, answer score: 16

Revisions (0)

No revisions yet.