10
\$\begingroup\$

I've spent the last weekend working on this basic block building game, which I'm quite proud of. I do think it can be improved in many places though, e.g, performance issues due to my badly designed tile loading, etc. etc...

/** Here are few instructions before you start...
  * IMPORTANT (ON KHANACADEMY ONLY): When you make changes to the code, all tiles will be black.
  * Just hit the restart button to fix that.
  *
  * CONTROLS:
  * MOUSE - Move around the current selected tile.
  * LEFT MOUSE BUTTON - Add a tile to the screen.
  * RIGHT MOUSE BUTTON - Delete a tile from the screen.
  * CONTROL KEY - Change the block type.
  * WASD or LEFT RIGHT DOWN UP - Move around the map.
*/

/* Global program constants */
var CURSOR_FONT = createFont("monospace", 15);
var BACKGROUND_COLOR = color(0, 0, 255);
var HITBOX_COLOR = color(255, 255, 255);
var CLOUD_COLOR = color(255, 255, 255);
var SUN_COLOR1 = color(194, 181, 33);
var SUN_COLOR2 = color(255, 140, 0);
var SUN_WIDTH = 100;
var SUN_HEIGHT = 100;
var TILE_SIZE = 10;
var COLORS = [
    color(180, 120, 20),
    color(20, 150, 20),
    color(100, 100, 100),
    color(240, 200, 10),
    color(5, 44, 117),
    color(255, 255, 255),
    color(110, 70, 10),
    color(10, 210, 20),
];
var TILE_TYPES = [
    "Dirt",
    "Grass",
    "Stone",
    "Sand",
    "Water",
    "Snow",
    "Wood",
    "Leaves",
];

/* Variables concerning map movement */
var movingUp = false;
var movingDown = false;
var movingLeft = false;
var movingRight = false;

/* Array containing cloud data */
var cloudArray = [];

/* Current selected color */
var selectedColor = 0;

/* Array contaning all tile data */
var tileArray = [];

/* Variables controlling the sun's position */
var sunYChange = -0.5;
var SUN_X_POS = 189;
var sunYPos = 200;

/* Render the day and night cycle*/
var renderDayNightCycle = function() {
    noStroke();
    fill(SUN_COLOR2);
    rect(SUN_X_POS, sunYPos, 45, 45);
    fill(SUN_COLOR1);
    rect(SUN_X_POS+5, sunYPos+5, 35, 35);
    sunYPos += sunYChange;
    if(sunYPos <= -50) {
        sunYChange = 0.5;
    }
    if(sunYPos >= 450) {
        sunYChange = -0.5;
    }
};

/* Generate new terrain for the world */
var generateTerrain = function() {
    var c = random(1, 3);
    var blockY = 300;
    for(var x = -1500; x <= 1500; x += TILE_SIZE) {
        tileArray.push([]);
        var rb = round(random(-1, 0));
        tileArray.push({
            xPos: x,
            yPos: blockY+TILE_SIZE*rb,
            colr: COLORS[1],
        });
        if(rb === -1) {
            tileArray.push({
                xPos: x,
                yPos: blockY,
                colr: COLORS[0],
            });
        }
        if(random() >= random()*random()/random()+random()) {
            var m = round(random(2, 6));
            for(var h = blockY-TILE_SIZE; h >= blockY-TILE_SIZE*m; h -= TILE_SIZE) {
                tileArray.push({
                    xPos: x,
                    yPos: h,
                    colr: COLORS[6],
                });
            }
            for(var i = 0; i <= 1; i++) {
                tileArray.push({
                    xPos: x,
                    yPos: blockY-TILE_SIZE*(m+i)-TILE_SIZE,
                    colr: COLORS[7],
                });
            }
            tileArray.push({
                xPos: x+TILE_SIZE,
                yPos: blockY-TILE_SIZE*m-TILE_SIZE,
                colr: COLORS[7]
            });
            tileArray.push({
                xPos: x-TILE_SIZE,
                yPos: blockY-TILE_SIZE*m-TILE_SIZE,
                colr: COLORS[7]
            });
        }
        for(var y = blockY; y <= blockY+TILE_SIZE*round(random(2, 4)); y += TILE_SIZE) {
            tileArray.push({
                xPos: x,
                yPos: y+TILE_SIZE,
                colr: COLORS[0],
            });
        }
        for(var y = blockY+TILE_SIZE*round(c); y <= blockY+TILE_SIZE*random(18, 22); y += TILE_SIZE) {
            tileArray.push({
                xPos: x,
                yPos: y+TILE_SIZE,
                colr: COLORS[2],
            });
        }
        blockY += (ceil(random(-1, 1)/TILE_SIZE)*TILE_SIZE)*round(random(-2, 2));
    }
};

