Added More Entitybag implementations

[AnderWoche]

How i can link this to a Issue?

[AnderWoche]

I made MutableEntityBag to an interface, and two implementations for it.

[AnderWoche]

Ref #156

[Quillraven]

thank you for the contribution - looks good to me! I wonder if the list version can be simplified with Kotlin's delegation? Maybe you can write:

class ListMutableEntityBag(
    @PublishedApi
    internal val list: MutableList<Entity>
) : MutableEntityBag by list {

But I am not sure if the delegation mechanism is smart enough to detect that the methods are the same. I guess they'd need a common interface and I am not sure if there is one ;)

[Quillraven]

Please rename it to values to be consistent with the other bag implementation.

[Quillraven]

Please add a test class for the list implementation as well. Most likely you can copy&paste the ArrayEntityBagTest file and just change the bag to the list version.

[AnderWoche]

by not working

[AnderWoche]

Many functions from the ListMutableEntityBag return an ArrayMutableEntityBag, which is intentional, so I have adapted the tests for it

[AnderWoche]

With the MutableList it is impossible to get the capacity variable so I have removed the tests.

[AnderWoche]

In the List implementation i have this one:

ListMutableEntityBag.kt

 /**
     * Resizes the bag to fit in the given [capacity] of [entities][Entity] if necessary.
     */
    override fun ensureCapacity(capacity: Int) {
        throw NotImplementedError("Not Supported")
    }

    /**
     * Resets [size] to zero, clears any [entity][Entity] of the bag, and if necessary,
     * resizes the bag to be able to fit the given [capacity] of [entities][Entity].
     */
    override fun clearEnsuringCapacity(capacity: Int) {
        throw NotImplementedError("Not Supported")
    }

yout are fine with this?

[Quillraven]

I was not aware of a NotImplementedError but imo it makes sense if the class does not have that.

Of course one would ask why the interface has it but the implementation doesn't. That is for sure not clean. Can we maybe remove it from the interface and just use the array implementation internally in Fleks? Need to have a look when I am at a computer again 😅

Out of curiosity: how do you actually use the list implementation? I mean how do you get entities in it?

[AnderWoche]

My usecase is this:

class ChildrenComponent : Component<ChildrenComponent> {

    val children = ListMutableEntityBag(ArrayList(8)) // on remove the order stays the same thats for me important.

    override fun World.onRemove(entity: Entity) {
        children.forEach { child ->
            child.configure {
                it -= ParentComponent
            }
        }
    }

    override fun type() = ChildrenComponent

    companion object : ComponentType<ChildrenComponent>()
}

[AnderWoche]

!!!! I have a new idea:

Why does the MutableEntityBag has its own interface?

couldn't we just replace the interface with the interface that comes from kotlin?

or maybe other interface like MutableCollection

[Quillraven]

If there is an interface that contains all those methods then sure, would be better to use it.

I still don't get in your example why you are not using a normal list? What is the advantage of using an entitybag which is actually a list in the background? 😅

[AnderWoche]

[AnderWoche]

I have just looked it up right now, I wouldn't need it. (because I have reprogrammed it again)
but before it was like this:

    private fun validate(parent: Entity?, children: EntityBag) {
        // validatre children
        children.forEach { newParent ->
            val newChildren = newParent.getOrNull(ChildrenComponent)?.children
            if (newChildren != null) {
                validate(newParent, newChildren)
            }
        }
    }

and i wantet to start the method like:

    override fun onTick() {
        super.onTick()

        validate(null, family.entities) // The family.entities has forced me to use the interface everywhere
    }

[AnderWoche]

The Problem was that the fast MutableEntityBag implementation had no sorting if you have these entities:
1
2
3

and you remove 1

the oder was:

3
2

and not:

2
3

thats why i needed a "sorted" entity bag

[Quillraven]

Ah okay now I get it. Well, a bag is a special data structure which doesn't care about order and is therefore a very fast and efficient way to add/remove elements. It is basically a dynamically growing array without the overhead of other data structures.

[AnderWoche]

I will test if we can use the collection interface because e.g. all extra functions like .all {} would be included automatically

[AnderWoche]

Okay i have a new idea:

Maybe it is enough to implement only the Iterable interace

[AnderWoche]

Ok pls take a look at this.

I have decided that the Collection interface makes more sense.

if its okay i going to write some tests

also take a look at the .forEach funktion now there will be the iterator i think it should be fast anyway couse the iterator is reused.
But can you do a performance test for safety?

[AnderWoche]

Take a Look at the _EntityBag.kt

The normal extension functions often return a list instead of an EntityBag. where you originally returned an EntityBag, I have added the function.

[Quillraven]

Thank you for your effort. I am currently on holiday for three weeks and don't have access to a computer, sorry.

I will have a look when I am back.

Regarding performance test: there is a Gradle task called jvmBenchmarks if I remember correctly. That should work for you as well.

There is also a manual.kt file where you can run benchmarks. However, you should run them a few times and take an average value.

For the Gradle benchmarks you don't need to do that but they also run for a longer time.

[Quillraven]

@AnderWoche: I just had a look now and that looks like way too many changes for what we are trying to achieve here. Also, the iterator seems to be limited to only two nested iterations? In general, it does not look like a clean solution imo.

I do like the Collection interface for the EntityBag. I will have a look at that when I am back from my holiday and have more time.

In the meanwhile - you already solved your initial problem in a different way, right? Or is there an easier "quick win" that we can introduce to help you?

edit: the Collection interface inherits from Iterable which has the iterator() method, I see. Then I don't want to go that route because initially the bag was just a helper collection to speed up certain parts of Fleks and it does not require an iterator and imo it does not make sense because it is basically a dynamic array and an array also has no iterator?

edit2: would a simple toList() method on an EntityBag help you? Or did you anyway solve it differently now and there is no need for it?

edit3: array actually has an iterator. I did not know that :D Well, I am back at home in two weeks then I will have a look again.

It returns btw an ArrayIterator. I would create something very similar for the bag then we can use the Collection interface. I still would then rather go for a toList() method instead of a list implementation of the bag because that is not the intention of a bag. It does not care about order of elements.

private class ArrayIterator<T>(val array: Array<T>) : Iterator<T> {
    private var index = 0
    override fun hasNext() = index < array.size
    override fun next() = try { array[index++] } catch (e: ArrayIndexOutOfBoundsException) { index -= 1; throw NoSuchElementException(e.message) }
}

[Quillraven]

Hi,

I had now some time and since you already solved your initial problem in a different way, I will close this PR.

The reason why I don't want to take over those changes are:

• the bag datastructure has no order. Adding an implementation that keeps the order is therefore not ideal in my opinion. A better approach is to use your own data structure and add the entities to it the way you need it.
• the Collection interface comes with an iterator which is in my opinion unnecessary. I don't see any use-case where an iterator is needed since we already have forEach implementations.
• renaming MutableEntityBag to something else is a breaking change, which we also should try to avoid.

However, I still want to thank you for your idea and time. I learned a few things because of that!