Review: Approve compile, review
Looks good to me, with Apple clang I get a lot less warnings.
You removed on break; that looks incorrect, but I am but sure,
see diff comments.
I prepared some changes that would remove some implicit float -> double
warnings, too.
Not sure if I should commit these, though.
Diff comments:
>
> === modified file 'src/ai/ai_help_structs.h'
> --- src/ai/ai_help_structs.h 2016-03-12 20:06:24 +0000
> +++ src/ai/ai_help_structs.h 2016-04-02 16:49:56 +0000
> @@ -351,8 +350,8 @@
> uint16_t mines_percent; // % of res it can mine
> uint32_t current_stats;
>
> - std::vector<int16_t> inputs;
> - std::vector<int16_t> outputs;
> + std::vector<Widelands::DescriptionIndex> inputs;
> + std::vector<Widelands::DescriptionIndex> outputs;
What _is_ a DescriptionIndex? Found it in src/logic/widelands.h but where is
the object this index is ued for?
> std::vector<Widelands::DescriptionIndex> critical_building_material;
>
> bool produces_building_material;
>
> === modified file
> 'src/editor/ui_menus/editor_tool_change_resources_options_menu.cc'
> --- src/editor/ui_menus/editor_tool_change_resources_options_menu.cc
> 2016-03-14 18:10:09 +0000
> +++ src/editor/ui_menus/editor_tool_change_resources_options_menu.cc
> 2016-04-02 16:49:56 +0000
> @@ -197,10 +197,7 @@
> case Change_By_Increase: change_by += change_by < 63; break;
> case Change_By_Decrease: change_by -= 1 < change_by; break;
> case Set_To_Increase: set_to += set_to < 63; break;
> - case Set_To_Decrease: set_to -= 0 < set_to; break;
> - default:
> - NEVER_HERE();
> - break;
> + case Set_To_Decrease: set_to -= 0 < set_to;
-=0 // thats a noop?
> }
> increase_tool_.set_change_by(change_by);
> increase_tool_.decrease_tool().set_change_by(change_by);
>
> === modified file 'src/logic/map_objects/bob.cc'
> --- src/logic/map_objects/bob.cc 2016-03-31 06:56:28 +0000
> +++ src/logic/map_objects/bob.cc 2016-04-02 16:49:56 +0000
> @@ -712,7 +712,7 @@
> // Slowing down a ship if two or more on same spot
> // Using probability of 1/8 and pausing it for 5, 10 or 15 seconds
> if (game.logic_rand() % 8 == 0) {
> - if (upcast(Ship, ship, this)) {
> + if (is_a(Ship, this)) {
Ah, much better!
> Map& map = game.map();
> const uint32_t ships_count
> =
> map.find_bobs(Widelands::Area<Widelands::FCoords>(get_position(), 0),
> nullptr, FindBobShip());
>
> === modified file 'src/scripting/lua_globals.cc'
> --- src/scripting/lua_globals.cc 2015-02-28 17:43:48 +0000
> +++ src/scripting/lua_globals.cc 2016-04-02 16:49:56 +0000
> @@ -104,7 +104,6 @@
> case LUA_TTHREAD:
> case LUA_TLIGHTUSERDATA:
> report_error(L, "Cannot format the
> given type %s at index %i", lua_typename(L, i), i);
> - break;
This looks wrong, perhaps:
NEVER_HERE(); // as report_error will never return
would be better?
>
> default:
> {
>
> === modified file 'src/ui_basic/multilinetextarea.h'
> --- src/ui_basic/multilinetextarea.h 2016-03-25 17:01:05 +0000
> +++ src/ui_basic/multilinetextarea.h 2016-04-02 16:49:56 +0000
> @@ -74,7 +74,7 @@
>
> /**
> * This prepares a non-richtext text for rendering. It escapes the
> source text and
> - * turns \n into <br> tags as needed, then creates the richtext style
> wrappers.
> + * turns '\\n' into '<br>' tags as needed, then creates the richtext
> style wrappers.
Thx, that was the most annyoing one
> */
> std::string make_richtext();
>
--
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/travis-clang-warnings.
_______________________________________________
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
