0

Right now I have a static ArrayList in my Object class, but both of the threads are accessing/changing data to that one list. I need an ArrayList that is shared between all instances of Object, but not between instances of Object that are in different threads. What's the best way to go about this? Here's the gist of what's going on:

public class Game extends Thread {

    private Actions actions;
    private Player whitePlayer;
    private Player blackPlayer;
    private String gameID;
    private boolean gameOver;
    private int movesTaken;
    private Command currentCommand;
    private Player winner;

    public ArrayList<Settlement> SettlementList = new ArrayList<Settlement>();
    public ArrayList<Hex> SettledHexes = new ArrayList<Hex>();

    public Game(String gameID){
        whitePlayer = new Player(PlayerSide.WHITE);
        blackPlayer = new Player(PlayerSide.BLACK);
        actions = new Actions(true);
        actions.setCurrentPlayer(whitePlayer);
        this.gameID = gameID;
        gameOver = false;
        movesTaken = 0;

    }

    public void run(){
        while(!gameOver){

            if(commandIsInQueue()){

                Message currentMessage = GameData.getIncomingMessages().poll();

                if (currentMessage.getPlayerID().equals(whitePlayer.getPlayerID())) {
                    actions.setCurrentPlayer(whitePlayer);
                }
                else if(currentMessage.getPlayerID().equals(blackPlayer.getPlayerID())){
                    actions.setCurrentPlayer(blackPlayer);
                }
                else{
                    endGame();
                }
                System.out.println("Move made by: " + actions.getCurrentPlayer().getPlayerID() + " " + "in game: " + gameID);
                setCurrentCommand(currentMessage.getCommand());
                executeCommand();
                checkEndGameConditions();
                mergeAllSettlement();
                movesTaken++;
            }

            try {
                Thread.sleep(1000000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

    private void executeCommand(){

        int x, y, z, orientation;
        TerrainType hexATerrain, hexBTerrain, terrainTypeExpansion;

        switch(currentCommand.getCommandType()){

            case ROTATE_TILE : rotate();
                break;
            case PLACE_TILE :
                x = currentCommand.getX();
                y = currentCommand.getY();
                z = currentCommand.getZ();
                orientation = currentCommand.getOrientation();
                hexATerrain = currentCommand.getHexATerrain();
                hexBTerrain = currentCommand.getHexBTerrain();
                try{
                    placeTile(x, y, z, orientation, hexATerrain, hexBTerrain);
                }catch (InvalidMoveException e){
                    endGame();
                }
                break;
            case FOUND_SETTLEMENT :
                x = currentCommand.getX();
                y = currentCommand.getY();
                z = currentCommand.getZ();
                try {
                    foundSettlement(x, y, z);
                } catch (InvalidMoveException e) {
                    endGame();
                }
                break;
            case PLACE_TOTORO :
                x = currentCommand.getX();
                y = currentCommand.getY();
                z = currentCommand.getZ();
                try {
                    placeTotoro(x, y, z);
                } catch (InvalidMoveException e) {
                    endGame();
                }
                break;
            case PLACE_TIGER :
                x = currentCommand.getX();
                y = currentCommand.getY();
                z = currentCommand.getZ();
                try {
                    placeTiger(x, y, z);
                } catch (InvalidMoveException e) {
                    endGame();
                }
                break;
            case EXPAND_SETTLEMENT :
                x = currentCommand.getX();
                y = currentCommand.getY();
                z = currentCommand.getZ();
                terrainTypeExpansion = currentCommand.getTerrainTypeExpansion();
                try {
                    expandSettlement(x, y, z, terrainTypeExpansion);
                } catch (InvalidMoveException e) {
                    endGame();
                }
                break;
        }
    }

    private void checkEndGameConditions(){

        if(whitePlayer.hasNoPiecesLeft()){
            winner = whitePlayer;
            endGame();
        }
        else if(blackPlayer.hasNoPiecesLeft()){
            winner = blackPlayer;
            endGame();

        }
        else if(whitePlayer.hasOnlyOneTypeOfPieceLeft()){
            winner = whitePlayer;
            endGame();
        }
        else if(blackPlayer.hasOnlyOneTypeOfPieceLeft()){
            winner = blackPlayer;
            endGame();

        }
        else if(actions.getBoard().getNumberOfTilesOnBoard() > 48){
            calculateWinnerByScore();
            endGame();
        }
    }

    private boolean commandIsInQueue(){
        try {
            if (GameData.getIncomingMessages().peek().getGameID().equals(null)) {
                return false;
            } else if (GameData.getIncomingMessages().peek().getGameID().equals(gameID)) {
                return true;
            } else {
                return false;
            }
        }
        catch (Exception e) {System.out.println("Error in commandIsInQueue() method..");}
        return false;
    }

    public void setCurrentCommand(Command command){
        currentCommand = command;
        movesTaken++;
    }

    public   void endGame(){
        System.out.println("end of: " + gameID);
        gameOver = true;
        //SEND END GAME MESSAGE
    }

    protected  void forfeit(Player player){
        if(player.getPlayerSide() == PlayerSide.BLACK){
            winner = whitePlayer;
        }
        else{
            winner = blackPlayer;
        }
        //SEND FORFEIT MESSAGE
    }

    public   void calculateWinnerByScore(){
        if(whitePlayer.getScore() > blackPlayer.getScore()){
            winner = whitePlayer;
        }
        else if(blackPlayer.getScore() > whitePlayer.getScore()){
            winner = blackPlayer;
        }
        else{
            if(whitePlayer.getTotoroAvailable() > blackPlayer.getTotoroAvailable()){
                winner = blackPlayer;
            }
            else if(blackPlayer.getTotoroAvailable() > whitePlayer.getTotoroAvailable()){
                winner = whitePlayer;
            }
            else if(whitePlayer.getTigerAvailable() > blackPlayer.getTigerAvailable()){
                winner = blackPlayer;
            }
            else if(whitePlayer.getTigerAvailable() < blackPlayer.getTigerAvailable()){
                winner = whitePlayer;
            }
        }
    }

    public   static Location convertCoordinates(int x, int y, int z){
        return new Location(x + 100, z + 100);
    }

    public   void rotate(){
        actions.invertTile();
        actions.rotateTile();
        actions.rotateTile();

        if(actions.getCurrentTile().getOrientation() == 6)  actions.getCurrentTile().setOrientation(1);
        else{
            int temp = actions.getCurrentTile().getOrientation() + 1;
            actions.getCurrentTile().setOrientation(temp);
        }
    }

    public   void placeTile(int x, int y, int z, int orientation, TerrainType terrainTypeA, TerrainType terrainTypeB) throws InvalidMoveException {
        Location coord = null;
        switch(orientation) {
            case 1:
                actions.setCurrentTile(new Tile(new Hex(TerrainType.VOLCANO), new Hex(terrainTypeA), new Hex(terrainTypeB)));
                actions.invertTile();
                actions.getCurrentTile().setOrientation(1);
                coord = convertCoordinates(x, y, z);
                break;
            case 2:
                actions.setCurrentTile(new Tile(new Hex(terrainTypeA), new Hex(terrainTypeB), new Hex(TerrainType.VOLCANO)));
                actions.getCurrentTile().setOrientation(2);
                coord = convertCoordinates(x + 1, y, z - 1);
                break;
            case 3:
                actions.setCurrentTile(new Tile(new Hex(terrainTypeB), new Hex(TerrainType.VOLCANO), new Hex(terrainTypeA)));
                actions.invertTile();
                actions.getCurrentTile().setOrientation(3);
                coord = convertCoordinates(x, y, z + 1);
                break;
            case 4:
                actions.setCurrentTile(new Tile(new Hex(TerrainType.VOLCANO), new Hex(terrainTypeA), new Hex(terrainTypeB)));
                actions.getCurrentTile().setOrientation(4);
                coord = convertCoordinates(x, y, z);
                break;
            case 5:
                actions.setCurrentTile(new Tile(new Hex(terrainTypeA), new Hex(terrainTypeB), new Hex(TerrainType.VOLCANO)));
                actions.invertTile();
                actions.getCurrentTile().setOrientation(5);
                coord = convertCoordinates(x - 1, y, z + 1);
                break;
            case 6:
                actions.setCurrentTile(new Tile(new Hex(terrainTypeB), new Hex(TerrainType.VOLCANO), new Hex(terrainTypeA)));
                actions.getCurrentTile().setOrientation(6);
                coord = convertCoordinates(x, y, z - 1);
                break;
            default:
                throw new InvalidMoveException("Invalid Orientation", 20);
        }

        actions.placeTile(coord);
    }

    public   void foundSettlement(int x, int y, int z) throws InvalidMoveException {
        Location coord = convertCoordinates(x, y, z);
        actions.foundSettlement(coord);
    }

    public   void placeTotoro(int x, int y, int z) throws InvalidMoveException {
        Location coord = convertCoordinates(x, y, z);
        actions.placeTotoro(coord);
    }

    public   void placeTiger(int x, int y, int z) throws InvalidMoveException {
        Location coord = convertCoordinates(x, y, z);
        actions.placeTiger(coord);
    }

    public  void expandSettlement(int x, int y, int z, TerrainType terrainType) throws InvalidMoveException {
        Location coord = convertCoordinates(x, y, z);
        actions.expandSettlement(coord, terrainType);
    }

    public void cyclePlayerTurn(){
        if(actions.getCurrentPlayer() == whitePlayer){
            actions.setCurrentPlayer(blackPlayer);
        }
        else{
            actions.setCurrentPlayer(whitePlayer);
        }
    }


    // deletes the current list of settlements and then re-merges each and adds them to a new list
    public void mergeAllSettlement()
    {
        // error if no settlements are on the board
        if(SettlementList.isEmpty()) return; //error?

        // create a list of Hex to be filled with Settled Hexes
        SettledHexes = new ArrayList<Hex>();
        for(int i=0; i<SettlementList.size(); i++)
        {
            for (int j=0; j<SettlementList.get(i).getSize(); j++)
            {
                SettledHexes.add(SettlementList.get(i).getHexInSettlementList().get(j));
            }
        }

        // Create a new SettlementList, deletes the old one
        SettlementList = new ArrayList<Settlement>();

        // Create a array of bool to check if that hex has been merged already
        boolean[] isChecked = new boolean[SettledHexes.size()];

        for (int i=0; i<SettledHexes.size(); i++)
        {
            if(isChecked[i] == false)
            {
                isChecked[i] = true;
                SettledHexes.get(i).setParentSettlement(new Settlement());
                SettledHexes.get(i).getParentSettlement().addSettlement(SettledHexes.get(i));

                merger(SettledHexes, isChecked, i);
            }
        }
    }

    public void merger( ArrayList<Hex> SettledHexes, boolean[] isChecked, int current_position)
    {
        for (int j = 0; j < SettledHexes.size(); j++) {
            if (isChecked[j]) continue;

            Hex start_point = SettledHexes.get(current_position);
            Hex possible_adj = SettledHexes.get(j);

            Location location_start = start_point.getLocationOfHex();
            Location location_possAdj = possible_adj.getLocationOfHex();

            if ((location_possAdj.getX() == location_start.getX() && location_possAdj.getY() == (location_start.getY() - 1))
                    || (location_possAdj.getX() == (location_start.getX() + 1) && location_possAdj.getY() == (location_start.getY() - 1))
                    || (location_possAdj.getX() == (location_start.getX() + 1) && location_possAdj.getY() == location_start.getY())
                    || (location_possAdj.getX() == location_start.getX() && location_possAdj.getY() == (location_start.getY() + 1))
                    || (location_possAdj.getX() == (location_start.getX() - 1) && location_possAdj.getY() == (location_start.getY() + 1))
                    || (location_possAdj.getX() == (location_start.getX() - 1) && location_possAdj.getY() == location_start.getY()))
            {
                if(start_point.getOwner() == possible_adj.getOwner())
                {
                    isChecked[j] = true;
                    SettlementList.get(SettlementList.size() - 1).addSettlement(SettledHexes.get(j));
                    merger(SettledHexes, isChecked, j);
                }
            }
        }

        return;
    }


    public Actions getActions(){
        return actions;
    }

    public Player getWhitePlayer(){
        return whitePlayer;
    }

    public Player getBlackPlayer(){
        return blackPlayer;
    }

    public void setGameID(String gid){
        gameID = gid;
    }

    public String getGameID(){
        return gameID;
    }

    public Command getCurrentCommand(){
        return currentCommand;
    }

    public boolean isGameOver(){
        return gameOver;
    }

    public Player getWinner(){
        return winner;
    }

    public ArrayList<Settlement> getSettlementList(){
        return SettlementList;
    }

    public ArrayList<Hex> getSettledHexes(){
        return SettledHexes;
    }
}    


public class Settlement {

    private ArrayList<Hex> HexInSettlementList;
    private int size;
    private boolean hasTotoro;
    private boolean hasTiger;
    private Game parentGame;

    public Settlement(){
        if(Thread.currentThread().getName() == null){
            parentGame = new Game(" ");
        }
        else if(Thread.currentThread().getName() == GameData.getGameOne().getGameID() ){
            parentGame = GameData.getGameOne();
        }
        else if(Thread.currentThread().getName() == GameData.getGameTwo().getGameID()){
            parentGame = GameData.getGameTwo();
        }
        else {
            parentGame = new Game(" ");
        }
        parentGame.getSettlementList().add(this);
        HexInSettlementList = new ArrayList<Hex>();
        size = 0;
        hasTotoro = false;
        hasTiger = false;
    }

    public void addSettlement(Hex hex){
        hex.setParentSettlement(this);
        this.HexInSettlementList.add(hex);
        if(hex.hasTotoro()) this.hasTotoro = true;
        if(hex.hasTiger()) this.hasTiger = true;
        this.size++;
    }

    public static void removeSettlement(Hex hex){
        if(hex.isSettled())
        {
            hex.getParentSettlement().size--;
            hex.getParentSettlement().getHexInSettlementList().remove(hex);
        }
        else return; //error?
    }


    // These set functions are used for testing purposes
    public int setSize(int size){ return this.size = size; }
    public boolean setHasTotoro(boolean hasTotoro) { return this.hasTotoro = hasTotoro; }
    public boolean setHasTiger(boolean hasTiger) { return this.hasTiger = hasTiger; }


    public int getSize(){ return size; }
    public boolean getHasTotoro() { return hasTotoro; }
    public boolean getHasTiger() { return hasTiger; }
    public ArrayList<Hex> getHexInSettlementList() { return HexInSettlementList; }
}
a.sapp
  • 41
  • 1
  • 9
  • 1
    Please show us some code that demonstrates the problem. – markspace Apr 09 '17 at 20:00
  • Your question also is somewhat confusing since Objects are not "in" threads, and a single object can have its methods called on several threads. – Hovercraft Full Of Eels Apr 09 '17 at 20:03
  • Updated my post. The short version of the problem is I have a game that multiple instances of are run simultaneously. Each game needs to have a list of settlements in it, but that list is being shared between both games. So moves in one game effect the other game. – a.sapp Apr 09 '17 at 20:06
  • I don't really figure out what is the problem... Can you provide external details (calls, external classes, thread launchs...) ? And it is not recommended to use an ArrayList in multi-thread configuration. You can pick a class from java.util.concurrent package (https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/package-summary.html). Just be aware of complexity (access/remove/search). It is more safe (but global safety depends on your whole code). – cactuschibre Apr 09 '17 at 20:34
  • Each game has a list of Settlements that are present on the game board. These lists need to be exclusive to each game. Every time a settlement is created, it adds itself to this list. The issue is that the list is being shared between games, when it should be exclusive to each one. – a.sapp Apr 09 '17 at 20:42
  • You still really need to show code that demonstrates that problem. Trying to explain it with words doesn't get the point across. I personally can't figure out what you're trying to actually do. Objects are either separate or shared, there's no way for them to be both. – markspace Apr 09 '17 at 20:46
  • You need to put listOfCurrentSettlements as a non-static element in order to create two different objects. Then, use a public getter to access it in the game. – cactuschibre Apr 09 '17 at 20:49
  • I think I figured it out. I took the list of settlements out of the Settlement class, along with the merge methods, placed them in the Game class(of which only one instance is created per game) and made it non static. Looks like it's working for the most part, just debugging my unit tests right now to account for the change. – a.sapp Apr 09 '17 at 21:17
  • @markspace, I apologize for being vague. I tried to dumb down the classes that I posted so no one had to wade through a few hundred lines of code. I can post both the class files if that would help though. Word of warning though: this is a group project so they're quite messy and unclean. – a.sapp Apr 09 '17 at 21:21

2 Answers2

0

I would avoid using Static.

Exactly as you stated, by making the ArrayList Static, it's shared between all of your threads. This is a problem if you want to run entirely separate, parallel instances of your code (or game) on each thread.

Try approaching this from a Design perspective.

Instead of trying to make your listOfCurrentSettlements static, I'd suggest having each instance of your game instantiate their own copy. There are a couple ways of doing this, but nearly all of them sacrifice some of the benefits and shortcuts provided by using static variables. One solution would be to pass this down to your Settlement classes through their constructors:

public class Settlement {

private ArrayList<Settlement> myListOfCurrentSettlements;

public Settlement(ArrayList<Settlement> currentSettlements){
     myListOfCurrentSettlements = currentSettlements;
     mylistOfCurrentSettlements.add(this);
}

or through an additional method:

public class Settlement {

private ArrayList<Settlement> myListOfCurrentSettlements = new ArrayList<>();

public Settlement(){
    // ...
}

public void setCurrentSettlements(ArrayList<Settlement> currentSettlements) {
    myListOfCurrentSettlements = currentSettlements;
    myListOfCurrentSettlements.add(this);
}

This comes at an obvious cost: your mergeSettlements() method can no longer be static, and now anytime you want to instantiate a Settlement in your code, you have to first make sure you're carrying around an instance of your listOfCurrentSettlements somewhere so you can pass it down. These aren't inherently bad things, but they are design problems for you to solve.

Whether or not you choose to rewrite your code is a design decision that is up to you, but note that static variables often make it difficult to expand upon on your code in certain situations (like this one). As Viktor suggested, you could always use ThreadLocal, but much like static variables, be mindful of the risks and consequences...

Community
  • 1
  • 1
  • Thanks! I really appreciate the in depth explanation. In the end I decided it would be easiest to pull the merge functionality out of the Settlement class and place it in Game (per thread there is only one instance of Game, while each thread may contain dozens of Settlement instances). So not it's non static inside Game and working great. – a.sapp Apr 10 '17 at 00:54
  • You're welcome! That sounds pretty close to something I wanted to suggest, but I wasn't quite sure how to without knowledge of the rest of your system. Good luck with your game! – DalenWBrauner Apr 10 '17 at 03:10
-1

Try to make your list ThreadLocal. In this case you will have one instance per thread

  • This is the exact opposite of what he is trying to achieve. – Hovercraft Full Of Eels Apr 09 '17 at 20:17
  • And improper use of ThreadLocal can cause severe memory leaks. – cactuschibre Apr 09 '17 at 20:26
  • As I understood he wants to avoid situation when one game( thread) affect another game(second thread). To achieve such requirements each thread should have own list. ThreadLocal variable can be used for that. ThreadLocal owned by thread. It means that gc will remove it after thread death( of course except famous issues with PermGen ileaks n web servers) – Viktor Chesnokov Apr 09 '17 at 20:54