Java last added button allows agent.move, rest don

Question

I'm new to java, and have been working on creating a code to get a small picture of a car to move using the keys. My problem is when I add more than 1 buttons to the panel. The function of the button in the code I post is nothing, just prints "button [i] clicked" message. The purpose is for them to read a file and update the locations of the car from the data in the file. This is supposed to be a piece on my Reinforcement Learning project. I thought it would be a good opportunity to learn java since this "graphics package" is not necessary for the project, just a "nice" addition. The code is here:

package graphics;

public class Board extends JPanel implements ActionListener {

private Timer timer;
private Agent agent;

private String button = "button.png";
private Image image;


protected JButton b1;
protected JButton b2;
protected JButton b3;

public Board() {

    //Keylistener added for the agent to respond to arrow keys

 addKeyListener(new TAdapter());
     agent = new Agent();
    timer = new Timer(10, this); //10ms timer calls action performed
    timer.start();     

            //This part for the button.
    ImageIcon i = new ImageIcon(this.getClass().getResource(button));
    image = i.getImage();

    b1 = new JButton("1", i);
    b1.setVerticalTextPosition(AbstractButton.CENTER);
    b1.setHorizontalTextPosition(AbstractButton.LEADING); 
    b1.setActionCommand("Active1");



    b2 = new JButton("2", i);
    b2.setVerticalTextPosition(AbstractButton.CENTER);
    b2.setHorizontalTextPosition(AbstractButton.LEADING); 
    b2.setActionCommand("Active2");



    b3 = new JButton("3", i);
    b3.setVerticalTextPosition(AbstractButton.CENTER);
    b3.setHorizontalTextPosition(AbstractButton.LEADING);
    b3.setActionCommand("Active3");



    b1.addActionListener(this); 
    b2.addActionListener(this);
    b3.addActionListener(this);

     add(b1);          add(b2);  add(b3);          

    setFocusable(true);
    setBackground(Color.BLACK);
    setDoubleBuffered(true);


  }


public void paint(Graphics g) {
    super.paint(g);

    Graphics2D g2d = (Graphics2D)g;

    //Transformations for the agent to be painted based upon its position and orientation

    AffineTransform trans = new AffineTransform();

    trans.rotate(Math.toRadians(agent.getTh()), agent.getX()+64, agent.getY()+64);  
    trans.translate(agent.getX(), agent.getY());

    g2d.drawImage(agent.getImage(), trans, this); // Draws agent with said transformations

    Toolkit.getDefaultToolkit().sync();
    g.dispose();
}


public void actionPerformed(ActionEvent ae) {

    b1.setEnabled(true);
    b2.setEnabled(true);
    b3.setEnabled(true);

    if (ae.getActionCommand()=="Active1") {
        b1.setEnabled(false);
        b2.setEnabled(true);
        b3.setEnabled(true);
       System.out.println("Clicked 1");
       agent.reset();
    } 



    if(ae.getActionCommand()=="Active2") {
        b1.setEnabled(true);
        b2.setEnabled(false);
        b3.setEnabled(true);
        System.out.println("Clicked 2");
        agent.reset();
    }

    if (ae.getActionCommand()=="Active3") {
        b1.setEnabled(true);
        b2.setEnabled(true);
        b3.setEnabled(false);
        System.out.println("Clicked 3");
        agent.reset();


    }

    agent.move();
    repaint();

}

private class TAdapter extends KeyAdapter {

    public void keyReleased(KeyEvent e) {
        agent.keyReleased(e);
    }

    public void keyPressed(KeyEvent e) {
        agent.keyPressed(e);
    }
}

}

Now, the problem is this. If I click button 1, or button 2, the other buttons are disabled and it says "Clicked 1 (or 2)" which is fine. But the agent.move() and repaint() aren't invoked. The car doesn't move when I press the keys. If I then click button 3, the other two buttons are disabled and the car moves with the keys.

If I add buttons in a different order add(b3); add(b2); add(b1); then the same happens but this time its button 1 that works fine.

Answer 1

It seems you need to use Threading. You can make your class to implements Runnable and then override the public void run(){} method and do the painting there.

Answer 2

Problems:

• Your main problem is one of focus -- When a JButton gets focus and the JPanel loses focus, the JPanel's KeyListener won't work, since the KeyListener requires that the listened to component have the focus, no exception.
• A bad solution is to force the JPanel to retain the focus at all times. This will be bad if your window has JButtons and will be disaster if you need to display JTextFields, JTextAreas, or other text components.
• A better solution is not to use a KeyListener as it has lots of problems with Swing applications and in particular it has focus issues as noted above. Use Key Bindings instead. Google the tutorial for the gory details on this.
• Don't use == to compare Strings, use equals(...) or equalsIgnoreCase(...) instead. The problem is that == checks for object equality, is String A the same object as String B, and you don't care about this. You want to know if the two Strings hold the same chars in the same order which is where the two methods come in.
• Don't dispose() of a Graphics object given you by the JVM as this may mess up the painting of a component's borders, children, and have even other side effects.
• don't draw in the JPanel's paint(...) method but rather in its paintComponent(...) method just as the tutorials tell you to do. Drawing in paint(...) can have side effects on a component's borders and children if you're not careful, and also does not have the benefit of default double buffering which is important for smooth animation. paintComponent(...) fixes all these problems.
• Speaking of which, you should Google and read the Swing graphics tutorials. You can't make stuff up and hope that it will work, and graphics programming will require a whole different approach from what you're used to.
• Ignore Andromeda's threading suggestion. While he means well, I suggest that you don't do painting in a background thread. Just move the car with your Swing Timer as you're doing. Background threading has its uses, but not here, as a Timer will work just fine. You will need to use a background thread when you have a long-running process that blocks the calling thread, something that you don't have in your current code, and so a thread "fix" is not needed and in fact is a potential source of problems if you don't take extreme care to make your Swing calls on the Swing event thread. The unknown for us though is what your "agent" is doing. If it is calling long-running code or code with Thread.sleep(...) or wait()/notify() in it, then yes, you will need to use background threading.
• But again, we know that's not your primary problem, since your problems began only after adding focus grabbers -- the JButtons. Again this is strong indication that your primary problem is not threading but is use of KeyListeners and their focus requirement.

