Java Key Bindings are firing very inconsistently

Question

I am working on a game that has a player of which I would like to move using WASD. I have decided to use Key Bindings in order to try and fix a problem I had with Key Listeners which is still happening now even after switching to Key Bindings. The problem is that even though pressing these keys is moving the player, after moving the player a few times, the input begins to stop working almost completely. This happens to the point of only maybe 1/10 key presses moving the player. What could I be doing wrong? Any help would be much appreciated.

Here is my game's main class with the Key Bindings: (Let me know if I should post the rest of my code)

import java.awt.*;
import javax.swing.*;
import java.awt.event.*;
import java.awt.Color;  
import java.awt.Graphics;  
import javax.swing.JComponent;
import java.lang.Math;
import java.util.LinkedList;
import java.awt.event.KeyEvent;
import javax.swing.Action;
import javax.swing.ActionMap;
import javax.swing.InputMap;
import javax.swing.KeyStroke;

public class ZombieMain extends JPanel implements ActionListener{

   private static int WIDTH = 1600;
   private static int HEIGHT = 900;

   private Action a,s,w,d,ra,rs,rw,rd;

   private LinkedList<Zombie> zombies = new LinkedList();
   Zombie z = new Zombie(100,100,50,30,50);
   static ZombiePlayer player = new ZombiePlayer(950,572,2,30);

   public ZombieMain(){
      this.setFocusable(true);
      this.requestFocus();

      Timer t = new Timer(10,this);
      t.start();

      Action w = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vY = -player.speed;
         }
      };
      Action s = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vY = player.speed;
         }
      };
      Action d = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vX = player.speed;
         }
      };
      Action a = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vX = -player.speed;
         }
      };
      Action rw = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vY = 0;
         }
      };
      Action rs = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vY = 0;
         }
      };
      Action rd = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vX = 0;
         }
      };
      Action ra = new AbstractAction() {
         public void actionPerformed(ActionEvent e) {
            player.vX = 0;
         }
      };

     getInputMap().put(KeyStroke.getKeyStroke("W"),"w");
     getInputMap().put(KeyStroke.getKeyStroke("S"),"s");
     getInputMap().put(KeyStroke.getKeyStroke("D"),"d");
     getInputMap().put(KeyStroke.getKeyStroke("A"),"a");
     getInputMap().put(KeyStroke.getKeyStroke("released W"),"rw");
     getInputMap().put(KeyStroke.getKeyStroke("released S"),"rs");
     getInputMap().put(KeyStroke.getKeyStroke("released D"),"rd");
     getInputMap().put(KeyStroke.getKeyStroke("released A"),"ra");

     getActionMap().put("w",w);
     getActionMap().put("s",s);
     getActionMap().put("d",d);
     getActionMap().put("a",a);
     getActionMap().put("rw",rw);
     getActionMap().put("rs",rs);
     getActionMap().put("rd",rd);
     getActionMap().put("ra",ra);
   }

   public void actionPerformed(ActionEvent e){
      repaint();
   }

   public void paint(Graphics g){
      g.setColor(new Color(40,40,40));
      g.fillRect(0,0,WIDTH,HEIGHT);
      z.draw((Graphics)g);
      player.draw((Graphics)g);
   }

   public int getWidth(){
      return WIDTH;
   }
   public int getHeight(){
      return HEIGHT;
   }
   public static void main(String[] args){
        JFrame frame = new JFrame();
        frame.add(new ZombieMain());
        frame.setSize(WIDTH,HEIGHT);  
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);    
        frame.setVisible(true);
        frame.setResizable(false);
   }
}

Answer 1

As to "why" you're having issues, I can only guess at, as we've only got an out of context snippet to deal with, however, there are a number of things which could be improved which might help resolve the issue.

• User WHEN_IN_FOCUSED_WINDOW when calling getInputMap

This applies context to when key events will actually be triggered, in the above case, key events will be triggered when ever the window has focus, regardless of which component currently has keyboard focus.

• Prefer overriding paintComponent over paint

Painting in Swing is some what complex, as general recommendation, it is best to override paintComponent when you want to perform custom painting. A nice side effect of this is, it will paint the components background color for your, one less thing you have to do ;)

• Prefer overriding getPreferredSize when you need to provide sizing hints

Overriding getWidth and getHeight will cause no end of possible issues and is just best avoided. Instead, override getPreferredSize, this way, you are participating within the layout API and get all its advantages, such as been able to call pack on the JFrame and have it take care of all the oddities revolving around frame decorations.

• Seperate the change of the state from the mechanisms used to affect that state

Nice wordy for "decouple your code". In your code, the state of the player is changed directly by the key actions. This is not only a bad idea, it can produce unexpected side effects and becomes increasingly difficult to manage as the requirements become more complex. It also makes it difficult to change the way that input is done. For example, you could include different input methods, such as joysticks, but you'd have to write the same code for it.

