Variables being changed for no reason

Question

I am making a very simple thing where things fall from the top of the screen and (once finished) you will have to grab them. I store all objects falling (bottles) inside an array. When there is one object, it draws fine and it's Y position increases normally, yet when there are more than one they are all given exactly the same X and Y. I have no idea what is causing this.

This is the whole file:

var canvas = document.getElementById("canvas");
var canvasWidth = 600;
var canvasHeight = 500;
canvas.width = canvasWidth;
canvas.height = canvasHeight;
var ctx = canvas.getContext("2d");

var bottle = {
    x: 0,
    y: 0,
    hasFell: false,
    height: 0,
    width: 0,
    src1: "water1.png",
    src2: "water2.png"
}

var bottles = new Array();
bottles.length = 20;
bottleImage = new Image();
bottleImage.src = bottle.src1;
fellBottleImage = new Image();
fellBottleImage.src = bottle.src2;

var makeBottle = function(){
    //If math.random above 7 add new bottle
    if(Math.random() > 0.7){
        var madeBottle = false;
        for(var i = 0; i < bottles.length; i++){
            if(bottles[i] == null){
                //It only goes in here once, after it has made one bottle it exits the loop
                madeBottle = true;
                bottles[i] = bottle;

                //These are the only things that change both the X and the Y, yet these cannot be the problem.
                bottles[i].x = Math.random() * 100;
                bottles[i].y = Math.random() * 100;


                console.log("Made bottle: " + i + " X: " + bottles[i].x + " Y: " + bottles[i].y);
                break;
            }
        }
        if(!madeBottle){
            for(var i = 0; i < bottles.length; i++){
                if(bottles[i].hasFell){
                    bottles[i] = null;
                    makeBottle();
                }
            }
        }
    }
}

var gameUpdate = function(){
    for(var i = 0; i < bottles.length; i++){
        if(bottles[i] != null){
            bottles[i].y += 1;
            if(bottles[i].y > canvasHeight) bottles[i] = null;
        }
    }
    gamePaint();
}

var gamePaint = function(){
    ctx.fillStyle = "#FFFFFF";
    ctx.fillRect(0, 0, canvasWidth, canvasHeight);

    for(var i = 0; i < bottles.length; i++){
        if(bottles[i] != null){
            ctx.drawImage(bottleImage, bottles[i].x, bottles[i].y);
        }
    }
}

var gameInterval = setInterval(gameUpdate, 10);
var bottleInterval = setInterval(makeBottle, 1000);

There must be, but I do not know what it is. – May 21 '13 at 17:13 — , May 21 '13 at 17:13

Mark Reed · Answer 1 · 2013-05-21T17:18:35.787

3

            bottles[i] = bottle;

That makes bottles[i] and bottle point to the same object. At the end of your loop, you still have only a single bottle with a lot of different names.

If you want to make bottles[i] a copy of bottle, you have to do the copying yourself. Here's one way:

bottles[i] = {}
for (var key in bottle) {
    bottles[i][key] = bottle[key];
}

Alternatively, if all bottles are going to start out with the same values, you could make a Bottle constructor function:

function Bottle() {
  this.x = 0;
  this.y = 0;
  this.hasFell = false;
  this.height = 0;
  this.width = 0;
  this.src1 = "water1.png";
  this.src2 = "water2.png";
}

bottle = new Bottle();

...

bottle[i] = new Bottle();

edited May 21 '13 at 17:18

answered May 21 '13 at 17:13

Mark Reed

91,912
16
138
175

@Hsenfow: http://stackoverflow.com/questions/122102/most-efficient-way-to-clone-an-object is an extremely good discussion on how to clone objects. – Sébastien Renauld May 21 '13 at 17:16
Thank you, although I think I'll do it the way Ben336 said, because it seems less complicated. – May 21 '13 at 17:18
@SébastienRenauld In this case it probably makes more sense to create a class than to clone a primitive object anyway. – Ben McCormick May 21 '13 at 17:18

score 3 · Answer 2 · edited May 21 '13 at 17:24

 //These are the only things that change both the X and the Y, yet these cannot be the problem.
 bottles[i].x = Math.random() * 100;
 bottles[i].y = Math.random() * 100;

Sure they can :) you're setting every member of the array to point to the bottle object, then modifying that object. You need to create different objects for each item in the array.

You can do this with

var Bottle = function(){
    this.x = 0;
    this.y = 0;
    this.hasFell = false;
    this.height = 0;
    this.width = 0;
    this.src1 = "water1.png";
    this.src2 = "water2.png";
}

and then

bottles[i] = new Bottle();

That creates a new constructor Bottle, and then instantiates it to create a new object with the property you want for each item in the array.

Those need to be assignment statements inside the constructor. — Mark Reed, May 21 '13 at 17:19

Variables being changed for no reason

2 Answers2