Entt: Automatic getters and setters for signals

[Kerndog73]

I found myself using setters as slots in Qt. I used this helper:

template <typename Object>
constexpr auto setter(Object *obj) {
  return [obj](auto &&value) {
    *obj = std::forward<decltype(value)>(value);
  };
}

It made me think that perhaps this would make sense as part of EnTT. Here's an example:

int value = 0;
entt::sigh<void(int)> somethingChanged;
entt::sink{somethingChanged}.connect(&value);
somethingChanged.publish(42);
// value == 42

You connect a pointer to an object and depending on the signature of the delegate (or sink), you'd either have a setter or a getter for that object. If you connect an int * to a void(int) then a setter is connected. If you connect an int * to an int() then a getter is connected. The types shouldn't need to be an exact match. You could connect a long * to a void(int) just fine.

The same effect can be achieved with the current API:

struct Object {
  int value = 0;
  void set(int newValue) {
    value = newValue;
  }
};
Object obj;
entt::sigh<void(int)> somethingChanged;
entt::sink{somethingChanged}.connect<&Object::set>(&obj);
somethingChanged.publish(42);
// obj.value == 42

The proposed API can help write cleaner code.

[skypjack]

int value = 0;
entt::sigh<void(int)> somethingChanged;
entt::sink{somethingChanged}.connect(&value);
somethingChanged.publish(42);
// value == 42

What's the use case for this? You can already attach data members or static variables. I don't see an use for attaching a local variable that is going to be deleted as soon as it goes out of scope. Can you give me more details?

[Kerndog73]

Currently, you can attach a data member as a getter but not a setter (because of std::invoke). I find that the current syntax is a little verbose.

sink.connect<&Object::value>(&obj); // current (getter)
sink.connect(&obj.value); // proposed (getter or setter)

[skypjack]

Indeed, std::invoke doesn't help to set values, you're right.
Let's look at this: sink.connect(&obj.value);. It boils down to a delegate that stores aside a pointer to function and a pointer to an instance. In your case, you've a pointer to member functions that isn't guaranteed to fit with the size of a pointer to function and you don't have the instance on which to _invoke_ it.
How do you propose to solve this? It's not clear to me yet.

[Kerndog73]

The data pointer would store &obj.value. fn would store this:

[](void *payload) {
  return *static_cast<int *>(payload);
}

...or this:

[](void *payload, int newValue) {
  *static_cast<int *>(payload) = newValue;
}

[skypjack]

A pointer to a data member (that is &obj.value unless you define it as static) isn't guaranteed to fit in a void * though. So data isn't enough for your purposes.

[Kerndog73]

&obj.value is an int *.

[Kerndog73]

struct Object {
  int value;
} obj;
int *ptr = &obj.value;

[skypjack]

Oh, ok, I thought you mistyped it from &obj::value. :smile:
Why can't you just use connect<&obj::value>(&obj)? It's equivalent and it works, it's how it is meant to be used. It's only a matter of typing less characters or there are other reasons?

[Kerndog73]

The proposed syntax is a little less characters for getters and a lot less characters for setters. That's really the whole point!

[skypjack]

Interesting. It would work also for whole objects to an extent, not only for data members or variables.

[skypjack]

I've still to write tests (and I cannot do that right now), but experimental contains the first draft of what you asked for. Does it look good to you?

---

Well, ok, I wrote also some tests and I hit a bug of MSVC. Again! :man_facepalming:

[skypjack]

Out of my head, not tested.
With the current implementation, you can already obtain it using data members as:

struct Object {
  int value = 0;
};
Object obj;
entt::delegate<int &()> somethingChanged;
somethingChanged.connect<&Object::value>(&obj);
somethingChanged() = 42;

Am I wrong? Why did you exclude this?

[Kerndog73]

