skypjack / entt

Gaming meets modern C++ - a fast and reliable entity component system (ECS) and much more
https://github.com/skypjack/entt/wiki
MIT License
10.01k stars 877 forks source link

Call for comments: signals on component creation/destruction #62

Closed skypjack closed 6 years ago

skypjack commented 6 years ago

Ok, people is asking this and I tried to figure out if it's possible. Probably, I found a nice way to add signalling stuff to the registry in a way that is in policy with the whole framework: pay only for what you use.. It means that there will be no performance hits for those components that aren't (let me say) observed. On the other side, it will be possible to be notified about creation and destruction of specific components if required.

I've still to review the whole idea and try to implement it. However, I'd like to have feedbacks about it.

Is it a feature in which you could be interested? Any suggestion, comment or request?

As a side note, it would unlock other features like implicit blueprints (when component A is attached, attach also B and C silently) and so on.

Let me know, feedbacks are appreciated!! I'll close the issue probably in a couple of days.

Thank you.

skypjack commented 6 years ago

@mhaemmerle I was exploring the different ways to notify listeners about a replacement and I found it. It seems interesting the idea of deducing that something has been replaced as a consequence of an add/remove sequence and as long as the entity identifier doesn't change in the meantime. The drawback is that listeners on add/remove are triggered as well if the replace works like an alias for sort of remove-then-add the component. Not sure if it's a viable approach and thus I was asking. ;-)

skypjack commented 6 years ago

@mhaemmerle

After adding the new event system to Entitas a couple of weeks ago, there's hardly any need for that anymore though.

Where is the documentation of the new event system? I'm curious to know if the behavior is similar to what I observed.

Thank you.

mhaemmerle commented 6 years ago

Didn't find documention either, but remembered this issue here; maybe even better than documentation as there's a discussion with the reasoning behind it.

skypjack commented 6 years ago

@mhaemmerle

