Component hooks will trigger family hooks before first configuration is finished for a given entity

[marcphilippebeaujean-abertay]

Not sure if this is the intended behaviour, but it seems strange to me. For example, lets take this case, a simple component:

data class HealthComponent(
    var initialHealth: Int = 10,
    var health: Int = initialHealth
) : Component<HealthComponent> {
    override fun type() = HealthComponent

    companion object : ComponentType<HealthComponent>() {
        private val LOG = logger<HealthComponent>()
    }

    override fun World.onAdd(entity: Entity) {
        if (entity hasNo HealthBarComponent) {
            LOG.info { "adding health bar for entity with hitpoints because it was not present" }
            // configure is called
            entity.configure {
                entity += HealthBarComponent()
            }
        }
    }
}

When I create my entity....

val enemyEntity = world.entity {
            it += EnemyComponent()
            it += HealthComponent(enemyStats.getStartingHitpoints()) // configure is called again, inside
        }

and I have such a hook:

// inside configureWorld
val enemyFamily = family { all(EnemyComponent) }
onAdd(enemyFamily) {
    LOG.info { "spawned enemy" } // will always run twice in a row, once for world.entity and once after entity.configure
}

The hook will trigger twice, because when configure runs inside the health component, the entity already has an enemy component. I know why this is the case and I can obviously circumvent it knowing this, but I'm not sure if this is very intuitive to the user and maybe a bit error prone (I didn't run into this issue for Fleks 1.6)? It seems more intuitive if the family hooks trigger only when an entity has been added to the world with "world.entity" and not whenever a configuration is run.

[Quillraven]

Imo family hooks also need to run after configure because obviously the component configuration can change after configure is called and therefore the entity will be removed or added from/to families.

However, if I understand you correctly you call configure within a world.entity block and here I'd agree that it should only run once at the end.

Why do you think it should not get called after configure?

[Quillraven]

One more remark: I personally would not add a component inside a component hook. That feels like a design flaw imo. I personally would create an extension function for the EntityCreateContext e.g. that configures Health and HealthBar together. That way you don't forget to add both components and it is also easier to follow the code because it happens together within a single method and you avoid the multiple hook notifications.

Just an idea 😉

[Quillraven]

I thought about it some more yesterday and now I'd definitely not follow the approach that you are using. You are basically doing something like this:

world.entity { entity ->
  entity.configure {

  }
}

This does not make sense because configure is unnecessary in the creation context of an entity. Also, as mentioned above, configure must notify all families and that's why you get the double notification (once for the configure block and once for the entity creation block).

You will also run in a similar issue if you modify your entities in a system, where you will end up with:

entity.configure {
  // add HealthComponent -> this trigger another configure call for HealthBar
  entity.configure {

  }
}

The same happens here again, that you get notified twice for the outer and inner configure block. I remember that we already looked into a solution for the nested configure calls to identify if different entities are passed, which will cause problems because families are not properly updated. In the end we decided to go for a documentation on this method to warn users that they need to take care themselves to not nest configure calls with different entities. The reason was, that this problem is not so easy to solve and had quite some impact on performance.

Similar reasons apply to your case and that's why I'd solve it differently. I don't know where you add a HealthComponent but it sounds like this is something "generic" that gets added to an entity at creation time and not later on. That's why I'd suggest something like:

fun EntityCreateContext.configureHealth(entity:Entity, health: Int) {
  it += HealthComponent(initialHealth = health)
  it += HealthBarComponent()
}

which you can then call:

world.entity {
  configureHealth(it, health = 10)
}

This solves the issue that you don't forget to add the HealthBarComponent and also you avoid multiple notifications for the same entity. In addition, I personally find it easier to understand and follow. I find it weird that when I add a HealthComponent that my entity automatically gets a HealthBarComponent as well without me adding it. By calling a separate method that clearly states that it adds both components, it is easier to read and follow. But that might be just my personal opinion :)

[Quillraven]

no activity for couple of days ... reopen if you have feedback again ;)

[Quillraven]

@marcphilippebeaujean-abertay: I will merge a PR soon that should optimize the notifications. Please try it out via the SNAPSHOT version and let me know if that solves your issue.

[marcphilippebeaujean-abertay]

Hi Quill, sorry I did not reply earlier because I went away for a bit.

| Why do you think it should not get called after configure?

I thought the main purpose of families was to be notified when an entity with a specific set of components was removed or created in the world, not whenever the configuration of those entities changed (this is what component hooks were for, at least from my understanding). If that is not the design philosophy, I feel like such a feature would be good to have because I think there is definitely a purpose for listening to specific create/remove events on the entity level, rather than just components.

I agree that avoiding nested configure calls is a good idea, but like you mentioned often it is not done on purpose. The configuration context is interesting, I am not sure if something like this is mentioned in the wiki actually it's great to be made aware of this!

Thanks for your awesome work, the library is great - will check out the PR.

[Quillraven]

I thought the main purpose of families was to be notified when an entity with a specific set of components was removed or created in the world, not whenever the configuration of those entities changed (this is what component hooks were for, at least from my understanding). If that is not the design philosophy, I feel like such a feature would be good to have because I think there is definitely a purpose for listening to specific create/remove events on the entity level, rather than just components.

This is not entirely correct. The purpose of a family is to store entities of a specific component configuration internally in order for us to iterate over those entities e.g. inside an IteratingSystem.

Since entity.configure is manipulating the components of an entity, it also must notify families about it because after the configuration is done, the entity can be part of new families and also can be removed from families where it currently is inside.

So we definitely need both notifications for families. One notification when an entity gets created and another notification whenever the components of an entity get changed.

Anyway, the new SNAPSHOT version should reduce the amount of notifications always to one which should help in your case. But as mentioned above, I'd vote for the extension method that I showed above which makes it easier to follow your code and also removes the duplicated notifications.