Spawn random objects without overlapping (Java)?

Question

I'm developing a game in Java, and part of it requires that objects spawn at the top of the screen and proceed to fall down. I have three objects that can possibly spawn, and three possible x coordinates for them to spawn at, all stored in an array called xCoordinate[].
One of the objects is of a class called Enemy, which inherits a class I have called FallingThings. In the FallingThings class, I have methods to generate new objects, my enemy method is below:

public static void generateNewEnemy() {
    xIndexEnemyOld = xIndexEnemy;
    xIndexEnemy = new Random().nextInt(3);
    if (delayTimer == 0) {
        while (xIndexEnemy == xIndexEnemyOld) {
            xIndexEnemy = new Random().nextInt(3);
        }
    }
    if (xIndexEnemy != xIndexMoney && xIndexEnemy != xIndexFriend) {
        Enemy enemy = new Enemy(xCoordinates[xIndexEnemy]);
        enemies.add((Enemy) enemy);
    } else {
        generateNewEnemy();
    }
}

xIndexEnemy represents the index of the xCoordinates array.
xIndexMoney and xIndexFriend are the indexes of the xCoordinates array for the two other objects (the comparisons with these values ensures that one object does not spawn directly on top of another).
The delayTimer variable represents the random delay between when new objects spawn, which was set earlier in my main class.
I store each instance of an Enemy object in an ArrayList.

Everything works except for the fact that sometimes, an object will spawn over itself (for example, the delay is 0, so two enemy objects spawn directly on top of each other, and proceed to fall down at the same speed at the same time).

I've been trying to crack this for the past two days, but I understand exactly why my code right now isn't working properly. I even tried implementing collision detection to check if another object already exists in the space, but that didn't work either.

I would be extremely grateful for any suggestions and ideas.

Answer 1

EDIT2

It seems that you still don't understand the problem with your function. It was addressed in the other answer but I'll try to make it more clear.

public static void generateNewEnemy() {
    xIndexEnemyOld = xIndexEnemy;

This is just wrong. You can't set the Old index without having actually used a new index yet.

    xIndexEnemy = new Random().nextInt(3);
    if (delayTimer == 0) {
        while (xIndexEnemy == xIndexEnemyOld) {
            xIndexEnemy = new Random().nextInt(3);
        }
    }

This is actually ok. You're generating an index until you get one that is different. It may not be the most elegant of solutions but it does the job.

    if (xIndexEnemy != xIndexMoney && xIndexEnemy != xIndexFriend) {
        Enemy enemy = new Enemy(xCoordinates[xIndexEnemy]);
        enemies.add((Enemy) enemy);
    } else {
        generateNewEnemy();
    }
}

This is your problem (along with setting the Old index back there). Not only do you have to generate an index thats different from the Old index, it must also be different from IndexMoney and IndexFriend.

Now, what happens if, for example, IndexOld = 0, IndexMoney = 1 and IndexFriend = 2? You have to generate an index that's different from 0, so you get (again, for instance) 1. IndexMoney is 1 too, so the condition will fail and you do a recursive call. (Why do you even have a recursive call?)

OldIndex was 0, and now in the next call you're setting it to 1. So IndexOld = 1, IndexMoney = 1 and IndexFriend = 2. Do you see the problem now? The overlapped index is now wrong. And the new index can only be 0 no matter how many recursive calls it takes.

You're shooting yourself in the foot more than once. The recursive call does not result in an infinite loop (stack overflow actually) because you're changing the Old index. (Which, again is in the wrong place)

That if condition is making it so the newly generated index cannot overlap ANY of the previous indexes. From what you said before it's not what you want.

You can simplify your function like this,

public static void generateNewEnemy() {
    xIndexEnemy = new Random().nextInt(3);
    if (delayTimer == 0) {
        while (xIndexEnemy == xIndexEnemyOld) {
            xIndexEnemy = new Random().nextInt(3);
        }
    }

    Enemy enemy = new Enemy(xCoordinates[xIndexEnemy]);
    enemies.add((Enemy) enemy);
    xIndexEnemyOld = xIndexEnemy;
    // Now that you used the new index you can store it as the Old one
}

Will it work? It will certainly avoid overlapping when the delayTimer is 0 but I don't know the rest of your code (nor do I want to) and what do you do. It's you who should know.

About my suggestions, they were alternatives for how to generate the index you wanted. I was assuming you would know how to fit them in your code, but you're still free to try them after you've fixed the actual problem.

---

Original Answer

Here's one suggestion.

One thing you could do is to have these enemies "borrow" elements from the array. Say you have an array,

ArrayList< Float > coordinates = new ArrayList< Float >();
// Add the coordinates you want ...

You can select one of the indexes as you're doing, but use the maximum size of the array instead and then remove the element that you choose. By doing that you are removing one of the index options.

int nextIndex = new Random().nextInt( coordinates.size() );
float xCoordinate = coordinates.get( nextIndex );
coordinates.remove( nextIndex ); // Remove the coordinate

Later, when you're done with the value (say, when enough time has passed, or the enemy dies) you can put it back into the array.

coordinates.add( xCoordinate );

Now the value is available again and you don't have to bother with checking indexes.

Well, this is the general idea for my suggestion. You will have to adapt it to make it work the way you need, specifically when you place the value back into the array as I don't know where in your code you can do that.

---

EDIT:

Another alternative is, you keep the array that you previously had. No need to remove values from it or anything.

When you want to get a new coordinate create an extra array with only the values that are available, that is the values that won't overlap other objects.

...
if (delayTimer == 0) {
    ArrayList< Integer > availableIndexes = new ArrayList< Integer >();
    for ( int i = 0; i < 3; ++i ) {
        if ( i != xIndexEnemyOld ) {
            availableIndexes.add( i );
        }
    }
    int selectedIndex = new Random().nextInt( availableIndexes.size() );
    xIndexEnemy = availableIndexes.get( selectedIndex );
}
// Else no need to use the array
else {
    xIndexEnemy = new Random().nextInt( 3 );
}
...

And now you're sure that the index you're getting should be different, so no need to check if it overlaps. The downside is that you have to create this extra array, but it makes your conditions simpler.

(I'm keeping the "new Random()" from your code but other answers/comments refer that you should use a single instance, remember that)

Answer 2

As I see, if delay == 0 all is good, but if not, you have a chance to generate new enemy with the same index. Maybe you want to call return; if delayTimer != 0?

UPDATED

Look what you have in such case: OldEnemyIndex = 1 NewEnemyIndex = random(3) -> 1 DelayTimer = 2

Then you do not pass to your if statement, then in the next if all is ok, if your enemy has no the same index with money or something else, so you create new enemy with the same index as previous