Thanks for your efforts on cleaning up the code!
Most of this branch looks good to me, although I have a a few comments. The
first two points should probably be fixed, the other ones are not so important.
Note that I haven't tried compiling or testing, I only looked at the code.
Diff comments:
>
> === modified file 'src/graphic/gl/fields_to_draw.h'
> --- src/graphic/gl/fields_to_draw.h 2017-05-13 18:48:26 +0000
> +++ src/graphic/gl/fields_to_draw.h 2017-06-28 08:25:35 +0000
> @@ -78,6 +78,7 @@
> };
>
> FieldsToDraw() {
> + reset(0, 0, 0, 0);
With this we end up with w_=h_=1 and field_.size() of 1. I think the values of
w_ and h_ are fine (checks are done for <w_) but a size() of 0 would be
preferable. Haven't checked whether anyone cares about the size, though.
> }
>
> // Resize this fields to draw for reuse.
>
> === modified file 'src/logic/field.cc'
> --- src/logic/field.cc 2017-01-25 18:55:59 +0000
> +++ src/logic/field.cc 2017-06-28 08:25:35 +0000
> @@ -23,6 +23,9 @@
>
> namespace Widelands {
>
> +Field::Field() : bobs(nullptr), immovable(nullptr) , caps(7), roads(6),
> height(0), brightness(0), owner_info_and_selections(Widelands::neutral()),
> resources(INVALID_INDEX), initial_res_amount(0), res_amount(0),
> terrains{INVALID_INDEX, INVALID_INDEX} {
Where are the values 7 and 6 are coming from? As far as I know, the ": 7" below
is not an initialization but a hint to the compiler, how many bits this
variable uses.
(Seems like someone was trying to save some memory there. Not sure whether this
try is successful.)
> +}
> +
> /**
> * Set the field's brightness based upon the slopes.
> * Slopes are calulated as this field's height - neighbour's height.
>
> === modified file 'src/logic/map_objects/tribes/militarysite.h'
> --- src/logic/map_objects/tribes/militarysite.h 2017-06-25 19:12:30
> +0000
> +++ src/logic/map_objects/tribes/militarysite.h 2017-06-28 08:25:35
> +0000
> @@ -34,6 +34,13 @@
> class Soldier;
> class World;
>
> +// I assume elsewhere, that enum SoldierPreference fits to uint8_t.
> +enum class SoldierPreference : uint8_t {
> + kNotSet,
I think we only need kNotSet for a few seconds when loading a savegame. Maybe
just drop it?
> + kRookies,
> + kHeroes,
> +};
> +
> class MilitarySiteDescr : public BuildingDescr {
> public:
> MilitarySiteDescr(const std::string& init_descname,
>
> === modified file 'src/logic/map_objects/world/map_gen.cc'
> --- src/logic/map_objects/world/map_gen.cc 2017-01-25 18:55:59 +0000
> +++ src/logic/map_objects/world/map_gen.cc 2017-06-28 08:25:35 +0000
> @@ -84,7 +84,7 @@
>
> MapGenAreaInfo::MapGenAreaInfo(const LuaTable& table,
> const World& world,
> - MapGenAreaType const areaType) {
> + MapGenAreaType const area_type) {
Why not also rename the parameter in the other methods?
> weight_ = get_positive_int(table, "weight");
>
> const auto read_terrains = [this, &table, &world](
>
> === modified file 'src/wui/debugconsole.cc'
> --- src/wui/debugconsole.cc 2017-01-25 18:55:59 +0000
> +++ src/wui/debugconsole.cc 2017-06-28 08:25:35 +0000
> @@ -90,6 +91,7 @@
> ChatMessage cm;
>
> cm.time = time(nullptr);
> + cm.playern = Widelands::neutral();
Maybe create a constructor for chat messages which only takes a message?
> cm.msg = msg;
> messages.push_back(cm);
>
--
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables/+merge/326256
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into
lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
