List copies final element to all other elements when converting from image to GameObject

Question

I am working on a small project using Java and Swing. I'm making a very, very small and simple RPG that is in the style of Earthbound for mainly it's combat style. Alongside this project, I'm working on a framework to make it easier to create games of a similar genre and feel (i.e. screen, game loop, state manager, audio manager, etc). The issue I'm running into is that the "image to game object" converter class I made is overwriting the list with the final game object that is created. I am converting from a PNG to a GameObject (class created within the framework).

I diagnosed this by printing out the position and type of all the elements in the list to receive the same X, Y, and types for each element. I checked to make sure the HashMap I passed through was setup correctly and that each tile type was added correctly. Below is the relevant section of code to the top along with it's output:

private void init() {
    tileMap.put(new Color(0,255,0), new Tile(16,16, TileType.GRASS));
    tileMap.put(new Color(250,255,0), new Tile(16,16, TileType.SAND));
    tileMap.put(new Color(135,135,135), new Tile(16,16, TileType.ROCK));
    tileMap.put(new Color(255,0,233), new Tile(16,16, TileType.DIRT_PATH_SIDE));

    for(Color t : tileMap.keySet()) {
        System.out.println(((Tile) tileMap.get(t)).getTileType());
    }
....
}

----- OUTPUT -----
GRASS
ROCK
DIRT_PATH_SIDE
SAND

The image loads correctly and the HashMap's values are added correctly so I decided to see where in the framework the issue was. I checked the position when it was being set and it was fine (ie (0,0), (0,16), (0,32)...) until I looped through the finished list where the game objects in the list are all the same.

I know that static variables cause an issue but the only "static" variable I am using is the enums to denote what the tile type is. I used an integer just to see what would happened and yielded the same result. I took the final list, made it a variable the class could read as opposed to just the function, and tried to see if that would work but still was just the final game object created after reading.

I've checked over SO and other websites but nothing is working. I fear that I looked at this problem the wrong way as I was trying to implement a solution I used in Unity/C# here but probably had a disconnect somewhere along the way.

Here is the full function from earlier:

private void init() {
    tileMap.put(new Color(0,255,0), new Tile(16,16, TileType.GRASS));
    tileMap.put(new Color(250,255,0), new Tile(16,16, TileType.SAND));
    tileMap.put(new Color(135,135,135), new Tile(16,16, TileType.ROCK));
    tileMap.put(new Color(255,0,233), new Tile(16,16, TileType.DIRT_PATH_SIDE));

    for(Color t : tileMap.keySet()) {
        System.out.println(((Tile) tileMap.get(t)).getTileType());
    }

    try {
        im.loadImage();
    } catch (IOException e) {
        e.printStackTrace();
    }

    im.readImage(tileMap);

    objects = im.getMap(); // objects is a list within the class that contains all Tile game objects.

}

And the converter class's relevant code:

public void readImage(HashMap<Color, GameObject> tileDict) {
    for(int x=0; x<img.getWidth(); x++) {
        for(int y=0; y<img.getHeight(); y++) {
            GameObject g = determineObject(tileDict, x, y);
            if(g != null) {
                map.add(g);
            }
        }
    }
}

private GameObject determineObject(HashMap<Color, GameObject> tileDict,
        int x, int y) {
    GameObject g = null;
    for(Color c : tileDict.keySet()) {
        if(c.getRGB() == img.getRGB(x, y)) {
            g = tileDict.get(c);
            g.setXPos(x*g.getWidth());
            g.setYPos(y*g.getHeight());
        }
    }
    return g;
}

--- EDIT: 8/10/2017 ---

I am sorry for not clarifying what the structure looks like. GameObject is an abstract class within the framework I've made that is extended by Tile in the game. As well, TileType is an enum type that I created within the game and not the framework. The framework is in a separate jar/project so it is unaware of what extends GameObject and what TileType is.

--- EDIT 2: 8/10/2017 ---

I tried implementing a solution using the Serializableclass but that didn't work. I followed the example from this question. The copy() function I created in the abstract GameObject looks like this:

public final GameObject copy() {
    try {
        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        new ObjectOutputStream(baos).writeObject(this);
        return (GameObject) new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray())).readObject();
    } catch (Exception e) {
        throw new AssertionError(); 
    }
}

I have also tried this as an abstract method and wrote the function in Tile but there is no current result. I'll keep mucking around here to see if anything comes up.

Resources

• Java ArrayList.add() replaces all elements in array with the final element
• List overriding objects java [closed]
• Add elements to Arraylist and it replaces all previous elements in Java [duplicate]
• Why does my ArrayList contain N copies of the last item added to the list?

Answer 1

I think, the problem here is, that you get the GameObject from the map and then modify it. So if you get the same GameObject twice, it gets modified two times and also gets added to the list twice. To have different GameObject's, you have to create a new one in determineObject().

BTW: it's very confusing to name a list map.

Answer 2

If I got it correctly tileMap is a static member of your class and you pass it to the readImage and thus to the determineObject method?!

In determineObject you change the xPos and yPos attributes of your objects in your static tile dictionary:

g = tileDict.get(c);
g.setXPos(x*g.getWidth());
g.setYPos(y*g.getHeight());

That is you modify the attributes of an entry in the global map.

You will need to create a copy of the GameObject instance you get from the map and continue with the copy instead of modifying the original object. Your code should do something like this:

GameObject globalGameObject = tileDict.get(c);
g = globalGameObject.createCopy();
g.setXPos(x*g.getWidth());
g.setYPos(y*g.getHeight());

Where createCopy() is a new method that creates a copy of the current instance. Each implementation of GameObject has to implement this method:

public abstract class GameObject {
  ...
  public abstract GameObject createCopy();
  ...
}

You could use the Cloneable feature of Java to create the copies for you as well, but Cloneable is easy to use wrong and I've actually not seen anyone using it for real life code till now.

Answer 3

I think your map should containe TileTypes instead of Tiles:

tileMap.put(new Color(0,255,0), TileType.GRASS);
tileMap.put(new Color(250,255,0), TileType.SAND);
tileMap.put(new Color(135,135,135), TileType.ROCK);
tileMap.put(new Color(255,0,233), TileType.DIRT_PATH_SIDE);

then method determineObject would become:

private GameObject determineObject(HashMap<Color, GameObject> tileDict,
        int x, int y) {
    GameObject g = null;
    for(Color c : tileDict.keySet()) {
        if(c.getRGB() == img.getRGB(x, y)) {
            TileType type = tileDict.get(c);
            g = new Tile(x, y, type);
            g.setXPos(x*g.getWidth());
            g.setYPos(y*g.getHeight());
            // BTW, how about a break here?
        }
    }
    return g;
}

Or even:

private GameObject determineObject(HashMap<Color, GameObject> tileDict,
        int x, int y) {
    GameObject g = null;
    Color c = new Color(img.getRGB(x, y));
    TileType type = tileDict.get(c);
    if (type != null) {
        g = new Tile(x, y, type);
        g.setXPos(x*g.getWidth());
        g.setYPos(y*g.getHeight());
    }
    return g;
}

UPDATE

I forgot to multiply x and y by the tile width. With my approach, you cannot use g.getWidth() and g.getHeight(), but I think in your case their values are both 16. Ill will add constants:

private static final int TILE_WIDTH = 16;
private static final int TILE_HEIGHT = 16;

and the method determineObject becomes:

private GameObject determineObject(HashMap<Color, GameObject> tileDict,
        int x, int y) {
    GameObject g = null;
    Color c = new Color(img.getRGB(x, y));
    TileType type = tileDict.get(c);
    if (type != null) {
        g = new Tile(TILE_WIDTH, TILE_HEIGHT, type);
        g.setXPos(x*TILE_WIDTH);
        g.setYPos(y*TILE_HEIGHT);
    }
    return g;
}

Answer 4

Alright so I found out the issue. While doing research on dpr's and Claus Radloff's answers, I came across the solution here. Seeing as in my case I was using an abstract class, I could not do what both Claus and dpr recommended per-say as you can't instantiate an abstract class. In order to solve this, I made the abstract class implement the Serializable class as I said in my question. I created the abstract method GameObject copy(); which the objects extending from this can implement or ignore (return null). From here, only one line of code is required inside said method:

return new Tile(this.width, this.height, this.type);

A big thank you to those that guided me in the right direction! I apologize again for not clarifying my structure in my initial post.