Trying to make a functioning Tetris game/ why does my board look weird?

[DeusExMachina90]

Hi all. For the final project in my introductory comp sci course, I've been instructed to recreate Tetris in java. I have been having some difficulties getting the board to display properly--the board that shows up on the screen has too many cells (in the paintComponent method of GameBoardPanel, change fillRect to drawRect and you'll see what I mean), and when I add blocks onto the board, the display sometimes shows additional unintended colored cells. Also, my canItFall method doesn't seem to be working properly, and I get the feeling that this might be a related problem issuing from the same cause.

I have posted my code thus far below (I've attached most of the code including one of the specific Tetromino block classes, though I'm fairly certain the problem is in GameBoardPanel). I tend to write/debug my codes as I go, hence the "under construction" appearance. I hugely appreciate any help you guys have to offer, as well as additional comments or suggestions on how I can improve my code/where I should go from here. Thanks!

import javax.swing.* ;
public class Tetris{

public static void main (String args[]){
	
JFrame frame = new JFrame("Ultimate Final Tetris Deathmatch Apocalypse III");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.getContentPane().add(new TetrisFrame());
frame.pack();
frame.setVisible(true);

}
}

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.text.*;
public class TetrisFrame extends JPanel{
	
private JButton uhpushityeah;
JPanel statspanel;
GameBoardPanel gamepanel;
private final int DELAY = 100000;
private Timer timer;
private int startX, startY, moveY, blockwidth, blockheight, rows, columns, xcoord, ycoord;
Block piece;

public TetrisFrame ()
{
setPreferredSize(new Dimension(800, 800));
gamepanel = new GameBoardPanel();
statspanel = new JPanel();
JSplitPane sp = new JSplitPane(JSplitPane.HORIZONTAL_SPLIT, gamepanel, statspanel);
add(sp);

statspanel.setLayout(new BorderLayout());
gamepanel.setPreferredSize(new Dimension(400, 600));
gamepanel.setFocusable(true);

uhpushityeah = new JButton ("COME TASTE THE BLOCK-STACKING FURY, CANDYASS.");
//uhpushityeah.addActionListener (new ButtonListener());
statspanel.add(uhpushityeah, BorderLayout.CENTER);

timer = new Timer(DELAY, new TetrisListener());
timer.start();

piece = gamepanel.createBlock();
gamepanel.addToBoard(piece);

}

/* 
private class ButtonListener implements ActionListener
{
public void actionPerformed(ActionEvent event)
{	
	
JTextField scorefield, levelfield;
JLabel label1, label2, label3;
int xcoord, ycoord;

statspanel.removeAll(); 
statspanel.updateUI();

scorefield= new JTextField(8);
scorefield.setEditable(false);
levelfield= new JTextField(3);
levelfield.setEditable(false);

label1 = new JLabel ("Score:");
label2 = new JLabel ("Level:");
label3 = new JLabel ("Next block:");

statspanel.add(label1, BorderLayout.NORTH);
statspanel.add(label2, BorderLayout.CENTER);
statspanel.add(label3, BorderLayout.SOUTH);

ycoord = 0;


} 	
} */
private class TetrisListener implements ActionListener{
public void actionPerformed(ActionEvent event){
		
}
}

}

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

