← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1395278-editor into lp:widelands

 

I think we should go through all the clang warnings on Travis sometime and fix.

Added replies to your comments.

Diff comments:

> 
> === modified file 'src/editor/tools/editor_increase_height_tool.h'
> --- src/editor/tools/editor_increase_height_tool.h	2016-01-28 05:24:34 +0000
> +++ src/editor/tools/editor_increase_height_tool.h	2016-03-18 12:52:47 +0000
> @@ -30,8 +30,8 @@
>  	 EditorSetHeightTool    &   the_set_tool)
>  		:
>  		EditorTool(the_decrease_tool, the_set_tool),
> -		m_decrease_tool(the_decrease_tool), m_set_tool(the_set_tool),
> -		m_change_by(1)
> +		decrease_tool_(the_decrease_tool), set_tool_(the_set_tool),
> +		change_by_(1)

I expect that's to avoid shadowing of functions set_tool() and decrease_tool(). We usually use input_decrease_tool etc. now.

>  	{}
>  
>  	int32_t handle_click_impl(const Widelands::World& world,
> 
> === modified file 'src/editor/tools/editor_tool.h'
> --- src/editor/tools/editor_tool.h	2016-02-01 08:21:18 +0000
> +++ src/editor/tools/editor_tool.h	2016-03-18 12:52:47 +0000
> @@ -62,23 +62,23 @@
>  		EditorInteractive& parent, EditorActionArgs* args, Widelands::Map* map)
>  	{
>  		return
> -		    (i == First ? *this : i == Second ? m_second : m_third)
> +			 (i == First ? *this : i == Second ? second_ : third_)
>  		    .handle_undo_impl(world, center, parent, args, map);
>  	}
>  
>  	const char * get_sel(const ToolIndex i) {
>  		return
> -		    (i == First ? *this : i == Second ? m_second : m_third)
> +			 (i == First ? *this : i == Second ? second_ : third_)
>  		    .get_sel_impl();
>  	}
>  
>  	EditorActionArgs format_args(const ToolIndex i, EditorInteractive & parent) {
>  		return
> -		    (i == First ? *this : i == Second ? m_second : m_third)
> +			 (i == First ? *this : i == Second ? second_ : third_)

Or what about a swich statement? We should also check whether First, Second, Third can be an enum class

cf. https://bugs.launchpad.net/widelands/+bug/1367725

>  		    .format_args_impl(parent);
>  	}
>  
> -	bool is_unduable() {return undoable;}
> +	bool is_undoable() {return undoable_;}
>  	virtual bool has_size_one() const {return false;}
>  	virtual EditorActionArgs format_args_impl(EditorInteractive & parent) {
>  		return EditorActionArgs(parent);


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-editor/+merge/289494
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1395278-editor.


References