Ping-Pong game in Java

Question

I wrote a simple game in Java using Swing. Code is working so far but I have no certainty that "spliting" into classes is done well. I would like to know what "design" problems I have in my code. Also I have a problem with understanding exceptions, I don't really know what to do when exceptions occurs. Terminate the program? Print stack trace? Print message on screen and terminate?

GameFrame.java

package tennisgame;

import java.awt.BorderLayout;
import java.awt.EventQueue;
import javax.swing.JFrame;

public class GameFrame extends JFrame{
    public final static  int WIDTH_GAME_FRAME = 800;
    public final static  int HEIGHT_GAME_FRAME = 700;

     public GameFrame() throws Exception{
       this.setTitle("Game");
       this.setLayout(new BorderLayout());
       this.setSize(WIDTH_GAME_FRAME, HEIGHT_GAME_FRAME);
       this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
       this.setLocationRelativeTo(null);

       Game game = new Game();
       this.add(game.getGamePanel());    
       this.setResizable(false);
       this.setVisible(true);     
    }

    public static void main(String[] args) {

        EventQueue.invokeLater(() -> {
            try{
                GameFrame tg = new GameFrame();
            }
            catch(Exception e){
                System.out.println("ERROR");
            }
        });                        
    }
}

Game.java

package tennisgame;

import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

public class Game implements Runnable {

    public static enum State {MENU, GAME}
    public static enum Direction {LEFT, RIGHT}
    private final GamePanel gPanel;
    private static State stateOfGame;
    private final MouseInput mouseInput;
    private final Ball ball = new Ball();
    private final int UPDATE_INTERVAL = 15;
    private final Paddle player = new Paddle(300, 600);
    private final Paddle pc = new Paddle(300, 42);
    private boolean win = false;
    private final Box box;
    private final KeyInput keyInput;


    public Game() throws Exception  {
       stateOfGame = State.MENU;
       gPanel = new GamePanel(player, pc, ball);    
       mouseInput = new MouseInput(this, gPanel);
       gPanel.addMouseListener(mouseInput);
       box = new Box(GameFrame.WIDTH_GAME_FRAME, GameFrame.HEIGHT_GAME_FRAME);
       keyInput = new KeyInput(gPanel, player, pc);
       player.setTurn(true);

       ScheduledThreadPoolExecutor executor, keyExecutor;
       keyExecutor = new ScheduledThreadPoolExecutor(10);

       keyExecutor.scheduleAtFixedRate
       (keyInput, 0L, UPDATE_INTERVAL, TimeUnit.MILLISECONDS);   

       executor = new ScheduledThreadPoolExecutor(10);
       keyExecutor = new ScheduledThreadPoolExecutor(10);

       executor.scheduleAtFixedRate
       (this, 0L, 15L, TimeUnit.MILLISECONDS);

       keyExecutor.scheduleAtFixedRate
       (keyInput, UPDATE_INTERVAL, 15L, TimeUnit.MILLISECONDS);       
    }

    @Override
    public void run() {      
       if (stateOfGame == State.GAME){
        ball.move();   
        checkCollisions();
       gPanel.repaint();
       }

   }
    public void changeStateOfGame(State state){
        stateOfGame = state; 
    }

    public void checkCollisions(){
        if (ball.getRectangle().intersects(player.getRectangle()) && player.isTurn()){
            ball.bouncePaddle();
            player.setTurn(false);
            pc.setTurn(true);
        }
        else if (ball.getRectangle().intersects(pc.getRectangle()) && pc.isTurn()){           
            ball.bouncePaddle();
            pc.setTurn(false);
            player.setTurn(true);            
       }
        else if (ball.getRectangle().intersects(box.getLeftWall()) ||
                ball.getRectangle().intersects(box.getRightWall()))
            ball.bounceWall();
        else if(ball.getRectangle().intersects(box.getUpperWall())){
            win = true;
            stateOfGame = State.MENU;
            gPanel.endOfGame(true);
            resetSettings();
        }
        else if (ball.getRectangle().intersects(box.getBottomWall())){
            win = false;
            stateOfGame = State.MENU;
            gPanel.endOfGame(false);
            resetSettings();
        }
    }

