Thread: Random Colors Problem

Alright so basically what I'm trying to do is create a 16x16 square board of tiles (256 total) and when I run the program I need it to set up the tiles randomly into the following colors
39 yellow tiles
46 red tiles
34 blue tiles
52 green tiles
41 magenta tiles
44 cyan tiles

When I run the code it sets the entire board to CYAN. I understand why it does that because it's the last for loop in my code but I have no idea how to fix it.

Heres the GUI class

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Container;
import java.awt.Dimension;
import java.awt.FlowLayout;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;
import java.util.Scanner;
 
import javax.swing.JButton;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;
import javax.swing.JPanel;
 
public class GameBoard extends JFrame{
	Tile[][] myTiles;
	private int numRows = 16, numCols = 16;
	//private MyEventHandler eh = new MyEventHandler();
 
	public static void main(String[] args) {
		new GameBoard();
	}
 
	public GameBoard() {
		myTiles = new Tile[numRows][numCols];
		setSize(800,800);
 
		Container c = getContentPane();
		c.setLayout(new BorderLayout());
 
		setDefaultCloseOperation(EXIT_ON_CLOSE);
		setTitle("Random Colors");
 
		JPanel tilePanel = buildTilePanel();
		c.add(tilePanel, BorderLayout.CENTER);
 
		setVisible(true);
	}
 
	private JPanel buildTilePanel() {
		JPanel tilePanel = new JPanel();
		tilePanel.setBackground(Color.BLACK);
		tilePanel.setPreferredSize(new Dimension(800, 800));
		tilePanel.setLayout(new GridLayout(numRows, numCols));
 
		for (int i = 0; i < myTiles.length; i++)
			for (int j = 0; j < myTiles[0].length; j++) {
				myTiles[i][j] = new Tile();
				myTiles[i][j].setColors();
				myTiles[i][j].shuffle();
				//myTiles[i][j].addActionListener(eh);
				tilePanel.add(myTiles[i][j]);
			}
		return tilePanel;
	}
}

and heres the tiles class

import java.awt.Color;
import java.awt.Dimension;
import javax.swing.JButton;
 
public class Tile extends JButton{
 
	private Tile[] tiles = new Tile[256];
 
 
	public Tile() {
		setPreferredSize(new Dimension(10, 10));
	}
 
	public void setColors() {
		for(int i = 0; i < 39; i++){
			setBackground(Color.YELLOW);
			paintImmediately(0, 0, getWidth(), getHeight());
 
		}
 
		for(int i = 39; i < 85; i++){
			setBackground(Color.RED);
			paintImmediately(0, 0, getWidth(), getHeight());
		}
 
		for(int i = 85; i < 119; i++){
			setBackground(Color.BLUE);
			paintImmediately(0, 0, getWidth(), getHeight());
		}
 
		for(int i = 119; i < 171; i++){
			setBackground(Color.GREEN);
			paintImmediately(0, 0, getWidth(), getHeight());
		}
 
		for(int i = 171; i < 212; i++){
			setBackground(Color.MAGENTA);
			paintImmediately(0, 0, getWidth(), getHeight());
		}
 
		for(int i = 212; i < 256; i++){
			setBackground(Color.CYAN);
			paintImmediately(0, 0, getWidth(), getHeight());
		}
	}
 
	public void shuffle() {
		int r = tiles.length;
		for(int i = 0; i < tiles.length; i++){
			Tile t = tiles[i];
			tiles[i] = tiles[r-1];
			tiles[r-1] = t;
		}
	}
}

I've also tried adding tiles[i]. in front of setBackground but it threw a nullpointer so I'm just stuck..Any help is greatly appreciated. Thanks!

Originally Posted by JonLane

I don't know much about GUI but I think I see a problem.

public void setColors() {

your code goes through 256 for loops, so that increments desirably however,
each time placing it over the exact same location unless paintImmediately parameters "getWidth()" & "getHeight()" increment.
(And getters methods should never do this!)
Thus each object in your array is painted 256 times always ending in cyan.

Before I tell you how to fix it, first consider what is the job of the method setColors.(seriously... take 30 seconds )


myTiles[i][j].setColors();

This method call wants to take a specific tile (located somewhere already in your array) and color it.
nothing illegal about this, but there is a contradiction in your ideas.
The method code on the other-hand looks like it wants an array to fill all on its own(almost)

you can either keep the tiles accessing the method one at a time and then change the method to some how keep track of what color to put in this one
(use if statements instead of for loops)

or have the method work on the entire array. (this is a little more work for your current code, but I feel is more proper)
Pass the method the entire array, and change your for loops to if statements, then wrap them in 1 for loop that starts at i and goes through to the 256.

Hope this helps,
Jonathan


}

so you're basically saying something like this

