entt: discussion - empty components should not be part of the each callback

skypjack was already refactoring the empty instance pools -> https://github.com/skypjack/entt/commit/e228cb66488fc6ae98365ce2f5f8db45834a92c3#diff-36792e38ee0671f0a02c94d88a91cfbb idk if that is gonna have any impact on this issue…

but basically something like

reg.group<Component, EmptyTag>().each([](auto id, auto comp) {});
reg.group<Component, EmptyTag>().each([](auto comp) {});

instead of

reg.group<Component, EmptyTag>().each([](auto id, auto comp, auto tag) {});
reg.group<Component, EmptyTag>().each([](auto comp, auto tag) {});

since there’s no point in having empty instance in the arguments… This is a api breaking change though so it’d have to be part of a major release cause I’m sure everyone like me has just been including the tags in their each callbacks.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 1
  • Comments: 46 (40 by maintainers)

Commits related to this issue

Most upvoted comments

I like your approach for input handling, I’ve never seen something like that before. Personally, I set up a model where entities announce their interest in specific inputs by means of components to which intents are attached. An intent system looks for matches and triggers these opaque objects, the intents. Someone else, somewhere else, if any, will do something as a consequence. It’s a sort of publisher/subscribers made with components.

That’s actually similar to mine. I had a system for components so, say, an InputImpulse component could be attached to something, and it was bound to a path (with a non-serialized cached Node entry, well, technically it was serialized just overwritten to be null upon deserialization, I tried to design everything possible to be simple memcpy’s for saving out for speed reasons, and consequently saving was super fast), and the system would just listen for events on the given node, aggregate them into a list inside the system, then drain it to perform the physics impulses or dumping inputs into the GUI system or whatever. It was pretty simple and not really the best design, but it worked and was fast enough since inputs didn’t happen thousands of times per tick. It was a great action-map system by using node events with transformers and all though. And I cannot state how useful it was for all inputs to be floats, whether a button, absolute axis, relative axis, whatever. A simple transformer could bring anything and everything into the right setup for whatever needed to process it.

I’ll look at the implementation offered by amethyst to see if it’s any way close to what I’ve in mind.

Their forums have a lot of discussion though a lot of it is on their Discord.

Out of curiosity, how did you construct the dependency tree? Just exploring it every time something was added to find the hook point or was it more clever than this?

It was pretty trivial and not super efficient, but as it was only done when a system was added to the context (registry in ENTT terms) and then an update was performed it wasn’t really that important since it only took a few hundred milliseconds at worst. All it did was this:

  1. Iterate all systems grabbing their aspects (an Aspect was basically a listen endpoint for a set of entities based on component types, so something like Aspect.of<PositionTable, VelocityTable>().exclude<DisabledTable>().build(ctx) linked to or created a set in the context aspect tree of what to map against and thus what should be updated, any of Aspect of the same information had the same list in memory of entities at the same location), and grabbing their concurrency settings (free to run concurrently with others, don’t run with XX system, always run after YY system, don’t run concurrently with anything, etc…).
  2. Once all aspects were touched then a bi-bitset was created of the same size of the count of all table types referenced (bi being it used 2 bits per table, 0 meant unused, 1 meant read-only, 2 meant read-write, and 3 meant exclude).
  3. The systems were iterated again getting their associated bitsets based on their aspects to get a set of those bitsets.
  4. The bitsets were sorted based on the number of the bi-bits set, then a tree was built based on that order for what was allowed to run together based on the system concurrency setting, it would assert if any systems were required to run before each other (circular failure) and it would fall back to non-concurrent if anything unexpected popped up.

It was not at all efficient and I meant to go over it again. Honestly it should be modeled like, say, a package dependency resolver library, of which is a well researched topic now. It really wasn’t that complicated though. If you want it to be ‘perfect’ then of course it is an NP-Complete problem, but I didn’t want perfect, I just found the first set that ‘fit’ well enough. Probably modeling it as a graph and using Dijkstra’s to solve it would be better and more simple actually.

@Kerndog73 good point. However, consider that this is rare, most of the times you’ll just use each without a list of components. My concern is instead that the list of parameters for each can be misleading for a newcomer, because it’s not immediately obvious if you look at the list of template parameters of a view or a group. That’s why I added less aside each initially.

I’m thinking of removing the empty components from each and get rid of less. Actually, it doesn’t make much sense to have fake instances of empty components.

BUT

I want also to add an optional list of components to each, so as to be able to get also empty components if required or even a shorter list of components when one isn’t interested in all of them. As an example:

registry.view<A, B, C>().each<A, B>([](auto &&...) { /* ... */ });

It will be faster for obvious reasons and more than once I needed something like this.

What about the two points above?

Yep, that’s exactly the point. Two versions to iterate, one returns all the components, one returns only non-empty ones.

What about the reference received by a signal?
Currently, the signature is something like void receive(registry &, entity, Component &);. The last parameter is pointless as well in case of empty types and it’s probably misleading.
What do you guys think about it?

very true in making the early exit optional, it keeps the callback cleaner too instead of doing a “return false” at the end of every callback that doesn’t care about early exit.

+1 for early exit

I suppose we could introduce a for_each method for this new signature callback order. I’d also suggest having it expect a boolean return for early exiting?