Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1346965-pointer-table into lp:widelands
Review: Approve compile, test, code review Looks good to me, compiled and opend all the tables I coud think of and sorted them. Found no crashes or Anomalies gcc 5 in travis just needed to long? Appveyor debug x64: could not read symbols: Memory exhausted. These are no real Code Problems, can we avoid these? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1346965-pointer-table/+merge/300974 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1346965-pointer-table. ___ 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_memleak into lp:widelands
Continuous integration builds have changed state: Travis build 1205. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/146859732. Appveyor build 1044. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_986611_cppcheck_memleak-1044. -- https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck_memleak/+merge/300979 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-986611-cppcheck_memleak 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_memleak into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-986611-cppcheck_memleak into lp:widelands.
Commit message:
Fixed memory leaks with Buttons in:
- src/wui/actionconfirm.cc
- src/wui/game_main_menu_save_game.cc
- src/ui_fsmenu/launch_mpg.cc.
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_memleak/+merge/300979
This branch fixes some of the memory leaks reported by cppcheck.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-986611-cppcheck_memleak into lp:widelands.
=== modified file 'src/ui_fsmenu/launch_mpg.cc'
--- src/ui_fsmenu/launch_mpg.cc 2016-04-15 09:01:09 +
+++ src/ui_fsmenu/launch_mpg.cc 2016-07-23 13:57:12 +
@@ -50,43 +50,38 @@
MapOrSaveSelectionWindow
(UI::Panel * parent, GameController * gc, uint32_t w, uint32_t h)
:
- /** TRANSLATORS: Dialog box title for selecting between map or saved game for new multiplayer game */
- Window(parent, "selection_window", 0, 0, w, h, _("Please select")),
- ctrl_(gc)
+ /** TRANSLATORS: Dialog box title for selecting between map or saved game for new multiplayer game */
+ Window(parent, "selection_window", 0, 0, w, h, _("Please select")),
+ // TODO(GunChleoc): Switch to Box layout
+ space_(get_inner_h() / 10),
+ butw_(get_inner_w() - 2 * space_),
+ buth_((get_inner_h() - 2 * space_) / 5),
+ button_map_(this, "map",
+ space_, space_, butw_, buth_,
+ g_gr->images().get("images/ui_basic/but0.png"),
+ _("Map"), _("Select a map"), true, false),
+ button_savegame_(this, "saved_game",
+ space_, space_ + buth_ + space_, butw_, buth_,
+ g_gr->images().get("images/ui_basic/but0.png"),
+ /** Translators: This is a button to select a savegame */
+ _("Saved Game"), _("Select a saved game"), true, false),
+ button_cancel_(this, "cancel",
+ space_ + butw_ / 4, space_ + 3 * buth_ + 2 * space_, butw_ / 2, buth_,
+ g_gr->images().get("images/ui_basic/but1.png"),
+ _("Cancel"), _("Cancel selection"), true, false),
+ ctrl_(gc)
{
center_to_parent();
- uint32_t y = get_inner_h() / 10;
- uint32_t space = y;
- uint32_t butw = get_inner_w() - 2 * space;
- uint32_t buth = (get_inner_h() - 2 * space) / 5;
- UI::Button * btn = new UI::Button
- (this, "map",
- space, y, butw, buth,
- g_gr->images().get("images/ui_basic/but0.png"),
- _("Map"), _("Select a map"), true, false);
- btn->sigclicked.connect
+ button_map_.sigclicked.connect
(boost::bind
(&MapOrSaveSelectionWindow::pressedButton, boost::ref(*this),
FullscreenMenuBase::MenuTarget::kNormalGame));
-
- btn = new UI::Button
- (this, "saved_game",
- space, y + buth + space, butw, buth,
- g_gr->images().get("images/ui_basic/but0.png"),
- /** Translators: This is a button to select a savegame */
- _("Saved Game"), _("Select a saved game"), true, false);
- btn->sigclicked.connect
+ button_savegame_.sigclicked.connect
(boost::bind
(&MapOrSaveSelectionWindow::pressedButton, boost::ref(*this),
FullscreenMenuBase::MenuTarget::kScenarioGame));
-
- btn = new UI::Button
- (this, "cancel",
- space + butw / 4, y + 3 * buth + 2 * space, butw / 2, buth,
- g_gr->images().get("images/ui_basic/but1.png"),
- _("Cancel"), _("Cancel selection"), true, false);
- btn->sigclicked.connect
+ button_cancel_.sigclicked.connect
(boost::bind
(&MapOrSaveSelectionWindow::pressedButton, boost::ref(*this),
FullscreenMenuBase::MenuTarget::kBack));
@@ -101,8 +96,13 @@
void pressedButton(FullscreenMenuBase::MenuTarget i) {
end_modal(i);
}
- private:
- GameController * ctrl_;
+
+private:
+ uint32_t space_, butw_, buth_;
+ UI::Button button_map_;
+ UI::Button button_savegame_;
+ UI::Button button_cancel_;
+ GameController * ctrl_;
};
FullscreenMenuLaunchMPG::FullscreenMenuLaunchMPG
=== modified file 'src/wui/actionconfirm.cc'
--- src/wui/actionconfirm.cc 2016-03-29 08:17:14 +
+++ src/wui/actionconfirm.cc 2016-07-23 13:57:12 +
@@ -49,6 +49,10 @@
protected:
Widelands::ObjectPointer object_;
+private:
+ UI::MultilineTextarea message_;
+ UI::Button button_ok_;
+ UI::Button button_cancel_;
};
/**
@@ -137,28 +141,22 @@
:
UI::Window
(&parent, "building_action_confirm", 0, 0, 200, 120, windowtitle),
- object_ (&map_object)
+ object_ (&map_object),
+ message_(this, 0, 0, 200, 74, message, UI::Align::kCenter),
+ button_ok_(this, "ok",
+ UI::g_fh1->fontset()->is_rtl() ? 6 : 114, 80, 80, 34,
+ g_gr->images().get("images/ui_basic/but4.png"),
+ g_gr->images().get("images/wui/menu_okay.png")),
+ bu
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1346965-pointer-table into lp:widelands
Continuous integration builds have changed state: Travis build 1202. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/146833401. Appveyor build 1041. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1346965_pointer_table-1041. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1346965-pointer-table/+merge/300974 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1346965-pointer-table 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-1346965-pointer-table into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1346965-pointer-table into lp:widelands.
Commit message:
Fix assert violation and memory leak with table column buttons. All table
columns now always have a button, even if the column title is empty.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1346965 in widelands: "Called C++ object pointer is null in
ui_basic/table.cc (set_column_title)"
https://bugs.launchpad.net/widelands/+bug/1346965
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1346965-pointer-table/+merge/300974
Completely removed the code that was causing scan-build to complain. The design
decision is that if you want a table without heading, use a listselect -
multiple columns should always have headings in the UI. We weren't using empty
column titles anyway.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1346965-pointer-table into lp:widelands.
=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc 2016-03-25 17:01:05 +
+++ src/ui_basic/table.cc 2016-07-23 09:24:01 +
@@ -76,6 +76,9 @@
for (const EntryRecord * entry : entry_records_) {
delete entry;
}
+ for (Column& column : columns_) {
+ delete column.btn;
+ }
}
/// Add a new column to this table.
@@ -99,17 +102,16 @@
{
Column c;
- c.btn = nullptr;
- if (title.size()) {
- c.btn =
-new Button
- (this, title,
- complete_width, 0, width, headerheight_,
- g_gr->images().get("images/ui_basic/but3.png"),
- title, tooltip_string, true, false);
- c.btn->sigclicked.connect
-(boost::bind(&Table::header_button_clicked, boost::ref(*this), columns_.size()));
- }
+ // All columns have a title button that is clickable for sorting.
+ // The title text can be empty.
+ c.btn =
+ new Button
+(this, title,
+ complete_width, 0, width, headerheight_,
+ g_gr->images().get("images/ui_basic/but3.png"),
+ title, tooltip_string, true, false);
+ c.btn->sigclicked.connect
+ (boost::bind(&Table::header_button_clicked, boost::ref(*this), columns_.size()));
c.width = width;
c.alignment = alignment;
c.is_checkbox_column = is_checkbox_column;
@@ -144,25 +146,8 @@
{
assert(col < columns_.size());
Column & column = columns_.at(col);
- if (!column.btn && !title.empty()) { // no title before, but now
- uint32_t complete_width = 0;
- for (uint8_t i = 0; i < col; ++i)
- complete_width += columns_.at(i).width;
- column.btn =
- new Button
-(this, title,
- complete_width, 0, column.width, headerheight_,
- g_gr->images().get("images/ui_basic/but3.png"),
- title, "", true, false);
- column.btn->sigclicked.connect
- (boost::bind(&Table::header_button_clicked, boost::ref(*this), col));
- } else if (title.empty()) { // had title before, not now
- if (column.btn) {
- delete column.btn;
- column.btn = nullptr;
- }
- } else
- column.btn->set_title(title);
+ assert(column.btn);
+ column.btn->set_title(title);
}
/**
___
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
