Quillraven / Fleks

Fast, lightweight, multi-platform entity component system in Kotlin
MIT License
180 stars 20 forks source link

Entity is in the world although it is deleted, the Contains function should also include delayRemoval entities. #153

Closed AnderWoche closed 2 months ago

AnderWoche commented 2 months ago

entity.remove() if(entity in world) // should be false becouse the in funktion ignore the delayRemoval entities

Quillraven commented 2 months ago

Hm but that is contradicting because it is actually still in the world, hence delayRemoval ;) Also, the only scenario where this can happen is if you delete an entity inside your system and check if it is in the world. I think there is no other case.

Can you provide your scenario where this is an issue?

AnderWoche commented 2 months ago

I am trying to build a parent child structure.

I have built a state system inspired by React which changes/rebuilds the UI depending on the state change, so if more than one state is changed per frame I have to rebuild the whole parent child structure several times.

To make the project clearer I thought I could outsource individual logics into components.

class ParentComponent(var parent: Entity) : Component<ParentComponent> {

    //[...]

    //If the entity is deleted, it should automatically remove itself from the parent.
    override fun World.onRemove(entity: Entity) {
        parent.getOrNull(ChildrenComponent)?.children?.minusAssign(entity)
    }
}

as soon as I want to rebuild everything from one point I assumed that entity.remove() works immediately and executes the logic of the onRemove of the component.

    private fun removeEntityAndChildrenTree(entity: Entity) {
        val children = entity.getOrNull(ChildrenComponent)?.children
        if (children != null) {
            for (i in children.size - 1 downTo 0) {
                removeEntityAndChildrenTree(children[i])
            }
        }
        println("delte entity ${entity.id}")
        //a solution would be to call children.clear() here.
        entity.remove()
    }

after long debugging I realized that the entities may only be removed later.

But because I already explicitly called remove, I thought that the entity would no longer be in the world.

if (family.contains(entity)) {
        if (entity[StateComponent].boundStates.contains(stateID)) {
            if(entity in world) { // I have called remove sometime before but the entity is still in the parent's CHildrenComponent. hence the check if it is still in the world/or has been marked as remove.
                doReInit(entity)
            }
            return
        }
}

Your point that you mentioned makes sense, that at that time the entity is really not deleted, but then I would like to have a method to check if the entity is marked as removeLater.

Quillraven commented 2 months ago

in is just a special syntax for contains. What about a contains(includeDelay: Boolean = false) default implementation of this method? When you set the parameter to true, then it will also check for delay removals. Or do you want a separate method like entity.isMarkedForRemoval()? Whatever you prefer :)

AnderWoche commented 2 months ago

Normally I would prefer that contains(includeDelay: Boolean = false) but I think to avoid confusion with the in syntax would use the other one.

this: entity.isMarkedForRemoval()

Quillraven commented 2 months ago

@AnderWoche: I just made the change in the master: https://github.com/Quillraven/Fleks/commit/3c0ece24a50ea5ff4fad106773bd6e0d0034d3ba

Should be available in the Snapshot soon. Please let me know, if that solves your problem :)

Thanks!

Edit: I broke something. Will need to have a look tomorrow, sorry.

Quillraven commented 2 months ago

Snapshot should be available now soon

AnderWoche commented 2 months ago

@AnderWoche: I just made the change in the master: https://github.com/Quillraven/Fleks/commit/3c0ece24a50ea5ff4fad106773bd6e0d0034d3ba

Should be available in the Snapshot soon. Please let me know, if that solves your problem :)

Thanks!

Edit: I broke something. Will need to have a look tomorrow, sorry.

Everything solved 👍