Put aside the fact that we cannot detect changes on data members in C++ (the registry doesn't even know what are the data members of a type actually), it seems that the event system of Entitas launches events only when components are added or removed. Am I wrong? I wouldn't add a per entity signal handler, mainly because it seems a waste in terms of memory usage and also because entities are nothing more than numbers in EnTT after all, so it would be tricky. That being said, all what remains are events on creation and destruction, that are already exposed as mentioned above:

registry.creation<Component>().connect<&MyFunction>();
registry.creation<Component>().connect<MyClass, &MyClass::myMemberFunction>(& instance);

Where a listener has the following function type:

void(Registry &, Entity);

Using them, we can easily define systems that react on changes and infere even a replacement from a destruction followed by a construction on an entity that doesn't change version. We can also collect changes if required and filters entities with these two signals.

What else do you think is required? Comments on this from whoever has experience with real world applications are welcome.

mhaemmerle commented 6 years ago

Basically everything in Entitas is using groups under hood one way or the other; those are kept up to date by subscribing to component changes on each entity. You're right in your assumption that the events feature in Entitas only runs when components are added/removed to/from an entity.

skypjack commented 6 years ago

@mhaemmerle

Sparse sets are kind of groups at the end of the day.
The behavior you described is more or less how persistent views work under the hood: they are (tightly packed) groups of entities kept aligned with the registry each time a component in which they are interested is added or removed.

The issue was that the signals to which they where attached weren't exported to the users.
I did it now, everything is made available to the users. I'm testing them and it works pretty well.
As it stands right now, we can create almost everything as an external tool then: blueprint infrastructure, reactive systems, all that comes to our mind and works by observing construction and destruction of components is possible.

Wasn't this what you were asking for after all?

mhaemmerle commented 6 years ago

Do you have an example of an EnTT reactive system?

skypjack commented 6 years ago

@mhaemmerle

Not yet. I'm working to refactor the signal stuff so as to separate signals from sinks. I want to avoid things like this:

registry.construction<Component>().publish(entity);

In other terms, publish will be be available only within the registry as it ought to be.

To define a reactive system isn't much difficult anyway.
It's trivial in case you want to observe only one component (sort of keep track of all the entities to which Component is attached during a tick or do something as soon as Component is attached to an entity and so on). It's a bit trickier to do when you want to observe multiple components, but still feasible.
Probably I'll provide users with some basic systems from which to inherit for the most common cases in future. I'll put a note in the TODO file so as not to forget it.

skypjack commented 6 years ago

A first draft of the changes is available from experimental.

The sole doubt I have is if it's worth it adding signals also for tags. I mean, a tag is quite easy to observe even without signals: store aside the attachee, in case it's different during the next tick we can deduce that a remove and attach took place and we have both the entity involved at once. An obvious advantage is that we reduce memory usage and we don't introduce any performance hit out of the box. The sole drawback is that the notifications aren't synchronous. What about this?

Any feedback on my doubt and the changes from experimental is appreciated.

Thank you.

mhaemmerle commented 6 years ago

If the intent of tags is to have guaranteed unique components, than they should have first class signal support. Otherwise the library is shifting the responsibility to it's users and forcing them to write boiler plate code.

How have you solved the generation of signals for components that are directly changed like this?

registry.get<Position>(entity) = { 0.f, 0.f };
skypjack commented 6 years ago

@mhaemmerle

Otherwise the library is shifting the responsibility to it's users and forcing them to write boiler plate code.

Well, built-in support for signals in the registry introduces performance hits. I can still define systems that do the job under the hood whether required for the users, so as not to force them to do it. The real issue is that it won't be synchronous. I'm trying to figure out if it can be a problem after all.

How have you solved the generation of signals for components that are directly changed

We cannot really solve it, so... If you get a reference to a component and modify it, the registry won't know about it in any case. The best I can do is to send a couple of added/removed signals within the replace member function, even though I'm not that sure it would help actually. It works only as long as users consistently use that function throughout the whole codebase.

So far, only assign, remove and destroy triggers events for components.

mhaemmerle commented 6 years ago

Well, built-in support for signals in the registry introduces performance hits. I can still define systems that do the job under the hood whether required for the users, so as not to force them to do it. The real issue is that it won't be synchronous. I'm trying to figure out if it can be a problem after all.

I have a more radical suggestion: split the current tag implementation into two distinctly different features (single responsibility principle).

The first one would use HashedString to tag entities with the guarantee of being unique in the context of a registry. This would solve the current semantic disconnect of a tag being more than just a tag.

The second one would use regular components, guarantee uniqueness in the context of a registry, manage the owning entity (if necessary at all) and would thus also need to offer the same semantics w.r.t. signals as regular components.

We cannot really solve it, so... If you get a reference to a component and modify it, the registry won't know about it in any case. The best I can do is to send a couple of added/removed signals within the replace member function, even though I'm not that sure it would help actually.

I agree, think that it is perfectly fine to do so (use replace) and also think that it would actually help. Examples in that vein would be STM's where you have to wrap your code e.g. in an atomic block or call commit when you're done with your changes. You can obviously leave out the atomic or change values directly on an object, but it either wouldn't work or would produce errors directly or further down the line. So developers actually use the API neccessitated by the library/concept and thus get the correct behaviour.

skypjack commented 6 years ago

@mhaemmerle Isn't this the same as what you have described as tag?

template<HashedString::hash_type>
struct Tag {};

I see a lot of people consider tags just like a name I can attach to something so as to retrieve it easily later on, but I don't see any drawbacks in letting tags be something more than a plain string. I mean, the latter still allows the former.

It makes sense probably to fire removed/added events on replace. Users can still avoid the performance hit using a get. I wouldn't add a third type of signal anyway (kind of replaced). I saw almost no ECS out there did it, probably there is a good reason for that.

mhaemmerle commented 6 years ago

I see a lot of people consider tags just like a name I can attach to something so as to retrieve it easily later on, but I don't see any drawbacks in letting tags be something more than a plain string. I mean, the latter still allows the former.

Maybe sometimes there's merit in what "a lot of people consider"; even if it would just help you decomplect an already pretty dense API and thus implementation. Maybe it would even help you implement signals on unique components in an easy manner. But alas, I get the feeling that you've made up your mind already and know better now than trying to convince you otherwise ;)