/* Initalize the array of clouds */
var generateClouds = function() {
    for(var i = 0; i <= round(random(2, 12)); i++) {
        cloudArray.push({
            xPos: random(50, 350),
            yPos: random(50, 150),
            w: random(30, 60),
            h: random(10, 20),
        });
    }
};

/* Draw the background */
var drawBackground = function() {
    for(var c = cloudArray.length-1; c >= 0; c--) {
        var cloud = cloudArray[c];
        noStroke();
        fill(CLOUD_COLOR);
        rect(cloud.xPos, cloud.yPos, cloud.w, cloud.h);
        cloud.xPos += random(0.01, 0.09);
        if(cloud.xPos >= 400) {
            cloud.xPos = 0-cloud.w;
        }
    }
};

/* Draw a hitbox over the selected position */
var drawHitbox = function(x, y) {
    noCursor();
    fill(HITBOX_COLOR);
    textFont(CURSOR_FONT);
    text("+", mouseX-1, mouseY+4);
    noFill();
    strokeWeight(1);
    stroke(HITBOX_COLOR);
    rect(x, y, TILE_SIZE, TILE_SIZE);
};

/* Add a block to the screen */
var addBlock = function(x, y) {
    drawHitbox(x, y);
    if(mouseIsPressed && mouseButton === LEFT) {
        if(x >= 20 && y >= 20) {
            tileArray.push({
                xPos: x,
                yPos: y,
                colr: COLORS[selectedColor],
            });
        }
    }
};

/* Delete a block from the screen */
var deleteBlock = function(x, y) {
    drawHitbox(x, y);
    if(mouseIsPressed && mouseButton === RIGHT) {
        for(var t = tileArray.length-1; t >= 0; t--) {
            var tile = tileArray[t];
            if(x === tile.xPos && y === tile.yPos) {
                tileArray.splice(t, 1);
            }
        }
    }
};

/* Render the tileArray */
var renderTiles = function() {
    for(var t = tileArray.length-1; t >= 0; t--) {
        var tile = tileArray[t];
        noStroke();
        fill(tile.colr);
        rect(tile.xPos, tile.yPos, TILE_SIZE, TILE_SIZE);
    }
};

/* Check for specific key actions */
var checkForKeyActions = function() {
    keyPressed = function() {
        if(keyCode === LEFT || keyCode === 65) {
            movingLeft = true;
        }
        if(keyCode === RIGHT || keyCode === 68) {
            movingRight = true;
        }
        if(keyCode === UP || keyCode === 87) {
            movingUp = true;
        }
        if(keyCode === DOWN || keyCode === 83) {
            movingDown = true;
        }
        if(keyCode === CONTROL) {
            selectedColor++;
            if(selectedColor >= COLORS.length) {
                selectedColor = 0;
            }
        }
    };
    keyReleased = function() {
        if(keyCode === LEFT || keyCode === 65) {
            movingLeft = false;
        }
        if(keyCode === RIGHT || keyCode === 68) {
            movingRight = false;
        }
        if(keyCode === UP || keyCode === 87) {
            movingUp = false;
        }
        if(keyCode === DOWN || keyCode === 83) {
            movingDown = false;
        }
    };
    if(movingUp) {
        for(var t = tileArray.length-1; t >= 0; t--) {
            var tile = tileArray[t];
            tile.yPos += TILE_SIZE;
        }
    }
    if(movingDown) {
        for(var t = tileArray.length-1; t >= 0; t--) {
            var tile = tileArray[t];
            tile.yPos -= TILE_SIZE;
        }
    }
    if(movingLeft) {
        for(var t = tileArray.length-1; t >= 0; t--) {
            var tile = tileArray[t];
            tile.xPos += TILE_SIZE;
        }
    }
    if(movingRight) {
        for(var t = tileArray.length-1; t >= 0; t--) {
            var tile = tileArray[t];
            tile.xPos -= TILE_SIZE;
        }
    }
};

