0

I am trying to make a simple game, and i need my spaceship to rotate on its centre (using A and D keys) , and move relative to the direction it is facing (W and S Keys).

I got the math for the movement and rotation mostly figured out, but im having an issue with the key listeners.

When i compile, the key listener works fine for th e first couple of key presses. But if i hold down A (rotate left while being pressed down) and then switch to D and then switch to A again, it stops working. Any other combination of key presses also results in this.

Basically, if i press the keys too much eventually it will stop working. If i go slow, it will start to work again after i press the key i want to work a few times. BUT if i press a bunch of keys at once (hit A, then D, then W in quick succession), then it completely stops working, and all keys become unresponsive.

This is not a processing speed issue i think, as the program does not crash, or produce any memory errors.

I have read up on similar issues, and saw that people suggested to not use KeyListeners, and use KeyBindings instead, but i tried both and got the exact same issues.

Here is the UPDATED code for my program (tried to make an MCVE): The portion i commented out in the middle was the code for the keybinding.

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.awt.geom.AffineTransform;
import java.io.IOException;
import javax.imageio.ImageIO;
import javax.swing.*;
import java.math.*;
import java.net.URL;

public class Game extends JPanel implements ActionListener{

private Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
JFrame frame = new JFrame();
private Ship ship = new Ship();
private Timer gameTimer, turnRight, turnLeft, moveForward, moveBackward;
private static final int IFW = JComponent.WHEN_IN_FOCUSED_WINDOW;

public Game(){
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setTitle("Study Buddy Menu");
    frame.setSize(800,700);
    frame.setLocation(dim.width/2 - 400, dim.height/2 - 350);

    turnLeft = new Timer(20, new ActionListener(){
        public void actionPerformed(ActionEvent e) {
            ship.setAngle(ship.getAngle() + ship.getTurnSpeed()); 
        }
    });
    turnRight = new Timer(20, new ActionListener(){
        public void actionPerformed(ActionEvent e) {
            ship.setAngle(ship.getAngle() - ship.getTurnSpeed()); 
        }
    });
    moveForward = new Timer(20, new ActionListener(){
        public void actionPerformed(ActionEvent e) {
            ship.setX(ship.getX() +  (float)Math.cos(Math.toRadians(ship.getAngle())));
            ship.setY(ship.getY() -  (float)Math.sin(Math.toRadians(ship.getAngle())));
            System.out.println("UP");
        }
    });
    moveBackward = new Timer(20, new ActionListener(){
        public void actionPerformed(ActionEvent e) {
            ship.setX(ship.getX() -  (float)Math.cos(Math.toRadians(ship.getAngle())));
            ship.setY(ship.getY() +  (float)Math.sin(Math.toRadians(ship.getAngle())));
            System.out.println("DOWN");
        }
    });
    this.setFocusable(true);
    this.requestFocus();
    /*getInputMap(IFW).put(KeyStroke.getKeyStroke('d'), "right");
    getActionMap().put("right", new MoveAction("d"));
    getInputMap(IFW).put(KeyStroke.getKeyStroke('a'), "left");
    getActionMap().put("left", new MoveAction("a"));
    getInputMap(IFW).put(KeyStroke.getKeyStroke('w'), "up");
    getActionMap().put("up", new MoveAction("w"));
    getInputMap(IFW).put(KeyStroke.getKeyStroke('s'), "down");
    getActionMap().put("down", new MoveAction("s"));*/

    this.addKeyListener(new KeyAdapter(){
        public void keyPressed(KeyEvent e){
            //TURN
            if(e.getKeyCode() == KeyEvent.VK_D){
                turnLeft.start();   
                turnRight.stop();
            }
            if(e.getKeyCode() == KeyEvent.VK_A){ 
                turnRight.start();
                turnLeft.stop();
                }
            //MOVE
            if(e.getKeyCode() == KeyEvent.VK_W){
                moveForward.start();
                moveBackward.stop();
            }
            if(e.getKeyCode() == KeyEvent.VK_S){
                moveBackward.start();
                moveForward.stop();
            }

        }
        public void keyReleased(KeyEvent e){
            turnRight.stop();
            turnLeft.stop();
            moveForward.stop();
            moveBackward.stop();
        }
    });
    frame.add(this);
    repaint();
    gameTimer = new Timer(20, this);
    gameTimer.start();
    frame.setVisible(true);
}

public void paintComponent(Graphics g){
    super.paintComponent(g);
    ship.draw(g);
}
public void actionPerformed(ActionEvent e) {
    repaint();

}
private class MoveAction extends AbstractAction {
    String d = null;
    MoveAction(String direction) {
        d = direction;
    } 
    public void actionPerformed(ActionEvent e) {
        switch (d){
            case "d": turnLeft.start(); turnRight.stop();break;
            case "a": turnRight.start(); turnLeft.stop();break;
            case "w": moveForward.start(); moveBackward.stop();break;
            case "s": moveBackward.start(); moveForward.stop();break;
        }

    }

}
class main {

public void main(String[] args) {
    Game game = new Game();
}

}

class Ship {
private float x, y, radius, speed, angle, turnSpeed;
private Image icon;
public Ship(){
    x = 400;
    y = 350;
    speed = 1;
    radius = 20;
    angle = 90;
    turnSpeed = 5;
    try {
        icon = ImageIO.read(new URL("https://i.stack.imgur.com/L5DGx.png"));
    } catch (IOException e) {
        e.printStackTrace();
    }
}


//GETTERS
public float getX(){
    return x;
}
public float getY(){
    return y;
}
public float getTurnSpeed(){
    return turnSpeed;
}
public float getAngle(){
    return angle;
}
public float getRadius(){
    return radius;
}
public float getSpeed(){
    return speed;
}
public Image getIcon(){
    return icon;
}
//SETTERS
public void setX(float X){
    x = X;
}
public void setTurnSpeed(float S){
    turnSpeed = S;
}
public void setY(float Y){
    y = Y;
}
public void setAngle(float A){
    angle = A;
}
public void setSpeed(float S){
    speed = S;
}
public void setRadius(float R){
    radius = R;
}
public void setIcon(Image image){
    icon = image;
}
//DRAW
public void draw(Graphics g){
    Graphics2D g2d = (Graphics2D) g;
    AffineTransform at = new AffineTransform();
    at.setToRotation(Math.toRadians(angle-90), x, y);
    //at.translate(x, y);
    g2d.setTransform(at);
    g2d.drawImage(icon,(int)(x - radius), (int)(y - radius),(int)(radius * 2),(int)(radius * 2), null);
    g2d.dispose();
}
}
}
  • 1
    `and use KeyBindings instead` - yes that is the solution. `Here is the portion of relevant code` - Actually its not. The code you posted doesn't help, since we don't know the context of how that code is used. Until a problem is solved you don't know what is or isn't relevant. All questions on the forum should include a proper [mcve] to demonstrate the problem. See [Motion Using the Keyboard](https://tips4java.wordpress.com/2013/06/09/motion-using-the-keyboard/) for working examples. The `KeyboardAnimation.java` is the most complete example. – camickr Apr 29 '17 at 05:13
  • Also, not sure what the point of doing a start/stop with the Timer is for. If you only want to execute some code once, then just invoke the code, there is no need for a Timer. By using the Timer the reaction to the event will be delayed by the Timer delay. My guess is that sometimes the Timer stops before it has a chance to fire the event. – camickr Apr 29 '17 at 05:18
  • @camickr Sorry, i thought that portion was all that was needed, the rest of my code is just setting up the GUI, and my "ship" class, which simply draws the ship image to the Panel. Also, i used a timer because i wanted the user to be able to hold down the direction keys, and have the ship move smoothly, as if to glide across the screen. Without a timer, if i held down a direction key, it would evoke the code once and only once, basically moving or rateting the ship by like 5 pixels, and then stopping. So i would have to rapidly tap the direction keys for it to move, hence the timer. – Ramin Kaviani Apr 29 '17 at 05:27
  • 1
    `Without a timer, if i held down a direction key, it would evoke the code once and only once` - then you have a problem with your code. When you hold down a key the KeyEvent is generated repeatedly. Starting/stopping the Timer makes no sense. The only reason it works is because the KeyEvent is continually generated so you continually start/stop the Timer. Instead, the proper solution when you use a Timer is to start the Timer on keyPressed and stop the Timer on keyReleased. Look at the working examples! – camickr Apr 29 '17 at 05:31
  • *"i thought that portion was all that was needed"* There's no accounting for what people think. An MCVE gives us the code we need to run the problem, and the motivation to do so & look at the problem more closely. Given there are two close reasons that mention 'no MCVE', I'll be selecting one of them now. – Andrew Thompson Apr 29 '17 at 05:38
  • @AndrewThompson Once again, sorry about that, ill work on making an MCVE to the best of my abilities. – Ramin Kaviani Apr 29 '17 at 05:44
  • 1
    If you're using images, it's helpful for an MCVE to substitute them with images from a `java.net.URL`, for example the ones [here](http://stackoverflow.com/a/19209651/2891664). If you don't know how to do that, I have an example of it [here](http://stackoverflow.com/a/30175751/2891664) if you scroll to the bottom of the code example to the `class Resources`. It's just `ImageIO.read(new URL("http://example.com/example.png"))`. – Radiodef Apr 29 '17 at 05:58
  • @Radiodef Thanks for the heads up, i made the change, (i hope its done right) – Ramin Kaviani Apr 29 '17 at 06:07
  • 1
    The edited code includes no `main(..)` method to put the app. on screen, so is not an MCVE. Drop a main method into the `Game` class, demote the `public class Ship {` to simply `class Ship {` and paste it into the end of the `Game` class then **check it is an MCVE** by pasting the lot into a new project, compiling and running it to see the problem. Good work on hot linking the image as suggested by @Radiodef. – Andrew Thompson Apr 29 '17 at 06:12
  • 1
    @AndrewThompson whoops, this is all new to me, ill add those changes in, thanks – Ramin Kaviani Apr 29 '17 at 06:19

1 Answers1

2

I'm just about to go to bed so I can't look too deeply in to your code, but a well-behaved method for doing movement is something like this:

  • Your KeyListener/key binding/whatever sets some variables like isTurningLeft, isTurningRight, isMovingForwards, isMovingBackwards.
  • Then you have just one timer that checks the control state on every iteration and computes the movement based on all of it at the same time.

Keeping track of every button simultaneously is important so that you have all the information. So for example, you could do something like this:

double turnRate = 0;
if (isTurningLeft) {
    turnRate -= maxTurnRate;
}
if (isTurningRight) {
    turnRate += maxTurnRate;
}
rotationAngle += timeElapsed * turnRate;

This way if both buttons are held at the same time, the turn rate is just 0. That will avoid a lot of weird sorts of behaviors, like sticking and stuttering. However, to do that you need to know the state of both buttons in one place.


edit

I modified your program to serve as an example:

import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.io.IOException;
import javax.imageio.ImageIO;
import javax.swing.*;
import java.net.URL;

public class KeyListenerGame extends JPanel implements ActionListener{
    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                new KeyListenerGame();
            }
        });
    }

