3

I'm building a Java based game in Swing, which is essentially a grid of Jbuttons

I have an Object called Cell, which is a custom JButton with additional parameters for storing objects. The game grid is represented by Cell[][]

I have an arraylist of type Cell[][] to allow me to store the state of the gamegrid after each move. If I want to undo the move, I need to copy the last element of the ArrayList to the game grid to allow it to be displayed on the UI.

My gamegrid is panelHolder and my arraylist is moveHolder.

So far I've tried Collections.copy(panelHolder, moveHolder.get(moveHolder.size())); which will not compile due to the "arguments not being applicable for the type Cell[][]"

I've also tried System.arraycopy(moveHolder.get(moveHolder.size()-1), 0, panelHolder, 0, panelHolder.length);, which throws and out of bounds exception. Initially I thought this was due to the moveHolder.size()-1, but even just as moveHolder.size() it has the same problem.

I've found numerous questions on StackOverflow and others that both show these two ways of doing it, but I can't seem to get it to work. Is there something more obvious I'm missing? Full class method below:

 public class UndoClass implements MoveCommand{

    public ArrayList<Cell[][]> moveHolder = new ArrayList<Cell[][]>();

    public Cell[][] execute(Cell[][] panelHolder) {
        if (moveHolder.size() > 0){
            Collections.copy(panelHolder, moveHolder.get(moveHolder.size()));       
            if (moveHolder.size() > 0){
                moveHolder.remove(moveHolder.size());
            }
        }
        System.out.println("Move Undone. Undos available:" + moveHolder.size());
        return panelHolder;
    }
    public void addMove(Cell[][] panelHolder){
        moveHolder.add(panelHolder);
    }

    public ArrayList<Cell[][]> getMoves(){  
        return moveHolder;
    }
}

Cell Class

public class Cell extends JButton {

    int co_x = 0;
    int co_y = 0;


    ArrayList<Players> current = new ArrayList <Players>();

}
jmo
  • 357
  • 1
  • 4
  • 12
  • Can you also post the `Cell` class so that it will be easier to try out a solution at our end? – Chetan Kinger Jul 07 '15 at 16:03
  • 2
    Since panelHolder and moveHolder.get(n) are arrays, System.arraycopy is the way to go. Dump the length of the two arrays to check they are what you think they are. – JP Moresmau Jul 07 '15 at 16:03
  • @ChetanKinger edited to add `Cell` class – jmo Jul 07 '15 at 16:05
  • This doesn't answer your question, but I'd personally recommend using LinkedList instead of ArrayList here. – celticminstrel Jul 07 '15 at 16:06
  • @JPMoresmau they should both be `[4][4]` considering the contents of `moveHolder` is a copy of `panelHolder` but in a different state. I'll go through and have a look now though, just in case I've overlooked something – jmo Jul 07 '15 at 16:07
  • 2
    I'm thinking that your problem lies in the fact that this is an array of arrays. Does this answer help? http://stackoverflow.com/q/1564832/ – celticminstrel Jul 07 '15 at 16:10
  • @celticminstrel is correct, you can't straight up copy 2d arrays in Java, you need to copy each array in the 2d array – BoDidely Jul 07 '15 at 16:11
  • And you should make a copy of each individual `Cell`, too. Also, why is the `Cell` storing a list of players? And it probably shouldn't extend `JButton` if you're going to be storing it in your undo list. Maybe you should use some sort of `Point` class and store that in your undo list instead? – celticminstrel Jul 07 '15 at 16:17
  • @celticminstrel that seems to be what I'm going for. I'll have a look at that and see if i can get it to work or work it into my question – jmo Jul 07 '15 at 16:18

3 Answers3

1

Just wanted to point our your execute(...) method accepts the Cell[][] both as a parameter and the return argument. That approach is going to force all of your commands to keep copying your input param arrays to the return statement array. Notice if you don't need to keep the two in sync and you just use the return arg, you don't have to worry about copying at all:

Cell[][] lastState = moveHolder.get(moveHolder.size()-1);
moveHolder.remove(moveHolder.size()-1);
return lastState;  // Not updating the panelHolder array, just returning

But of course now the input parm and return are out of sync. Instead you might want to encapsulate that state into a single object to make your life easier. Something like this (note that the execute now returns a void):

public ArrayList<GameState> previousStates = new ArrayList<GameState>();

public void execute(GameState currentState) {
    if (previousStates .size() > 0) {
         GameState lastState = previousStates.get(previousStates.size()-1);
         currentState.restoreFrom(lastState);
         previousStates .remove(moveHolder.size()-1);
    }   
 }

Good luck on the game!

brariden
  • 124
  • 7
0
if (moveHolder.size() > 0) {
    for (int i = 0; i < panelHolder.length; i++) {
        panelHolder[i] = moveHolder.get(moveHolder.size()-1)[i].clone();   
    }
    moveHolder.remove(moveHolder.size()-1);
}

Try this. You need to make copies of each internal array when copying 2D arrays.

BoDidely
  • 504
  • 3
  • 13
  • @celticminstrel thanks, idk why but SO is the place I make the most typos – BoDidely Jul 07 '15 at 16:21
  • This works to copy! Hasn't entirely solved my problem (something wrong with my logic for adding the states at the moment), but this does exactly what I need it to – jmo Jul 07 '15 at 16:37
0

Try a Linked List

    LinkedList<Cell[][]> ll = new LinkedList();
    ll.removeLast();
    panelHolder = ll.clone();
Jan
  • 205
  • 3
  • 9