Instead, you should simply have a "state manager" which carries the current state of the input, when your "main loop" is ready, it will apply that state to the model, as a seperate step.

The "main loop" doesn't care how the state is updated, only that it can obtain the information it needs to make decisions about how to apply it.

Below is a rough example of everything I've discussed above and in my testing I had no issues with the bindings "stalling"

import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Rectangle;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import javax.swing.AbstractAction;
import javax.swing.Action;
import javax.swing.ActionMap;
import javax.swing.InputMap;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.KeyStroke;
import javax.swing.Timer;

public class ZombieMain extends JPanel implements ActionListener {

    enum VerticalDirection {
        UP, DOWN, NONE
    }

    enum HorizontalDirection {
        LEFT, RIGHT, NONE
    }

    public class VerticalStateController {

        private VerticalDirection state = VerticalDirection.NONE;

        public void setState(VerticalDirection state) {
            this.state = state;
        }

        public VerticalDirection getState() {
            return state;
        }

    }

    public class HorizontalStateController {

        private HorizontalDirection state = HorizontalDirection.NONE;

        public void setState(HorizontalDirection state) {
            this.state = state;
        }

        public HorizontalDirection getState() {
            return state;
        }

    }

    public class VerticalAction extends AbstractAction {

        private VerticalStateController controller;
        private VerticalDirection state;

        public VerticalAction(VerticalStateController controller, VerticalDirection state) {
            this.controller = controller;
            this.state = state;
        }

        @Override
        public void actionPerformed(ActionEvent e) {
            controller.setState(state);
        }

    }

    public class HorizontalAction extends AbstractAction {

        private HorizontalStateController controller;
        private HorizontalDirection state;

        public HorizontalAction(HorizontalStateController controller, HorizontalDirection state) {
            this.controller = controller;
            this.state = state;
        }

        @Override
        public void actionPerformed(ActionEvent e) {
            controller.setState(state);
        }

    }

    private static int WIDTH = 400;
    private static int HEIGHT = 400;

    private Rectangle player = new Rectangle(0, 0, 20, 20);
    private VerticalStateController verticalStateController = new VerticalStateController();
    private HorizontalStateController horizontalStateController = new HorizontalStateController();

    public ZombieMain() {
        setBackground(new Color(40, 40, 40));

        Timer t = new Timer(10, this);
        t.start();

        InputMap im = getInputMap(WHEN_IN_FOCUSED_WINDOW);
        ActionMap am = getActionMap();

        // Pressed
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_W, 0, false), "Pressed.up");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_S, 0, false), "Pressed.down");
        // Released
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_W, 0, true), "Released.up");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_S, 0, true), "Released.down");

        // Pressed
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_A, 0, false), "Pressed.left");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_D, 0, false), "Pressed.right");
        // Released
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_A, 0, true), "Released.left");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_D, 0, true), "Released.right");

        am.put("Pressed.up", new VerticalAction(verticalStateController, VerticalDirection.UP));
        am.put("Pressed.down", new VerticalAction(verticalStateController, VerticalDirection.DOWN));
        am.put("Released.up", new VerticalAction(verticalStateController, VerticalDirection.NONE));
        am.put("Released.down", new VerticalAction(verticalStateController, VerticalDirection.NONE));

        am.put("Pressed.left", new HorizontalAction(horizontalStateController, HorizontalDirection.LEFT));
        am.put("Pressed.right", new HorizontalAction(horizontalStateController, HorizontalDirection.RIGHT));
        am.put("Released.left", new HorizontalAction(horizontalStateController, HorizontalDirection.NONE));
        am.put("Released.right", new HorizontalAction(horizontalStateController, HorizontalDirection.NONE));
    }

    @Override
    public Dimension getPreferredSize() {
        return new Dimension(WIDTH, HEIGHT);
    }

    public void actionPerformed(ActionEvent e) {
        switch (verticalStateController.getState()) {
            case UP:
                player.y -= 4;
                break;
            case DOWN:
                player.y += 4;
                break;
        }
        switch (horizontalStateController.getState()) {
            case LEFT:
                player.x -= 4;
                break;
            case RIGHT:
                player.x += 4;
                break;
        }
        repaint();
    }

    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        Graphics2D g2d = (Graphics2D) g.create();
        g2d.setColor(Color.RED);
        g2d.fill(player);
        g2d.dispose();
    }

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                JFrame frame = new JFrame();
                frame.add(new ZombieMain());
                frame.pack();
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setVisible(true);
            }
        });
    }
}

I should point out that this is just one approach. Could also put a bunch of "flags" in a Set of some kind