    public static State getStateOfGame(){
        return stateOfGame;
    }

    public GamePanel getGamePanel() {
        return gPanel;
    }

    public void resetSettings(){
        ball.reset();
        player.reset();
        pc.reset();
    }
}

GamePanel.java

package tennisgame;

import java.awt.Graphics;
import java.awt.image.BufferedImage;
import java.io.IOException;
import javax.imageio.ImageIO;
import javax.swing.JOptionPane;
import javax.swing.JPanel;


public class GamePanel extends JPanel {

    private BufferedImage playImg, helpImg, exitImg, backgroundImg, titleImg;
    private BufferedImage paddleImg, ballImg;
    private final int X_MENU_IMG_POSITION;
    private String winnerInfo;
    private final Paddle player, pc;
    private final Ball ball;

    public GamePanel(Paddle player, Paddle pc, Ball ball) throws Exception{
        this.X_MENU_IMG_POSITION = 251;
        loadImages();
        this.player = player;
        this.pc = pc;
        this.ball = ball;
    }

    public final void loadImages() throws IOException{
        playImg       = ImageIO.read(getClass().getResource
                                                      ("/images/play.png"));
        helpImg       = ImageIO.read(getClass().getResource
                                                      ("/images/help.png"));
        exitImg       = ImageIO.read(getClass().getResource
                                                      ("/images/exit.png"));
        backgroundImg = ImageIO.read(getClass().getResource
                                                      ("/images/bitmap.png"));
        titleImg      = ImageIO.read(getClass().getResource
                                                      ("/images/title220.png"));
        paddleImg     = ImageIO.read(getClass().getResource
                                                      ("/images/yellow.png")); 
        ballImg       = ImageIO.read(getClass().getResource
                                                    ("/images/football18.png"));
    }

    public int getXMenuImgPosition(){
        return X_MENU_IMG_POSITION;
    }

     public void endOfGame(boolean win)   {
        if (win)  {
            winnerInfo = "You win!";
            JOptionPane.showMessageDialog(this, winnerInfo,
                    "End of game", JOptionPane.INFORMATION_MESSAGE);
        }

        else{
            winnerInfo = "You lost";
            JOptionPane.showMessageDialog(this, winnerInfo,
                    "End of game", JOptionPane.INFORMATION_MESSAGE);
        }       
    }  

    @Override
    protected void paintComponent(Graphics g)   {
       super.paintComponent(g);
       g.drawImage(backgroundImg, 0, 0, null);        

      if (Game.getStateOfGame() == Game.State.MENU)
          paintMenu(g);
      else
       paintGame(g);      
    }

    private void paintMenu(Graphics g){     
       g.drawImage(titleImg, 280, 25, null);
       g.drawImage(playImg, X_MENU_IMG_POSITION, 280, null);
       g.drawImage(helpImg, X_MENU_IMG_POSITION, 400, null);
       g.drawImage(exitImg, X_MENU_IMG_POSITION, 520, null);
    }

    private void paintGame(Graphics g) {
       g.drawImage(ballImg, ball.getRectangle().x, ball.getRectangle().y, null);

       g.drawImage(paddleImg, player.getRectangle().x,
                                                 player.getRectangle().y, null);
       g.drawImage(paddleImg, pc.getRectangle().x,
                                                  pc.getRectangle().y, null);
    }
}

Paddle.java

package tennisgame;

import java.awt.Rectangle;

public class Paddle {
    private int xPaddlePosition;
    private int yPaddlePosition;
    private final int startingXPosition, startingYPosition; 
    private final int WIDTH = 120, HEIGHT = 18;
    private final int PADDLE_MOVE = 5;
    private boolean myTurnBounce;

