GunChleoc has proposed merging
lp:~widelands-dev/widelands/options_spinbox_cleanups into lp:widelands.
Commit message:
Options and Spinbox cleanups:
- Reversed semantics of "Do not zip" checkbox.
- More intelligent handling of units in spinboxes.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/options_spinbox_cleanups/+merge/290192
I reversed the "Do not zip...." checkbox, because negation is hard to process.
Also, better handling of the units and their plural forms in spinboxes - the
hard coded word order was not good. I also cleaned up the spinbox code
regarding units (e.g. use a map for direct access instead of iterating over a
vector).
For testing: Spinbox is used in Editor new map / new random map, and in Options.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/options_spinbox_cleanups into lp:widelands.
=== modified file 'src/editor/ui_menus/editor_main_menu_new_map.cc'
--- src/editor/ui_menus/editor_main_menu_new_map.cc 2016-03-02 17:11:16 +0000
+++ src/editor/ui_menus/editor_main_menu_new_map.cc 2016-03-28 10:48:00 +0000
@@ -50,10 +50,12 @@
box_(this, margin_, margin_, UI::Box::Vertical, 0, 0, margin_),
width_(&box_, 0, 0, box_width_, box_width_ / 3,
0, 0, 0,
- _("Width:"), "", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kValueList),
+ _("Width:"), UI::SpinBox::Units::kNone, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kValueList),
height_(&box_, 0, 0, box_width_, box_width_ / 3,
0, 0, 0,
- _("Height:"), "", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kValueList),
+ _("Height:"), UI::SpinBox::Units::kNone, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kValueList),
list_(&box_, 0, 0, box_width_, 330),
// Buttons
button_box_(&box_, 0, 0, UI::Box::Horizontal, 0, 0, margin_),
=== modified file 'src/editor/ui_menus/editor_main_menu_random_map.cc'
--- src/editor/ui_menus/editor_main_menu_random_map.cc 2016-03-02 17:11:16 +0000
+++ src/editor/ui_menus/editor_main_menu_random_map.cc 2016-03-28 10:48:00 +0000
@@ -52,14 +52,17 @@
// Size
width_(&box_, 0, 0, box_width_, box_width_ / 3,
0, 0, 0,
- _("Width:"), "", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kValueList),
+ _("Width:"), UI::SpinBox::Units::kNone, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kValueList),
height_(&box_, 0, 0, box_width_, box_width_ / 3,
0, 0, 0,
- _("Height:"), "", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kValueList),
+ _("Height:"), UI::SpinBox::Units::kNone, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kValueList),
max_players_(2),
players_(&box_, 0, 0, box_width_, box_width_ / 3,
2, 1, max_players_,
- _("Players:"), "", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kSmall),
+ _("Players:"), UI::SpinBox::Units::kNone, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kSmall),
// World + Resources
world_descriptions_(
{
@@ -104,13 +107,15 @@
mountainsval_(100 - waterval_ - landval_ - wastelandval_),
water_(&box_, 0, 0, box_width_, box_width_ / 3,
waterval_, 0, 60,
- _("Water:"), "%", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kSmall, 5),
+ _("Water:"), UI::SpinBox::Units::kPercent, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kSmall, 5),
land_(&box_, 0, 0, box_width_, box_width_ / 3,
landval_, 0, 100,
- _("Land:"), "%", g_gr->images().get("images/ui_basic/but1.png"), UI::SpinBox::Type::kSmall, 5),
+ _("Land:"), UI::SpinBox::Units::kPercent, g_gr->images().get("images/ui_basic/but1.png"),
+ UI::SpinBox::Type::kSmall, 5),
wasteland_(&box_, 0, 0, box_width_, box_width_ / 3,
wastelandval_, 0, 70,
- _("Wasteland:"), "%", g_gr->images().get("images/ui_basic/but1.png"),
+ _("Wasteland:"), UI::SpinBox::Units::kPercent, g_gr->images().get("images/ui_basic/but1.png"),
UI::SpinBox::Type::kSmall, 5),
mountains_box_(&box_, 0, 0, UI::Box::Horizontal, 0, 0, margin_),
mountains_label_(&mountains_box_, 0, 0, _("Mountains:")),
=== modified file 'src/ui_basic/spinbox.cc'
--- src/ui_basic/spinbox.cc 2016-02-17 08:48:02 +0000
+++ src/ui_basic/spinbox.cc 2016-03-28 10:48:00 +0000
@@ -19,6 +19,7 @@
#include "ui_basic/spinbox.h"
+#include <map>
#include <vector>
#include <boost/format.hpp>
@@ -35,14 +36,6 @@
namespace UI {
-struct IntValueTextReplacement {
- /// Value to be replaced
- int32_t value;
-
- /// Text to be used
- std::string text;
-};
-
struct SpinBoxImpl {
/// Value hold by the spinbox
int32_t value;
@@ -55,13 +48,13 @@
std::vector<int32_t> values;
/// The unit of the value
- std::string unit;
+ UI::SpinBox::Units unit;
/// Background tile style of buttons.
const Image* background;
- /// Special names for specific Values
- std::vector<IntValueTextReplacement> valrep;
+ /// Special names for specific values
+ std::map<int32_t, std::string> value_replacements;
/// The UI parts
Textarea * text;
@@ -84,7 +77,7 @@
const int32_t x, const int32_t y, const uint32_t w, const uint32_t unit_w,
int32_t const startval, int32_t const minval, int32_t const maxval,
const std::string& label_text,
- const std::string& unit,
+ const SpinBox::Units& unit,
const Image* background,
SpinBox::Type type,
int32_t step_size, int32_t big_step_size)
@@ -231,27 +224,17 @@
*/
void SpinBox::update()
{
- bool was_in_list = false;
- for (const IntValueTextReplacement& value : sbi_->valrep) {
- if (value.value == sbi_->value) {
- sbi_->text->set_text(value.text);
- was_in_list = true;
- break;
- }
- }
- if (!was_in_list) {
+ if (sbi_->value_replacements.count(sbi_->value) == 1) {
+ sbi_->text->set_text(sbi_->value_replacements.at(sbi_->value));
+ } else {
if (type_ == SpinBox::Type::kValueList) {
if ((sbi_->value >= 0) && (sbi_->values.size() > static_cast<size_t>(sbi_->value))) {
- /** TRANSLATORS: %i = number, %s = unit, e.g. "5 pixels" in the advanced options */
- sbi_->text->set_text((boost::format(_("%1$i %2$s"))
- % sbi_->values.at(sbi_->value)
- % sbi_->unit.c_str()).str());
+ sbi_->text->set_text(unit_text(sbi_->values.at(sbi_->value)));
} else {
sbi_->text->set_text("undefined"); // The user should never see this, so we're not localizing
}
} else {
- /** TRANSLATORS: %i = number, %s = unit, e.g. "5 pixels" in the advanced options */
- sbi_->text->set_text((boost::format(_("%1$i %2$s")) % sbi_->value % sbi_->unit.c_str()).str());
+ sbi_->text->set_text(unit_text(sbi_->value));
}
}
@@ -310,16 +293,6 @@
/**
- * manually sets the used unit to a given string
- */
-void SpinBox::set_unit(const std::string & unit)
-{
- sbi_->unit = unit;
- update();
-}
-
-
-/**
* \returns the value
*/
int32_t SpinBox::get_value() const
@@ -335,27 +308,6 @@
}
}
-/**
- * \returns the unit
- */
-std::string SpinBox::get_unit() const
-{
- return sbi_->unit;
-}
-
-
-/**
- * Searches for value in sbi->valrep
- * \returns the place where value was found or -1 if the value wasn't found.
- */
-int32_t SpinBox::find_replacement(int32_t value) const
-{
- for (uint32_t i = 0; i < sbi_->valrep.size(); ++i)
- if (sbi_->valrep[i].value == value)
- return i;
- return -1;
-}
-
/**
* Adds a replacement text for a specific value
@@ -363,34 +315,25 @@
*/
void SpinBox::add_replacement(int32_t value, const std::string& text)
{
- if (int32_t i = find_replacement(value) >= 0)
- sbi_->valrep[i].text = text;
- else {
- IntValueTextReplacement newtr;
- newtr.value = value;
- newtr.text = text;
- sbi_->valrep.push_back(newtr);
- }
+ sbi_->value_replacements[value] = text;
update();
}
-/**
- * Removes a replacement text for a specific value
- */
-void SpinBox::remove_replacement(int32_t value)
-{
- if (int32_t i = find_replacement(value) >= 0) {
- sbi_->valrep[i].text = (boost::format(_("%1$i %2$s")) % value % sbi_->unit.c_str()).str();
+const std::string SpinBox::unit_text(int32_t value) const {
+ switch (sbi_->unit) {
+ case (Units::kMinutes):
+ /** TRANSLATORS: A spinbox unit */
+ return (boost::format(ngettext("%d minute", "%d minutes", value)) % value).str();
+ case (Units::kPixels):
+ /** TRANSLATORS: A spinbox unit */
+ return (boost::format(ngettext("%d pixel", "%d pixels", value)) % value).str();
+ case (Units::kPercent):
+ /** TRANSLATORS: A spinbox unit */
+ return (boost::format(_("%i %%")) % value).str();
+ default:
+ return (boost::format("%u") % value).str();
}
}
-/**
- * \returns true, if find_replacement returns an int >= 0
- */
-bool SpinBox::has_replacement(int32_t value) const
-{
- return find_replacement(value) >= 0;
-}
-
-}
+} // namespace UI
=== modified file 'src/ui_basic/spinbox.h'
--- src/ui_basic/spinbox.h 2016-02-07 16:31:06 +0000
+++ src/ui_basic/spinbox.h 2016-03-28 10:48:00 +0000
@@ -31,7 +31,6 @@
namespace UI {
struct SpinBoxImpl;
-struct IntValueTextReplacement;
/// A spinbox is an UI element for setting the integer value of a variable.
/// w is the overall width of the SpinBox and must be wide enough to fit 2 labels and the buttons.
@@ -45,12 +44,19 @@
kValueList // Uses the values that are set by set_value_list().
};
+ enum class Units {
+ kNone,
+ kPixels,
+ kMinutes,
+ kPercent
+ };
+
SpinBox
(Panel*,
int32_t x, int32_t y, uint32_t w, uint32_t unit_w,
int32_t startval, int32_t minval, int32_t maxval,
const std::string& label_text = std::string(),
- const std::string& unit = std::string(),
+ const Units& unit = Units::kNone,
const Image* buttonbackground = g_gr->images().get("images/ui_basic/but3.png"),
SpinBox::Type = SpinBox::Type::kSmall,
// The amount by which units are increased/decreased for small and big steps when a button is pressed.
@@ -62,18 +68,14 @@
// otherwise you will confuse the user.
void set_value_list(const std::vector<int32_t>&);
void set_interval(int32_t min, int32_t max);
- void set_unit(const std::string&);
int32_t get_value() const;
- std::string get_unit() const;
void add_replacement(int32_t, const std::string&);
- void remove_replacement(int32_t);
- bool has_replacement(int32_t) const;
const std::vector<UI::Button*>& get_buttons() {return buttons_;}
private:
void update();
void change_value(int32_t);
- int32_t find_replacement(int32_t value) const;
+ const std::string unit_text(int32_t value) const;
const SpinBox::Type type_;
SpinBoxImpl* sbi_;
=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc 2016-02-16 09:22:53 +0000
+++ src/ui_fsmenu/options.cc 2016-03-28 10:48:00 +0000
@@ -154,7 +154,7 @@
sb_maxfps_(&box_interface_, 0, 0, column_width_ / 2, column_width_ / 4,
opt.maxfps, 0, 99,
- _("Maximum FPS:"), ""),
+ _("Maximum FPS:")),
// Windows options
@@ -166,17 +166,13 @@
sb_dis_panel_
(&box_windows_, 0, 0, column_width_, 200,
opt.panel_snap_distance, 0, 99, _("Distance for windows to snap to other panels:"),
- /** TRANSLATORS: Options: Distance for windows to snap to other panels: */
- /** TRANSLATORS: This will have a number added in front of it */
- ngettext("pixel", "pixels", opt.panel_snap_distance)),
+ UI::SpinBox::Units::kPixels),
sb_dis_border_
(&box_windows_, 0, 0, column_width_, 200,
opt.border_snap_distance, 0, 99,
_("Distance for windows to snap to borders:"),
- /** TRANSLATORS: Options: Distance for windows to snap to borders: */
- /** TRANSLATORS: This will have a number added in front of it */
- ngettext("pixel", "pixels", opt.border_snap_distance)),
+ UI::SpinBox::Units::kPixels),
// Sound options
music_ (&box_sound_, Point(0, 0), _("Enable Music"), "", column_width_),
@@ -188,18 +184,16 @@
sb_autosave_
(&box_saving_, 0, 0, column_width_, 250,
opt.autosave / 60, 0, 100, _("Save game automatically every:"),
- /** TRANSLATORS: Options: Save game automatically every: */
- /** TRANSLATORS: This will have a number added in front of it */
- ngettext("minute", "minutes", opt.autosave / 60),
+ UI::SpinBox::Units::kMinutes,
g_gr->images().get("images/ui_basic/but3.png"), UI::SpinBox::Type::kBig),
sb_rolling_autosave_
(&box_saving_, 0, 0, column_width_, 250,
opt.rolling_autosave, 1, 20, _("Maximum number of autosave files:"),
- "",
+ UI::SpinBox::Units::kNone,
g_gr->images().get("images/ui_basic/but3.png"), UI::SpinBox::Type::kBig),
- nozip_(&box_saving_, Point(0, 0), _("Do not zip widelands data files (maps, replays and savegames)"),
+ zip_(&box_saving_, Point(0, 0), _("Compress widelands data files (maps, replays and savegames)"),
"", column_width_),
write_syncstreams_(&box_saving_, Point(0, 0), _("Write syncstreams in network games to debug desyncs"),
"", column_width_),
@@ -267,7 +261,7 @@
// Saving
box_saving_.add(&sb_autosave_, UI::Align::kLeft);
box_saving_.add(&sb_rolling_autosave_, UI::Align::kLeft);
- box_saving_.add(&nozip_, UI::Align::kLeft);
+ box_saving_.add(&zip_, UI::Align::kLeft);
box_saving_.add(&write_syncstreams_, UI::Align::kLeft);
// Game
@@ -288,25 +282,6 @@
/** TRANSLATORS Options: Save game automatically every: */
sb_autosave_ .add_replacement(0, _("Off"));
- for (UI::Button* temp_button : sb_autosave_.get_buttons()) {
- temp_button->sigclicked.connect
- (boost::bind
- (&FullscreenMenuOptions::update_sb_autosave_unit,
- boost::ref(*this)));
- }
- for (UI::Button* temp_button : sb_dis_panel_.get_buttons()) {
- temp_button->sigclicked.connect
- (boost::bind
- (&FullscreenMenuOptions::update_sb_dis_panel_unit,
- boost::ref(*this)));
- }
-
- for (UI::Button* temp_button : sb_dis_border_.get_buttons()) {
- temp_button->sigclicked.connect
- (boost::bind
- (&FullscreenMenuOptions::update_sb_dis_border_unit,
- boost::ref(*this)));
- }
// Fill in data
// Interface options
@@ -365,8 +340,8 @@
message_sound_ .set_state(opt.message_sound);
// Saving options
- nozip_ .set_state(opt.nozip);
- write_syncstreams_ .set_state(opt.write_syncstreams);
+ zip_ .set_state(opt.zip);
+ write_syncstreams_ .set_state(opt.write_syncstreams);
// Game options
auto_roadbuild_mode_ .set_state(opt.auto_roadbuild_mode);
@@ -379,17 +354,6 @@
language_list_.focus();
}
-void FullscreenMenuOptions::update_sb_autosave_unit() {
- sb_autosave_.set_unit(ngettext("minute", "minutes", sb_autosave_.get_value()));
-}
-
-void FullscreenMenuOptions::update_sb_dis_panel_unit() {
- sb_dis_panel_.set_unit(ngettext("pixel", "pixels", sb_dis_panel_.get_value()));
-}
-
-void FullscreenMenuOptions::update_sb_dis_border_unit() {
- sb_dis_border_.set_unit(ngettext("pixel", "pixels", sb_dis_border_.get_value()));
-}
void FullscreenMenuOptions::add_languages_to_list(const std::string& current_locale) {
@@ -478,8 +442,8 @@
// Saving options
os_.autosave = sb_autosave_.get_value();
os_.rolling_autosave = sb_rolling_autosave_.get_value();
- os_.nozip = nozip_.get_state();
- os_.write_syncstreams = write_syncstreams_.get_state();
+ os_.zip = zip_.get_state();
+ os_.write_syncstreams = write_syncstreams_.get_state();
// Game options
os_.auto_roadbuild_mode = auto_roadbuild_mode_.get_state();
@@ -546,7 +510,7 @@
// Saving options
opt.autosave = opt_section_.get_int("autosave", DEFAULT_AUTOSAVE_INTERVAL * 60);
opt.rolling_autosave = opt_section_.get_int("rolling_autosave", 5);
- opt.nozip = opt_section_.get_bool("nozip", false);
+ opt.zip = !opt_section_.get_bool("nozip", false);
opt.write_syncstreams = opt_section_.get_bool("write_syncstreams", true);
// Game options
@@ -589,8 +553,8 @@
// Saving options
opt_section_.set_int("autosave", opt.autosave * 60);
opt_section_.set_int("rolling_autosave", opt.rolling_autosave);
- opt_section_.set_bool("nozip", opt.nozip);
- opt_section_.set_bool("write_syncstreams", opt.write_syncstreams);
+ opt_section_.set_bool("nozip", !opt.zip);
+ opt_section_.set_bool("write_syncstreams", opt.write_syncstreams);
// Game options
opt_section_.set_bool("auto_roadbuild_mode", opt.auto_roadbuild_mode);
=== modified file 'src/ui_fsmenu/options.h'
--- src/ui_fsmenu/options.h 2016-02-14 21:18:03 +0000
+++ src/ui_fsmenu/options.h 2016-03-28 10:48:00 +0000
@@ -63,7 +63,7 @@
// Saving options
int32_t autosave; // autosave interval in minutes
int32_t rolling_autosave; // number of file to use for rolling autosave
- bool nozip;
+ bool zip;
bool write_syncstreams;
// Game options
@@ -98,10 +98,6 @@
OptionsCtrl::OptionsStruct get_values();
private:
- void update_sb_autosave_unit();
- void update_sb_dis_panel_unit();
- void update_sb_dis_border_unit();
-
// Fills the language selection list
void add_languages_to_list(const std::string& current_locale);
@@ -149,7 +145,7 @@
// Saving options
UI::SpinBox sb_autosave_;
UI::SpinBox sb_rolling_autosave_;
- UI::Checkbox nozip_;
+ UI::Checkbox zip_;
UI::Checkbox write_syncstreams_;
// Game options
_______________________________________________
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
