MouseListener Lag in Piano Tiles game

Question

I was attempting to write my own simple version of the Piano Tiles game app in Java just as a fun exercise.

GameLogic.java

import javax.swing.*;

import java.awt.*;
import java.awt.event.*;
import java.util.*;

public class GameLogic extends JPanel
{
    Tile[][] tileArray = new Tile[4][10];

    int lowestIndex;
    int targetTileIndex;

    int counter = 0;

    boolean calculating = false;

    boolean gameStart = false;

    public GameLogic()
    {
        super();
        this.setPreferredSize(new Dimension(500, 400));

        int tempY = 200;
        int tempX = 0;
        lowestIndex = 0;
        targetTileIndex = 0;

        for(int y = 0; y<10; y++)
        {
            int tempRandom = randomNum();
            for(int x = 0; x<4; x++)
            {
                tileArray[x][y] = new Tile(tempX, tempY);
                if(x == tempRandom)
                {
                    tileArray[x][y].activate();
                }

                tempX+=65;
            }

            tempY-=100;
            tempX = 0;
        }

        final javax.swing.Timer moveTimer = new javax.swing.Timer(1, new ActionListener()
        {
            public void actionPerformed(ActionEvent e)
            {
                for(int y = 0; y<10; y++)
                {
                    for(int x = 0; x<4; x++)
                    {
                        tileArray[x][y].moveDown();

                    }
                }

                if(tileArray[0][lowestIndex].getYPos()>=400)
                {
                    int tempRandom = randomNum();

                    for(int x = 0; x<4; x++)
                    {
                        tileArray[x][lowestIndex].setYPos(-600);

                        //temp
                        tileArray[x][lowestIndex].deactivate();

                        if(x == tempRandom)
                        {
                            tileArray[x][lowestIndex].activate();
                        }
                    }

                    if(lowestIndex!=9)
                    {
                        lowestIndex++;
                    }
                    else
                    {
                        lowestIndex = 0;
                    }
                }
                repaint();
            }
        }
                );

        class MyMouseListener implements MouseListener
        {

            @Override
            public void mouseClicked(MouseEvent e) 
            {
                counter++;
                System.out.println(counter);
                System.out.println("Key!");
                if(gameStart == false)
                {
                    moveTimer.start();
                }
                // TODO Auto-generated method stub
                int x = e.getX();
                int y = e.getY();
                int targetTileNumber = -1;
                int targetX;
                int targetY;

                for(int a = 0; a<4; a++)
                {
                    if(tileArray[a][targetTileIndex].getStatus())
                    {
                        targetTileNumber = a;
                    }
                }

                targetX = targetTileNumber*65;
                targetY = tileArray[targetTileNumber][targetTileIndex].getYPos();

                if(x>=targetX && x<=(targetX+65) && y>=targetY && y<=(targetY+100))
                {
                    tileArray[targetTileNumber][targetTileIndex].deactivate();

                    if(targetTileIndex!=9)
                    {
                        targetTileIndex++;
                    }
                    else
                    {
                        targetTileIndex = 0;
                    }
                }
                else
                {

                }

                repaint();

            }

            @Override
            public void mouseEntered(MouseEvent arg0) {
                // TODO Auto-generated method stub

            }

            @Override
            public void mouseExited(MouseEvent arg0) {
                // TODO Auto-generated method stub

            }

            @Override
            public void mousePressed(MouseEvent arg0) {
                // TODO Auto-generated method stub

            }

            @Override
            public void mouseReleased(MouseEvent arg0) {
                // TODO Auto-generated method stub

            }

        }

        this.addMouseListener(new MyMouseListener());

    }

    public void paintComponent(Graphics g)
    {
        super.paintComponent(g);
        Graphics2D g2 = (Graphics2D) g;
        g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);

        for(int y = 0; y<10; y++)
        {
            for(int x = 0; x<4; x++)
            {
                tileArray[x][y].paintComponent(g2);
            }
        }
    }

    public int randomNum()
    {
        Random rand = new Random();
        return rand.nextInt(4);
    }

    public static void main(String[] args)
    {
        JFrame f = new JFrame();
        f.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        f.setLayout(new FlowLayout());

        f.add(new GameLogic());

        f.pack();
        f.setVisible(true);
    }
}

Tile.java

import javax.swing.*;

import java.awt.*;

public class Tile extends JComponent
{
    boolean activated = false;
    int length;
    int width;

    int xPos;
    int yPos;

    public Tile(int xPos, int yPos)
    {
        super();
        length = 100;
        width = 65;

        this.xPos = xPos;
        this.yPos = yPos;

        repaint();
    }

    public void paintComponent(Graphics g)
    {
        super.paintComponent(g);
        Graphics2D g2 = (Graphics2D) g;
        g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);

        if(activated)
        {
            g2.setColor(Color.BLACK);
        }
        else
        {
            g2.setColor(Color.WHITE);
        }