For example:

import java.awt.BasicStroke;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.RenderingHints;
import java.awt.Stroke;
import java.awt.event.*;
import java.util.EnumMap;
import javax.swing.*;

@SuppressWarnings("serial")
public class KeyBindingPanel extends JPanel {
   private static final int PREF_W = 800;
   private static final int PREF_H = PREF_W;
   private static final Stroke THICK_STROKE = new BasicStroke(5f,
         BasicStroke.CAP_ROUND, BasicStroke.JOIN_ROUND);
   private static final int OVAL_WIDTH = 30;
   private static final int OVAL_HEIGHT = 30;
   private static final Color OVAL_COLOR = Color.red;
   private static final Color BKGRD_COLOR = Color.black;
   private static final int TIMER_DELAY = 20;
   public static final int STEP = 2;
   private int myX = 0;
   private int myY = 0;
   private JButton[] buttons = new JButton[3];
   private int condition = WHEN_IN_FOCUSED_WINDOW;
   private InputMap inputMap = getInputMap(condition);
   private ActionMap actionMap = getActionMap();
   private EnumMap<Direction, Boolean> directionMap = new EnumMap<Direction, Boolean>(
         Direction.class);

   public KeyBindingPanel() {
      for (int i = 0; i < buttons.length; i++) {
         buttons[i] = new JButton(new ButtonAction());
         add(buttons[i]);
      }
      setBackground(BKGRD_COLOR);

      for (final Direction direction : Direction.values()) {
         directionMap.put(direction, Boolean.FALSE);

         Boolean[] onKeyReleases = { Boolean.TRUE, Boolean.FALSE };
         for (Boolean onKeyRelease : onKeyReleases) {
            KeyStroke keyStroke = KeyStroke.getKeyStroke(
                  direction.getKeyCode(), 0, onKeyRelease);
            inputMap.put(keyStroke, keyStroke.toString());
            actionMap.put(keyStroke.toString(), new DirAction(direction,
                  onKeyRelease));
         }
      }

      new Timer(TIMER_DELAY, new GameTimerListener()).start();
   }

   @Override
   public Dimension getPreferredSize() {
      return new Dimension(PREF_W, PREF_H);
   }

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

      Graphics2D g2b = (Graphics2D) g.create();
      g2b.setRenderingHint(RenderingHints.KEY_ANTIALIASING,
            RenderingHints.VALUE_ANTIALIAS_ON);
      g2b.setStroke(THICK_STROKE);
      g2b.setColor(OVAL_COLOR);
      g2b.drawOval(myX, myY, OVAL_WIDTH, OVAL_HEIGHT);

      g2b.dispose(); // since I created this guy
   }

   private class GameTimerListener implements ActionListener {
      @Override
      public void actionPerformed(ActionEvent e) {
         for (Direction direction : Direction.values()) {
            if (directionMap.get(direction)) {
               myX += STEP * direction.getRight();
               myY += STEP * direction.getDown();
            }
         }
         repaint();
      }
   }

   private class DirAction extends AbstractAction {
      private Direction direction;
      private boolean onRelease;

      public DirAction(Direction direction, boolean onRelease) {
         this.direction = direction;
         this.onRelease = onRelease;
      }

      @Override
      public void actionPerformed(ActionEvent evt) {
         directionMap.put(direction, !onRelease); // it's the opposite!
      }
   }

   private class ButtonAction extends AbstractAction {
      public ButtonAction() {
         super("Press Me!");
      }

      @Override
      public void actionPerformed(ActionEvent e) {
         JButton thisBtn = (JButton) e.getSource();
         for (JButton btn : buttons) {
            if (btn == thisBtn) {
               btn.setEnabled(false);
            } else {
               btn.setEnabled(true);
            }
         }
      }
   }

   private static void createAndShowGui() {
      JFrame frame = new JFrame("KeyBindingPanel");
      frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
      frame.getContentPane().add(new KeyBindingPanel());
      frame.pack();
      frame.setLocationRelativeTo(null);
      frame.setVisible(true);
   }

   public static void main(String[] args) {
      SwingUtilities.invokeLater(new Runnable() {
         public void run() {
            createAndShowGui();
         }
      });
   }
}

enum Direction {
   UP(KeyEvent.VK_UP, -1, 0), DOWN(KeyEvent.VK_DOWN, 1, 0), LEFT(
         KeyEvent.VK_LEFT, 0, -1), RIGHT(KeyEvent.VK_RIGHT, 0, 1);

   private int keyCode;
   private int down;
   private int right;

   private Direction(int keyCode, int down, int right) {
      this.keyCode = keyCode;
      this.down = down;
      this.right = right;
   }

   public int getKeyCode() {
      return keyCode;
   }

   public int getDown() {
      return down;
   }

   public int getRight() {
      return right;
   }

}