/* Draw the current selected tile */
var drawSelectedTile = function() {
    strokeWeight(1.5);
    stroke(255, 255, 255);
    fill(COLORS[selectedColor]);
    rect(5, 5, 15, 15);
    fill(255, 255, 255);
    textFont(CURSOR_FONT);
    text(TILE_TYPES[selectedColor], 25, 17.4); 
};

/* Load the world and clouds before the draw loop begins */
generateTerrain();
generateClouds();

/* Main draw loop */
draw = function() {

    /* Reset the background on each loop */
    background(BACKGROUND_COLOR);

    /* Render the day/night cycle */
    renderDayNightCycle();

    /* Draw the background clouds */
    drawBackground();

    /* Render the tiles */
    renderTiles();

    /* Draw the current selected tile */
    drawSelectedTile();

    /* Check for certain key actions */
    checkForKeyActions();

    /* Check if the user wants to add a block */
    addBlock(ceil(mouseX/TILE_SIZE)*TILE_SIZE-TILE_SIZE, 
             ceil(mouseY/TILE_SIZE)*TILE_SIZE-TILE_SIZE);

    /* Check if the user wants to delete a block */
    deleteBlock(ceil(mouseX/TILE_SIZE)*TILE_SIZE-TILE_SIZE, 
                ceil(mouseY/TILE_SIZE)*TILE_SIZE-TILE_SIZE);

    /* Draw a hitbox over the current selected block */
    drawHitbox(ceil(mouseX/TILE_SIZE)*TILE_SIZE-TILE_SIZE, 
               ceil(mouseY/TILE_SIZE)*TILE_SIZE-TILE_SIZE);
};

I know that there is plenty to be improved, so what can be improved? You can find the link to play it here: here.

\$\endgroup\$
0

2 Answers 2

6
\$\begingroup\$

Your code is much improved from the last time that I looked at it. Well done! I do have some comments, and I will just address them in the order they appear.

RIGHT MOUSE BUTTON - Delete a tile from the screen.

Not all computers have a right mouse button. Suppose that someone is using a Mac, for example?

var SUN_COLOR1 = color(194, 181, 33);
var SUN_COLOR2 = color(255, 140, 0);

I think that you would be much better off calling these colors what they are instead of calling them COLOR1 and COLOR2. backgroundColor and foregroundColor would make more sense here.

var COLORS = [
    color(180, 120, 20),
    color(20, 150, 20),
    color(100, 100, 100),
    color(240, 200, 10),
    color(5, 44, 117),
    color(255, 255, 255),
    color(110, 70, 10),
    color(10, 210, 20),
];

When first encountering this, it is not clear to the reader what these COLORS will be used for. I thought that maybe they would be randomly assigned at some point. If that was the case, randomColors would be a better name. However, looking through the code it appears that they are used throughout when rendering any of the tiles. It looks like there is some relationship between selectedColor, the TILE_TYPES array, and the COLORS array. Unfortunately the exact relationship is difficult to comprehend from the code.

This suggests that it would be better to have some sort of Tile object that contained the information about its name and color, and whatever other important information necessary. Writing Javascript in an object oriented way is somewhat difficult since the features are not built into the language the way that they are built into a more traditional language such as Java, but it is still possible to do so and it would increase the readability of your code.