It makes sense probably to fire removed/added events on replace. Users can still avoid the performance hit using a get. I wouldn't add a third type of signal anyway (kind of replaced). I saw almost no ECS out there did it, probably there is a good reason for that.

Cool.

skypjack commented 6 years ago

@mhaemmerle

Maybe it would even help you implement signals on unique components in an easy manner.

The problem is all in the fact the components already required signals because of the persistent views, so the performance hit was already there. Any other feature do not use signals, so it would introduce a performance hit. To be honest, creation and destruction of entities, tags and components aren't usually along critical paths, so I don't think it's a problem to introduce signals all around. My two cents.

Actually I haven't made up my mind already, that's why I'm trying to explore your point of view. What I'm saying is that what you mentioned as tags is already possible. Do you see it? The difference in splitting the whole thing in two parts would be that the call point changes from this:

registry.attach<MyStringTag<HashedString{"foo"}>>(entity);

To this:

registry.attach<HashedString{"foo"}>(entity);

Of course, unless you want to use runtime strings, but it doesn't make sense in a high performance tool to do something like this, so I'm excluding it. So, is it shorter? Right. Drawbacks? An extra data structure to work with, probably a map, something that isn't properly memory friendly. Anything else? No, at least I don't see any other difference. Therefore, it goes without saying that what a lot of people consider is already possible with EnTT, so I don't see why you blame me. :-(

mhaemmerle commented 6 years ago

a lot of people consider

Actually thinking about printing this on a T_Shirt :)

Any other feature do not use signals, so it would introduce a performance hit. An extra data structure to work with, probably a map, something that isn't properly memory friendly.

These two assertions are super abstract to me, as there's no number attached. How much slower is it going to be? How much more memory usage are we talking about? Where should the line be drawn w.r.t. to EnTT's promise on performance? Also isn't in line with the EnTT creed that you pay for what you use? Maybe it's time for another round of benchmarks? (On that note, what do you think about converting the unit from seconds to milliseconds? The former is rather pointless for such small numbers ;))

This is purely theoretical at this point: e.g. if a second data structure for string tags would get introduced, it would increase memory usage by the baseline of the structure itself and then should have similar characteristics to the existing one. And if there's someone I'd trust to pick the best data structure for the job, than it's you w.r.t. to EnTT and C++ :)

So what is the big problem that restricts tags from taking part in the new signal goodness? :)

skypjack commented 6 years ago

How much slower is it going to be? How much more memory usage are we talking about?

Much, much slower actually!! :-D At a first glance, in case of no listeners it's a matter of an extra:

while(begin != end) { /* ... */ }

So, a comparison each time a tag is attached or removed?

Memory usage is the one of an empty vector in case of no listeners, so it's mainly implementation defined. However I'd say at least the size of an extra couple of pointers for each tag.

what do you think about converting the unit from seconds to milliseconds?

Actually it could make sense. :-) I used the same benchmarks of EntityX initially, so as to compare the two libraries, therefore I kept the same pattern on benchmarks.

skypjack commented 6 years ago

@mhaemmerle

So what is the big problem that restricts tags from taking part in the new signal goodness? :)

Actually no problem at all apparently. What about:

void(Registry<Entity> &, Entity)

Vs:

void(Registry<Entity> &, Entity, tag_type)

The main difference is a couple of signals for each tag type vs a couple of signals shared between the tags and the burden of an if user side.

mhaemmerle commented 6 years ago

Much, much slower actually!! :-D

That is still rather ... abstract ;) And it still sounds too complex for what it's doing IMHO, though let's agree to disagree here, ok? :) As the number of tags is probably way, way smaller than the number of entities it's probably alright no matter what.

void(Registry<Entity> &, Entity, tag_type)

Cool, that's something people can work with and don't have to worry about different behaviours.

        std::pair<
            SigH<void(Registry &, Entity)>,
            SigH<void(Registry &, Entity)>
        > listeners;

