Function starts being repeated over and over again

I've been developing a simple system that is supposed to change between two different scenes when you press a button.

gameOne();

var game = 1;

function gameOne()
game = 1;
console.log("Game 1");
$( "body" ).keyup(function( event )
if ( event.which == 49 && game == 1) // Number 1 key
gameTwo();

);


function gameTwo()
game = 2;
console.log("Game 2");
$( "body" ).keyup(function( event )
if ( event.which == 49 && game == 2) // Number 1 key
gameOne();

);



Expected behaviour - I want it to say Game 1, after after pressing the 1 key and then Game 2 after pressing the 1 key again, and then repeat this as I press 1.

Actual behaviour - It does the expected behaviour a few times, and then it starts repeating 1 and 2 over and over again, to the point it lags the browser out.

JSFiddle: https://jsfiddle.net/a0npotm8/10/

I'm really sorry if this is a basic question or anything, I'm still fairly new to Javascript and JQuery and this is really confusing me currently.

All help is appreciated.
Thank you :)

4 Answers
4

The problem here is that you are rebinding the keyup event recuresively inside the keyup callback, so it ends up by breaking the browser.

keyup

keyup

What you need to do is to get the keyup binding code out of the two functions:

keyup

gameOne();

var game = 1;

$("body").keyup(function(event)
if (event.which == 49 && game == 1) // Number 1 key
gameTwo();
else if (event.which == 49 && game == 2) // Number 1 key
gameOne();

);


function gameOne()
game = 1;
console.log("Game 1");


function gameTwo()
game = 2;
console.log("Game 2");



what about something like:

let game = 1;
document.onkeyup = ev =>
if (ev.which === 49)
console.log(`Game $game`);
game = game === 1 ? 2 : 1;

;

will it solve your issue?

You can use a delegate event handler to control actions like this, so you do not have to juggle event bindings around.

var $container = $('#container').focus();

$(document.body)
.on('keyup', '#container.game1', function(e)
if (e.which == 49)
console.log('Game 1');
$container.removeClass('game1').addClass('game2');

)
.on('keyup', '#container.game2', function(e)
if (e.which == 49)
console.log('Game 2');
$container.removeClass('game2').addClass('game1');

);

#container
min-width: 100vw;
min-height: 100vh;
background-color: rgb(128, 128, 128);


<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="container" class="game1" tabindex="0">
</div>

This logic creates two different delegate event handlers for the body. Both filter out events for the #container child element of the body, but also filter based on an additional class on the container; game1 and game2. Depending on which class the element has, only one of the event handlers will process.

#container

Whenever you call keyup on an element, you attach another event handler. To catch events, you only need to call it once. The callback functions that handle the event will fire every time the event happens.

keyup

Thanks for contributing an answer to Stack Overflow!

But avoid …

To learn more, see our tips on writing great answers.

Required, but never shown

Required, but never shown

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.