Review: Needs Fixing
This will break once translations come in.
Diff comments:
>
> === modified file 'src/wui/economy_options_window.cc'
> --- src/wui/economy_options_window.cc 2019-02-23 11:00:49 +0000
> +++ src/wui/economy_options_window.cc 2019-05-16 17:28:26 +0000
> @@ -186,35 +281,402 @@
> const Widelands::Economy::TargetQuantity& tq = is_wares
> ?
>
> economy->ware_target_quantity(index) :
>
> economy->worker_target_quantity(index);
> - // Don't allow negative new amount.
> - if (amount >= 0 || -amount <=
> static_cast<int>(tq.permanent)) {
> - if (is_wares) {
> - game.send_player_command(*new
> Widelands::CmdSetWareTargetQuantity(
> - game.get_gametime(),
> player_->player_number(), serial_, index,
> - tq.permanent + amount));
> + // Don't allow negative new amount
> + const int old_amount = static_cast<int>(tq.permanent);
> + const int new_amount = std::max(0, old_amount + delta);
> + if (new_amount == old_amount) {
> + continue;
> + }
> + if (is_wares) {
> + game.send_player_command(*new
> Widelands::CmdSetWareTargetQuantity(
> + game.get_gametime(),
> player_->player_number(), serial_, index, new_amount));
> + } else {
> + game.send_player_command(*new
> Widelands::CmdSetWorkerTargetQuantity(
> + game.get_gametime(),
> player_->player_number(), serial_, index, new_amount));
> + }
> + }
> + }
> +}
> +
> +void EconomyOptionsWindow::EconomyOptionsPanel::reset_target() {
> + Widelands::Game& game =
> dynamic_cast<Widelands::Game&>(player_->egbase());
> + const bool is_wares = type_ == Widelands::wwWARE;
> + const auto& items = is_wares ? player_->tribe().wares() :
> player_->tribe().workers();
> + const PredefinedTargets settings =
> economy_options_window_->get_selected_target();
> +
> + bool anything_selected = false;
> + bool second_phase = false;
> +
> +run_second_phase:
> + for (const Widelands::DescriptionIndex& index : items) {
> + if (display_.ware_selected(index) || (second_phase &&
> !display_.is_ware_hidden(index))) {
> + anything_selected = true;
> + if (is_wares) {
> + game.send_player_command(*new
> Widelands::CmdSetWareTargetQuantity(
> + game.get_gametime(),
> player_->player_number(), serial_, index, settings.wares.at(index)));
> + } else {
> + game.send_player_command(*new
> Widelands::CmdSetWorkerTargetQuantity(
> + game.get_gametime(),
> player_->player_number(), serial_, index, settings.workers.at(index)));
> + }
> + }
> + }
> +
> + if (!second_phase && !anything_selected) {
> + // Nothing was selected, now go through the loop again and
> change everything
> + second_phase = true;
> + goto run_second_phase;
You can use a do... while instead and get rid of the goto.
https://homepages.cwi.nl/~storm/teaching/reader/Dijkstra68.pdf
> + }
> +}
> +
> +constexpr unsigned kThinkInterval = 200;
> +
> +void EconomyOptionsWindow::think() {
> + const uint32_t time = player_->egbase().get_gametime();
> + if (time - time_last_thought_ < kThinkInterval ||
> !player_->get_economy(serial_)) {
> + // If our economy has been deleted, die() was already called,
> no need to do anything
> + return;
> + }
> + time_last_thought_ = time;
> + update_profiles();
> +}
> +
> +std::string EconomyOptionsWindow::applicable_target() {
> + const Widelands::Economy* eco = player_->get_economy(serial_);
> + for (const auto& pair : predefined_targets_) {
> + bool matches = true;
> + if (tabpanel_.active() == 0) {
> + for (const Widelands::DescriptionIndex& index :
> player_->tribe().wares()) {
> + const auto it = pair.second.wares.find(index);
> + if (it != pair.second.wares.end() &&
> eco->ware_target_quantity(index).permanent != it->second) {
> + matches = false;
> + break;
> + }
> + }
> + } else {
> + for (const Widelands::DescriptionIndex& index :
> player_->tribe().workers()) {
> + const auto it = pair.second.workers.find(index);
> + if (it != pair.second.workers.end() &&
> eco->worker_target_quantity(index).permanent != it->second) {
> + matches = false;
> + break;
> + }
> + }
> + }
> + if (matches) {
> + return pair.first;
> + }
> + }
> + return "";
> +}
> +
> +void EconomyOptionsWindow::update_profiles() {
> + const std::string current_profile = applicable_target();
> +
> + for (const auto& pair : predefined_targets_) {
> + if (last_added_to_dropdown_.count(pair.first) == 0) {
> + return update_profiles_needed(current_profile);
> + }
> + }
> + for (const auto& string : last_added_to_dropdown_) {
> + if (!string.empty() && predefined_targets_.find(string) ==
> predefined_targets_.end()) {
> + return update_profiles_needed(current_profile);
> + }
> + }
> + if (last_added_to_dropdown_.count("") == (current_profile.empty() ? 0 :
> 1)) {
> + return update_profiles_needed(current_profile);
> + }
> +
> + update_profiles_select(current_profile);
> +}
> +
> +void EconomyOptionsWindow::update_profiles_needed(const std::string&
> current_profile) {
> + dropdown_.clear();
> + last_added_to_dropdown_.clear();
> + for (const auto& pair : predefined_targets_) {
> + dropdown_.add(_(pair.first), pair.first);
> + last_added_to_dropdown_.insert(pair.first);
> + }
> + if (current_profile.empty()) {
> + // Nothing selected
> + dropdown_.add("", "");
How about having _("–") as a display label here?
> + last_added_to_dropdown_.insert("");
> + }
> + update_profiles_select(current_profile);
> +}
> +
> +void EconomyOptionsWindow::update_profiles_select(const std::string&
> current_profile) {
> + if (dropdown_.is_expanded()) {
> + return;
> + }
> + const std::string select = current_profile.empty() ? "" :
> _(current_profile);
You are selecting the display label here instead of the actual entry. This will
stop working once translations come in.
> + if (!dropdown_.has_selection() || dropdown_.get_selected() != select) {
> + dropdown_.select(select);
> + }
> + assert(dropdown_.has_selection());
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::update_save_enabled() {
> + const std::string& text = profile_name_.text();
> + if (text.empty() || text == kDefaultEconomyProfile) {
> + save_.set_enabled(false);
> + save_.set_tooltip(text.empty() ? _("The profile name cannot be
> empty") :
> + _("The default profile cannot be overwritten"));
> + } else {
> + save_.set_enabled(true);
> + save_.set_tooltip(_("Save the profile under this name"));
> + }
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::table_selection_changed() {
> + if (!table_.has_selection()) {
> + delete_.set_enabled(false);
> + delete_.set_tooltip("");
> + return;
> + }
> + const std::string& sel = table_[table_.selection_index()];
> + if (economy_options_->get_predefined_targets().at(sel).undeletable) {
> + delete_.set_tooltip(_("The predefined profiles cannot be
> deleted"));
> + delete_.set_enabled(false);
> + } else {
> + delete_.set_tooltip(_("Delete the selected profiles"));
> + delete_.set_enabled(true);
> + }
> + profile_name_.set_text(sel);
> + update_save_enabled();
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::update_table() {
> + table_.clear();
> + for (const auto& pair : economy_options_->get_predefined_targets()) {
> + table_.add(pair.first).set_string(0, _(pair.first));
> + }
> + layout();
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::save() {
> + const std::string name = profile_name_.text();
> + assert(!name.empty());
> + assert(name != kDefaultEconomyProfile);
> + for (const auto& pair : economy_options_->get_predefined_targets()) {
> + if (pair.first == name) {
> + UI::WLMessageBox m(this, _("Overwrite?"),
> + _("A profile with this name already
> exists.\nDo you wish to replace it?"),
Replace the \n with a blank space.
> + UI::WLMessageBox::MBoxType::kOkCancel);
> + if (m.run<UI::Panel::Returncodes>() !=
> UI::Panel::Returncodes::kOk) {
> + return;
> + }
> + break;
> + }
> + }
> + economy_options_->do_create_target(name);
> + die();
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::delete_selected() {
> + assert(table_.has_selection());
> +
> + auto& map = economy_options_->get_predefined_targets();
> + const std::string& name = table_[table_.selection_index()];
> +
> + assert(name != kDefaultEconomyProfile);
> + assert(!map.at(name).undeletable);
> + auto it = map.find(name);
> + assert(it != map.end());
> + map.erase(it);
> +
> + economy_options_->save_targets();
> + economy_options_->update_profiles();
> + update_table();
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::unset_parent() {
> + economy_options_ = nullptr;
> + die();
> +}
> +
> +void EconomyOptionsWindow::SaveProfileWindow::think() {
> + if (!economy_options_) {
> + die();
> + }
> + UI::Window::think();
> +}
> +
> +EconomyOptionsWindow::SaveProfileWindow::SaveProfileWindow(UI::Panel*
> parent, EconomyOptionsWindow* eco)
> + : UI::Window(parent, "save_economy_options_profile", 0, 0, 0, 0, _("Save
> Profile")),
> + economy_options_(eco),
> + main_box_(this, 0, 0, UI::Box::Vertical),
> + table_box_(&main_box_, 0, 0, UI::Box::Vertical),
> + table_(&table_box_, 0, 0, 460, 120, UI::PanelStyle::kWui),
> + buttons_box_(&main_box_, 0, 0, UI::Box::Horizontal),
> + profile_name_(&buttons_box_, 0, 0, 240, 0, 0, UI::PanelStyle::kWui),
> + save_(&buttons_box_, "save", 0, 0, 80, 34,
> UI::ButtonStyle::kWuiPrimary, _("Save")),
> + cancel_(&buttons_box_, "cancel", 0, 0, 80, 34,
> UI::ButtonStyle::kWuiSecondary, _("Cancel")),
> + delete_(&buttons_box_, "delete", 0, 0, 80, 34,
> UI::ButtonStyle::kWuiSecondary, _("Delete")) {
> + table_.add_column(200, _("Existing Profiles"));
> + update_table();
> +
> + table_.selected.connect([this](uint32_t) { table_selection_changed();
> });
> + profile_name_.changed.connect([this] { update_save_enabled(); });
> + profile_name_.ok.connect([this] { save(); });
> + profile_name_.cancel.connect([this] { die(); });
> + save_.sigclicked.connect([this] { save(); });
> + cancel_.sigclicked.connect([this] { die(); });
> + delete_.sigclicked.connect([this] { delete_selected(); });
> +
> + table_box_.add(&table_, UI::Box::Resizing::kFullSize);
> + buttons_box_.add(&profile_name_, UI::Box::Resizing::kFullSize);
> + buttons_box_.add(&save_);
> + buttons_box_.add(&cancel_);
> + buttons_box_.add(&delete_);
> + main_box_.add(&table_box_, UI::Box::Resizing::kFullSize);
> + main_box_.add(&buttons_box_, UI::Box::Resizing::kFullSize);
> + set_center_panel(&main_box_);
> +
> + table_selection_changed();
> + update_save_enabled();
> +}
> +
> +EconomyOptionsWindow::SaveProfileWindow::~SaveProfileWindow() {
> + if (economy_options_) {
> + economy_options_->close_save_profile_window();
> + }
> +}
> +
> +void EconomyOptionsWindow::create_target() {
> + if (save_profile_dialog_) {
> + // Already open
> + return;
> + }
> + save_profile_dialog_ = new SaveProfileWindow(get_parent(), this);
> +}
> +
> +void EconomyOptionsWindow::close_save_profile_window() {
> + assert(save_profile_dialog_);
> + save_profile_dialog_ = nullptr;
> +}
> +
> +void EconomyOptionsWindow::do_create_target(const std::string& name) {
> + assert(!name.empty());
> + assert(name != kDefaultEconomyProfile);
> + const Widelands::Tribes& tribes = player_->egbase().tribes();
> + const Widelands::TribeDescr& tribe = player_->tribe();
> + Widelands::Economy* economy = player_->get_economy(serial_);
> + PredefinedTargets t;
> + for (Widelands::DescriptionIndex di : tribe.wares()) {
> + if (tribes.get_ware_descr(di)->has_demand_check(tribe.name())) {
> + t.wares[di] =
> economy->ware_target_quantity(di).permanent;
> + }
> + }
> + for (Widelands::DescriptionIndex di : tribe.workers()) {
> + if (tribes.get_worker_descr(di)->has_demand_check()) {
> + t.workers[di] =
> economy->worker_target_quantity(di).permanent;
> + }
> + }
> + predefined_targets_[name] = t;
> +
> + save_targets();
> + update_profiles();
> +}
> +
> +void EconomyOptionsWindow::save_targets() {
> + const Widelands::Tribes& tribes = player_->egbase().tribes();
> + Profile profile;
> +
> + std::map<std::string, uint32_t> serials;
> + for (const auto& pair : predefined_targets_) {
> + if (pair.first != kDefaultEconomyProfile) {
> + serials.emplace(pair.first, serials.size());
> + }
> + }
> + Section& global_section =
> profile.create_section(kDefaultEconomyProfile.c_str());
> + for (const auto& pair : serials) {
> + global_section.set_string(std::to_string(pair.second).c_str(),
> pair.first);
> + }
> +
> + for (const auto& pair : predefined_targets_) {
> + if (pair.first == kDefaultEconomyProfile) {
> + continue;
> + }
> + Section& section =
> profile.create_section(std::to_string(serials.at(pair.first)).c_str());
> + for (const auto& setting : pair.second.wares) {
> +
> section.set_natural(tribes.get_ware_descr(setting.first)->name().c_str(),
> setting.second);
> + }
> + for (const auto& setting : pair.second.workers) {
> +
> section.set_natural(tribes.get_worker_descr(setting.first)->name().c_str(),
> setting.second);
> + }
> + }
> +
> + Section& section = profile.create_section("undeletable");
> + for (const auto& pair : predefined_targets_) {
> + if (pair.first != kDefaultEconomyProfile) {
> +
> section.set_bool(std::to_string(serials.at(pair.first)).c_str(),
> pair.second.undeletable);
> + }
> + }
> +
> + g_fs->ensure_directory_exists(kEconomyProfilesDir);
> + std::string complete_filename = kEconomyProfilesDir +
> g_fs->file_separator() + player_->tribe().name();
> + profile.write(complete_filename.c_str(), false);
> +
> + // Inform the windows of other economies of new and deleted profiles
> + Notifications::publish(NoteEconomyProfile(serial_));
> +}
> +
> +void EconomyOptionsWindow::read_targets() {
> + predefined_targets_.clear();
> + const Widelands::Tribes& tribes = player_->egbase().tribes();
> + const Widelands::TribeDescr& tribe = player_->tribe();
> +
> + {
> + PredefinedTargets t;
> + t.undeletable = true;
> + for (Widelands::DescriptionIndex di : tribe.wares()) {
> + const Widelands::WareDescr* descr =
> tribes.get_ware_descr(di);
> + if (descr->has_demand_check(tribe.name())) {
> + t.wares.emplace(di,
> descr->default_target_quantity(tribe.name()));
> + }
> + }
> + for (Widelands::DescriptionIndex di : tribe.workers()) {
> + const Widelands::WorkerDescr* descr =
> tribes.get_worker_descr(di);
> + if (descr->has_demand_check()) {
> + t.workers.emplace(di,
> descr->default_target_quantity());
> + }
> + }
> + predefined_targets_.emplace(kDefaultEconomyProfile, t);
> + }
> +
> + std::string complete_filename = kEconomyProfilesDir +
> g_fs->file_separator() + player_->tribe().name();
> + Profile profile;
> + profile.read(complete_filename.c_str());
> +
> + Section* global_section = profile.get_section(kDefaultEconomyProfile);
> + if (global_section) {
> + std::map<std::string, std::string> serials;
> + while (Section::Value* v = global_section->get_next_val()) {
> + serials.emplace(std::string(v->get_name()),
> v->get_string());
> + }
> +
> + for (const auto& pair : serials) {
> + Section* section = profile.get_section(pair.first);
> + PredefinedTargets t;
> + while (Section::Value* v = section->get_next_val()) {
> + const std::string name =
> std::string(v->get_name());
> + Widelands::DescriptionIndex di =
> tribes.ware_index(name);
> + if (di == Widelands::INVALID_INDEX) {
> + di = tribes.worker_index(name);
> + assert(di != Widelands::INVALID_INDEX);
> + t.workers.emplace(di, v->get_natural());
> } else {
> - game.send_player_command(*new
> Widelands::CmdSetWorkerTargetQuantity(
> - game.get_gametime(),
> player_->player_number(), serial_, index,
> - tq.permanent + amount));
> + t.wares.emplace(di, v->get_natural());
> }
> }
> + predefined_targets_.emplace(pair.second, t);
> }
> - }
> -}
>
> -void EconomyOptionsWindow::EconomyOptionsPanel::reset_target() {
> - Widelands::Game& game =
> dynamic_cast<Widelands::Game&>(player_->egbase());
> - const bool is_wares = type_ == Widelands::wwWARE;
> - const auto& items = is_wares ? player_->tribe().wares() :
> player_->tribe().workers();
> - for (const Widelands::DescriptionIndex& index : items) {
> - if (display_.ware_selected(index)) {
> - if (is_wares) {
> - game.send_player_command(*new
> Widelands::CmdResetWareTargetQuantity(
> - game.get_gametime(),
> player_->player_number(), serial_, index));
> - } else {
> - game.send_player_command(*new
> Widelands::CmdResetWorkerTargetQuantity(
> - game.get_gametime(),
> player_->player_number(), serial_, index));
> + if (Section* section = profile.get_section("undeletable")) {
> + while (Section::Value* v = section->get_next_val()) {
> +
> predefined_targets_.at(serials.at(std::string(v->get_name()))).undeletable =
> v->get_bool();
> }
> }
> }
> +
> + update_profiles();
> }
--
https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366987
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/economy-target-profiles.
_______________________________________________
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
