Cjmarkham
Cjmarkham

Reputation: 9681

Key Event happening twice when pressed with another key

In my game, I use the event listeners keyup and keydown to track which keys are pressed (this.input is a reference to the Input function below):

$(document.body).on('keydown', this.input.keyPress.bind(this.input));
$(document.body).on('keyup', this.input.keyPress.bind(this.input));

Everything works as expected, until I press 2 keys at once. The left key moves the player left, the shift key fires a weapon. Separately, they work fine. If both are pressed at once, however, and held, the player shoots twice.

Here is the code I am using to handle the inputs (removed other key events for brevity)

shootSpecial should only be called once since keyMap.shift is only true on keydown. Once keyup is triggered, it is set to false: keyMap[key] = e.type === 'keydown';

var Input = function () {
  var keyMap = {};

  this.handleKeyPress = function () {
    if (keyMap.arrowleft === true) {
      game.player.moving.left = true;
    } else if (keyMap.shift === true) {
      game.player.shootSpecial();
    }
  };

  this.keyPress = function (e) {
    var key = e.key.toLowerCase();
    if (key === ' ') {
      key = 'space';
    }
    keyMap[key] = e.type === 'keydown';
    console.log(key, keyMap[key]);
    this.handleKeyPress();
  };
};

The console.log reports that left and shift are true on keydown and then both false on keyup so I'm not too sure why it would be triggering the shootSpecial event twice.

var keyMap = {};
var handleKeyPress = function () {
  if (keyMap.arrowleft === true) {
    console.log('left');
  } else if (keyMap.shift === true) {
    console.log('shift');  
  }
};

var keyPress = function (e) {
  var key = e.key.toLowerCase();
  keyMap[key] = e.type === 'keydown';
  if (document.getElementById(key))
    document.getElementById(key).innerText = e.type === 'keydown';
  
  handleKeyPress();
}

document.addEventListener('keydown', keyPress);
document.addEventListener('keyup', keyPress);
LEFT:<div id="arrowleft">false</div>
SHIFT:<div id="shift">false</div>

As you can see by the fiddle, one event gets called twice.

Upvotes: 1

Views: 2823

Answers (2)

AmericanUmlaut
AmericanUmlaut

Reputation: 2837

Your problem is that you're handling keydown and keyup with the same code, with the result that releasing key1 while pressing key2 has the same effect as pressing key2.

The fix is pretty easy: don't call your handleKeyPress function at all on keyup events.

var keyMap = {};
var handleKeyPress = function () {
  if (keyMap.arrowleft === true) {
    console.log('left');
  } else if (keyMap.shift === true) {
    console.log('shift');  
  }
};

var keyPress = function (e) {
  var key = e.key.toLowerCase();
  keyMap[key] = e.type === 'keydown';
  if (document.getElementById(key))
    document.getElementById(key).innerText = e.type === 'keydown';
  
  // Only call handleKeyPress if the key is pressed
  if (keyMap[key]) {
    handleKeyPress();
  }
}

document.addEventListener('keydown', keyPress);
document.addEventListener('keyup', keyPress);
LEFT:<div id="arrowleft">false</div>
SHIFT:<div id="shift">false</div>


Adding a suggestion in response to OP's comment. Your problem is that your handleKeyPress() has no information about whether the key was pressed down or up, and it also doesn't know which key was even pressed, only which keys are down at the time it is called. To get what you're aiming for, I would do something like this:

var keyPress = function (e) {
  var key = e.key.toLowerCase(),
      isKeyDown = e.type === "keydown",
      output = key + (isKeyDown ? "-down" : "-up") + ": ";
  
  switch (key) {
    case "shift":
      if (isKeyDown) {
        output += "pew pew";
      } else {
        output += "doing nothing"
      }
      break;
      
    case "arrowleft":
      output += "updating walking_left";
      document.getElementById("walking_left").innerText = isKeyDown;
      break;
      
    default:
      output += "doing nothing";
  }

  console.log(output);
};

document.addEventListener('keydown', keyPress);
document.addEventListener('keyup', keyPress);
Walking left:<div id="walking_left">false</div>

Upvotes: 1

trincot
trincot

Reputation: 350147

If you press two keys, you'll fire two events, whether you press them at the same time or not. The same for the keyup event. So in total the handleKeypress function is called four times.

The order might well happen to be:

key        | event   | keyMap.arrowleft | keyMap.shift | effect
-----------+---------+------------------+--------------+----------
shift      | keydown |     false        |     true     | shoot
arrowleft  | keydown |     true         |     true     | moving.left=true
arrowleft  | keyup   |     false        |     true     | shoot
shift      | keyup   |     false        |     false    | nothing

So in that scenario you shoot twice. Of course, this will not always be the case, as the order might be different.

Solution

Pass the key as argument to the handleKeyPress function to make sure you only execute an action that corresponds to that key.

var Input = function () {
  var keyMap = {};

  this.handleKeyPress = function (key) {
    switch (key) {
    case 'arrowleft':
      game.player.moving.left = keyMap[key]; // also sets back to false
      break;
    case 'shift':
      if (keyMap.shift) game.player.shootSpecial();
      break;
    }
  };

  this.keyPress = function (e) {
    var key = e.key.toLowerCase();
    if (key === ' ') {
      key = 'space';
    }
    keyMap[key] = e.type === 'keydown';
    this.handleKeyPress(key); // pass key
  };
};

Upvotes: 0

Related Questions