canvas drawImage loop issue

Question

I'm trying to modify this canvas game by replacing fillRect with images.

No problems when changing the ship code in spaceinvaders.js on line 529:

ship = new Image();
ship.src = "images/ship.svg";

ctx.drawImage(ship, this.ship.x - (this.ship.width / 2), this.ship.y - (this.ship.height / 2), this.ship.width, this.ship.height);

but having trouble with invaders loop:

for(var i=0; i<this.invaders.length; i++) {

    var invader = this.invaders[i] = new Image();
    this.invaders[i].src = "images/space_invader.png";

    //ctx.fillRect(invader.x - invader.width/2, invader.y - invader.height/2, invader.width, invader.height);

    this.invaders[i].onload = function(){

        var thisX = invader.x * 18;
        var thisY = invader.y * 14;

        return function() {
            ctx.drawImage(invader,invader.x - invader.width/2, invader.y - invader.height/2, invader.width, invader.height);
            //ctx.drawImage(this, thisX, thisY, invader.width, invader.height);
        };

    };
}

When I get the invader image to finally show, it stacks them all in top of each other. - Even with thisX and thisY variables.

EDIT:

img = new Image();

img.onload = function(){

    for(var i=0; i<this.invaders.length; i++) {
        for(var j=0; j<this.invaders.length; j++) {
            var invader = this.invaders[i];
            ctx.drawImage(img,j*18, i*14, invader.width, invader.height);
        }
    }
}

img.src = "images/space_invader.png";

Gives: cannot read property 'length' of undefined.

EDIT 2:

img = new Image();

img.onload = function(){

    for(var i=0; i<5; i++) {
        for(var j=0; j<10; j++) {
            ctx.drawImage(img,j*22, i*16, 18, 14);
        }
    }
}

img.src = "images/space_invader.png";

This way I'm atleast able to see invaders but they are static:

EDIT 3:

var invaders=[{x:0,y:0},{x:20,y:0},{x:40,y:0},{x:60,y:0},{x:80,y:0},{x:100,y:0},{x:120,y:0},{x:140,y:0},{x:160,y:0},{x:180,y:0},{x:0,y:16},{x:20,y:16},{x:40,y:16},{x:60,y:16},{x:80,y:16},{x:100,y:16},{x:120,y:16},{x:140,y:16},{x:160,y:16},{x:180,y:16},{x:200,y:16},{x:0,y:32},{x:20,y:32},{x:40,y:32},{x:60,y:32},{x:80,y:32},{x:100,y:32},{x:120,y:32},{x:140,y:32},{x:160,y:32},{x:180,y:32},{x:200,y:32},{x:0,y:48},{x:20,y:48},{x:40,y:48},{x:60,y:48},{x:80,y:48},{x:100,y:48},{x:120,y:48},{x:140,y:48},{x:160,y:48},{x:180,y:48},{x:200,y:48},{x:0,y:64},{x:20,y:64},{x:40,y:64},{x:60,y:64},{x:80,y:64},{x:100,y:64},{x:120,y:64},{x:140,y:64},{x:160,y:64},{x:180,y:64},{x:200,y:64}];

for (var i=0; i<this.invaders.length; i++) {

    invaders[i] = new Image();

    invaders[i].onload = function() {
        ctx.drawImage(this, x, y);         
    };

    invaders[i].src = "images/space_invader.png";
}

Gives x is not defined.

EDIT 4:

function createImage(i){   

var image = new Image();  
    image.src = "images/space_invader.png";
    image.onload = function(){  
        ctx.drawImage(image,imagePos[i][0],imagePos[i][1], 20, 16);
    } 
}

