Simplify C++ QueryInterface syntax

[Philip]

Currently C++ components have to say:

ICmpPosition* cmpPosition = static_cast<ICmpPosition*>
  (context.GetComponentManager().QueryInterface(GetEntityId(), IID_Position));

which is horrid.

JS just says

var cmpPosition = Engine.QueryInterface(this.entity, IID_Position);

which is nicer. C++ has got to have more type declarations, and there's tradeoffs with other desirable features of the code, but it should be possible to do better than the current syntax.

[Philip]

An obvious thing to try:

#define QUERY_INTERFACE(ent, type) static_cast<ICmp##type*> (context.GetComponentManager().QueryInterface((ent), IID_##type));

CmpPosition* cmpPosition = QUERY_INTERFACE(GetEntityId(), Position);

which makes the assumption that context is available. Under the current model, usually it's an argument to the current method, so that's fine; but sometimes you'd need to do CSimContext& context = m_Context; first, which is nasty since the use is hidden in the macro. Unless we change the way the context is passed around within components, the macro should probably be:

#define QUERY_INTERFACE(context, ent, type) static_cast<ICmp##type*> (context.GetComponentManager().QueryInterface((ent), IID_##type));

CmpPosition* cmpPosition = QUERY_INTERFACE(context, GetEntityId(), Position);

so it's explicit.

An alternative is to have a real entity object, instead of entity IDs, and the object can provide methods like ICmpPosition* GetPosition() (probably still implemented with the global interface/component maps - the entity object itself wouldn't contain any data (other than the ID and maybe a pointer to the component manager), and wouldn't be reference-counted or anything).

In JS, entity objects mean you could write var cmpPosition = this.entity.Position. (That's independent of whether the C++ implementation has entity objects or entity IDs). But I don't really like how it makes the relatively expensive map lookup look like a simple property access; the explicit QueryInterface discourages its use, without being painful when you really do want to use it.

[jan]

• The basic idea of a macro sounds reasonable, it'd be nice to get rid of some of the duplication.

• However, specifying Position instead of IID_Position reduces the number of characters, but probably requires *more* typing (IID_* allows autocompletion). there's also the issue of being able to look up the symbol defintion (e.g. to see what other IIDs there are), which is always an argument against token-pasting via preprocessor

• I wouldn't have any problem with context being an implicit parameter (not exactly a rarity when macros are used), since it seems likely for it to be called that (I'd expect m_context to be rarer). Maybe we could just create a second QUERY_INTERFACE_FROM_CONTEXT(context, ent, type) macro?

• The argument that 'entity.Position' is misleadingly cheap/normal is a good one, I wouldn't mind writing QueryInterface in JS.

[Philip]

Yeah, autocompletion would be nice. m_Context is quite common in the current design, since it's used whenever a method (other than those inherited from IComponent) needs to access other components, so I'd prefer not to make that case harder.

olsner suggested something like

ICmpPosition* cmpPosition = ICmpPosition::GetFrom(context, GetEntityId());

Then there's also

class IComponent {
    template <typename T>
    T* QueryInterface(context, ent) {
        return static_cast<T*>(context.GetComponentManager().QueryInterface(ent, T::GetInterfaceId()));
    }
    template <typename T>
    T* QueryInterface(context) {
        return QueryInterface<T*>(context, GetEntityId());
    }
};

...

ICmpPosition* cmpPosition = QueryInterface<ICmpPosition>(context);
ICmpPosition* cmpPosition = QueryInterface<ICmpPosition>(context, otherentity);

I think that'd work, and it's about as concise as possible, and it doesn't need macros, and the default-to-self is convenient (and a common case).

[jan]

Nice! I like the template idea more than a macro - it's just regrettable that argument deduction doesn't work. It also seems better than the GetFrom suggestion (as discussed in the meeting, that would have to be defined by a macro and can't be in the base class and is therefore a little less obvious).

[Philip]

Deducing T from the return type, in the variable initialisation? I suppose that could actually work if the type wasn't a raw pointer... If not using raw pointers, perhaps something like:

template <typename T>
class CmpPtr {
    T* m;
public:
    CmpPtr(context, ent) {
        m = static_cast<T*>(context.GetComponentManager().QueryInterface(ent, T::GetInterfaceId()));
    }
    T* operator->() { return m; }
};

...

CmpPtr<ICmpPosition> cmpPosition(context, GetEntityId());

That can't do default-to-self but it means you only have to specify the interface type once. But maybe IDEs can't autocomplete method names after you type cmpPosition-> when it's not a real pointer.

There's too many options to choose from :-(

[jan]

No worries, even VA 10.2 on VC2005 is able to autocomplete class members of smart pointers (including the basic CmpPtr you sketched above). I think this new idea improves upon the previous ICmpPosition* cmpPosition = QueryInterface<ICmpPosition>(context);

Writing GetEntityId() isn't particularly onerous, and this solution avoids any and all redundancy without macro hackery.

[Philip]

I was more worried about Eclipse, since its autocomplete is far less good than VA's. But it seems to cope fine here. I've done the CmpPtr thing now (see changeset 6a8a0491d4a2), so hopefully it won't need many more changes.

[anonymous]

Milestone Simulation stage 1 deleted