widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #07916
[Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck_performance-ui into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-986611-cppcheck_performance-ui into lp:widelands.
Commit message:
Fixed performance issues in UI classes that were flagged up by cppcheck and fixed a textdomain issue in wui/mapdata.
Performance issues fixed are:
- Prefer prefix ++ operator in wui/waresdisplay.cc.
- Constructor initialization in ui_fsmenu/base and wui/mapdata.
- Passing parameters per reference.
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_performance-ui/+merge/300992
Fixed performance issues in UI classes that were flagged up by cppcheck and fixed a textdomain issue in wui/mapdata.
Performance issues fixed are:
- Prefer prefix ++ operator in wui/waresdisplay.cc.
- Constructor initialization in ui_fsmenu/base and wui/mapdata.
- Passing parameters per reference.
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-986611-cppcheck_performance-ui into lp:widelands.
=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc 2016-03-25 17:01:05 +0000
+++ src/ui_basic/listselect.cc 2016-07-24 11:56:05 +0000
@@ -249,7 +249,7 @@
* index.
*/
void BaseListselect::set_entry_color
- (const uint32_t n, const RGBColor col)
+ (const uint32_t n, const RGBColor& col)
{
assert(n < entry_records_.size());
=== modified file 'src/ui_basic/listselect.h'
--- src/ui_basic/listselect.h 2016-02-09 21:14:53 +0000
+++ src/ui_basic/listselect.h 2016-07-24 11:56:05 +0000
@@ -73,7 +73,7 @@
void switch_entries(uint32_t, uint32_t);
- void set_entry_color(uint32_t, RGBColor);
+ void set_entry_color(uint32_t, const RGBColor&);
uint32_t size () const {return entry_records_.size ();}
bool empty() const {return entry_records_.empty();}
=== modified file 'src/ui_basic/slider.cc'
--- src/ui_basic/slider.cc 2016-03-30 07:20:05 +0000
+++ src/ui_basic/slider.cc 2016-07-24 11:56:05 +0000
@@ -538,7 +538,7 @@
DiscreteSlider::DiscreteSlider
(Panel * const parent,
const int32_t x, const int32_t y, const uint32_t w, const uint32_t h,
- const std::vector<std::string> labels_in,
+ const std::vector<std::string>& labels_in,
uint32_t value_,
const Image* background_picture_id,
const std::string & tooltip_text,
=== modified file 'src/ui_basic/slider.h'
--- src/ui_basic/slider.h 2016-02-03 18:09:15 +0000
+++ src/ui_basic/slider.h 2016-07-24 11:56:05 +0000
@@ -185,7 +185,7 @@
DiscreteSlider
(Panel * const parent,
const int32_t x, const int32_t y, const uint32_t w, const uint32_t h,
- const std::vector<std::string> labels_in,
+ const std::vector<std::string>& labels_in,
uint32_t value_,
const Image* background_picture_id,
const std::string & tooltip_text = std::string(),
=== modified file 'src/ui_basic/table.h'
--- src/ui_basic/table.h 2016-02-21 09:39:30 +0000
+++ src/ui_basic/table.h 2016-07-24 11:56:05 +0000
@@ -126,7 +126,7 @@
const Image* get_picture(uint8_t col) const;
const std::string & get_string(uint8_t col) const;
void * entry() const {return entry_;}
- void set_color(const RGBColor c) {
+ void set_color(const RGBColor& c) {
use_clr = true;
clr = c;
}
=== modified file 'src/ui_fsmenu/base.cc'
--- src/ui_fsmenu/base.cc 2016-02-07 18:10:53 +0000
+++ src/ui_fsmenu/base.cc 2016-07-24 11:56:05 +0000
@@ -48,10 +48,9 @@
* Args: bgpic name of the background picture
*/
FullscreenMenuBase::FullscreenMenuBase(const std::string& bgpic)
- : UI::Panel(nullptr, 0, 0, g_gr->get_xres(), g_gr->get_yres()) {
-
- background_image_ = bgpic;
-}
+ : UI::Panel(nullptr, 0, 0, g_gr->get_xres(), g_gr->get_yres()),
+ background_image_(bgpic)
+{}
FullscreenMenuBase::~FullscreenMenuBase()
{
=== modified file 'src/wui/field_overlay_manager.cc'
--- src/wui/field_overlay_manager.cc 2016-01-28 05:24:34 +0000
+++ src/wui/field_overlay_manager.cc 2016-07-24 11:56:05 +0000
@@ -117,7 +117,7 @@
}
void FieldOverlayManager::register_overlay
- (Widelands::TCoords<> const c,
+ (const Widelands::TCoords<>& c,
const Image* pic,
int32_t const level,
Point hotspot,
=== modified file 'src/wui/field_overlay_manager.h'
--- src/wui/field_overlay_manager.h 2016-01-24 20:11:53 +0000
+++ src/wui/field_overlay_manager.h 2016-07-24 11:56:05 +0000
@@ -85,7 +85,7 @@
/// Register an overlay at a location (node or triangle). hotspot is the point
/// of the picture that will be exactly over the location. If hotspot is
/// Point::invalid(), the center of the picture will be used as hotspot.
- void register_overlay(const Widelands::TCoords<> coords,
+ void register_overlay(const Widelands::TCoords<>& coords,
const Image* pic,
int32_t level,
Point hotspot = Point::invalid(),
=== modified file 'src/wui/game_debug_ui.cc'
--- src/wui/game_debug_ui.cc 2016-05-17 16:40:22 +0000
+++ src/wui/game_debug_ui.cc 2016-07-24 11:56:05 +0000
@@ -467,7 +467,7 @@
===============
*/
void show_field_debug
- (InteractiveBase & parent, Widelands::Coords const coords)
+ (InteractiveBase& parent, const Widelands::Coords& coords)
{
new FieldDebugWindow(parent, coords);
}
=== modified file 'src/wui/game_debug_ui.h'
--- src/wui/game_debug_ui.h 2014-09-10 08:55:04 +0000
+++ src/wui/game_debug_ui.h 2016-07-24 11:56:05 +0000
@@ -28,6 +28,6 @@
// Open debug window for the given coordinates
void show_mapobject_debug(InteractiveBase & parent, Widelands::MapObject &);
-void show_field_debug(InteractiveBase & parent, Widelands::Coords coords);
+void show_field_debug(InteractiveBase & parent, const Widelands::Coords& coords);
#endif // end of include guard: WL_WUI_GAME_DEBUG_UI_H
=== modified file 'src/wui/interactive_base.h'
--- src/wui/interactive_base.h 2016-05-03 08:45:23 +0000
+++ src/wui/interactive_base.h 2016-07-24 11:56:05 +0000
@@ -186,7 +186,7 @@
struct SelData {
SelData
(const bool Freeze = false, const bool Triangles = false,
- const Widelands::NodeAndTriangle<> Pos =
+ const Widelands::NodeAndTriangle<>& Pos =
Widelands::NodeAndTriangle<>
(Widelands::Coords(0, 0),
Widelands::TCoords<>
=== modified file 'src/wui/interactive_gamebase.h'
--- src/wui/interactive_gamebase.h 2016-03-16 09:22:00 +0000
+++ src/wui/interactive_gamebase.h 2016-07-24 11:56:05 +0000
@@ -77,7 +77,7 @@
protected:
void draw_overlay(RenderTarget &) override;
- virtual int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords> c) = 0;
+ virtual int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords>& c) = 0;
GameMainMenuWindows main_windows_;
ChatProvider * chat_provider_;
=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc 2016-04-02 08:28:29 +0000
+++ src/wui/interactive_player.cc 2016-07-24 11:56:05 +0000
@@ -266,7 +266,7 @@
return player_number_;
}
-int32_t InteractivePlayer::calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords> c) {
+int32_t InteractivePlayer::calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords>& c) {
assert(get_player());
return get_player()->get_buildcaps(c);
}
=== modified file 'src/wui/interactive_player.h'
--- src/wui/interactive_player.h 2016-01-24 20:11:53 +0000
+++ src/wui/interactive_player.h 2016-07-24 11:56:05 +0000
@@ -77,12 +77,12 @@
void cleanup_for_load() override;
void think() override;
- void set_flag_to_connect(Widelands::Coords const location) {
+ void set_flag_to_connect(const Widelands::Coords& location) {
flag_to_connect_ = location;
}
void popup_message(Widelands::MessageId, const Widelands::Message &);
- int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords> c) override;
+ int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords>& c) override;
private:
void cmdSwitchPlayer(const std::vector<std::string> & args);
=== modified file 'src/wui/interactive_spectator.cc'
--- src/wui/interactive_spectator.cc 2016-03-16 10:32:50 +0000
+++ src/wui/interactive_spectator.cc 2016-07-24 11:56:05 +0000
@@ -143,7 +143,7 @@
}
-int32_t InteractiveSpectator::calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords> c) {
+int32_t InteractiveSpectator::calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords>& c) {
const Widelands::PlayerNumber nr_players = game().map().get_nrplayers();
iterate_players_existing(p, nr_players, game(), player) {
=== modified file 'src/wui/interactive_spectator.h'
--- src/wui/interactive_spectator.h 2016-01-24 20:11:53 +0000
+++ src/wui/interactive_spectator.h 2016-07-24 11:56:05 +0000
@@ -51,7 +51,7 @@
void toggle_statistics();
void exit_btn();
void save_btn();
- int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords> c) override;
+ int32_t calculate_buildcaps(const Widelands::TCoords<Widelands::FCoords>& c) override;
bool can_see(Widelands::PlayerNumber) const override;
bool can_act(Widelands::PlayerNumber) const override;
Widelands::PlayerNumber player_number() const override;
=== modified file 'src/wui/mapdata.cc'
--- src/wui/mapdata.cc 2016-06-07 07:13:03 +0000
+++ src/wui/mapdata.cc 2016-07-24 11:56:05 +0000
@@ -21,42 +21,62 @@
#include "io/filesystem/filesystem.h"
-MapData::MapData() : authors(""), nrplayers(0), width(0), height(0),
- maptype(MapData::MapType::kNormal), displaytype(MapData::DisplayType::kMapnamesLocalized) {}
-
- MapData::MapData(const Widelands::Map& map, const std::string& init_filename,
- const MapData::MapType& init_maptype,
- const MapData::DisplayType& init_displaytype) :
- MapData() {
- i18n::Textdomain td("maps");
- filename = init_filename;
- name = map.get_name().empty() ? _("No Name") : map.get_name();
- localized_name = _(name);
- // Localizing this, because some author fields now have "edited by" text.
- const std::string& author = map.get_author();
- authors = MapAuthorData(author.empty() ? _("No Author") : _(author));
- description = map.get_description().empty() ? "" : _(map.get_description());
- hint = map.get_hint().empty() ? "" : _(map.get_hint());
- nrplayers = map.get_nrplayers();
- width = map.get_width();
- height = map.get_height();
- suggested_teams = map.get_suggested_teams();
- tags = map.get_tags();
- maptype = init_maptype;
- displaytype = init_displaytype;
-
- if (maptype == MapData::MapType::kScenario) {
- tags.insert("scenario");
- }
- }
+MapData::MapData(const std::string& init_filename,
+ const std::string& init_localized_name,
+ const std::string& init_author,
+ const MapData::MapType& init_maptype,
+ const MapData::DisplayType& init_displaytype) :
+ filename(init_filename),
+ name(init_localized_name),
+ localized_name(init_localized_name),
+ authors(init_author),
+ description(""),
+ hint(""),
+ nrplayers(0),
+ width(0),
+ height(0),
+ maptype(init_maptype),
+ displaytype(init_displaytype)
+{}
+
+MapData::MapData(const Widelands::Map& map, const std::string& init_filename,
+ const MapData::MapType& init_maptype,
+ const MapData::DisplayType& init_displaytype) :
+ MapData(init_filename,
+ _("No Name"),
+ _("No Author"),
+ init_maptype,
+ init_displaytype) {
+
+ i18n::Textdomain td("maps");
+ if (!map.get_name().empty()) {
+ name = map.get_name();
+ }
+ localized_name = _(name);
+ // Localizing this, because some author fields now have "edited by" text.
+ if (!map.get_author().empty()) {
+ authors = map.get_author();
+ }
+ description = map.get_description().empty() ? "" : _(map.get_description());
+ hint = map.get_hint().empty() ? "" : _(map.get_hint());
+ nrplayers = map.get_nrplayers();
+ width = map.get_width();
+ height = map.get_height();
+ suggested_teams = map.get_suggested_teams();
+ tags = map.get_tags();
+
+ if (maptype == MapData::MapType::kScenario) {
+ tags.insert("scenario");
+ }
+}
MapData::MapData(const std::string& init_filename, const std::string& init_localized_name) :
- MapData() {
- filename = init_filename;
- name = init_localized_name;
- localized_name = init_localized_name;
- maptype = MapData::MapType::kDirectory;
- }
+ MapData(init_filename,
+ init_localized_name,
+ "",
+ MapData::MapType::kDirectory,
+ MapData::DisplayType::kMapnamesLocalized)
+{}
bool MapData::compare_names(const MapData& other) {
// The parent directory gets special treatment.
=== modified file 'src/wui/mapdata.h'
--- src/wui/mapdata.h 2016-05-15 09:03:01 +0000
+++ src/wui/mapdata.h 2016-07-24 11:56:05 +0000
@@ -71,10 +71,14 @@
kMapnamesLocalized
};
-
- /// For incomplete data
- MapData();
-
+private:
+ /// For common properties
+ MapData(const std::string& init_filename,
+ const std::string& init_localized_name,
+ const std::string& init_author,
+ const MapData::MapType& init_maptype,
+ const MapData::DisplayType& init_displaytype);
+public:
/// For normal maps and scenarios
MapData(const Widelands::Map& map, const std::string& init_filename,
const MapData::MapType& init_maptype,
=== modified file 'src/wui/plot_area.cc'
--- src/wui/plot_area.cc 2016-05-20 07:01:10 +0000
+++ src/wui/plot_area.cc 2016-07-24 11:56:05 +0000
@@ -491,7 +491,7 @@
*/
void WuiPlotArea::draw_plot_line
(RenderTarget & dst, std::vector<uint32_t> const * dataset,
- uint32_t const highest_scale, float const sub, RGBColor const color, int32_t const yoffset)
+ uint32_t const highest_scale, float const sub, const RGBColor& color, int32_t const yoffset)
{
if (!dataset->empty()) {
float posx = get_inner_w() - kSpaceRight;
=== modified file 'src/wui/plot_area.h'
--- src/wui/plot_area.h 2016-05-20 07:01:10 +0000
+++ src/wui/plot_area.h 2016-07-24 11:56:05 +0000
@@ -96,7 +96,7 @@
uint32_t highest_scale);
void draw_plot_line
(RenderTarget & dst, std::vector<uint32_t> const * dataset,
- uint32_t const highest_scale, float const sub, RGBColor const color, int32_t yoffset);
+ uint32_t const highest_scale, float const sub, const RGBColor& color, int32_t yoffset);
uint32_t get_plot_time() const;
/// Recalculates the data
virtual void update();
=== modified file 'src/wui/vector.h'
--- src/wui/vector.h 2014-07-05 16:41:51 +0000
+++ src/wui/vector.h 2016-07-24 11:56:05 +0000
@@ -39,12 +39,12 @@
}
// vector addition
- Vector operator+ (Vector const other) const {
+ Vector operator+ (const Vector& other) const {
return Vector(x + other.x, y + other.y, z + other.z);
}
// inner product
- float operator* (Vector const other) const {
+ float operator* (const Vector& other) const {
return x * other.x + y * other.y + z * other.z;
}
=== modified file 'src/wui/waresdisplay.cc'
--- src/wui/waresdisplay.cc 2016-03-08 21:14:48 +0000
+++ src/wui/waresdisplay.cc 2016-07-24 11:56:05 +0000
@@ -464,7 +464,7 @@
std::vector<Widelands::DescriptionIndex>::iterator j;
Widelands::TribeDescr::WaresOrder order = tribe.wares_order();
- for (i = order.begin(); i != order.end(); i++)
+ for (i = order.begin(); i != order.end(); ++i)
for (j = i->begin(); j != i->end(); ++j)
if ((c = map.find(*j)) != map.end()) {
ret += "<sub width=30 padding=2><p align=center>"
=== modified file 'src/wui/watchwindow.cc'
--- src/wui/watchwindow.cc 2016-04-03 16:18:24 +0000
+++ src/wui/watchwindow.cc 2016-07-24 11:56:05 +0000
@@ -386,7 +386,7 @@
===============
*/
void show_watch_window
- (InteractiveGameBase & parent, Widelands::Coords const coords)
+ (InteractiveGameBase & parent, const Widelands::Coords& coords)
{
if (g_options.pull_section("global").get_bool("single_watchwin", false)) {
if (g_watch_window)
=== modified file 'src/wui/watchwindow.h'
--- src/wui/watchwindow.h 2014-09-10 08:55:04 +0000
+++ src/wui/watchwindow.h 2016-07-24 11:56:05 +0000
@@ -24,6 +24,6 @@
class InteractiveGameBase;
-void show_watch_window(InteractiveGameBase &, Widelands::Coords);
+void show_watch_window(InteractiveGameBase &, const Widelands::Coords&);
#endif // end of include guard: WL_WUI_WATCHWINDOW_H
Follow ups
