Review: Approve

Code LGTM, just 1 code style nit.

I noticed that you use variable type "auto" a lot - please double-check if you 
can be more specific. Strong typing is good ;)


Diff comments:

> 
> === modified file 'src/editor/editorinteractive.h'
> --- src/editor/editorinteractive.h    2016-01-07 12:47:17 +0000
> +++ src/editor/editorinteractive.h    2016-01-16 12:55:32 +0000
> @@ -129,17 +153,16 @@
>  
>       //  state variables
>       bool m_need_save;
> -     struct PlayerReferences {
> -             int32_t      player;
> -             void const * object;
> -     };
>       std::vector<PlayerReferences> m_player_tribe_references;
>  
>       uint32_t m_realtime;
>       bool m_left_mouse_button_is_down;
>  
> -     EditorHistory m_history;
> +     std::unique_ptr<Tools> m_tools;
> +     std::unique_ptr<EditorHistory> m_history;

Time to rename to tools_, history_ etc?

>  
> +     
> std::unique_ptr<Notifications::Subscriber<Widelands::NoteFieldResourceChanged>>
> +        field_resource_changed_subscriber_;
>       UI::UniqueWindow::Registry m_toolmenu;
>  
>       UI::UniqueWindow::Registry m_toolsizemenu;


-- 
https://code.launchpad.net/~widelands-dev/widelands/fix_resource_overlay/+merge/282680
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/fix_resource_overlay.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to