1
\$\begingroup\$

I am using Android API's and using the RectF class for rectangles. I have a list of blocks that are on the screen at one time. Each block is given a random position, and then I go through all of the tiles previous, to check if there is an intersection. If there is, it recursively calls the method again to get a new random position and check it against all the other tiles.

I feel like this should work perfectly, except for one thing that may be affecting it. Since recursion opens up so many instances of a method, it needs to finish them off, so I don't know if the rectangle is just being drawn at the initial position, and the rest of the calls are doing nothing?

I have tried quite a few different methods and this seems to be the closest, but something weird is happening and I cant figure it out.

Start method:

    private void start() {

    timerTask = new TimerTask() {
        @Override
        public void run() {

            if(Game.STATE == Game.State.Playing) {                  
                removeFailedTiles();

                Random deathRate = new Random();
                int i = deathRate.nextInt(100);
                if(i % 10 == 0) {
                    System.out.println("Death tile");
                    activeDeathTiles.add(new DeathTile(getRandomPosition()));
                    //System.out.println(activeDeathTiles.toString());
                } else {
                    System.out.println("Normal Tile");
                    activeTiles.add(new Tile(tileLifetime, getRandomPosition()));
                }

            }

            if(Game.STATE == Game.State.Lost) {
                timerTask.cancel();
                timer.cancel();
            }
        }
    };

    timer.schedule(timerTask, 1000, tileDelay);

}

Get Random Position method:

private RectF getRandomPosition() {
    int screenWidth = (int) Game.SCREEN_WIDTH;
    int screenHeight = (int) Game.SCREEN_HEIGHT;

    RectF pos = new RectF();

    Random random = new Random();
    int maxX = (int) (screenWidth - (Tile.TILE_WIDTH * 1.5f)), minX = (int) (Tile.TILE_WIDTH / 1.5f);
    int maxY = (int) (screenHeight - (Tile.TILE_HEIGHT * 1.5f)), minY = (int) (Tile.TILE_HEIGHT / 1.5f);

    pos.top = random.nextInt(maxY - minY) + minY;
    pos.left = random.nextInt(maxX - minX) + minX;
    pos.bottom = pos.top + Tile.TILE_HEIGHT;
    pos.right = pos.left + Tile.TILE_HEIGHT;

    if(isOverlap(pos)) {
        return getRandomPosition();
    } else {
        return pos;
    }
}

Helper methods:

private List<RectF> getAllTiles() {
    List<RectF> allTiles = new ArrayList<RectF>();

    for(RectF t : activeDeathTiles) 
        allTiles.add(t);
    for(RectF t : activeTiles) 
        allTiles.add(t);

    return allTiles;
}

private boolean isOverlap(RectF position) {

    for(RectF r : getAllTiles()) {
        if(RectF.intersects(position, r)) {
            return true;
        }
    }

    return false;
}

If someone could please help me figure out what is going wrong here that would be stellar!

\$\endgroup\$
  • \$\begingroup\$ What exactly is going wrong with this code? \$\endgroup\$ – MadEqua Jun 21 '15 at 15:08
  • \$\begingroup\$ The blocks are still placed on top of each other, although im saying if there is a collision to get a new position \$\endgroup\$ – Kyle Jensen Jun 21 '15 at 15:30
  • \$\begingroup\$ Please accept the answer given by MadEqua if this has solved your problem, by hitting the checkmark. Thanks. \$\endgroup\$ – Engineer Oct 22 '15 at 10:14
1
\$\begingroup\$

All of your code seems fine exept for this typo on getRandomPosition():

pos.right = pos.left + Tile.TILE_HEIGHT;

which should be

pos.right = pos.left + Tile.TILE_WIDTH;

I'm not sure if that can be the root cause of the problem, but there's always the option of not using recursion and use a loop, should be easier to debug:

private RectF getRandomPosition() {
    RectF pos = new RectF();

    int screenWidth = (int) Game.SCREEN_WIDTH;
    int screenHeight = (int) Game.SCREEN_HEIGHT;

    Random random = new Random();
    int maxX = (int) (screenWidth - (Tile.TILE_WIDTH * 1.5f)), minX = (int) (Tile.TILE_WIDTH / 1.5f);
    int maxY = (int) (screenHeight - (Tile.TILE_HEIGHT * 1.5f)), minY = (int) (Tile.TILE_HEIGHT / 1.5f);

    do {
        pos.top = random.nextInt(maxY - minY) + minY;
        pos.left = random.nextInt(maxX - minX) + minX;
        pos.bottom = pos.top + Tile.TILE_HEIGHT;
        pos.right = pos.left + Tile.TILE_WIDTH;
    }
    while(isOverlap(pos));

    return pos;
}

Bear in mind that if the screen gets too full the loop may not be able to finish. To fix it, something like a maximum number of loops before giving up can be added.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ The tiles are squares so width height is the same but technically yes thats right but wouldnt do it. The way this works the screen will never be that full so it should be okay on that end. Il try the loop \$\endgroup\$ – Kyle Jensen Jun 21 '15 at 16:16
  • \$\begingroup\$ The loop you have there doesnt do anything? Shouldnt i be resetting the position in there? \$\endgroup\$ – Kyle Jensen Jun 21 '15 at 16:18
  • \$\begingroup\$ Writing on pos.top, pos.left, etc is effectively resetting the position. \$\endgroup\$ – MadEqua Jun 21 '15 at 16:25
  • \$\begingroup\$ Ohh i didnt see the do { thats why i was confused \$\endgroup\$ – Kyle Jensen Jun 21 '15 at 16:56
  • \$\begingroup\$ +1 Yep - avoid recursion wherever possible - although elegant, it's more difficult to reason about and can hurt performance by the way it abuses the stack. \$\endgroup\$ – Engineer Oct 22 '15 at 10:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.