public class GameBoardPanel extends JPanel implements Runnable{
private final int DELAY = 25;

private int initX, initY, moveY, blockWidth, blockHeight;	
private int xcoord, ycoord, startX, startY, whichblock, ycheck;
private Square[][] gameboard;

private Block[] gameObjects;
public static final int  MAX_BLOCKS = 1000;
public int numBlocks = 0;
Thread t;
Block nextblock;


public GameBoardPanel(){
	
addKeyListener(new ControlListener());
	
t = new Thread(this);
startX = 4;
startY = 1;
gameboard = new Square[21][11];	
gameObjects = new Block[MAX_BLOCKS];
for(int i = 0; i < 20; i++){
	for (int j = 0; j < 10; j++){
		gameboard[i][j] = new Square();
	}	
}
setBackground(Color.blue);	
t.start();
}

public void run(){

    while(true){ 
	//clear the board
	for(int i = 0; i < 20; i++){
	    for(int j = 0; j < 10; j++){
		gameboard[i][j].setColor(Color.black);
		gameboard[i][j].empty = true;		
	    }
	}
	
	

	for(int i = 0; i < numBlocks; i++){
		if(canItFall(gameObjects[i]) == true){
			gameObjects[i].fallDown();
			situateBlock(gameObjects[i]);
		}
	}

	repaint();

	try{
	    t.sleep(1000);
	} catch (InterruptedException e) {
	    throw new RuntimeException(e);
	}
    }
}

public void paintComponent (Graphics page){
super.paintComponent (page);


ycoord = 20;
for(int i = 0; i < 20; i++){
	xcoord = 20;
	for (int j = 0; j < 10; j++){
		gameboard[i][j].setX(xcoord);
		gameboard[i][j].setY(ycoord);
		xcoord += 20;
		page.setColor(gameboard[i][j].getColor());
		page.fillRect(gameboard[i][j].getX(), gameboard[i][j].getY(), 90, 90);
	}
	
	ycoord += 20;
}	
}
		
	
public Block createBlock(){
	Random generator = new Random();
	whichblock = generator.nextInt(7);
	switch(whichblock){
		case 0:
			nextblock = new Tblock();			
		break;
		case 1:
			nextblock = new Oblock();			
		break;
		case 2:
			nextblock = new Zblock();			
		break;
		case 3:
			nextblock = new Sblock();			
		break;
		case 4:
			nextblock = new Jblock();			
		break;
		case 5:
			nextblock = new Iblock();			
		break;
		case 6:
			nextblock = new Lblock();			
		break;
	}
//	Tblock test = new Tblock();
	gameObjects[numBlocks] = nextblock;
	addToBoard(nextblock);
	numBlocks++;
	return nextblock;
}
public void addToBoard(Block newblock){
	int[][] geom = newblock.getGeometry();
	newblock.setX(startX);
	newblock.setY(startY);		
	repaint();	
	}
public void situateBlock(Block curblock){
	int[][] geom = curblock.getGeometry();
	for (int i = 0; i < 4; i++){
		gameboard[curblock.getY() + geom[i][1]][curblock.getX() + geom[i][0]].fillSquare();
		gameboard[curblock.getY() + geom[i][1]][curblock.getX() + geom[i][0]].setColor(curblock.getColor());
		}		
	}
public boolean canItFall(Block curblock){
	int[][] geom = curblock.getGeometry();
	for(int i = 0; i < 4; i++){
		System.out.println(gameboard[curblock.getY() + geom[i][1]+1][curblock.getX() + geom[i][0]].isItEmpty());
		if(getY() + geom[i][1] == 10)
			return false;
		else if(gameboard[curblock.getY() + geom[i][1]+1][curblock.getX() + geom[i][0]].isItEmpty() == false)
			return false;
		
	}
	return true;
	}
private class ControlListener implements KeyListener{
public void keyPressed(KeyEvent event){
	switch(event.getKeyCode()){
	case KeyEvent.VK_UP:
		gameObjects[numBlocks-1].rotate();
	break;
	case KeyEvent.VK_DOWN:
		
	break;
	case KeyEvent.VK_LEFT:
		gameObjects[numBlocks-1].moveLeft();
	break;
	case KeyEvent.VK_RIGHT:
		gameObjects[numBlocks-1].moveRight();
	break;
	}
	repaint();
}
public void keyTyped(KeyEvent event){}
public void keyReleased(KeyEvent event){}
}
}

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

public class Square{
private Color color;
public boolean empty;
private int x, y;

public Square(){
	color = Color.black;
	empty = true;
}	
public void setX(int xcoord){
	x = xcoord;	
}
public void setY(int ycoord){
	y = ycoord;	
}
public int getX(){
	return x;
}
public int getY(){
	return y;
}
public void setColor(Color anycoloryoulike){
	color = anycoloryoulike;
}
public Color getColor(){	
	return color;
}
public void fillSquare(){
	empty = false;	
}
public boolean isItEmpty(){
	return empty;
}


}

import java.awt.Color;

abstract public class Block
{
	int[][] geometry;
	protected int x, y, orientation;
	protected Color colorofshape;
	protected boolean isitfalling;

	public Block(){
		geometry = new int[4][2];
		orientation = 0;
		colorofshape = Color.black;
	}

	public int[][] getGeometry()
	{
		return geometry;
	}

	public int getX()
	{
		return x;
	}

	public int getY()
	{
		return y;
	}

	public void setX(int x)
	{
		this.x = x;
	}

	public void setY(int y)
	{
		this.y = y;
	}
	
	public Color getColor()
	{
		return colorofshape;
	}

	public void setColor(Color c)
	{
		colorofshape = c;
	}
	
	public abstract void rotate ();
	
	public void moveLeft(){
		x = x-1;	
	}
	
	public void moveRight(){
		x += 1;	
	}
	
	public void fallDown(){
		y += 1;
	}	
	
}

import java.awt.Color;