    public Paddle(int xPaddlePosition, int yPaddlePosition){
        this.xPaddlePosition = startingXPosition = xPaddlePosition;
        this.yPaddlePosition = startingYPosition = yPaddlePosition;
        myTurnBounce = false;     
    }

    public Rectangle getRectangle(){
        return new Rectangle(xPaddlePosition,  yPaddlePosition, WIDTH, HEIGHT);
    }

    public void move(Game.Direction dir){
        if (Game.Direction.LEFT == dir){
            if (xPaddlePosition >= PADDLE_MOVE)
                xPaddlePosition -= PADDLE_MOVE;
        }                   
        else {
            if (xPaddlePosition + WIDTH <= 792)
                xPaddlePosition += PADDLE_MOVE;
        } 
    }

    public void reset(){
        xPaddlePosition = startingXPosition;
        yPaddlePosition = startingYPosition;
    }

    public void setTurn(boolean turn){
        myTurnBounce = turn;
    }

    public boolean isTurn(){
        return myTurnBounce;
    }  
}

Box.java

package tennisgame;

import java.awt.Rectangle;

public class Ball {
    private int xPosition;
    private int yPosition;
    private int xMovement = 3;
    private int yMovement = 3;
    private final int WIDTH = 25, HEIGHT = 20;

    public Ball(){
         xPosition =  (int) (Math.random() * 200 + 33) * 3;
         yPosition =  (int) (Math.random() * 33 + 100) * 3;
    }    

    public void move(){
        xPosition += xMovement;
        yPosition += yMovement;
    }

    public void bouncePaddle(){
       yMovement = -yMovement;
    }

    public void bounceWall(){
        xMovement = -xMovement;
    }

    public void reset(){
         xPosition =  (int) (Math.random() * 200 + 33) * 3;
         yPosition =  (int) (Math.random() * 33 + 100) * 3;
    }

    public Rectangle getRectangle(){
        return new Rectangle(xPosition, yPosition, WIDTH, HEIGHT);
    }
}

MouseInput.java

package tennisgame;

import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import javax.swing.JOptionPane;

public class MouseInput extends MouseAdapter{
    private final GamePanel gPanel;
    private final Game game;
    private final String helpInfo = "This is a very simple game which imitates"
            + " world-known game Ping Pong \n\n " + "First player use arrows"
            + ", second player use A and D keys";

    public MouseInput(Game game, GamePanel gPanel){
        this.game = game;
        this.gPanel = gPanel;
    }

    @Override
    public void mousePressed(MouseEvent e){
      int xMousePosition = e.getX();
      int yMousePosition = e.getY();

      if (Game.getStateOfGame() == Game.State.GAME)
          return;

      if (xMousePosition < gPanel.getXMenuImgPosition() || xMousePosition > 551)
          return;

      if (yMousePosition > 280 && yMousePosition < 350)                       
                game.changeStateOfGame(Game.State.GAME);

      else if (yMousePosition > 400 && yMousePosition < 470)        
            JOptionPane.showMessageDialog(gPanel, helpInfo,
                    "Help information", JOptionPane.INFORMATION_MESSAGE);

      else if (yMousePosition > 520 && yMousePosition < 590)
              System.exit(0);             
    }
}

KeyInput.java

package tennisgame;

import java.awt.event.ActionEvent;
import java.awt.event.KeyEvent;
import javax.swing.AbstractAction;
import javax.swing.ActionMap;
import javax.swing.InputMap;
import javax.swing.JComponent;
import javax.swing.KeyStroke;
import java.util.EnumMap;

public final class KeyInput implements Runnable{

    private static final String PRESSED = "pressed";    
    private static final String RELEASED = "released"; 
    private final EnumMap<Key, Boolean> keyMap; 
    private final GamePanel gPanel;
    private final Paddle player, pc;

