rttrorg / rttr

C++ Reflection Library
https://www.rttr.org
MIT License
3.13k stars 433 forks source link

Multi-metadata #7

Open seanmiddleditch opened 8 years ago

seanmiddleditch commented 8 years ago

A very useful feature for reflection systems like RTTR is to be able to attach multiple copies of the same metadata key, each with a different value.

A simplified example taken from some production code in a game engine with a reflection system very similar to RTTR:

REFLECT(FooNode)
(
  // used by node graph UI to enumerate the events an instance of FooNode can emit
  node::event<EventA>(),
  node::event<EventB>(),
  node::event<EventC>()
)
.method("something", &type::something)
.property("thing", &type::thing);

Notably that interface doesn't even require an explicitly-named metadata key and instead just keys metadata by the type of the metadata itself. However, an interface that still requires the user to specify keys but allowed multiple values would be good on its own. Key-less metadata is not explicitly required, though it can be a further convenience (perhaps by just using the type of the value as the key if no key is explicitly specified?).

acki-m commented 8 years ago

You can do this right now.

RTTR_REGISTRATION
{
    registration::class_<foo>("foo")
        .method("func", &foo::func)
        (
            metadata("one key", std::make_tuple(node::event<EventA>(), 
                                                node::event<EventB>(),
                                                node::event<EventC>()))
        )
}

Making the metadata implicit is not something I would prefer. It just nice to read, that the following data is metadata. Just think about following situation:

RTTR_REGISTRATION
{
    registration::class_<foo>("foo")
        .method("func", &foo::func)
        (
            metadata("A", "value"),
            metadata("B", "value"),
            metadata("C", "value"),
            default_arguments(nullptr),
            policy::meth::discard_return
        )
}

instead of:

RTTR_REGISTRATION
{
    registration::class_<foo>("foo")
        .method("func", &foo::func)
        (
            node::event<EventA>(),
            node::event<EventB>(),
            node::event<EventC>(),
            default_arguments(nullptr),
            policy::meth::discard_return
        )
}

Having this metadata function makes the registration code more readable. Btw. the metadata gets moved into an std::array, so no memory is wasted.

A quote from boost python:

Remember the Zen, Luke:

"Explicit is better than implicit"

"In the face of ambiguity, refuse the temptation to guess"

seanmiddleditch commented 8 years ago

A single key is non composable (can't have functions that easily add groups of metadata to a class, or macros, etc.), and more explicit to boot. :)

seanmiddleditch commented 8 years ago

I mean that separate keys is more explicit.

acki-m commented 8 years ago

I do not really get it. Do you want something like JSON?

variant var = prop.get_metadata("key1.key2.key3");
seanmiddleditch commented 8 years ago

No.

I think the Event case I gave early is a good example, but I'm not sure how to better phrase it.

Adding multiple values to a single key has composability problems. I'm forced to list out all Event metadata in a single place and can't easily build up that list using helper functions. This can be important for composition-based class design, among other things.

It also ends up being a problem for inheritance, though an easier one to deal with: you just end up forcing the end-user querying metadata to walk the inheritance hierarchy to find all base's metadata. Helper functions could then deal with combining the metadata from the class hierarchy. But such workarounds shift the composition cost to every query instead of once at registration time.

acki-m commented 8 years ago

This can be important for composition-based class design, among other things.

What has the metadata to do with "composition-based class design"?

Maybe you can show me the other way around, show me an interface where you can retrieve your multiple key-to-value metadata? Sorry, but I don't get it otherwise.

What I can assure you is, this metadata wrapper function is needed and cannot be left away.

seanmiddleditch commented 8 years ago

What has the metadata to do with "composition-based class design"?

I need metadata of the same type on a single class that comes from multiple sources. Sticking with the Event example, it is very common to have a type that can receive a great many events: the ones it can receive, the ones that parents can receive, and the ones that inner data members can receive.

Maybe you can show me the other way around, show me an interface where you can retrieve your multiple key-to-value metadata

// inefficient way that fills in a vector
// take vector as inout so user can opt to reuse the allocated space
vector<Event> events;
my_type->query_metadata<Event>(key, events);
for (Event const& ev : events) ...

// faster way
strongly prefering a returned range and not a returned vector
for (Event const& ev : m_type->query_metadata<Event>(key)) ...

Our codebase is littered with constructs like those for network protocol generation, UI generation, component validation, asset conditioning, etc.

For registration, that can look like:

auto& r = registration::class_<foo>("foo")
  (
    foo::Inner::compose()
   )
  .method(...)
  ( ... );

Assuming foo::Inner::compose() returns a function object whose operator() takes the registration builder type, RTTR can invoke that object (akin to how IOStreams manipulator functions compose). That then allows for Foo to have metadata injected into it from other sources that are not necessarily under the direct control of Foo.

Not being able to do that means that the loops up above have to not only manually iterate over Foo to find its copy of key and then manually search parents for key but then also iterate through each and every member of both Foo and all parents to see if they have a new composable_key or such. Turns into a pain, and a pain that's pushed out into every single call sight (or a helper) instead of localized in the internals of type registration.

If that hasn't made my case, I admit defeat and am perfectly fine with this request being closed. :)

What I can assure you is, this metadata wrapper function is needed and cannot be left away.

Of course. It's a perfectly reasonable API design.