widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

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.