Regarding CircularBuffer's store() method

[no-trick-pony]

Hi!

I wanted to use the ai.utils.CircularBuffer as part of my project, however, I encountered some issues:

Regarding CircularBuffer's store() method ( https://github.com/libgdx/gdx-ai/blob/master/gdx-ai/src/com/badlogic/gdx/ai/utils/CircularBuffer.java ):

    public boolean store (T item) {
        if (isFull()) {
            if (!resizable) return false;

            // Resize this queue
            resize(Math.max(8, (int)(items.length * 1.75f)));
        }
        items[tail++] = item;
        if (tail == items.length) tail = 0;
        return true;
    }

Why does it abort and return false when the circular buffer is full and not resizable? Ring buffers would usually overwrite the oldest entry, but apparently this never happens here.

Also, if the buffer is full and resizable, it will get resized. Thus f (tail == items.length) tail = 0; will probably never get executed.

Another problem:

        CircularBuffer<Float> buffer = new CircularBuffer<Float>(3, false);

        System.out.println(buffer.store(3.2f));
        System.out.println(buffer.store(3.2f));
        System.out.println(buffer.store(3.2f));
        System.out.println(buffer.store(3.2f));

prints true, true, false, false.

So, not only does the "circular" buffer simply don't store new values like a circular buffer should but it also stores one element less than its capacity. This is likely caused by a bug in the isFull() method.

[davebaol]

Why does it abort and return false when the circular buffer is full and not resizable? Ring buffers would usually overwrite the oldest entry, but apparently this never happens here.

Well, in my use case I don't want to overwrite the oldest entry because I use the buffer to implement a queue, see PathFinderQueue. In your case, If you want to overwrite the oldest entry you can just read and store again when you get false.

Also, if the buffer is full and resizable, it will get resized. Thus f (tail == items.length) tail = 0; will probably never get executed.

Should not be a problem

So, not only does the "circular" buffer simply don't store new values like a circular buffer should but it also stores one element less than its capacity. This is likely caused by a bug in the isFull() method.

I'll look into it soon, thanks.

[davebaol]

The problem is that when head == tail the buffer can be empty or full. I think the simplest solution is to use a size field that is incremented on store and decremented on read. This way the buffer is full when size == items.length and is empty when size == 0.