public class Tblock extends Block{

int[][] geom1 = {{-1,0},{0,0},{1,0},{0,1}};
int[][] geom2 = {{0,1},{0,0},{0,-1},{-1,0}};
int[][] geom3 = {{0,-1},{1,0},{0,0},{-1,0}};
int[][] geom4 = {{0,0},{1,0},{0,1},{0,-1}};
	
	public Tblock ()
	{
		super();
		setColor(Color.magenta);
		geometry = geom1;
	}
	public void rotate ()
	{
		if (orientation == 0)
		{
			geometry = geom1;
		}

		if (orientation == 1)
		{
			geometry = geom2;
		}

		if (orientation == 2)
		{
			geometry = geom3;
		}

		if (orientation == 3)
		{
			geometry = geom4;
		}

		orientation = ((orientation + 1) % 4);
	}
}

[zweibieren]

Any time two threads are updating one set of data trouble can arise.
Here gameObjects are modified both by the run() method on its thread
and the ControlListener running on the swing event thread.
These problems are avoided with synchronization: tutorial.

Other problems can arise from implementing a local timer loop such as the run() method.
You may have better success with the Swing timer for animations.

[Fubarable]

What are Oblock, Zblock, Tblock, Iblock, Sblock, etc...? Where are their classes?

[zweibieren]

One way to avoid synchronization problems is with a synchronized queue.
The user interface would put operations on the queue and the operations would be processed by the run() loop.

Examples of an operation: rotate, shift-left, shift-right. The ControlListener puts an Enum value on the queue each time the user types a key. The run() methods qhecks for operations on the queu, executes any it finds, and only then does its repaint.

For tetris, the best choice is a ConcurrentLinkedQueue.

[DeusExMachina90]

Thank you both for your responses. I'm sorry if my description of the problem is somewhat sketchy, I'm fairly new to java.


@ Fubarable: Those classes all represent different the parameters for the different tetris blocks, they're all polymorphs of Block. I didn't include them because I thought it would be redundant (there are 7 of them, all basically the same). If you want to try to compile and run the program, just limit the createBlock method to making Tblocks.

@ zweibieren: Thanks for the suggestion, I will read up on timers and look into using that instead. I'm still wondering though if that's the cause of the error (or the only cause, anyway). I say this because even before I started adding blocks to the board, I noticed that the drawing of the board contained more cell than I instructed it to draw. The way it looks to me, it seems as though some of the cells I have on the board don't actually correspond to Square objects, yet somehow a rectangle got drawn there anyway?

[zweibieren]

Consider paintComponent, which is called in both threads:

		gameboard[i][j].setX(xcoord);
		gameboard[i][j].setY(ycoord);
		xcoord += 20;
		page.setColor(gameboard[i][j].getColor());
		page.fillRect(gameboard[i][j].getX(), gameboard[i][j].getY(), 90, 90);

What happens if two different threads are executing this code?
Suppose xcoord is incremented by both threads.
What do we get? We get xcoord values greater than 400.
It would help to make xcoord a local variable.
But other more subtle problems will appear until variable access is solved.

And what about those setX and setY operations?
Doesn't each gameboard[i][j] element always get assigned the same X and Y?
Indeed, the coordinate can be computed from i and j,
so why does Block have x and y values.

The program is somewhat confusing in that Block and Square both have x and y,
but they are in different coordinate spaces. Removing X and Y from Square will help.

[Fubarable]

One of the reasons that your grid looks "funny" is that you start each new block 20 points from the previous, but each block is 90 points in width meaning that you're going to see a lot of overlap. For e.g.,

import java.awt.*;
import javax.swing.*;

@SuppressWarnings("serial")
public class SimpleGameBoard {
  protected static final int STEP = 20;
  protected static final int SIDE = 90;
  protected static final int ROWS = 20;
  protected static final int COLS = 10;

  private JPanel mainPanel = new JPanel(){
    protected void paintComponent(Graphics g) {
      super.paintComponent(g);
      for (int i = 0; i < ROWS; i++) {
        int y = STEP * i + STEP;
        for (int j = 0; j < COLS; j++) {
          int x = STEP * j + STEP;
          g.drawRect(x, y, SIDE, SIDE);
        }
      }
    }
  };

  public SimpleGameBoard() {
    mainPanel.setPreferredSize(new Dimension(320, 510));
  }

  public JComponent getComponent() {
    return mainPanel;
  }

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

  public static void main(String[] args) {
    java.awt.EventQueue.invokeLater(new Runnable() {
      public void run() {
        createAndShowUI();
      }
    });
  }
}

Also, please try to 1) separate your program logic more from your GUI with intelligent refactoring, and 2) avoid all those "magic" numbers.