Out of curiousity, what's the reason for using a pair here (especially when listeners is private)?

skypjack commented 6 years ago

@mhaemmerle

I was joking. It's a comparison between pointers in case of no listeners. We are speaking of stardust here. :-D

void(Registry<Entity> &, Entity, tag_type)

So, the if to check if it's the right tag is in charge to the users this way. Right? It makes sense for me.

Out of curiousity, what's the reason for using a pair here (especially when listeners is private)?

The pair is because I keep two separate lists of listeners here: the ones that want to be notified about construction and the ones that want to be notified about destruction. Why does it sound strange to you? It's exactly the same as using two data members once compiled, but I had to find only one name this way. :-D

mhaemmerle commented 6 years ago

So, the if to check if it's the right tag is in charge to the users this way. Right? It makes sense for me.

Got a more full-fledged example here?

It's exactly the same as using two data members once compiled, but I had to find only one name this way. :-D

It's just ... odd ;) Also godbolt.org says it's ~40 instructions more :P

constructionSignal, destructionSignal onConstruction, onDestruction

Splitting hairs here.

skypjack commented 6 years ago

@mhaemmerle

Got a more full-fledged example here?

Sort of:

void f(Registry<Entity> &registry, Entity entity) {
    // ... You already know that is the right tag
}

registry.construction<MyTag>().connect<&f>();

Vs:

void f(Registry<Entity> &registry, Entity entity, tag_type type) {
    if(type == registry.tag<MyTag>()) {
        // ... You check the type
    }
}

registry.connect<&f>();

Out of my mind, this won't be probably the real API, but it gives you an idea of what I'm speaking about.

Also godbolt.org says it's ~40 instructions more

Really? Using -O3? O.o It's definitely odd, I agree.

mhaemmerle commented 6 years ago

Out of my mind, this won't be probably the real API, but it gives you an idea of what I'm speaking about.

Obviously not having to do the comparison would be better, but it's a good enough start for now.

Really? Using -O3? O.o It's definitely odd, I agree.

Added -O3 and the difference is even bigger. I'am probably doing something wrong, so here's the link :)

skypjack commented 6 years ago

@mhaemmerle

An issue regarding tags and signals is just popped out. Unlike components, tags aren't explicitly updated when an entity is destroyed. Because of how entity identifiers are defined (entity + version), it isn't necessary to update tags and thus I never do it (just an optimization). In other terms, a tag is available to use if it has never been assigned to an entity or in case the entity to which it belongs has a different version (actually it means that the tag no longer belongs to that entity because the latter has been destroyed meanwhile). So the performance hit signals for tags would introduce is O(N) with the number of tags, at least for some operations.

I'm going to try a different approach. As far as I can see, you're interested in a sort of reactive systems and they look like a good tool to have actually. Right? Well, I think we can easily implement them even without signals on tags because of how those systems work. It's possible probably because of their update step that changes a bit the rules of the game. Tags can be treated differently than components and we can still get the same result.

mhaemmerle commented 6 years ago

You could also use two maps for tags to trade memory for speed.

Component -> Entity
Entity -> Component

If you'd split the EnTT tags feature into actual tags and singleton components, you could use two maps for the tags and one map for the singleton components, as they actually don't need an entity to work.

skypjack commented 6 years ago

@mhaemmerle

as they actually don't need an entity to work.

I didn't get this point. Single instance components need an entity to work actually.
As an example of use I've been told of by an user, imagine a Camera component you want to be unique. It's attached to an entity that has also a Transform and who knows what other components. When you want to update all the positions of your entities, you can get the transform from the entity that has the camera (attacchee), then perform a multiplication.

mhaemmerle commented 6 years ago

You could solve that with string tags.

I may be biased, but I really like this explanation of unique components. You can get at the entity behind the unique component, but it's frowned upon.

skypjack commented 6 years ago

@mhaemmerle

In Entitas, unique components are components. So they need an entity. Am I wrong?

mhaemmerle commented 6 years ago

