CODEWITHSUNDEEP
javascriptcanvas
Kevin Young
Kevin Young

Reputation: 49

Canvas flashing

I'm a sophomore in High School and am in a comp sci class that uses (I believe) pretty old school approaches to things. I know my code is not going to be fantastic.

When any of the arrow keys are held down the map spazzes and flashes. I have to redraw the map because the canvas needs to be cleared, because otherwise I would have a ton of each character when I move him. I just want to draw one, obviously.

What can I do to make the map not flash when the arrow keys are held down?

http://gitastudents.com/youngkevin/zelda/ZELDA.html

Upvotes: 0

Views: 864

Answers (2)

Blindman67
Blindman67

Reputation: 54026

Old school and a mess.

Well that code is a mess (and I don't believe that is the teachers doing), so before you hand it in, and even while you are writing it just apply a code formatter or a beautifier on the source code. It will look better and be a lot easier to read and find problems.

A lot of the code is old school so the teacher should get updated code.

Load images only once

The reason that you are getting flickering is because every time you render the map you recreate and reload the images. You do the same for the link and the sword images (which are the same image BTW).

To fix

Add to the top of the script, just after the tag add

var canvas; // I put this here because of the mess is too much to clean
var grass = new Image();
var stone = new Image();
var sword = new Image();
var link = sword; // same image 
sword.src = 'http://gitastudents.com/youngkevin/zelda/img/sprites-link.png'
grass.src = 'http://gitastudents.com/youngkevin/zelda/img/grass.png';
stone.src = 'http://gitastudents.com/youngkevin/zelda/img/tree.png';

Then make the following changes

Yours in original format functions drawLink and drawSword (plus my comments). Don't know what is with the canvas size changes maybe old school clear method?

You had..

function drawLink(){
    var c = document.getElementById("myCanvas");  // Not needed
    var ctx = c.getContext("2d");                 // Not needed
    // ctx.fillStyle ="#FCD8A8";                  // remove dead code
    // ctx.fillRect(linkx - 20,linky-20, 30, 30); // remove dead code
    ctx.canvas.height = 575 ;                     // ?? only size canvas once
    ctx.canvas.width = 835  ;                     // remove



    link = new Image();                          // remove
    link.src = 'img/sprites-link.png'            // remove
    
    ctx.drawImage(link,srcX,srcY,srcW,srcH,linkx,linky,linkW,linkH);
    // drawSword();                              // remove dead code
    drawMap();                                   // why is this here???


}
        function drawSword(){
            var c = document.getElementById("myCanvas"); // Not needed
    var ctx = c.getContext("2d");                         // Not needed
    ctx.canvas.width = window.innerHeight +900;           // ???? remove
    ctx.canvas.height = window.innerHeight  ;              // remove

    sword = new Image();                                   // Not needed remove
    sword.src = 'img/sprites-link.png'                      // Not needed remove
    
    ctx.drawImage(sword,30,200,15,30,200,200,30,30);
        }

...change to (formated with jsbeautifier)

function drawLink() {
    ctx.clearRect(0, 0, canvas.width, canvas.height);
    if (link.complete) {
        ctx.drawImage(link, srcX, srcY, srcW, srcH, linkx, linky, linkW, linkH);
    }
    drawMap();  // ??
}

function drawSword() {
    if (sword.complete) { // make sure image has loaded
        ctx.drawImage(sword, 30, 200, 15, 30, 200, 200, 30, 30);
    }
}

then change the drawMap function to look like

function drawMap() {
    var posX = 0;
    var posY = 0;
    if (stone.complete && grass.complete) {
        for (var i = 0; i < mapArray.length; i++) {
            for (var j = 0; j < mapArray[i].length; j++) {
                if (mapArray[i][j] == 1) {
                    ctx.drawImage(stone, posX, posY, 32, 32);
                } else if (mapArray[i][j] == 2) {
                    ctx.drawImage(grass, 0, 0, 17, 17, posX, posY, 32, 32);
                } else if (mapArray[i][j] == 3) {
                    ctx.drawImage(grass, 20, 0, 17, 17, posX, posY, 32, 32);
                }
                posX += 32;
            }
            posY += 32;
            posX = 0;
        }
    }
}

And the function init now looks like

function init() {
    document.addEventListener('keydown', doKeyDown, true);
    document.addEventListener('keyup', keyUp, false);
    srcY = 30;
    canvas = document.getElementById("myCanvas");
    c = canvas; // you should change all the vars c to canvas
    canvas.height = 575;
    canvas.width = 835;
    ctx = canvas.getContext("2d");
    setMap7();
}

And that will stop the flickering as the images are now loaded at the start and will not be readered until they are complete.

For the rest of the code it is too much of a mess to fix without a complete rewrite from the ground up.

Code review.

Maybe once you have handed in the assignment and to help to gently nudge teacher in the right direction, submit the original code (from the teacher) to Code review telling them what it is. They will review that code and you could then respectively show the teacher, who may then take some time to get up to date with Javascript.

Upvotes: 1

Kyler Love
Kyler Love

Reputation: 1021

You can do lots of things to enhance this code. My first idea would be to get rid of the double array and create objects of trees and rocks with x y coords... you will still have to redraw on each render, but would allow you to get rid of that nested loop and only have an array of trees/rocks. not the whole map.

Also look into making a gameloop. you are rerendering on keypress which is fine if you were not intending to hold a button down such as a card game.

Upvotes: 0

Related Questions