//    private static final int IFW = JComponent.WHEN_IN_FOCUSED_WINDOW;
//    private Dimension   dim     = Toolkit.getDefaultToolkit().getScreenSize();
    private JFrame frame;
    private Ship   ship;
    private Timer  gameTimer;//, turnRight, turnLeft, moveForward, moveBackward;
    private KeyBindingController controller;

    // This is just an example. I recommend using
    // the key bindings instead, even though they
    // are a little more complicated to set up.
    class KeyListenerController extends KeyAdapter {
        boolean isTurningLeft;
        boolean isTurningRight;
        boolean isMovingForward;
        boolean isMovingBackward;

        @Override
        public void keyPressed(KeyEvent e) {
            switch (e.getKeyCode()) {
                case KeyEvent.VK_A:
                    isTurningLeft = true;
                    break;
                case KeyEvent.VK_D:
                    isTurningRight = true;
                    break;
                case KeyEvent.VK_W:
                    isMovingForward = true;
                    break;
                case KeyEvent.VK_S:
                    isMovingBackward = true;
                    break;
            }
        }
        @Override
        public void keyReleased(KeyEvent e) {
            switch (e.getKeyCode()) {
                case KeyEvent.VK_A:
                    isTurningLeft = false;
                    break;
                case KeyEvent.VK_D:
                    isTurningRight = false;
                    break;
                case KeyEvent.VK_W:
                    isMovingForward = false;
                    break;
                case KeyEvent.VK_S:
                    isMovingBackward = false;
                    break;
            }
        }
    }

    // This could be simplified a lot with Java 8 features.
    class KeyBindingController {
        boolean isTurningLeft;
        boolean isTurningRight;
        boolean isMovingForward;
        boolean isMovingBackward;

        JComponent component;

        KeyBindingController(JComponent component) {
            this.component = component;
            // Bind key pressed Actions.
            bind(KeyEvent.VK_A, true, new AbstractAction("turnLeft.pressed") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isTurningLeft = true;
                }
            });
            bind(KeyEvent.VK_D, true, new AbstractAction("turnRight.pressed") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isTurningRight = true;
                }
            });
            bind(KeyEvent.VK_W, true, new AbstractAction("moveForward.pressed") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isMovingForward = true;
                }
            });
            bind(KeyEvent.VK_S, true, new AbstractAction("moveBackward.pressed") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isMovingBackward = true;
                }
            });
            // Bind key released Actions.
            bind(KeyEvent.VK_A, false, new AbstractAction("turnLeft.released") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isTurningLeft = false;
                }
            });
            bind(KeyEvent.VK_D, false, new AbstractAction("turnRight.released") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isTurningRight = false;
                }
            });
            bind(KeyEvent.VK_W, false, new AbstractAction("moveForward.released") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isMovingForward = false;
                }
            });
            bind(KeyEvent.VK_S, false, new AbstractAction("moveBackward.released") {
                @Override
                public void actionPerformed(ActionEvent e) {
                    isMovingBackward = false;
                }
            });
        }

        void bind(int keyCode, boolean onKeyPress, Action action) {
            KeyStroke keyStroke = KeyStroke.getKeyStroke(keyCode, 0, !onKeyPress);
            String   actionName = (String) action.getValue(Action.NAME);
            component.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW)
                .put(keyStroke, actionName);
            component.getActionMap()
                .put(actionName, action);
        }
    }

    public KeyListenerGame(){
        frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setTitle("Study Buddy Menu");

        frame.add(this);
        frame.pack();
        frame.setLocationRelativeTo(null);

        gameTimer  = new Timer(20, this);
        controller = new KeyBindingController(this);
        ship       = new Ship(getSize());

        gameTimer.start();
        frame.setVisible(true);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        double secsElapsed = gameTimer.getDelay() / 1000.0;

        double maxSpeed     = ship.getSpeed();
        double maxTurnSpeed = ship.getTurnSpeed();
        double theta = Math.toRadians( ship.getAngle() );
        double x     = ship.getX();
        double y     = ship.getY();

        double turnSpeed = 0;
        if (controller.isTurningLeft) {
            turnSpeed -= maxTurnSpeed;
        }
        if (controller.isTurningRight) {
            turnSpeed += maxTurnSpeed;
        }

        theta += secsElapsed * Math.toRadians(turnSpeed);

        double speed = 0;
        if (controller.isMovingForward) {
            speed += maxSpeed;
        }
        if (controller.isMovingBackward) {
            speed -= maxSpeed;
        }

        double velX = speed * Math.cos(theta);
        double velY = speed * Math.sin(theta);

        x += secsElapsed * velX;
        y += secsElapsed * velY;

        ship.setX( (float) x );
        ship.setY( (float) y);
        ship.setAngle( (float) Math.toDegrees(theta) );

        repaint();
    }

    @Override
    public Dimension getPreferredSize() {
        Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize();
        // Computes a preferred size based on the
        // dimensions of the screen size.
        int prefWidth  = screenSize.width / 2;
        // Compute 4:3 aspect ratio.
        int prefHeight = prefWidth * 3 / 4;
        return new Dimension(prefWidth, prefHeight);
    }

    @Override
    protected void paintComponent(Graphics g){
        super.paintComponent(g);
        ship.draw(g);
    }

    class Ship {
        private float x, y, radius, speed, angle, turnSpeed;
        private Image icon;
        public Ship(Dimension gameSize) {
//            x = 400;
//            y = 350;
//            speed = 1;
//            radius = 20;
//            angle = 90;
//            turnSpeed = 5;

            x = gameSize.width  / 2;
            y = gameSize.height / 2;
            radius = 20;
            angle  = -90;
            // 1/4 of the game height per second
            speed  = gameSize.height / 4;
            // 180 degrees per second
            turnSpeed = 180;

            try {
                icon = ImageIO.read(new URL("https://i.stack.imgur.com/L5DGx.png"));
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        //GETTERS
        public float getX(){
            return x;
        }
        public float getY(){
            return y;
        }
        public float getTurnSpeed(){
            return turnSpeed;
        }
        public float getAngle(){
            return angle;
        }
        public float getRadius(){
            return radius;
        }
        public float getSpeed(){
            return speed;
        }
        public Image getIcon(){
            return icon;
        }
        //SETTERS
        public void setX(float X){
            x = X;
        }
        public void setTurnSpeed(float S){
            turnSpeed = S;
        }
        public void setY(float Y){
            y = Y;
        }
        public void setAngle(float A){
            angle = A;
        }
        public void setSpeed(float S){
            speed = S;
        }
        public void setRadius(float R){
            radius = R;
        }
        public void setIcon(Image image){
            icon = image;
        }
        //DRAW
        public void draw(Graphics g){
            Graphics2D g2d = (Graphics2D) g.create();
            //
            // Draw the ship's movement vector.
            double theta = Math.toRadians(angle);
            double velX  = speed * Math.cos(theta);
            double velY  = speed * Math.sin(theta);
            g2d.setColor(java.awt.Color.blue);
            g2d.draw(new java.awt.geom.Line2D.Double(x, y, x + velX, y + velY));
            //
            g2d.rotate(theta, x, y);
            int imgX = (int) ( x - radius );
            int imgY = (int) ( y - radius );
            int imgW = (int) ( 2 * radius );
            int imgH = (int) ( 2 * radius );
            g2d.drawImage(icon, imgX, imgY, imgW, imgH, null);
            //
            g2d.dispose();
        }
    }
}

In addition to the controls, I changed some other things:

  • Always begin a Swing program with a call to SwingUtilities.invokeLater(...). This is because Swing runs on a different thread than the one which main gets run on. https://docs.oracle.com/javase/tutorial/uiswing/concurrency/initial.html
  • Instead of setting the size of a JFrame directly, I overrode getPreferredSize and computed an initial size based on the screen dimensions, then called pack() on the JFrame. This is much more well-behaved than trying to set pixel sizes statically. Also, the size of a JFrame includes its insets (for example the title bar), so if you call e.g. jFrame.setSize(800,600) the content pane which displays the game actually ends up being a little smaller than 800x600.
  • I added a line which displays the ship's movement vector, just to visualize what's going on. This sort of thing is nice for debugging.
  • In ship.draw I create a copy of g with g.create(), because g is the graphics object which Swing personally uses. Doing things like disposing or setting a transform on the Graphics passed in isn't a good idea.
  • I fixed the rotation code a little. Because the y-axis in AWT coordinates is upside-down, positive angles actually rotate clockwise and negative angles rotate counter-clockwise.

There are ways to make the key binding code nicer, but I just did it the most obvious way so you can clearly see what I was actually doing. You have to set up bindings both for pressed and released events. You'll notice, though, that there is a pattern to it, so you could program a small class to perform the generic boolean logic instead of copy-and-pasting like I did.

Java 8 lambdas would also be very nice, but my computer with Java 8 is broken. With Java 8 you could end up with something like this, though:

bind(KeyEvent.VK_A, "moveLeft", b -> isMovingLeft = b);

And:

void bind(int keyCode, String name, Consumer<Boolean> c) {
    Action pressAction = new AbstractAction(name + ".pressed") {
        @Override
        public void actionPerformed(ActionEvent e) {
            c.accept(true);
        }
    };
    Action releaseAction = new AbstractAction(name + ".released") {
        @Override
        public void actionPerformed(ActionEvent e) {
            c.accept(false);
        }
    };
    // Then bind it to the InputMap/ActionMap like I did
    // in the MCVE.
}
Radiodef
  • 37,180
  • 14
  • 90
  • 125
  • [for example](http://stackoverflow.com/a/7940227/714968) but only +1:-) – mKorbel Apr 29 '17 at 13:19
  • @Radiodef i tried doing what you suggested, and initially my problem was that i had no way of stopping the ship from turning, as in 'isTurningLeft' would always be true after pressing 'd'. so i made it so that pressing any key makes the other 3 directions false, but that didnt seem to work either. Then in a keyReleased i set all 4 directions to false. This worked, except that like before, it only worked SOMETIMES, and if i pressed lots of keys at once the ship just froze up and wouldn't move anymore, and the key listener become unresponsive. – Ramin Kaviani Apr 29 '17 at 15:37
  • Here is a debug i ran to test when different things were responsive: https://hastebin.com/rimikefava.css as you can see, after a few key presses only the Key Released is functional. Both KeyBinders and KeyListeners stop responding. – Ramin Kaviani Apr 29 '17 at 16:01
  • Thank you for taking the time to write all this, i really appreciate the extra edits, i learned a lot. I tried your MCVE, and it ran very smoothly, however, i still ran into the issue where after a bit of moving and gliding around, everything freezes up and the game no longer accepts key inputs. It lasts much longer than mine though haha, i wonder if this has to do with my computer or compiler rather than the code? – Ramin Kaviani Apr 30 '17 at 02:15
  • I don't know, unfortunately. In the original code in the question I don't think you were doing anything that would degrade performance that badly. ([Here's a recent example](http://stackoverflow.com/a/43566748/2891664) of stuff not to do.) You could try using a profiler to see if the program is doing something unusual. The mundane reasons a program could be slow are because it's doing stuff like allocating huge amounts of memory or doing IO too frequently, like in the link. – Radiodef Apr 30 '17 at 02:35
  • Hmm, thats weird. Could it be an issue with KeyBinders then? or the way in which im trying to do everything as a whole? maybe im taking the wrong approach altogether? – Ramin Kaviani Apr 30 '17 at 05:08
  • The code in my answer should be perfectly fine, so if you're running it as-is and you're stilling running in to a performance issue, then it could be your computer or some issue with your Java version. If the code you're running is different then I can't really comment on it without seeing it. I think the next thing I would do is set up a profiler and see if there's a memory leak in the Java SE platform code or something. It's probably unlikely, but it's possible. – Radiodef Apr 30 '17 at 06:02