        g2.fillRect(xPos, yPos, width, length);
        g2.setColor(Color.BLACK);
        g2.drawRect(xPos, yPos, width, length);
    }

    public void activate()
    {
        activated = true;
    }

    public void deactivate()
    {
        activated = false;
    }

    public boolean getStatus()
    {
        return activated;
    }

    public void moveDown()
    {
        this.yPos+=1;
        repaint();
    }
    public int getYPos()
    {
        return yPos;
    }
    public void setYPos(int yPos)
    {
        this.yPos = yPos;
    }
    public void paint()
    {
        this.repaint();
    }
}

Now one issue I am having is regarding the mouse listener. Rapid clicking causes it to not log every click skipping some instances of the click, which kind of causes issues regarding the logic checking at higher speeds of scrolling. I wasn't sure whether this is an issue with me writing in too many calculations at the instance of a mouse click. The only reasons I could come up with were either that or me creating some kind of buffer in order to facilitate rapid clicking.

Additionally, is my code okay overall or is my approach completely inefficient/off/wrong in any way?

Answer 1

Reviewing the code.

At first sight it look all nice and clean.
Nice formatting of the code.
Java standarts are a little differtent then you use, but you are consistent and that is good.
The deviation is where you set your {.
This is normally done at the same line and not a new line.
Still some improvement can be done to make your code easier to maintain.

Magic numbers

Yeah, for some people they are the hell. I'm not saying to convert all your "magic numbers" to private static final int but some you have to do.

Tile[][] tileArray = new Tile[4][10];

in combination with :

for(int y = 0; y<10; y++)
{
    int tempRandom = randomNum();
    for(int x = 0; x<4; x++)
    {
        tileArray[x][y] = new Tile(tempX, tempY);
        if(x == tempRandom)
        {
            tileArray[x][y].activate();
        }
        tempX+=65;
    }
 }

Why not making your bounds as constants and the Tile final so you can't initialize it again?

private static final int X_BOUND_TILES = 4;
private static final int Y_BOUND_TILES = 10;
private final Tile[][] tileArray = new Tile[X_BOUND_TILES][Y_BOUND_TILES];

and the for refactorred to this :

for(int y = 0; y < Y_BOUND_TILES; y++)
{
    int tempRandom = randomNum();
    for(int x = 0; x < X_BOUND_TILES; x++)
    {
        tileArray[x][y] = new Tile(tempX, tempY);
        if(x == tempRandom)
        {
            tileArray[x][y].activate();
        }
        tempX+=65;
    }
 }

This is much easier to refactor. Just change your constant, compile and your off without having errors that could be raised if you forget to change your for loop.

Now why do you count 65 plus? What is the meaning of 65?
You use multiple time 65. When you need to change one, do they have to change all?
If the answer is yes => certainly make a constant, otherwise I suggest to refactor to a constant but this is not so important.

Scoping of variables

The default scope is package-private. All classes in the same package can access the method/field/class. Package-private is stricter than protected and public scopes, but more permissive than private scope.

Are you sure you want that scope for all your global variables?

Tile[][] tileArray = new Tile[4][10];
int lowestIndex;
int targetTileIndex;
int counter = 0;
boolean calculating = false;
boolean gameStart = false;

This should be :

private static final int X_BOUND_TILES = 4;
private static final int Y_BOUND_TILES = 10;
private final Tile[][] tileArray = new Tile[X_BOUND_TILES][Y_BOUND_TILES];
private int lowestIndex;
private int targetTileIndex;
private int counter = 0; 
private boolean calculating = false;
private boolean gameStart = false;

Default values

This is a part I have to say.
It's educated to me so I educate further but I'm not a fan of it.

private int counter = 0; 
private boolean calculating = false;
private boolean gameStart = false;

Is the same as :

private int counter; 
private boolean calculating;
private boolean gameStart;

Default value of a boolean is false.
Default value of a int is 0.
Take care, default of Integer and Boolean is null.

Defensive programming

if (lowestIndex!=9) {
    lowestIndex++;
} else {
    lowestIndex = 0;
}

We all know what you are trying to do. But what happens if lowestIndex is magically turned 10?
You have a bug that will not correct by himself. Let's say we do the following :

if (lowestIndex<9) {
    lowestIndex++;
} else {
    lowestIndex = 0;
}

Execution is almost the same. But now we are sure when lowestIndex is changed magically the next step it will correct itself.

Further, this could be refactorred a ternary operator :

lowestIndex = lowestIndex<9?lowestIndex + 1:0;

Legacy code

You know how it's going. You write and erease code constantly. I see in your code an else block with nothing in it.

Remove the else block cause it's not used anymore, so you get lesser and cleaner code.

Method's complexity - Separation of concerns

Most of your methods are a little long(we have seen a lot more worse then yours) and can be refactorred to smaller methods with a small specific problem.

GameLogic can be lot smaller and make 4-5 methods for it.
Example : The code of your actionlistener.

final javax.swing.Timer moveTimer = new javax.swing.Timer(1, getActionListenerForTimer());

Or something like that.
Let the method return the actionlistener, but in the creation of your actionlistener use also at least 2-3 methods.

Edit :

Your problem :

Use mousePressed instead of mouseClicked with your eventlistener.
mouseClicked looks for multiple button clicks, so it will merge some events.