public void setColor() {
		for(int i = 0; i < tiles.length; i++)
			if(tiles[i] == null){
				setBackground(Color.YELLOW);
				paintImmediately(0, 0, getWidth(), getHeight());
			}

but obviously I need some kind of code to limit yellow to be set to only 39 tiles. how would I go about doing that?

edit: or maybe something like this?

public void setColor() {
		tiles = new Tile[numRows];
		for(int numRows = 0; numRows < tiles.length; numRows++)
			if(numRows < 39){
				setBackground(Color.YELLOW);
				paintImmediately(0, 0, getWidth(), getHeight());
			}
 
 
	}

Originally Posted by captain

edit: or maybe something like this?

public void setColor() {
		tiles = new Tile[numRows];
		for(int numRows = 0; numRows < tiles.length; numRows++)
			if(numRows < 39){
				setBackground(Color.YELLOW);
				paintImmediately(0, 0, getWidth(), getHeight());
			}
 
	}

I assume the end product you want to be a 2d array. The current method structure will work for a 1d array.

but getting to your new code,
when you call: myTiles[i][j].setColors();

you are using an object reference to access the method, this means your method already knows exactly which tile you want it to fill.
(not sure if that is needed, because I have no idea how "setBackground(Color.YELLOW);" works. But if it does indeed fill something then no need to change anything.)

All you need to do is write your method to choose what color you want, then to fill it with that color.

Something like:

public void setColor(int chooser) {
		for(int idx = 0; idx < chooser; idx++)
			if(idx < 39){
				setBackground(Color.YELLOW);
			}
			else if(idx < ????){
				setBackground(Color.????);
			}
			else {
				setBackground(Color.????);
			}
 			paintImmediately(0, 0, getWidth(), getHeight()); //NOTE: this can be outside the if statements!
	}

so I have added a parameter your method requires, now you have to consider how to get that to be the right number

when you call: myTiles[i][j].setColors( //TODO );

use i and j to create a non repeating sequence from 1 to 256 or 0 to 255 if that is easier.
This sequence represents the unique ID of the current tile.

Also, I am interested in using your project to learn more about GUI's. If you didn't mind posting the entire thing.(before or after it compiles) I would like to play around with some things for my own understanding.

Thanks,
Jonathan

Here I did some of it for you. You need to finish out the logic of how to set colors but you should get the idea. Really you do not even need the Tile class but I left it in because I am assuming you will be doing more with it later. Make sure you understand what I did and if you have any questions let me know.

Your shuffle function never would have worked because you didn't add anything to the array on your Tile class. That is probably where you are throwing your null pointer. And it does not make sense that you have an array of tiles in that class, it should have been on your game board class. But really you did not need it because you already had all of your tiles stored in myTiles.

Also, you were shuffling and adding your tiles to the gameboard before they were all created. You need to make all your tiles and then shuffle then add.

import java.awt.Color;
import java.awt.Dimension;
import java.awt.GridLayout;
import java.util.ArrayList;
import java.util.Collections;
 
import javax.swing.JFrame;
import javax.swing.JPanel;
 
public class GameBoard extends JPanel
{
	private static final int YELLOW = 39;
	private static final int RED = YELLOW+46;
	private static final int BLUE = RED+34;
	private static final int GREEN = BLUE+52;
	private static final int MAGENTA = GREEN+41;
	private static final int CYAN = MAGENTA+44;
 
	private ArrayList<Tile> tiles;
	private int numRows = 16;
	private int numCols = 16;
	private int numTiles = numRows*numCols;
 
 
	public GameBoard() {
		tiles = new ArrayList<Tile>();
		setSize(800,800);
		buildTilePanel();
	}
	private void buildTilePanel() {
		this.setBackground(Color.BLACK);
		this.setPreferredSize(new Dimension(800, 800));
		this.setLayout(new GridLayout(numRows, numCols));
 
		for (int i = 0; i < numTiles; i++)
		{
			System.out.println("here: "+i);
				tiles.add(new Tile());	
				tiles.get(i).setColor(getColor(i));
				//myTiles[i][j].addActionListener(eh);
		}
			Collections.shuffle(tiles);
 
			for(Tile t: tiles)
			{
				this.add(t);
			}
	}
	private Color getColor(int color)
	{
		Color rtn = Color.BLACK;
		if(color<YELLOW)
		{
			rtn = Color.YELLOW;
		}
		else if(color<RED)
		{
			rtn = Color.RED;
		}
		else if( color<BLUE)
		{
			rtn = Color.BLUE;
		}
		//...Do the rest of the logic here
 
		return(rtn);
	}
 
	public static void main( String args[] )
	{
		JPanel baseView = new GameBoard();
		baseView.setPreferredSize(new Dimension(800,800));
		baseView.setOpaque(false);
		JFrame frame = new JFrame();
		frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		frame.getContentPane().add(baseView);
		frame.setSize(800,800);
 
		frame.setVisible(true);
		frame.repaint();
	}
}

import java.awt.Color;
import java.awt.Dimension;
 
import javax.swing.JButton;
 
public class Tile extends JButton
{
	public Tile() {
		super();
		setPreferredSize(new Dimension(10, 10));
	}
 
	public void setColor(Color c)
	{
		this.setBackground(c);
	}
}

**EDIT**
also this may or may not be the best way to do this. That is for you to decide and modify where needed