1

I have a Sprite class:

public class Sprite {
    public static ArrayList<Sprite> sprites = null;
    private Texture texture = null;

    public Sprite(Texture texture) {
        sprites.add(this);
        this.texture = texture;
    }

    public void render(SpriteBatch batch) {
        batch.draw(texture);
    }

    public void dispose() {
        sprites.remove(this);
    }
}

Before I create any sprites, I make sure that the Sprite class has a reference to the pool of sprites that are going to be rendered:

public class Main extends ApplicationAdapter {
    private static ArrayList<Sprite> sprites = new ArrayList<Sprite>();

    public Main() {
        Sprite.sprites = sprites;

        new Sprite("texture.png");
    }

    @Override
    public void render(SpriteBatch batch) {
        for (int i = 0; i < sprites.size(); i++)
            sprites.get(i).render(batch);
    }
}

So now when I create a new sprite, it will automatically render without me having to manually add it into the ArrayList.

The problem is that sprites can be disposed during the time that for loop is running, and thus some sprites won't be rendered for that specific frame.

I do not wish to loop through the items backwards such as:

for (int i = sprites.size() - 1; i >= 0; i--)
    sprites.get(i).render(batch);

Because this will render my sprites out of order (sprites that should be on top will be covered).

My only solution so far has been to have another ArrayList that keeps track of which objects to dispose, as such:

public class Sprite {
    public static ArrayList<Sprite> sprites = null, garbage = null;

    //everything else the same

    public void dispose() {
        garbage.add(this);
    }
}

And then during render, I first remove all the garbage from sprites, then render sprites:

for (int i = 0; i < garbage.size(); i++)
    sprites.remove(garbage.get(i));
garbage.clear();

for (int i = 0; i < sprites.size(); i++)
            sprites.get(i).render(batch);

This method works, but I feel it is inefficient. (Having two arraylists? Then doing two for loops?)

Is there any way I can loop through Sprites without it skipping a Sprite when a Sprite is disposed? I don't want to loop through Sprites backwards though because it will mess up the order.

I've now tested this with a synchronized list, but I am unable to get this working properly.

Testable code, (with java.util.ConcurrentModificationException)

import java.util.*;

class Ideone {

    public static class Sprite {
        public static List<Sprite> sprites = null;
        public int age = 0;

        public Sprite() {
            synchronized(sprites) {
                sprites.add(this);
            }
        }

        public void render() {
            age++;
            if (age > 30)
                dispose();
        }

        public void dispose() {
            synchronized(sprites) {
                sprites.remove(this);
            }
        }
    }

    private static List<Sprite> sprites = Collections.synchronizedList(new ArrayList<Sprite>());

    public static void main(String[] args) {
        Sprite.sprites = sprites;

        new Sprite();
        new Sprite();

        for (int i = 0; i < 60; i++)
            render();

    }

    public static void render() {
        synchronized(sprites) {
            Iterator<Sprite> iterator = sprites.iterator();
            while (iterator.hasNext())
                iterator.next().render();
        }
    }
}
1
  • You could synchronize access to the List so that it cannot be modified whilst rendering, but this isn't ideal whilst rendering if the UI thread gets locked out. Alternatively, could queue a 'request' to modify the thread and handle this queue before rendering each frame. Commented May 27, 2015 at 16:41

3 Answers 3

2

First of all if sprites list is accessed from multiple threads you need to synchronize access to it or use thread safe data structure (e.g. CopyOnWriteArrayList or CopyOnWriteArraySet).

Sprite.dispose can directly remove itself from sprites in this case.

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

9 Comments

I just tried List<Sprite> sprites = Collections.synchronizedList(new ArrayList<Sprite>(); and synchronized (sprites) + and iterator, but I got a ConcurrentModificationException because Sprites also gets new items during the loop too. I also took a peak at CopyOnWriteArrayList, and this seems more inefficient than my two ArrayLists (because you're copying the entire array, as opposed to only referencing the ones to delete).
If you use Collections.synchronizedList you still need to synchronize access while iterating: docs.oracle.com/javase/7/docs/api/java/util/…
Are you talking about a synchronized block? I am using one while iterating.
If you use synchronized properly there is no way it can happen, because calling sprites.remove or add will wait until synchronized (sprites) block is completed. Are you sure that you are iterating in the synchronized (sprites) block too?
Actually, the synchronized block only guarantees that if you are using iterator.add/remove. If you're using add/remove from the list directly, you'll still hit the exception. And of course, using the iterator just to add or remove stuff would be way more inefficient than just keeping another list.
|
2

If you just have to take care of remove, then use Iterator like

Iterator<Sprite> itr = sprites.iterator();
while (itr.hasNext()) {
  // do something with itr.next()
  // call itr.remove() to remove the current element in the loop
}

Iterator provide remove method to remove any item from the list but make sure you don't add any new item to the list otherwise it will throw ConcurrentModificationException.

7 Comments

I like this idea, but how will Sprite.dispose() run itr.remove()? Note that the only place sprites will change is from dispose(). In other words, I'm not removing any sprites within the loop, it's happening somewhere else but at the same time the loop is running.
This link can help you out in that case:- stackoverflow.com/questions/5849154/…
I can't use an Iterator either because new items will be added to the ArrayList too.
then you can use ListIterator - which provides you to add new element to the list
I'm still getting a ConcurrentModificationException, I'm not using itr.remove() because I'm not removing the sprites within the while/forloop. The ArrayList is being modified from dispose(), which happens outside of the loop, but happens at the same time as the loop. I've done synchronized (sprites) with an Iterator and a ListIterator, and both throw the same exception.
|
0

Here's what worked for me clicky:

import java.util.*;

class Ideone {

    public static class Sprite {
        public static ArrayList<Sprite> sprites = null;
        public int age = 0;

        private boolean removed = false;

        public Sprite() {
            sprites.add(this);
        }

        public boolean render() {
            if (removed)
                return false;
            age++;
            if (age > 30)
                dispose();
            return true;
        }

        public void dispose() {
            removed = true;
        }
    }

    private static ArrayList<Sprite> sprites = new ArrayList<Sprite>();

    public static void main(String[] args) {
        Sprite.sprites = sprites;

        new Sprite();
        new Sprite();

        for (int i = 0; i < 60; i++)
            render();

    }

    public static void render() {
        Iterator<Sprite> iterator = sprites.iterator();
        while (iterator.hasNext())
            if (!iterator.next().render())
                iterator.remove();
    }
}

So, when a sprite should be removed, I set a quick bool that tells the main render loop to remove it with the iterator. I'll have to adapt this to use a sync'd list (so that adding items doesn't throw an exception).

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.