Code review comment for lp:~gordallott/unity/hud

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

There are a few introspection / autopilot issues with this branch:

Your HUDController derives from Introspectable, but you never use it. You need to:
 1) Add it to the introspection tree.At Diff line 2729 (where you create the HUDController instance) you need to call "AddChild(hud_controller_.get());". If the HUDController is ever deleted, you need to call "RemoveChild(hud_controller_.get());" BEFFORE the deletion takes place.

 2) Add properties to the HUD controller that will be useful in autopilot tests.

You need to repeat this process with all child objects of the HUDController that derive from Introspectable.

Finally, the string returned from "GetName" should be "HUDController" - you don't need to qualify it with "unity.hud".

In HUDIcon.h you #include "Introspectable.h", but never use it.

In HUDView.h, you've declared a "const gchar* GetName()" method, which is from Introspectable, but there's two problems:
 1) You don't derive from Introsopectable (as noted above, you should).
 2) That's not the correct method signiture - the return type is "std::string", and the method is const.

In the same class, make sure you add the search bar to the introspection tree - that way we can get the current search string.

Finally, it'd be really nice to have some autopilot tests written when this is merged. I don't mind helping you with this.

Cheers,

review: Needs Fixing

« Back to merge proposal