var imagePos=[[0,0],[20,0],[40,0],[60,0],[80,0],[100,0],[120,0],[140,0],[160,0],[180,0],[0,16],[20,16],[40,16],[60,16],[80,16],[100,16],[120,16],[140,16],[160,16],[180,16],[0,32],[20,32],[40,32],[60,32],[80,32],[100,32],[120,32],[140,32],[160,32],[180,32],[0,48],[20,48],[40,48],[60,48],[80,48],[100,48],[120,48],[140,48],[160,48],[180,48],[0,64],[20,64],[40,64],[60,64],[80,64],[100,64],[120,64],[140,64],[160,64],[180,64]];

for(var i = 0; i <this.invaders.length; i += 1){
    createImage(i); 
}
return

Shows invaders but they keep flickering. (main game loop?)

Answer 1

One variable can only hold one value.

There is a simple rule that will help you avoid problems like this. Never declare functions inside loops.

Why??

Let look at a loop

var imagePos = [[10,10],[10,20],[10,30],[10,40],[10,50],[20,10],[20,20],[20,30],[20,40],[20,50]];
for(var i = 0; i < 10; i += 1){
    var image = new Image();
    image.src = "imagename";
    image.onload = function(){
         ctx.drawImage(image,imagePos[i][0],imagePos[i][1]);
    } 
}
// end
return

You are expecting 10 images to start to load, when each image has loaded you expect them to be draw at the correct position. But that will never happen.

The onload function will not fire until the for loop has completed and the execution has returned. You have 2 variables, i and image, by the time the first onload event fires i = 10 and it will stay that way for each onload. Also the image you referance inside the onload function is also only one variable and can not hold more than one image. Even though each onload is correctly bound to the image, the image variable will be pointing to the last images created in the loop.

You are kind of expecting i and image to hold 10 separate values each and know what needs what value when.

The correct way to do that is as follows

function createImage(i){   // load an image function 
                           // creates a new i each time it is called
    var image = new Image();  // create a new image variable 
    image.src = "imagename";
    image.onload = function(){  // now the onload will have the correct variables 
         ctx.drawImage(image,imagePos[i][0],imagePos[i][1]);
    } 
 }
var imagePos = [[10,10],[10,20],[10,30],[10,40],[10,50],[20,10],[20,20],[20,30],[20,40],[20,50]];
for(var i = 0; i < 10; i += 1){
    createImage(i); // call the function so it can create unique variables for each image
}
// end
return

You could also have done it with the function inside the loop but you would have needed to use the functions bound object to store the values you wanted.

var imagePos = [[10,10],[10,20],[10,30],[10,40],[10,50],[20,10],[20,20],[20,30],[20,40],[20,50]];
for(var i = 0; i < 10; i += 1){
    var image = new Image();
    image.src = "imagename";
    image.posIndex = i; // attach the index to the image
    image.onload = function(){ // instead of using image use the image bound to the function 
                                // and referenced via the this token
         ctx.drawImage(this, imagePos[this.posIndex][0], imagePos[this.posIndex][1]);
    } 
    
}
// end
return

The is nothing wrong with doing it this way, apart from the trap you found. That is why the general rule is, if you are unfamiliar with javascript, do not declare functions inside loops.

Answer 2

Great! Now all you need is some kind of game loop to get them moving. I would move the x and y positions (from your current for loop) into individual objects so you can modify each invader per (update) loop.

var invaders = [ {x: 100, y: 100, speed: 10}, {x: 140, y: 100, speed: 10}, {x: 180, y: 100, speed: 10}, // and so on ];

Try reading more information regarding game loops and how they work here: https://www.isaacsukin.com/news/2015/01/detailed-explanation-javascript-game-loops-and-timing

edit: Something like this...

for(var i = 0; i < invaders.length; i++) {

    var invader = new Image();

    invader.onload = function() {
        ctx.drawImage(this, invaders[i].x, invaders[i].y);         
    };

    invaders[i].src = "images/space_invader.png";
}

Answer 3

You probably shouldn't use the x and y properties of the Image object. First it's read only and second it's probably not set and there for always 0 since you are creating a new Image instance.

See https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/Image and also https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement