ArrayList select coordinates, if there are less clients than coordinates, give each client a coordinate

Question

    Coordinate[] coords = {
        new Coordinate(3093, 3630), new Coordinate(3095, 3632), new Coordinate(3098, 3633),
        new Coordinate(3101, 3633), new Coordinate(3104, 3631), new Coordinate(3106, 3629), 
        new Coordinate(3107, 3627), new Coordinate(3108, 3624), new Coordinate(3109, 3620),
        new Coordinate(3108, 3617), new Coordinate(3106, 3614), new Coordinate(3102, 3613),
        new Coordinate(3099, 3613), new Coordinate(3097, 3613), new Coordinate(3093, 3614),
        new Coordinate(3090, 3617), new Coordinate(3087, 3619)
    };


    int random = Misc.random(coords.length - 1);
    Coordinate coord = coords[random];
    boolean found = false;

    if (insidePlayers.size() < coords.length) {
        if (spawnPoints.contains(coord)) {
            found = false;
        }
        while (!found) {
            random = Misc.random(coords.length - 1);
            coord = coords[random];
            if (!spawnPoints.contains(coord)) {
                player.spawnPointX = coords[random].getX();
                player.spawnPointY = coords[random].getY();
                spawnPoints.add(coord);
                found = true;
                break;
            }
        }
    }
    else {
        player.spawnPointX = coords[random].getX();
        player.spawnPointX = coords[random].getY();
    }

Basically what I am trying to do here is, if there are more clients than available coordinates (spots), then give each player his own coordinate (So other clients can't have the same coords).

But unfortunately it doesn't work, sometimes clients are getting the same coordinates. Why is it happening? what did I do wrong?

Coordinate class:

public class Coordinate {

    private int x = 0;
    private int y = 0;

    public Coordinate(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }
}

ArrayList:

public static ArrayList<Coordinate> spawnPoints = new ArrayList<Coordinate>();

So what's wrong there?

Where's the loop to assign coordinates to more than one player? Also, what's `spawnPoints`? Some kind of `Set`? — Ted Hopp, Aug 09 '13 at 15:15
spawnPoint saves the coord X/Y of the client for future uses, the loop is outside of this code, basically loops through all clients, and runs the method startGame(Client player), so each player goes through this code. — Junatanm Degraded, Aug 09 '13 at 15:25

score 1 · Answer 1 · edited May 23 '17 at 11:50

1

You need to override equals() in your Coordinate class.

@override
public void equals(Object o){
if(o==null)
 return false;
if(!(o instanceof Coordinate)){
  return false;

Coordinate newO = (Coordinate) o;
if(this.x == newO.x && this.y == newO.y)
  return true

return false;
}

This is because ArrayList#contains uses ArrayList#indexOf() which is defined as:

public int indexOf(Object o) {
         if (o == null) {
             for (int i = 0; i < size; i++)
                 if (elementData[i]==null)
                     return i;
         } else {
             for (int i = 0; i < size; i++)
                 if (o.equals(elementData[i])) /// <--- Uses .equals()
                     return i;
         }
         return -1;
     }

Note: While you are at it also override hashCode() as well. You don't really need it in your case but its a good practice and will help you if you use hash based data structures. This will help.

edited May 23 '17 at 11:50

Community

1
1

answered Aug 09 '13 at 15:06

rocketboy

9,573
2
34
36

Why though? I am not using equals() ? Kinda new to java by the way ^^ – Junatanm Degraded Aug 09 '13 at 15:09
I've already tried doing the checking class-sided, and it didn't work, almost the same way as you, then tried this way I am using now and it didn't work aswell. What causing it to not work? – Junatanm Degraded Aug 09 '13 at 15:16
How are you populating `spawnPoints`? – rocketboy Aug 09 '13 at 15:22
@rocketboy - It's inside the inner `if`: a `Coordinate` is added when it is assigned to a `player`. – Ted Hopp Aug 09 '13 at 15:24

score 1 · Answer 2 · answered Aug 09 '13 at 15:21

So essentially you have a set of spawn points, and you want to spawn players on these points but no two players can have the same spawnpoint. A simpler way of doing this would be to remove a coordinate from the list once it has been given to a player. Not sure how you feed in the clients (players), please elaborate on that if this solution doesn't help.

 Coordinate[] coords = {
    new Coordinate(3093, 3630), new Coordinate(3095, 3632), new Coordinate(3098, 3633),
    new Coordinate(3101, 3633), new Coordinate(3104, 3631), new Coordinate(3106, 3629), 
    new Coordinate(3107, 3627), new Coordinate(3108, 3624), new Coordinate(3109, 3620),
    new Coordinate(3108, 3617), new Coordinate(3106, 3614), new Coordinate(3102, 3613),
    new Coordinate(3099, 3613), new Coordinate(3097, 3613), new Coordinate(3093, 3614),
    new Coordinate(3090, 3617), new Coordinate(3087, 3619)
};

public static List<Coordinate> coordinates = new ArrayList<>(Arrays.asList(coords));
public static final Random rnd = new java.util.Random();

if(!coordinates.isEmpty())
    int randomIndex = rnd.nextInt(coordinates.size());
    Coord randomCoord = coordinates.get(randomIndex);
    player.spawnPointX = randomCoord.getX();
    player.spawnPointY = randomCoord.getY();
    coordinates.remove(randomIndex);
else
    System.out.println("No more coordinates left to assign to player");

score 0 · Answer 3 · answered Aug 09 '13 at 16:52

I think the problem is a combination of two factors: 1) you are allocating a new coords array each time into the method; and 2) your Coordinate class does not implement equals() and hashCode(). The reason these cause a problem is that the default implementation of equals() and hashCode() inherited from Object are based on object identity, not equality of values. The result is that if, in one call, the Coordinate with values of (say) x=3093 and y=3630 is added to spawnPoints, on the next call, a new Coordinate object with the same x and y values will test as not contained in spawnPoints.

I think this can be fixed in one of two ways:

Declare the coords array to be a static final field instead of allocating a new array each time into the method. Then object identity will suffice for your logic.
Implement Coordinate.equals(Object) and Coordinate.hashCode() so that Coordinate objects with the same x and y field values test as equal and have the same hash code.

Even if the first approach fixes the problem, you should strongly consider implementing equals and hashCode, particularly if you are adding Coordinate objects to other collections.

ArrayList select coordinates, if there are less clients than coordinates, give each client a coordinate

3 Answers3