0
var canvas = document.getElementById('gameCanvas');
var ctx = canvas.getContext('2d');
var xPosition = 0;
var yPosition = 0;
var frameLength = 20; 

function gameLoop(){

    gameUpdate();
    setTimeout(function(){gameLoop()}, frameLength);
}

function gameUpdate(){

ctx.clearRect(0, 0, 480, 640);
window.addEventListener('keydown', function(event){switch(event.keyCode){case 38: yPosition -= 1; break; case 40: yPosition += 1; break;}}, false);
ctx.fillRect(xPosition, yPosition, 50, 50);

}

$(document).ready(function () {
gameLoop();
});

I'm trying to move a rectangle in canvas via arrow keys. X and Y positions are variables xPosition and yPosition. Dealing strictly with the Y-direction for now, I have a switch function triggered by an eventlistener for "keydown." When, for example, the DOWN-Arrow key is pressed, it increments the Y value in fillRect(X, Y, originX, OriginY). This changes the global variable, then the new rectangle with new coordinates is drawn on the Canvas. GameLoop resets, rinse repeat. That's how I perceive it to work in theory.

For some reason, however, even if I do nothing at all, the longer I let the script run, the "farther" the square jumps when I press UP or DOWN. That is, pressing down launches it off the canvas ever so many pixels that is determined by the number of loops setTimeout has gone through, it appears. Why is this, and how do I address it?

Zakaria Acharki
  • 66,747
  • 15
  • 75
  • 101
Jake L.
  • 11
  • 1
  • 2
    You don't need to add the event listener everytime you go through the loop. Do it on the $(document).ready and you'll be fine. – ericjbasti Jan 09 '14 at 00:24
  • It's called **add**EventListener for a reason - it doesn't remove previously-added event listeners!! – Pointy Jan 09 '14 at 00:24
  • You don't need to wrap `gameLoop` into a `function`, just write `setTimeout(gameLoop, frameLength);` – Olaf Dietsche Jan 09 '14 at 00:32
  • Appreciate all the advice—really helped clarify what went wrong. Also, thanks for pointing that out, Olaf. – Jake L. Jan 09 '14 at 00:50

1 Answers1

1

You are adding the event several times. Just move this line to global scope:

window.addEventListener('keydown', function(event){switch(event.keyCode){case...

If not the key will be detected several times and therefor accumulate

You can also throw in a preventDefault in the handler while you're at it:

window.addEventListener('keydown', function(event){
    if (event.preventDefault) event.preventDefault();
    switch(event.keyCode){case ...
  • Much appreciated! To further understand what went wrong, how did the function from the EventListener run without a keydown trigger? What I assume happened is the EventListeners stacked each time. And thus by the time one would press the up or down arrow, you were actually running that function, say, 50 times... ? – Jake L. Jan 09 '14 at 00:47
  • @JakeL. it will produce a bunch of listeners which the event need to be distributed to. As JS is single-threaded the processing of this event chain will eventually consume most of the time available. If you have 50 listeners and each listener increments with one the result would be 50 increments for each key press. Next time you might have 150 listeners and so forth. –  Jan 09 '14 at 02:50