They are components, Entitas generates the code to extend the Entity-class as well as the Context-class (using C# extension feature), so they still need an entity. When accessing the component, Entitas does an iteration over a collection containing exactly one entity. Which is ... sub-optimal IMO :)

Edit: Entitas uses partial classes to extend Entity/Context and C# extensions for the Matcher.

When removing the component, the entity gets destroyed as seen here.

So, instead of doing the proper thing and building up an index in the context, they've bent the existing tools quite a lot to enable unique components...

To summarize: if one would use an index for unique components, no entities are needed. They'd be regular structs which can be called components to describe their POD nature.

skypjack commented 6 years ago

@mhaemmerle

If one doesn't want to assign a single instance component to an entity (to be able to retrieve other components as in the example I made), why putting that component in a registry or ECS or whatever? I don't understand. I mean, make it a singleton and that's all. Is it only so as to be able to say - I don't use singletons for they are bad? Come on, that's exactly the same thing, used the same way, pretty well buried into a data structure so that it's hard to recognize. Right?

mhaemmerle commented 6 years ago

Of all the things mentioned in my comment and the Entitas cookbook ... that's the one thing you cherry-pick to get mad about? ;) Dude, I don't think I can help you at this point ;)

skypjack commented 6 years ago

@mhaemmerle

Ahahahah... Well, we were speaking about tags and single instance components, right? :-) At the end of the day, Entitas just uses two different data structures to handle exactly the same thing. It doesn't make much sense for me but I'm pretty sure there is a reason for that.

With a C++ ECS (not necessarily EnTT) you can use:

And that's all. This is how C++ works. The rest is syntactic sugar.

Do you want to see something like this as part of the API?

registry.tag<HashedString {"foobar"}>(entity);

It's trivial to do and the library already supports something similar out of the box. Why similar? Because what is missed is syntactic sugar and nothing more as I said, under the hood it would work exactly the same way it does today.
There is no reason to add a map to the registry to keep track of something we can already map to tags quite easily.

The fact is that you asked to separate tags in two parts a few comments ago: one (let me say) string based to assign to entities, the other one component based and entity free. An example, you said? Entitas. I pointed out that Entitas doesn't work this way and you replied that it should instead. I pointed out that it wouldn't make sense anyway because those would be only singletons buried within the registry or within Entitas and that's it...

It's me that noted only this in the Entitas cookbook, now. :-)

I'm not looking for help right now. I'm trying to understand why a singleton disguised as something else should be so tempting. It is not, at least from my point of view and you have no arguments so far to prove the opposite. :-)

So, it's time to reset the discussion. I think we went far from the topic. What was the initial request? I even forgot it.

mhaemmerle commented 6 years ago

I'm trying to understand why a singleton disguised as something else should be so tempting.

They already exist in EnTT:

registry.attach<PausedComponent>(registry.create());
bool paused = registry.has<PausedComponent>();

registry.attach<ScoreComponent>(registry.create(), 0);
ScoreComponent &score = registry.get<ScoreComponent>();

I don't want to create an entity for that:

registry.attach<PausedComponent>();
bool paused = registry.has<PausedComponent>();

registry.attach<ScoreComponent>(0);
ScoreComponent &score = registry.get<ScoreComponent>();

I want to revel in the glorious signal goodness without any nagging what-if's:

void onScoreAdded(Registry &registry)
{
    std::cout << "Score: " << registry.get<ScoreComponent>().score << std::endl;
}

void onScoreRemoved(Registry &registry)
{
    // ...
}

registry.tagAdded<ScoreComponent>.connect<&onScoreAdded>();
registry.tagRemoved<ScoreComponent>.connect<&onScoreRemoved>();

It is not, at least from my point of view and you have no arguments so far to prove the opposite. :-)

They're convenient AF. The registry contains my application's complete model. No need for setting up singleton(s). I'am lazy.

skypjack commented 6 years ago

Created a dedicated issue to discuss this (let me say) singleton mode for tags. I'm closing this issue because I made available signals for components on experimental. Thank you @mhaemmerle and all you guys for participating.

skypjack commented 6 years ago

@mhaemmerle

In the meantime I added signals for tags. Branch experimental.