    public KeyInput(GamePanel gPanel, Paddle player, Paddle pc)  {
      this.keyMap =  new EnumMap<>(Key.class);
      this.gPanel = gPanel;
      this.player = player;
      this.pc = pc;

      resetKeyInput();

      ActionMap actionMap = gPanel.getActionMap();
      int condition = JComponent.WHEN_IN_FOCUSED_WINDOW;
      InputMap inputMap = gPanel.getInputMap(condition);

      for (Key key : Key.values())      {
         KeyStroke pressedKeyStroke = KeyStroke.getKeyStroke
                                     (key.getKeyCode(), 0, false);
         KeyStroke releasedKeyStroke = KeyStroke.getKeyStroke
                                     (key.getKeyCode(), 0, true);

         inputMap.put(pressedKeyStroke, key.getText() + PRESSED);
         inputMap.put(releasedKeyStroke, key.getText() + RELEASED);
         actionMap.put(key.getText() + PRESSED, new MyArrowBinding(key, false));
         actionMap.put(key.getText() + RELEASED, new MyArrowBinding(key, true));
      }
    }

    @Override
    public void run()    {         
        int order = 1;

          for (Key key : keyMap.keySet())
         {
            if (keyMap.get(key) && order == 1)      
                pc.move(Game.Direction.LEFT);

            else if (keyMap.get(key) && order == 2)           
                pc.move(Game.Direction.RIGHT);

            else if (keyMap.get(key) && order == 3)            
                player.move(Game.Direction.LEFT);

            else if (keyMap.get(key) && order == 4)           
                player.move(Game.Direction.RIGHT);

            order++;
         }     
    }

    public void resetKeyInput()   {
         keyMap.put(Key.A, false);
         keyMap.put(Key.D, false);
         keyMap.put(Key.LEFT, false);
         keyMap.put(Key.RIGHT, false);
    }   

    private class MyArrowBinding extends AbstractAction    {
      private final Key key;
      private final boolean released;

      public MyArrowBinding(Key key, boolean released)     {
         this.key = key;
         this.released = released;
      }

      @Override
      public void actionPerformed(ActionEvent aEvt)     {
         keyMap.put(key, !released);
      }
   }

    enum Direction    {
      LEFT("Left"), RIGHT("Right"), NEUTRAL("Neutral");
      private final String text;

      private Direction(String text)     {
         this.text = text;
      }

      public String getText()     {
         return text;
      }
    }

    enum Key   {
      A("A", Direction.LEFT, KeyEvent.VK_A),
      D("D", Direction.RIGHT, KeyEvent.VK_D), 
      LEFT("Left", Direction.LEFT, KeyEvent.VK_LEFT),
      RIGHT("Right", Direction.RIGHT, KeyEvent.VK_RIGHT);

      private final String text;
      private final Direction direction;
      private final int keyCode;

      private Key(String text, Direction direction, int keyCode)     {
         this.text = text;
         this.direction = direction;
         this.keyCode = keyCode;
      }

      public String getText()     {
         return text;
      }

      public Direction getDirection()    {
         return direction;
      }

      public int getKeyCode()    {
         return keyCode;
      }
    }
}

Box.java

package tennisgame;

import java.awt.Rectangle;

public class Box {
    private final int WIDTH_FRAME, HEIGHT_FRAME;
    private final int MARGIN = 10; 

    public Box(int width, int height){
        WIDTH_FRAME = width;
        HEIGHT_FRAME = height;
    }

    public Rectangle getLeftWall(){
        return new Rectangle(-MARGIN, 0, MARGIN, HEIGHT_FRAME);
    }

    public Rectangle getRightWall(){
        return new Rectangle(WIDTH_FRAME, 0, MARGIN, HEIGHT_FRAME);
    }

    public Rectangle getBottomWall(){
        return new Rectangle(0, HEIGHT_FRAME, WIDTH_FRAME, MARGIN);
    }