@skypjack Returning a reference works well for delegate but for sigh (the primary use case) you have to use a collector so the calling code is a little ugly. Perhaps I'll try to write a more fleshed-out example. When I wrote int value = 0;, what I meant was "an int that is somewhere and has the correct lifetime". I think I should avoid doing that in future.

---

I started writing a more fleshed-out example but then I realised that maybe this isn't as useful as I first thought. I was going to explain why this feature is useful but instead, I'm going to do the opposite.

Let's say that you're making an editor using EnTT and Dear Imgui. I've never made an editor, nor have I used Dear Imgui so you'll have to fill in the gaps. There's a class for keeping track of the current entity. There are also various classes for manipulating the current entity, such as adding components, removing components, editing components and destroying the entity. When the current entity changes, a signal lets the other classes know.

/// Keeps track of the current entity
class CurrentEntity {
public:
  void display() {
    if (/* the user clicked on a different entity */) {
      entity = /* new entity */;
      changed.publish(entity);
    }
  }

  auto onChanged() {
    return entt::sink{changed};
  }

private:
  entt::entity entity;
  entt::sigh<void(entt::entity)> changed;
};

/// Destroys the current entity
class DestroyEntity {
public:
  void display(entt::registry &reg) {
    if (/* the user clicked a button to destroy the current entity */) {
      reg.destroy(entity);
    }
  }

  // Using new feature
  void setEntity(entt::sink<void(entt::entity)> changed) {
    changed.connect(&entity);
    // We can easily create and store a scoped_connection
    // That seems to be the only pro
  }

  // Using current features
  void setEntity(entt::entity newEntity) {
    entity = newEntity;
  }

private:
  entt::entity entity;
};

int main(int argc, char **argv) {
  entt::registry reg;
  CurrentEntity current;
  DestroyEntity destroy;
  // Using new feature
  destroy.setEntity(current.onChanged());
  // Using current features
  current.onChanged().connect<&DestroyEntity::setEntity>(&destroy);
  while (true) {
    current.display();
    destroy.display(reg);
  }
}

The purpose of this feature was to write less code. As you can see from the example, you end up writing about the same amount of code. DestroyEntity is also coupled to the full signature of the onChanged signal so if the signal emitted some more parameters that we didn't care about, we'd still have to include them in the parameter type of setEntity. Of course, we could make setEntity a template but ugh, then it has to be in a header.

The code in main becomes much cleaner using the new feature (so I wouldn't be tempted to use a macro) but the receiver classes are kind of ugly. If you're passing a sink to the receiver object then the receiver class needs a function for connecting the slot to the signal and a function that is the slot. That's two functions instead of one if you want to be consistent. I've been trying to find a way to fit the feature into this example but I keep running into problems. Every time I try to solve a problem, I just create more problems. That's enough proof for me to realise that this feature really isn't useful.

I'm not even using this in my own code anymore. I was using this macro, then I opened this issue, then I refactored some things, and now I've just realised that I'm no longer using it.

If this feature does have a use, I certainly haven't found it. I'm really sorry for wasting your time on this.

[skypjack]

Not a problem, don't worry. You're welcome.

---

As a side note, I really like this idea:

changed.connect(&entity);

But I don't like much this:

void setEntity(entt::sink<void(entt::entity)> changed) {
    // ...
}

In general, my classes expose slots but they never accept sinks or connect directly to other instances. There exists always a sort of _director_ that takes the sender and the receiver and connects them.
In a sense, I don't want a class to _know_ too much. Not sure it's clear what I mean.
By the way, this is probably why this feature sounded odd to me initially (I've no use cases as well) but now I see what was your goal at least. :+1:

Thanks for the details. :wink:

[Kerndog73]

I agree with you completely. That's exactly what's wrong with this feature. In order to use it, you either have to expose private data or do weird stuff.

[skypjack]

I could imagine an use as an example for input handling.
If you use components for that, the input system could trigger a signal and update directly their data members (that are already public after all). However, it isn't much different from iterating them while it's probably less clear to a reader.