Java: Optimizing algorithm in game

Question

I'm creating a game (Java) with scene2d.

I wrote function for collision detection but I think it's bad function. It looks bad.

How can I optimize it? Make faster and more beautiful.

private void deleteEnemies()
{
    for(int i = 0; i < getActors().size - 1; i++)
    {
        if(getActors().get(i) != null && getActors().get(i) instanceof Enemy)
        {
            ////////////////
            for (int j = 0; j < getActors().size - 1; j++)
            {
                if(getActors().get(j) != null && getActors().get(j) instanceof Ball)
                {
                    if (actorsIntersecting(getActors().get(i), getActors().get(j)))
                    {
                        getActors().get(i).remove();
                        getActors().get(j).remove();
                    }
                }
            }
            //////////////
        }
    }
}

quick note - make sure this line is right: j < getActors().size - 1 - it seems you may miss the last list item. — Damian Walczak
– Damian Walczak, Commented Oct 1, 2014 at 19:38

tomer.z · Accepted Answer · 2014-10-01 19:30:18Z

1

Put getActors().get(i) in a variable, dont call it twice in the outer if
Same for getActors().get(j) in the inner if
use these variable in the most inner if's condition and body
save the size in a variable because now the .size function is being called on every iteration when the for condition is checked
You shouldn't use a size that can dynamically change during the loop for the loop condition (because you are removing items as you go) which brings us back to #4. Other than that its pretty much ok coding style perspective and I doubt you can make it more efficient than with what I told you (Other than using threads)

answered Oct 1, 2014 at 19:30

tomer.z

1,0831 gold badge11 silver badges27 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

spudone · Accepted Answer · 2014-10-01 19:36:18Z

1

Since you will do this frequently, consider storing the Enemies and Balls in their own structures (List or Set or whatever works). That prevents you from looping through actors you don't need, and avoids the instanceof checks.

answered Oct 1, 2014 at 19:36

spudone

1,2777 silver badges10 bronze badges

Comments

Tomek · Accepted Answer · 2014-10-02 06:18:10Z

1

Well, my first idea was to check only "nearest" enemies and not all of them. Somehow try to decrease size of that list.

~~2. Second one - please check your and conditions in and one by one - now you are checking 2 conditions always. Try to put "heavier" if later, for example:~~

from:

if(getActors().get(i) != null && getActors().get(i) instanceof Enemy)

to:

if(getActors().get(i) != null) { if(getActors().get(i) instanceof Enemy) { ..... } }

3. call your getActors().get(i) one time - save to variable. 4. I'm thinking why is it necessary to check if an actor is null, maybe just remove nulls from list or keep uninitialized actors on another list. Also try this with Balls and Enemies, please don't keep every actor on a single list.

edited Oct 2, 2014 at 6:18

answered Oct 1, 2014 at 19:32

Tomek

5434 silver badges12 bronze badges

2 Comments

Rick77 Over a year ago

Thumbs up for a check on proximity in order to reduce the order of algorithm from O(N^2)ish to some sort of O(NlogN) order. Threads would help, but a change of strategy is IMO the way to go

Dan Harms Over a year ago

Conditions are evaluated left to right. In a condition with all &&'s the first false will stop the condition checking. Splitting up the if gets you nothing but more lines.

Damian Walczak · Accepted Answer · 2014-10-01 19:48:10Z

0

I would rewrite the models a bit, so they can test the intersection itself and then do the delete like that (probably it can still be improved)

private void deleteEnemies () {
    List<Actor> actors = getActors();
    List<Actor> toRemove = new ArrayList<Actor>();
    int actorsSize = actors.size();
    Actor first = null, second = null;
    for(int i = 0; i < actorsSize; ++i) {
        first = actors.get(i);
        for(int j = 0; j < actorsSize; ++j) {
            if(i == j) continue;
            second = actors.get(j);
            if(first.intersects(second)) {
                toRemove.add(first);
                toRemove.add(second);

            }
        }
    }
    actors.removeAll(toRemove);
}

answered Oct 1, 2014 at 19:48

Damian Walczak

1,33414 silver badges21 bronze badges

Comments

eduyayo · Accepted Answer · 2014-10-01 19:35:03Z

0

Don't use size(), define a variable Try not to cast. Try not to uae instanceof. Maybe, sort lists by zsort or the like so u can, sometimes, start and or stop the loops sooner??

answered Oct 1, 2014 at 19:35

eduyayo

2,0382 gold badges15 silver badges37 bronze badges

Comments

Rick77 · Accepted Answer · 2014-10-01 19:39:56Z

Adding to the (very good) suggestions of the other participant: cache the enemies and projectiles in separate structures, so you don't have to check what they are at all.

Use the time vs space trade-off as much as you can: the standard approach, as hinted by Tomek, in this kind of situations is to reduce the number of checks (=iterations) by pruning the enemies and projectiles that cannot possibly collide within the current frame (they are way to far).

Anyway, a word of advice: go on with the game, complete as much as you can so that it will run correctly (if slowly), and only then go for the optimization.

That because

by optimizing preemptively in this way you will never finish it
you don't know how the final game really will be, perhaps: maybe after finishing 90% of it, you will see some easy chances for optimization.

Dan Harms · Accepted Answer · 2014-10-01 19:48:49Z

0

As others have said, the real improvement to speed would be two collections, one with balls and the other with enemies. As for making it look nicer, you could something like this:

for (Actor enemy : getActors()) {
    if (enemy != null && enemy instanceof Enemy) {
        for (Actor ball : getActors()) {
            if (ball != null && ball instanceof Ball && actorsIntersecting(enemy, ball)) {
                ball.remove();
                enemy.remove();
            }
        }
    }
}

answered Oct 1, 2014 at 19:48

Dan Harms

4,8702 gold badges21 silver badges30 bronze badges

Collectives™ on Stack Overflow

Java: Optimizing algorithm in game

7 Answers 7

Comments

Comments

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

Comments

Comments

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related