    public Rectangle getUpperWall(){
        return new Rectangle(0, -MARGIN, WIDTH_FRAME, MARGIN);
    }
}

oopexpert · Accepted Answer · 2017-02-10 12:54:20Z

Model vs UI

Your classes have basic separation of concerns. But as you can see by yourself some model elements like "Ball" or "Box" or "Paddle" or even "Game" have a dependency to the package java.awt.*. Try to reformulate your model not to point to UI elements. This is not just because you may provide another UI technology like JavaFX. Sure you can do that afterwards. But the real issue is code quality because you satisfy the "single responsibility principle". My advice is to have a look at the observer pattern.

Doing the separation of UI and model is very hard while saying to do so is easy. I even saw experienced developers struggling with it as the do not put the fact into account that a proper separation implicitly makes the UI interchangable. I often heard "we do not need to exchange the UI but we have proper separation". But that is inherent contradictory. I see UI exchangability not as a feature. I see it as a measurement for a pretended separation.

Game state

You formulate the state of the game within an enum. My suggestion is to use a full state pattern. There you not only provide an artefact that says you are in state 1 or in state 2. You also encapsulate the corresponding behaviour and use polimorphism. Currently the state specific behaviour is spread all over the place within if-then-else-statements.

Furthermore you should extend your state handling. The game is a process that follows some process states. And one other important process state is the end of the process. I do not say to model ALL states. But if you have behaviour that should handle the end of the game you should represent it as a separate state.

Application State

I think you mixed "application state" with "game state". State.MENU seems to be semantical anorganic to State.GAME if you model the game state with it. Either you should have two types of state (application state AND game state) or (my suggestion) omit the representation of the application state as the "menu" has nothing to do with the game itself.

Magic numbers

You have some magic numbers left you should name. You already did this with "HEIGHT_FRAME" or "WIDTH_FRAME". But the checks for the mouse position are comparing against magic numbers.

Game vs. game instance

You also mixed some further semantics as a game may be the set of rules you follow OR a concrete game currently in progress. I suggest to separate these responsibilities.

Direction

As the amount of directions are limitted in yur usecase using an enumeration is totally fine. But here again you have external behaviour you can internalize using polymorphism. Especially the move-behaviour can be encapsulated within the enumeration.

Avoid early return-statements

"return", "break" and "continue" are structured goto statements and they come with a heritage.

They break the control flow and say "I am not interested in the statements below me".

I do not say you have to have a single exit per method but I encourage you to have it. My point of view is: Code is never perfect. As code is never perfect it is continuously improved by refactorings. Maybe the statements below the return statement become important at some point or you want to some extract code into a new method then you will have a hard time to do so without reformulating the whole control flow.

But if you think your code fragment does currently not violate the single responsibility principle and will not in the future you will not face the burden of such a refactoring.

Swing event dispatcher thread

If you are working in a single thread environment you have no real problem. But you are working with multiple threads so the Swing-UI can asynchronously change for different reasons. The "Ball" will continue to move and your paddle as well if you press the direction keys.

To enforce a consistent UI state I suggest to make ANY change to your UI in the EDT (event dipatcher thread). Mostly you will not face the problem. But concurrency problems are those that are hard to interprete if they occur.

You already use the "EventQueue" when you start the appication.

Timothy Truckle · Accepted Answer · 2017-02-10 12:08:06Z

Thanks for sharing this.

Looks quite reasonable to me. I have only a few thing I'd change:

unnecessary inheritance

Your Game class extends JPanel but it does not change the behavior of a JPanel, it only confirures its content. This can also be done from the outside:

public class GameFrame{
    public final static  int WIDTH_GAME_FRAME = 800;
    public final static  int HEIGHT_GAME_FRAME = 700;

     public GameFrame(JFrame theFrame) throws Exception{
       theFrame.setTitle("Game");
       theFrame.setLayout(new BorderLayout());
       theFrame.setSize(WIDTH_GAME_FRAME, HEIGHT_GAME_FRAME);
       theFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
       theFrame.setLocationRelativeTo(null);

       Game game = new Game();
       theFrame.add(game.getGamePanel());    
       theFrame.setResizable(false);
    }

