[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors into lp:widelands
Continuous integration builds have changed state: Travis build 2358. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/246562869. Appveyor build 2186. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_986611_cppcheck_explicit_constructors-2186. -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors/+merge/326255 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors into lp:widelands. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into lp:widelands
Continuous integration builds have changed state: Travis build 2356. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/246523458. Appveyor build 2184. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_986611_cppcheck_uninitialized_variables-2184. -- 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 : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/00_soldier_control into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/00_soldier_control into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/00_soldier_control/+merge/325902 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/00_soldier_control. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ai-post-b19-2 into lp:widelands
I presume that clang-format will take care of that, will not? Once all changes are done I will run clang-format and retest and let you know. in 8163 I introduced new special wares, investigate diff for this revision if you are interested. I did not introduce the helper function, I think we can manage without it, especially now, when I shortened the code. But if you insist I can. -- https://code.launchpad.net/~widelands-dev/widelands/ai-post-b19-2/+merge/325796 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai-post-b19-2 into lp:widelands. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference into lp:widelands
Thanks for the review - I have reverted those changes. -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference/+merge/326257 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-unused into lp:widelands
Thanks - removed :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-unused/+merge/326236 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck-unused. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference into lp:widelands
Review: Needs Fixing Diff comments: > > === modified file 'src/graphic/text/rt_render.cc' > --- src/graphic/text/rt_render.cc 2017-06-01 08:52:15 + > +++ src/graphic/text/rt_render.cc 2017-06-24 11:37:18 + > @@ -969,7 +969,7 @@ > public: > TagHandler(Tag& tag, > FontCache& fc, > -NodeStyle ns, This changes semantics and is almost certainly wrong (also a few times below). Either NodeStyle -> const NodeStyle& or keep passing by value. > +NodeStyle& ns, > ImageCache* image_cache, > RendererStyle& renderer_style, > const UI::FontSets& fontsets) -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference/+merge/326257 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-unused into lp:widelands
Review: Approve
more stuff can be removed, see inline comment.
Diff comments:
>
> === modified file 'src/logic/editor_game_base.cc'
> --- src/logic/editor_game_base.cc 2017-02-28 12:59:39 +
> +++ src/logic/editor_game_base.cc 2017-06-23 17:42:19 +
> @@ -419,31 +419,6 @@
> return lasttrackserial_;
> }
>
> -/*
> -===
> -Retrieve a previously stored pointer using the serial number.
> -Returns 0 if the pointer has been removed.
> -===
> -*/
> -void* EditorGameBase::get_trackpointer(uint32_t const serial) {
> - std::map::iterator it = trackpointers_.find(serial);
add_trackpointer() is also not used in the codebase. Also not trackkpointers_
and last_track_serial_ members. Remove them all?
> -
> - if (it != trackpointers_.end())
> - return it->second;
> -
> - return nullptr;
> -}
> -
> -/*
> -===
> -Remove the registered track pointer. Subsequent calls to get_trackpointer()
> -using this serial number will return 0.
> -===
> -*/
> -void EditorGameBase::remove_trackpointer(uint32_t serial) {
> - trackpointers_.erase(serial);
> -}
> -
> /**
> * Cleanup for load
> *
--
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-unused/+merge/326236
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-986611-cppcheck-unused.
___
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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference into
lp:widelands.
Commit message:
Pass function arguments per (const) reference where possible.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #986611 in widelands: "Issues reported by cppcheck"
https://bugs.launchpad.net/widelands/+bug/986611
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference/+merge/326257
This cleanup should gain us a bit of performance. Changes are pretty trivial
(argument -> (const) argument&)
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-986611-cppcheck-pass-by-reference into
lp:widelands.
=== modified file 'src/base/i18n.cc'
--- src/base/i18n.cc 2017-01-25 18:55:59 +
+++ src/base/i18n.cc 2017-06-24 11:37:18 +
@@ -76,7 +76,7 @@
/**
* Set the localedir. This should usually only be done once
*/
-void set_localedir(std::string dname) {
+void set_localedir(const std::string& dname) {
localedir = dname;
}
@@ -157,7 +157,7 @@
* Set the locale to the given string.
* Code inspired by wesnoth.org
*/
-void set_locale(std::string name) {
+void set_locale(const std::string& name) {
const std::map kAlternatives = {
{"ar", "ar,ar_AR,ar_AE,ar_BH,ar_DZ,ar_EG,ar_IN,ar_IQ,ar_JO,ar_KW,ar_LB,ar_LY,ar_MA,ar_OM,ar_"
"QA,ar_SA,ar_SD,ar_SY,ar_TN,ar_YE"},
=== modified file 'src/base/i18n.h'
--- src/base/i18n.h 2017-01-25 18:55:59 +
+++ src/base/i18n.h 2017-06-24 11:37:18 +
@@ -52,10 +52,10 @@
};
void init_locale();
-void set_locale(std::string);
+void set_locale(const std::string&);
const std::string& get_locale();
-void set_localedir(std::string);
+void set_localedir(const std::string&);
const std::string& get_localedir();
// Localize a list of 'items'. The last 2 items are concatenated with "and" or
=== modified file 'src/editor/tools/tool_action.h'
--- src/editor/tools/tool_action.h 2017-01-25 18:55:59 +
+++ src/editor/tools/tool_action.h 2017-06-24 11:37:18 +
@@ -46,7 +46,7 @@
Widelands::Map& m,
Widelands::NodeAndTriangle<> c,
EditorInteractive& p,
- EditorActionArgs nargs)
+ const EditorActionArgs& nargs)
: tool(t), i(ind), map(m), center(c), parent(p) {
args = new EditorActionArgs(parent);
*args = nargs;
=== modified file 'src/graphic/text/rt_errors.h'
--- src/graphic/text/rt_errors.h 2017-01-25 18:55:59 +
+++ src/graphic/text/rt_errors.h 2017-06-24 11:37:18 +
@@ -28,7 +28,7 @@
class Exception : public std::exception {
public:
- Exception(std::string msg) : std::exception(), msg_(msg) {
+ explicit Exception(const std::string& msg) : std::exception(), msg_(msg) {
}
const char* what() const noexcept override {
return msg_.c_str();
@@ -41,7 +41,7 @@
#define DEF_ERR(Name) \
class Name : public Exception { \
public: \
- Name(std::string msg) : Exception(msg) { \
+ explicit Name(const std::string& msg) : Exception(msg) { \
}\
};
=== modified file 'src/graphic/text/rt_errors_impl.h'
--- src/graphic/text/rt_errors_impl.h 2017-01-25 18:55:59 +
+++ src/graphic/text/rt_errors_impl.h 2017-06-24 11:37:18 +
@@ -28,7 +28,7 @@
struct SyntaxErrorImpl : public SyntaxError {
SyntaxErrorImpl(
- size_t line, size_t col, std::string expected, std::string got, std::string next_chars)
+ size_t line, size_t col, const std::string& expected, const std::string& got, const std::string& next_chars)
: SyntaxError(
(boost::format(
"Syntax error at %1%:%2%: expected %3%, got '%4%'. String continues with: '%5%'") %
=== modified file 'src/graphic/text/rt_parse.h'
--- src/graphic/text/rt_parse.h 2017-01-25 18:55:59 +
+++ src/graphic/text/rt_parse.h 2017-06-24 11:37:18 +
@@ -104,7 +104,7 @@
}
Child(Tag* t) : tag(t) {
}
- Child(std::string t) : tag(nullptr), text(t) {
+ Child(const std::string& t) : tag(nullptr), text(t) {
}
~Child() {
if (tag)
=== modified file 'src/graphic/text/rt_render.cc'
--- src/graphic/text/rt_render.cc 2017-06-01 08:52:15 +
+++ src/graphic/text/rt_render.cc 2017-06-24 11:37:18 +
@@ -969,7 +969,7 @@
public:
TagHandler(Tag& tag,
FontCache& fc,
- NodeStyle ns,
+ NodeStyle& ns,
ImageCache* image_cache,
RendererStyle& renderer_style,
const UI::FontSets&
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into
lp:widelands.
Commit message:
Initialize a bunch of uninitialized member variables, adding constructors where
necessary. Turned some enums into enum classes.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #986611 in widelands: "Issues reported by cppcheck"
https://bugs.launchpad.net/widelands/+bug/986611
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables/+merge/326256
I'm on a code cleanup rampage today... this one needs a bit of looking at to
make sure that I initialized everything correctly.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables into
lp:widelands.
=== modified file 'src/ai/defaultai_warfare.cc'
--- src/ai/defaultai_warfare.cc 2017-06-23 16:16:28 +
+++ src/ai/defaultai_warfare.cc 2017-06-24 10:45:33 +
@@ -612,9 +612,9 @@
if (ms->economy().warehouses().size()) {
uint32_t const j = ms->soldier_capacity();
- if (MilitarySite::kPrefersRookies != ms->get_soldier_preference()) {
+ if (SoldierPreference::kRookies != ms->get_soldier_preference()) {
game().send_player_militarysite_set_soldier_preference(
- *ms, MilitarySite::kPrefersRookies);
+ *ms, SoldierPreference::kRookies);
} else if (j > 1) {
game().send_player_change_soldier_capacity(*ms, (j > 2) ? -2 : -1);
}
@@ -672,9 +672,9 @@
changed = true;
// and also set preference to Heroes
- if (MilitarySite::kPrefersHeroes != ms->get_soldier_preference()) {
+ if (SoldierPreference::kHeroes != ms->get_soldier_preference()) {
game().send_player_militarysite_set_soldier_preference(
- *ms, MilitarySite::kPrefersHeroes);
+ *ms, SoldierPreference::kHeroes);
changed = true;
}
@@ -683,9 +683,9 @@
} else { // otherwise decrease soldiers
uint32_t const j = ms->soldier_capacity();
- if (MilitarySite::kPrefersRookies != ms->get_soldier_preference()) {
+ if (SoldierPreference::kRookies != ms->get_soldier_preference()) {
game().send_player_militarysite_set_soldier_preference(
- *ms, MilitarySite::kPrefersRookies);
+ *ms, SoldierPreference::kRookies);
} else if (j > 1) {
game().send_player_change_soldier_capacity(*ms, (j > 2) ? -2 : -1);
}
=== modified file 'src/base/md5.h'
--- src/base/md5.h 2017-01-25 18:55:59 +
+++ src/base/md5.h 2017-06-24 10:45:33 +
@@ -74,7 +74,7 @@
*/
template class MD5Checksum : public Base {
public:
- MD5Checksum() {
+ MD5Checksum() : Base() {
Reset();
}
explicit MD5Checksum(const MD5Checksum& other)
=== modified file 'src/economy/shippingitem.h'
--- src/economy/shippingitem.h 2017-01-25 18:55:59 +
+++ src/economy/shippingitem.h 2017-06-24 10:45:33 +
@@ -59,6 +59,8 @@
void remove(EditorGameBase&);
struct Loader {
+ Loader () : serial_(0U) {
+ }
void load(FileRead& fr);
ShippingItem get(MapObjectLoader& mol);
=== modified file 'src/game_io/game_preload_packet.cc'
--- src/game_io/game_preload_packet.cc 2017-01-25 18:55:59 +
+++ src/game_io/game_preload_packet.cc 2017-06-24 10:45:33 +
@@ -46,6 +46,9 @@
constexpr uint16_t kCurrentPacketVersion = 6;
constexpr const char* kMinimapFilename = "minimap.png";
+GamePreloadPacket::GamePreloadPacket() : minimap_path_(""), mapname_(""), background_(""), win_condition_(""), gametime_(0), player_nr_(Widelands::neutral()), version_(""), savetimestamp_(0), gametype_(GameController::GameType::kUndefined) {
+}
+
std::string GamePreloadPacket::get_localized_win_condition() const {
i18n::Textdomain td("win_conditions");
return _(win_condition_);
=== modified file 'src/game_io/game_preload_packet.h'
--- src/game_io/game_preload_packet.h 2017-01-25 18:55:59 +
+++ src/game_io/game_preload_packet.h 2017-06-24 10:45:33 +
@@ -36,6 +36,8 @@
*/
struct GamePreloadPacket : public GameDataPacket {
+ GamePreloadPacket();
+
void read(FileSystem&, Game&, MapObjectLoader* = nullptr) override;
void write(FileSystem&, Game&, MapObjectSaver* = nullptr) override;
=== modified file 'src/graphic/gl/fields_to_draw.h'
--- src/graphic/gl/fields_to_draw.h 2017-05-13 18:48:26 +
+++ src/graphic/gl/fields_to_draw.h 2017-06-24 10:45:33 +
@@ -78,6 +78,7 @@
};
FieldsToDraw() {
+ reset(0, 0, 0, 0);
}
// Resize this fields to draw for reuse.
=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc 2017-05-31 21:27:07 +
+++ src/graphic/graphic.cc 2017-06-24 10:45:33 +
@@ -59,7 +59,7 @@
} // namespace
-Graphic::Graphic() : image_cache_(new ImageCache()), animation_manager_(new AnimationManager()) {
+Graphic::Graphic() : window_mode_width_(0), window_mode_height_(0), sdl_window_(nullptr), max_texture_size_(kMinimumSizeForTextures), image_cache_(new ImageCache()),
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-unused into lp:widelands
Continuous integration builds have changed state: Travis build 2348. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/246309258. Appveyor build 2176. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_986611_cppcheck_unused-2176. -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-unused/+merge/326236 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-986611-cppcheck-unused into lp:widelands. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors into
lp:widelands.
Commit message:
Made a bunch of constructors explicit.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #986611 in widelands: "Issues reported by cppcheck"
https://bugs.launchpad.net/widelands/+bug/986611
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors/+merge/326255
This change is big but trivial. These are all "stupid" cases, but might protect
us from a few regression in the future.
The principal aim is to get rid of a lot of noise in the cppcheck reports so
that we can look into the interesting cases some time.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-986611-cppcheck-explicit-constructors into
lp:widelands.
=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h 2017-01-25 18:55:59 +
+++ src/ai/ai_help_structs.h 2017-06-24 09:44:24 +
@@ -171,7 +171,7 @@
// Fishers and fishbreeders must be built near water
struct FindNodeWater {
- FindNodeWater(const World& world);
+ explicit FindNodeWater(const World& world);
bool accept(const Map& /* map */, const FCoords& coord) const;
@@ -183,7 +183,7 @@
// Open water is a field where all 6 adjacent triangles are water
struct FindNodeOpenWater {
// 'world' is unused, but we need to fit the template.
- FindNodeOpenWater(const World& /* world */) {
+ explicit FindNodeOpenWater(const World& /* world */) {
}
bool accept(const Map& /* map */, const FCoords& coord) const;
@@ -228,7 +228,7 @@
};
struct BuildableField {
- BuildableField(const Widelands::FCoords& fc);
+ explicit BuildableField(const Widelands::FCoords& fc);
int32_t own_military_sites_nearby_();
@@ -288,7 +288,7 @@
};
struct MineableField {
- MineableField(const Widelands::FCoords& fc);
+ explicit MineableField(const Widelands::FCoords& fc);
Widelands::FCoords coords;
uint32_t field_info_expiration;
@@ -299,7 +299,7 @@
};
struct EconomyObserver {
- EconomyObserver(Widelands::Economy& e);
+ explicit EconomyObserver(Widelands::Economy& e);
Widelands::Economy& economy;
std::list flags;
@@ -517,7 +517,7 @@
// list of candidate flags to build roads, with some additional logic
struct FlagsForRoads {
- FlagsForRoads(int32_t mr) : min_reduction(mr) {
+ explicit FlagsForRoads(int32_t mr) : min_reduction(mr) {
}
struct Candidate {
=== modified file 'src/ai/ai_hints.h'
--- src/ai/ai_hints.h 2017-05-25 07:16:05 +
+++ src/ai/ai_hints.h 2017-06-24 09:44:24 +
@@ -34,7 +34,7 @@
/// buildings conf file. It is used to tell the computer player about the
/// special properties of a building.
struct BuildingHints {
- BuildingHints(std::unique_ptr);
+ explicit BuildingHints(std::unique_ptr);
~BuildingHints() {
}
=== modified file 'src/base/i18n.h'
--- src/base/i18n.h 2017-01-25 18:55:59 +
+++ src/base/i18n.h 2017-06-24 09:44:24 +
@@ -43,7 +43,7 @@
/// released when the object goes out of scope. This is exception-safe, unlike
/// calling grab_textdomain and release_textdomain directly.
struct Textdomain {
- Textdomain(const std::string& name) {
+ explicit Textdomain(const std::string& name) {
grab_textdomain(name);
}
~Textdomain() {
=== modified file 'src/economy/economy.h'
--- src/economy/economy.h 2017-02-12 09:10:57 +
+++ src/economy/economy.h 2017-06-24 09:44:24 +
@@ -110,7 +110,7 @@
Time last_modified;
};
- Economy(Player&);
+ explicit Economy(Player&);
~Economy();
Player& owner() const {
=== modified file 'src/economy/economy_data_packet.h'
--- src/economy/economy_data_packet.h 2017-01-25 18:55:59 +
+++ src/economy/economy_data_packet.h 2017-06-24 09:44:24 +
@@ -31,7 +31,7 @@
class EconomyDataPacket {
public:
- EconomyDataPacket(Economy* e) : eco_(e) {
+ explicit EconomyDataPacket(Economy* e) : eco_(e) {
}
void read(FileRead&);
=== modified file 'src/economy/expedition_bootstrap.h'
--- src/economy/expedition_bootstrap.h 2017-02-17 21:52:41 +
+++ src/economy/expedition_bootstrap.h 2017-06-24 09:44:24 +
@@ -49,7 +49,7 @@
class ExpeditionBootstrap {
public:
- ExpeditionBootstrap(PortDock* const portdock);
+ explicit ExpeditionBootstrap(PortDock* const portdock);
virtual ~ExpeditionBootstrap();
// Start bootstrapping an expedition. This will request all wares and workers.
@@ -118,7 +118,7 @@
ExpeditionBootstrap* bootstrap;
- NoteExpeditionCanceled(ExpeditionBootstrap* const init_bootstrap) : bootstrap(init_bootstrap) {
+ explicit NoteExpeditionCanceled(ExpeditionBootstrap* const init_bootstrap) : bootstrap(init_bootstrap) {
}
};
=== modified file 'src/economy/fleet.h'
--- src/economy/fleet.h 2017-04-23 12:11:19 +
+++ src/economy/fleet.h 2017-06-24 09:44:24 +
@@ -75,7 +75,7 @@
const FleetDescr& descr() const;
- Fleet(Player&
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1697242-fileview into lp:widelands
Review: Approve testing Works :-) Opened all windows and found no problems. Should Ken Cunnningham be asked if the initial bug is still fixed for him? Looks like this is complicated stuff... maybe add some comments to the code? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1697242-fileview/+merge/325456 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1697242-fileview. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/00_soldier_control into lp:widelands
Rebutalled some comments, the rest are done.
Thanks for the review!
@bunnybot merge
Diff comments:
>
> === modified file 'src/logic/map_objects/tribes/militarysite.cc'
> --- src/logic/map_objects/tribes/militarysite.cc 2017-06-06 05:26:58
> +
> +++ src/logic/map_objects/tribes/militarysite.cc 2017-06-19 07:06:15
> +
> @@ -43,6 +43,114 @@
>
> namespace Widelands {
>
> +// TODO(sirver): This method should probably return a const reference.
remode the TODOs for now.
> +std::vector MilitarySite::SoldierControl::present_soldiers() const
> {
> + std::vector soldiers;
> +
> + for (Worker* worker : military_site_->get_workers()) {
> + if (upcast(Soldier, soldier, worker)) {
> + if (military_site_->is_present(*soldier)) {
> + soldiers.push_back(soldier);
> + }
> + }
> + }
> + return soldiers;
> +}
> +
> +// TODO(sirver): This method should probably return a const reference.
> +std::vector MilitarySite::SoldierControl::stationed_soldiers()
> const {
> + std::vector soldiers;
> +
> + for (Worker* worker : military_site_->get_workers()) {
> + if (upcast(Soldier, soldier, worker)) {
> + soldiers.push_back(soldier);
> + }
> + }
> + return soldiers;
> +}
> +
> +Quantity MilitarySite::SoldierControl::min_soldier_capacity() const {
> + return 1;
> +}
> +
> +Quantity MilitarySite::SoldierControl::max_soldier_capacity() const {
> + return military_site_->descr().get_max_number_of_soldiers();
> +}
> +
> +Quantity MilitarySite::SoldierControl::soldier_capacity() const {
> + return military_site_->capacity_;
> +}
> +
> +void MilitarySite::SoldierControl::set_soldier_capacity(uint32_t const
> capacity) {
> + assert(min_soldier_capacity() <= capacity);
> + assert(capacity <= max_soldier_capacity());
> + assert(military_site_->capacity_ != capacity);
> + military_site_->capacity_ = capacity;
> + military_site_->update_soldier_request();
> +}
> +
> +void MilitarySite::SoldierControl::drop_soldier(Soldier& soldier) {
> + Game& game = dynamic_cast(military_site_->owner().egbase());
> +
> + if (!military_site_->is_present(soldier)) {
> + // This can happen when the "drop soldier" player command is
> delayed
> + // by network delay or a client has bugs.
> + military_site_->molog("MilitarySite::drop_soldier(%u): not
> present\n", soldier.serial());
> + return;
> + }
> + if (present_soldiers().size() <= min_soldier_capacity()) {
> + military_site_->molog("cannot drop last soldier(s)\n");
> + return;
> + }
> +
> + soldier.reset_tasks(game);
> + soldier.start_task_leavebuilding(game, true);
> +
> + military_site_->update_soldier_request();
> +}
> +
> +int MilitarySite::SoldierControl::incorporate_soldier(EditorGameBase&
> egbase, Soldier& s) {
> + if (s.get_location(egbase) != military_site_) {
> + s.set_location(military_site_);
> + }
> +
> + // Soldier upgrade is done once the site is full. In soldier upgrade, we
> + // request one new soldier who is better suited than the existing ones.
> + // Normally, I kick out one existing soldier as soon as a new guy
> starts walking
> + // towards here. However, since that is done via infrequent polling, a
> new soldier
> + // can sometimes reach the site before such kick-out happens. In those
> cases, we
> + // should either drop one of the existing soldiers or reject the new
> guy, to
> + // avoid overstocking this site.
> +
> + if (stationed_soldiers().size() >
> military_site_->descr().get_max_number_of_soldiers()) {
> + return military_site_->incorporate_upgraded_soldier(egbase, s)
> ? 0 : -1;
> + }
> +
> + if (!military_site_->didconquer_) {
> + military_site_->conquer_area(egbase);
> + // Building is now occupied - idle animation should be played
> + military_site_->start_animation(egbase,
> military_site_->descr().get_animation("idle"));
> +
> + if (upcast(Game, game, )) {
> + military_site_->send_message(
> +*game, Message::Type::kEconomySiteOccupied,
> military_site_->descr().descname(),
> +military_site_->descr().icon_filename(),
> military_site_->descr().descname(),
> +military_site_->descr().occupied_str_, true);
> + }
> + }
> +
> + if (upcast(Game, game, )) {
> + // Bind the worker into this house, hide him on the map
> + s.reset_tasks(*game);
> + s.start_task_buildingwork(*game);
> + }
> +
> + // Make sure the request count is reduced or the request is deleted.
> + military_site_->update_soldier_request(true);
> +
>
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1697242-fileview into lp:widelands
Review: Resubmit *headdesk* next try. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1697242-fileview/+merge/325456 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1697242-fileview. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ai-post-b19-2 into lp:widelands
BTW comments like //Should happen only rarely so we print a warning here will break codecheck, always use a blank space: // Should happen only rarely so we print a warning here Regarding your question in the code, size_t is an unsigned type http://www.cplusplus.com/reference/cstddef/size_t/?kw=size_t -- https://code.launchpad.net/~widelands-dev/widelands/ai-post-b19-2/+merge/325796 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai-post-b19-2 into lp:widelands. ___ 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
