Platformer with collisions

Today I started to build my first platformer, first time working with collisions and stuff. Now, surprisingly, everything seems to work, but since this is my first time creating a platformer and I'm still a beginner with collisions I don't know if the way I coded it is good practise.

The game is a ball with a fixed x coordinate and it has to jump to different platforms that move from the right to the left. If the ball falls through the bottom or gets too high, i.e. not being visible anymore, the ball freezes. To see if the ball has fallen through the bottom of the screen I used the variable HEIGHT (declared in class GamePanel), but instead of the ball freezing outside of the screen, you can still see a little bit of it on the screen when it has frozen.

If anything in the code is unclear, please tell me, because I want to learn to write readable code. Did I use enough comments? Too many? On the wrong spots?

BasicPlatformer.java

package BasicPlatformer;

import javax.swing.*;

public class BasicPlatformer {

    public static void main(String[] args){
        GamePanel gamePanel = new GamePanel();
        JFrame frame = new JFrame("Basic Platforrmer");
        frame.getContentPane().add(gamePanel);
        frame.pack();
        frame.setVisible(true);
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        frame.setLocation(2000, 50);
        frame.setResizable(false);
        gamePanel.requestFocusInWindow();
    }
}

GamePanel.java

```
package BasicPlatformer;

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.util.ArrayList;

public class GamePanel extends JPanel {
public static final int WIDTH = 1000;
public static final int HEIGHT = 900;
public static final int PLATFORMSTARTHEIGHT = 700;

public static ArrayList platforms;

Timer platformTimer, playerTimer, creationTimer;
Player player;

public GamePanel(){
Dimension panelSize = new Dimension(WIDTH, HEIGH

-
All GUI updates must be performed on the GUI thread. But you create new GUI elements in the main method of the BasicPlatformer, which runs on the main thread. It is not correct. You should use the invokeLater method and a Runnable(possibly with a lambda expression) instead.

-
It looks like there is no need for multiple timers here. One timer(with the shortest delay) can do fine(it just requires small changes in the event listener).

-
Overall design: the Player class knows to much about the internals of the GamePlatform and the Platform. It seems to me that intersection methods should belong to the Platform class and bounds checks should be in the GamePlatform class.

You code also violates the single responsibility principle: the game logic is tightly coupled with the GUI. I'd recommend creating separate classes for performing computations and rendering the game.

-
Variables names. There is no need to make them so short. Keep them descriptive. For example, this piece of code:

int bX = x + w / 2; //x of the middle bottom of the player
int bY = y + h; //y of the bottom of the player

is not good. It requires writing additional comments instead of storing all the information in the code itself:

int playerBottomX = x + w / 2; 
int playerBottomY = y + h;

Now it is clear that these are the coordinates of the bottom of the player without any comments(it would also be good to change x and y to something more descriptive: what part of the player do they represent? Why do we add w / 2 to x? Ideally, the code should be written in such a way that answers to these questions are obvious).

One more example of an unclear method:

for (Platform platform : GamePanel.platforms) {
    if (bX > platform.getX() && bX = platform.getY() && bY < platform.getY() + velY) { //because velY is added to y, some y values are skipped. But a y value of the player must be between the y of the platform and the value of velY below the y of the platform
            velY = 0;
            y = platform.getY() - h; //because the player can land a bit off of the platform, set the y of the player to the y of the platform and ajust with the height of the player
        }
        break;
    }
}

There is too much stuff going on here. And the fact that you need to write comments to explain what the code does is a smell. How to improve it? Well, you can make it much more readable by decomposing it into several smaller methods:

for (Platform platform : GamePanel.platforms) {
    if (platform.isAboveHorizontally(playerBottomX)) { 
        if (platform.intersectsVertically(playerBottomY, velocityY)) { 
            landOnPlatform(platform);
        }
        break;
    }
}

I think that intersection methods should belong to the Platform class.

A rule of thumb: if you need to write comments inside a method to clarify what's going on, you can either rename variables or extract a method instead. The code should be self-documenting.

-
As I have said above, it is often(not always) a smell to have comments inside a method. However, it is a good practice to write comments for all public classes and methods(which explain what they do, not how).

-
Do not make fields static unless it is really necessary. For instance, the platforms field of the GamePanel class does not look like a good candidate to be static. It belongs to an instance, not to a class(what happens if you create more than instance?).

-
Avoid "generic" names for your methods. For example, move does not tell much about what the method does. The questions is: move where? The same is true for fields.

-
Use consistent indentation: always have one whitespace before an opening curly bracket.

-

Now, surprisingly, everything seems to work

It looks like you are not confident in your code. It is a good practice to test the code in a systematic manner(using unit tests) to be sure that it works(of course, tests cannot prove its correctness, but they can help to ensure that it behaves properly for specific cases. It is much better than nothing).

StackExchange Code Review Q#90853, answer score: 5