handle multiple key presses ignoring repeated key

Question

I had asked this in the comments section of another question (> How do I handle simultaneous key presses in Java?), and was asked to make a new question altogether.

My problem is that when I create an ArrayList of keypresses they are not removed fast enough via the keyReleased event if the user holds down the keys. I want movement to be with "asdf" and North, East, South, West, NorthEast... etc.

Here is my code for both events:

@Override
public void keyPressed(KeyEvent e) {
    if(chatTextField.isFocusOwner() == true){
        //do nothing - don't walk
    } else {
        logger.debug("Key Pressed: " + e.getKeyChar());
        lastKey = keysPressed.get(keysPressed.size()-1);

        for (String key : keysPressed){
            if (!key.contains(String.valueOf(e.getKeyChar())) && !lastKey.contains(String.valueOf(e.getKeyChar()))){
                keysPressed.add(String.valueOf(e.getKeyChar()));
                System.out.println("ADDED: " + keysPressed);
            }
        }

        String keysList = keysPressed.toString();
        if (keysList.contains("w")){
            if (keysList.contains("d")){
                requestCharacterMove("NorthEast");
            } else if(keysList.contains("a")){
                requestCharacterMove("NorthWest");
            } else{
                requestCharacterMove("North");
            }
        } else if (keysList.contains("s")){
            if (keysList.contains("d")){
                requestCharacterMove("SouthEast");
            } else if(keysList.contains("a")){
                requestCharacterMove("SouthWest");
            } else{
                requestCharacterMove("South");
            }
        } else if (keysList.contains("d")){
            requestCharacterMove("East");
        } else if (keysList.contains("a")){
            requestCharacterMove("West");
        }
    }
}

@Override
public void keyReleased(KeyEvent e) {
    if(chatTextField.isFocusOwner() == true){
        //do nothing - don't walk
    } else {
        logger.debug("Key Released: " + e.getKeyChar());
        for (String key : keysPressed){
            if (key.contains(String.valueOf(e.getKeyChar()))){
                keysPressed.remove(String.valueOf(e.getKeyChar()));
                System.out.println("REMOVED: " + keysPressed);
            }
        }
    }
}

@Override
public void keyTyped(KeyEvent arg0) {
    // TODO Auto-generated method stub

}

Until I added the second check in there via the lastKey(String) variable the pyramid created was enormous. Even with that second check the list grows and almost always has two-three duplicates. Any help on this would be great as my character is moving awkwardly. :(

Also any way to remove duplicate conversions to char, string, arrayList would be great as I'm nervous I used too many types for something "simple".

Reactor

for better help sooner post an SSCCE – Judsen

Answer 1

Your obseravtion that things are handled slowly most likely is caused solely be the many System.out.println() statements.

Your problem that you do not get diagonal movement stems from your somewhat faulty checking logic - instead of explicitly checking if (for example) keys A and B are pressed, just check them independently - key A moves the character in one direction, B in another. In total (e.g.), by moving WEST and NORTH you will have effectively moved NORTHWEST.

Instead of a list of pressed keys, you could use a java.util.BitSet and just set the bit for each key that is currently pressed. That should also drastically reduce the amount of code you need to write (keyPressed just sets the bit indicated by key code, keyReleased clears it). To check if a key is pressed you ask the BitSet then if the bit for the code is currently set.

EDIT: Example of using BitSet instead of a list

public class BitKeys implements KeyListener {

    private BitSet keyBits = new BitSet(256);

    @Override
    public void keyPressed(final KeyEvent event) {
        int keyCode = event.getKeyCode();
        keyBits.set(keyCode);
    }

    @Override
    public void keyReleased(final KeyEvent event) {
        int keyCode = event.getKeyCode();
        keyBits.clear(keyCode);
    }

    @Override
    public void keyTyped(final KeyEvent event) {
        // don't care
    }

    public boolean isKeyPressed(final int keyCode) {
        return keyBits.get(keyCode);
    }

}

I made the example implement KeyListener, so you could even use it as is. When you need to know if a key is pressed just use isKeyPressed(). You need to decide if you prefer with raw key code (like I did) or go with key character (like you currently do). In any case, you see how using the BitSet class the amount of code for recording the keys reduces to a few lines :)

Flied

thanks for the help, I am at work now so I admit I haven't explored your solution(java.util.BitSet) much yet, but is there some example you could link me to? Or some sort of psuedo/real example? That would be awesomely appreciated. :D – Reactor

It should be pretty straight forward to the point where I'd say if it gives you trouble to implement you are in trouble in many other regards, but here you go. – Flied

I know it took me forever to get back to this, but have been busy with work. It works nicer now thanks for the help. I am wondering why the BitSet seems to make things quicker. Is it more efficient than a list? This is without printlns on either way(list vs BitSet). – Reactor

It says here that it uses an array, so I assume the BitSet has less overhead than an ArrayList – Andrey

Upon closer examination, I have realized that an array of what are essentially booleans would be faster than adding and removing `KeyEvent's to/from a list. – Andrey

Answer 2

As an alternative, this game uses the numeric keypad to implement each (semi-) cardinal direction with a single keystroke. The default arrangement is shown in the Design section. The keys may be individually reassigned to map a similar rosette anywhere on the keyboard.

Angeliaangelic

I appreciate your alternative however, I am not interested in that functionality. Thanks. – Reactor

Answer 3

Looks like you are not handling threading in Java right. There are three threads (minimum) to any Java program. They are the main program thread, the event dispatch thread, and one more that i can't remember right now.

Whenever you get an event it is delivered to you by a special thread (I believe it's the event dispatch thread, but that is besides the point). You are not allowed to do anything (that takes time) on this thread, that will freeze up your input and cause you to miss events, making Java look unresponsive. So what has happened is you have broke the event system in java. What you should do is store the result in some sort of buffer, which is the fasted thing you can be expected to do with the event, then it is handled later as I will describe.

[Aside: A funny application is to make a simple gui, and on the press of the button call wait on the thread for like 5 seconds. Your entire gui will freeze until the delay has finished!]

You should have a different thread running on the side (probably your main thread). It will run some sort of loop, which controls the frames in your program, completing once per game cycle. Once each cycle this thread reads the results stored in the input buffer and processes them. The theory behind this is simple, but the execution can be a little messy, because you will need to make sure that no input events are dropped or read more then once. Either way, good luck with your game!

Heliotropism