var CLOUD_COLOR = color(255, 255, 255);
var SUN_COLOR1 = color(194, 181, 33);
var SUN_COLOR2 = color(255, 140, 0);
var movingDown = false;
var movingLeft = false;
var movingRight = false;

Choose a naming style and stick with it. Do not mix ALLCAPITAL variable naming with camelCase. It is important to write your code as consistently as possible. For Javascript, most people will recommend that you do all variables in camelCase.

var generateTerrain = function() {

There is a lot going on in this function and it is difficult to follow exactly how the terrain is generated. I would be surprised if you are able to understand the exact flow of what is happening when you look at this code after a few months. Just a few pieces that I do not like:

var c = random(1, 3);
var blockY = 300;
for(var x = -1500; x <= 1500; x += TILE_SIZE) {

All of these numbers seem like magic numbers, and should have variable names describing what they mean.

if(random() >= random()*random()/random()+random()) {

I'm not even going to try to figure out what this is doing, but it seems really complicated and obtuse.

for(var y = blockY; y <= blockY+TILE_SIZE*round(random(2, 4)); y += TILE_SIZE) {
for(var y = blockY+TILE_SIZE*round(c); y <= blockY+TILE_SIZE*random(18, 22); y += TILE_SIZE) {

Again, no clue what these lines might mean. Things like this make it difficult to know the rules used to generate the terrain. At the very least, there should be comments explaining the stages of the terrain generation. However, I think that if you restructure the function and use more descriptive names and clearer logic, it should be understandable without a lot of comments.

/* Draw the background */
var drawBackground = function() {

Comments such as the one above are redundant and unnecessary. Comments should generally explain why you are doing something, and not what you are doing. The exception to this is to explain a difficult to understand line of code.

draw = function() {

I really like that drawing everything is separated into its own method. The method is easy to understand, and it would be easy to alter it later. This is a very good way to do things.

\$\endgroup\$
3
\$\begingroup\$

I love this project,

I would definitely create a helper function to create tiles like this:

function createTile( x, y, color ){
  tileArray.push({
    x: x,
    y: y,
    color: color,
  });
}

Notice that I would drop xPos to x because everybody parses x correctly, and I changed colr to color because colr just looks weird.

Then your generateTerrain function becomes so much more readable that it actually invites the reader to start messing around with world generation:

for(var x = -1500; x <= 1500; x += TILE_SIZE) {
    tileArray.push([]);
    var rb = round(random(-1, 0));
    tileArray.push( createTile( x, blockY+TILE_SIZE*rb, COLORS[1] ) );
    if(rb === -1) {
        tileArray.push( createTile( x, blockY, COLORS[0] ) );
    }
//etc etc etc

One more change I would suggest is to make the colors more intuitive by naming them:

var tiles = {
  'dirt':  color(180, 120, 20),    //0
  'grass': color(20, 150, 20),     //1
  'stone': color(100, 100, 100),   //2
  'sand': color(240, 200, 10),     //3
  'water': color(5, 44, 117),      //4
  'snow': color(255, 255, 255),    //5
  'wood': color(110, 70, 10),      //6
  'leaves': color(10, 210, 20),    //7
};
var TILE_TYPES = Object.keys( tiles );

Then further in the code you can replace COLOR statements like this:

    for(var y = blockY; y <= blockY+TILE_SIZE*round(random(2, 4)); y += TILE_SIZE) {
        tileArray.push( newTile( x, y+TILE_SIZE, tiles.dirt ) );
    }
    for(var y = blockY+TILE_SIZE*round(c); y <= blockY+TILE_SIZE*random(18, 22); y += TILE_SIZE) {
        tileArray.push( newTile( x, y+TILE_SIZE, tiles.stone ) );
    }

As for the background, check out this: http://cssdeck.com/labs/4ksohwya It might take a while to understand how the background is done, but it would be so worth it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.