Review: Approve
Found only intended renamings or improvements,
Added some comments about minor things.
New for me are warnings like:
[ 44%] Building CXX object
src/logic/CMakeFiles/logic.dir/map_objects/tribes/requirements.cc.o
.../bug-1395278-editor/src/logic/map_objects/tribes/requirements.cc:247:5:
warning: format specifies type 'unsigned int' but the argument has underlying
type 'uint8_t' (aka 'unsigned char') [-Wformat] TrainingAttribute::kHealth,
I think someone reenabled that clang warning.
Can be merged
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)
Why use the_ as a prefix, strange?
> {}
>
> 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_)
This (i == First ? *this : i == Second ? second_ : third_) would benefit from
an inline function
> .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.
_______________________________________________
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
