Feature: Entity Builder pipe method for passing a function to dot-chain

[garrett-is-a-swann]

Background

As discussed previously on discord, the mechanism to emplace with additional arguments was removed in this commit, which makes on_add hooks or observers the only in-api way to have custom logic to occur during entity construction.

Hooks and observers both have the same restriction of calls not being able to nest -- if one hook/observer creates an entity/sets component values that would invoke another hook/observer, those will not occur until the current hook/observer has finished resolving.

Out-of-api, we can stop building, perform logic, and continue building:

auto my_entity = ecs.entity().add<SomeTag>();

if (something) {
  my_entity.add<...>(....);
}
... etc

but if we want to reuse this sort of logic in a function

template<class... Args>
flecs::entity optionalAdd(flecs::entity e, Args... args) {
  if (e.has<SomeTag>()) {
    e.set<...>({args...});
  }
  return e;
}

we must break the flow of dot chain

auto my_entity = ecs.entity()
  .add<SomeTag>();
optionalAdd(my_entity, data1, data2);
my_entity.add<...>();

or by getting creative with parenthesis

auto my_entity = optionalAdd(ecs.entity()
  .add<SomeTag>(), data1, data2)
.add<...>();

Proposal

A way to easily solve this in-api, is to have a "piping" function that takes a function call and argument list, that "pipes" the entity to the first argument of the given function and forwards the rest of the arguments:

auto my_entity = ecs.entity()
  .add<SomeTag>()
  .pipe(optionalAdd, data1, data2)
  .add<...>(...);

This allows users to create functions for self-contained builder logic, and use/reuse them as wanted directly during entity building.

[SanderMertens]

I'm not sure if having to break up the dot chain is a strong enough reason to add a new method to the builder class. My thoughts:

• It's aesthetically more pleasing but also slightly less readable (what are the semantics of "pipe"?)
• The (current) behavior is confusing when done from a deferred context. Should "pipe" also be enqueued as command?
• I'm not sure if this solves the nesting problem, except if you mean that optionalAdd can also call a reusable function?
• If so, couldn't you solve the nesting problem by having a single observer on SomeTag and then in the observer just do the same as in optionalAdd?

[garrett-is-a-swann]

I'm not sure if having to break up the dot chain is a strong enough reason to add a new method to the builder class.

That's very fair. 🙂 This is mostly just to include something I find nice/useful, and assumed others might appreciate as well. If it doesn't make it in, no big deal. Admittedly, not the largest value add.

It's aesthetically more pleasing but also slightly less readable (what are the semantics of "pipe"?)

Pipe could be renamed to "call", "thread", or a number of other things. The semantics though, are from unix pipelines (| operator) and functional programming "function composition pipelines" (clojure, various, haskell)

The (current) behavior is confusing when done from a deferred context. Should "pipe" also be enqueued as command?

It doesn't really solve the problem when called from a deferred context -- this is a solution for a non-deferred scenarios since you can't expect entity modifications to occur until after observer/hook callbacks have fully resolved.

I'm not sure if this solves the nesting problem, except if you mean that optionalAdd can also call a reusable function?

That is what I mean. You can do

void Image::create(entity image) {
  VkImage vk_image;
  VkCreateImage(..., vk_image);

  vk_image.set(vk_image);

  auto image_view = image.world().entity()
    .add<VkImage>(image)
    .pipe(ImageView::create);

  image.add<DefaultView>(image_view);
}

void ImageView::create(entity image_view) {
  VkImageView vk_image_view;
  VkCreateImageView(..., image_view.ensure<VkImage>(), vk_image_view);
  image_view.set(vk_image_view);
}

// create an image with a default image view is now just 
ecs.entity().pipe(Image:create); // Only marginally handwavy

If so, couldn't you solve the nesting problem by having a single observer on SomeTag and then in the observer just do the same as in optionalAdd?

In my testing and per our prior conversations, I believe you cannot. There isn't a stack for observer calls, so if inside one observer you modify state on an entity such that it should induce another observer call, that call will not occur until after the current observer resolves. Therefore, if you depend on state from that second observer, you can't ensure it's available -- thus the restructuring to manually calling builders and the want for ergonomics that the builder pipe method offers 🙂

[SanderMertens]

There isn't a stack for observer calls, so if inside one observer you modify state on an entity such that it should induce another observer call

That's correct, but inside an observer you can call other functions, e.g. in an Image OnAdd observer you could call ImageView::create, which is essentially the same as the pipe approach. An observer has the advantage that the correct construction logic is applied unconditionally (what if an entity/component is instantiated through some other means, like the explorer)

I think the use case for this is a bit too specific for it to get upstreamed. What you could do however is add this as a mixin to the entity_builder class (see how the API currently does mixins). That lets you keep the changes in a separate files, and should cause minimal conflicts when merging with master.

[garrett-is-a-swann]

That's correct, but inside an observer you can call other functions, e.g. in an Image OnAdd observer you could call ImageView::create, which is essentially the same as the pipe approach.

I think the only catch here is if the xxx::create functions require state that was only just added to entities during the observer's runtime. I think the nail in the coffin for the way I previously structured my code was having a function either create a new entity or return a cached entity if a similar one already existed... but creating a new entity would always result in errors because the entity I just created didn't have its state yet.

That being said, I agree that it's niche and the problem is solvable in many ways other than this.

What you could do however is add this as a mixin to the entity_builder class (see how the API currently does mixins). That lets you keep the changes in a separate files, and should cause minimal conflicts when merging with master.

Awesome, I'll try that out :^) Very cool that you have that in your build system.