    public static void main(String[] args) {

        EventQueue.invokeLater(() -> {
            try{
                JFrame theFrame = new JFrame();
                GameFrame tg = new GameFrame(theFrame);
                theFrame.setVisible(true);
            }
            catch(Exception e){
                System.out.println("ERROR");
            }
        });                        
    }
}

replace `if`/`else` cascade by polymorpism

your method checkCollisions yould be simplified by using polymorphism:

You need another file:

CollisionCheck.java

interface CollisionCheck {
   State checkCollisionWith(Ball ball);
}

The you create instances of that interface in your Game class:

private final List<CollisionCheck> collisionChecks = new ArrayList<>(); 
public Game() throws Exception  {
   // same as you have
   // add collision checks
   // as anonymous class
   collisionChecks.add(new CollisionCheck () {
      @Override
      public  State checkCollisionWith(Ball ball) {
         if (ball.getRectangle().intersects(player.getRectangle()) && player.isTurn()){
           ball.bouncePaddle();
           player.setTurn(false);
           pc.setTurn(true);
         }
         return stateOfGame;
      }
   });
   // as java 8 lambda
   collisionChecks.add(movedBall -> {
     if (movedBall.getRectangle().intersects(pc.getRectangle()) && pc.isTurn()){           
          ball.bouncePaddle();
          pc.setTurn(false);
          player.setTurn(true);            
      }
      return stateOfGame;
   });
   // as instance of another top level class holding the implementation:
   collisionChecks.add(new PaddleBounceChec(ball,pc,player));
   // same schema for all bounce checks

your method checkCollisions changes to this:

public void checkCollisions(){
  for(CollisionCheck cc :  collisionChecks)
       stateOfGame= cc.checkCollisionWith(ball);
 // that's all

There is another place where you could use inheritance instead of if/else:

public void move(Game.Direction dir){
    if (Game.Direction.LEFT == dir){
        if (xPaddlePosition >= PADDLE_MOVE)
            xPaddlePosition -= PADDLE_MOVE;
    }                   
    else {
        if (xPaddlePosition + WIDTH <= 792)
            xPaddlePosition += PADDLE_MOVE;
    } 
}

This logic could be implemented in the enum:

 public static enum Direction {
     LEFT {
       public int move(int xPaddlePosition){
         if (xPaddlePosition >= Paddle.PADDLE_MOVE)
            xPaddlePosition -= Paddle.PADDLE_MOVE;
         }
         return xPaddlePosition 
     },
     RIGHT {
       public int move(int xPaddlePosition){
         if (xPaddlePosition + Paddle.WIDTH <= 792)
             xPaddlePosition += Paddle.PADDLE_MOVE;
         }
         return xPaddlePosition 
     };
    abstact public int move(int xPaddlePosition);
  }

this would change your method to:

 public void move(Game.Direction dir){
    xPaddlePosition = dir.move(xPaddlePosition);
 }

Stack Exchange Network

current community

your communities

more stack exchange communities

Ping-Pong game in Java

2 Answers 2

Model vs UI

Game state

Application State

Magic numbers

Game vs. game instance

Direction

Avoid early return-statements

Swing event dispatcher thread

unnecessary inheritance

replace `if`/`else` cascade by polymorpism

Your Answer

Not the answer you're looking for? Browse other questions tagged java game swing or ask your own question.

Hot Network Questions

Ping-Pong game in Java

2 Answers 2

Model vs UI

Game state

Application State

Magic numbers

Game vs. game instance

Direction

Avoid early return-statements

Swing event dispatcher thread

unnecessary inheritance

replace if/else cascade by polymorpism

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java game swing or ask your own question.

Related

Hot Network Questions

replace `if`/`else` cascade by polymorpism