[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands. Commit message: Users can define and save their own profiles of economy target quantities. Redesigned the economy options menu. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1827696 in widelands: "Allow users to define their own economy default settings" https://bugs.launchpad.net/widelands/+bug/1827696 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 For each tribe, I added two profiles: "Efficiency" is equal to the changes proposed previously, and "Stockpile" is for people who, well, like to stockpile stuff. Additionally there is an unchangeable "Default" profile that resets stuff to the default settings. To apply a profile, select the profile and the wares you wish to change and click Apply (replaces Reset). Use "Save" to save your current settings as a profile. The save window also allows you to delete profiles. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands. === added directory 'data/economy_profiles' === added file 'data/economy_profiles/atlanteans' --- data/economy_profiles/atlanteans 1970-01-01 00:00:00 + +++ data/economy_profiles/atlanteans 2019-05-04 19:31:09 + @@ -0,0 +1,85 @@ +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug) + +[Efficiency] +blackroot_flour="1" +atlanteans_bread="20" +bread_paddle="0" +buckets="0" +coal="5" +cornmeal="3" +diamond="3" +fire_tongs="1" +fishing_net="2" +gold="1" +gold_ore="1" +gold_thread="0" +granite="10" +hammer="0" +hook_pole="0" +hunting_bow="1" +iron="5" +iron_ore="1" +milking_tongs="0" +pick="1" +planks="1" +quartz="3" +saw="0" +scythe="0" +shield_advanced="0" +shield_steel="0" +shovel="0" +smoked_fish="5" +smoked_meat="3" +spidercloth="5" +spider_silk="5" +tabard="1" +tabard_golden="0" +trident_double="0" +trident_heavy_double="0" +trident_light="1" +trident_long="0" +trident_steel="0" +atlanteans_horse="1" +atlanteans_soldier="10" + +[Stockpile] +blackroot_flour="20" +atlanteans_bread="30" +bread_paddle="1" +buckets="2" +coal="25" +cornmeal="20" +diamond="10" +fire_tongs="1" +fishing_net="2" +gold="20" +gold_ore="15" +gold_thread="5" +granite="30" +hammer="2" +hook_pole="1" +hunting_bow="1" +iron="25" +iron_ore="20" +milking_tongs="1" +pick="3" +planks="40" +quartz="10" +saw="2" +scythe="1" +shield_advanced="1" +shield_steel="1" +shovel="2" +smoked_fish="40" +smoked_meat="25" +spidercloth="20" +spider_silk="15" +tabard="30" +tabard_golden="1" +trident_double="1" +trident_heavy_double="1" +trident_light="30" +trident_long="1" +trident_steel="1" +atlanteans_horse="20" +atlanteans_soldier="20" === added file 'data/economy_profiles/barbarians' --- data/economy_profiles/barbarians 1970-01-01 00:00:00 + +++ data/economy_profiles/barbarians 2019-05-04 19:31:09 + @@ -0,0 +1,77 @@ +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug) + +[Efficiency] +ax="1" +ax_battle="0" +ax_broad="0" +ax_bronze="0" +ax_sharp="0" +ax_warriors="0" +beer="0" +beer_strong="1" +blackwood="40" +barbarians_bread="5" +bread_paddle="0" +cloth="10" +coal="20" +felling_ax="0" +fire_tongs="1" +fishing_rod="0" +gold="1" +gold_ore="1" +granite="10" +grout="1" +hammer="1" +helmet="0" +helmet_mask="0" +helmet_warhelm="0" +hunting_spear="0" +iron="5" +iron_ore="5" +kitchen_tools="0" +meal="5" +pick="1" +ration="20" +scythe="0" +shovel="0" +snack="0" +barbarians_ox="1" +barbarians_soldier="10" + +[Stockpile] +ax="30" +ax_battle="1" +ax_broad="1" +ax_bronze="1" +ax_sharp="1" +ax_warriors="1" +beer="15" +beer_strong="20" +blackwood="45" +barbarians_bread="25" +bread_paddle="1" +cloth="10" +coal="25" +felling_ax="5" +fire_tongs="1" +fishing_rod="1" +gold="20" +gold_ore="15" +granite="30" +grout="20" +hammer="2" +helmet="1" +helmet_mask="1" +helmet_warhelm="1" +hunting_spear="1" +iron="25" +iron_ore="20" +kitchen_tools="1" +meal="15" +pick="2" +ration="30" +scythe="1" +shovel="1" +snack="20" +barbarians_ox="20" +barbarians_soldier="20" === added file 'data/economy_profiles/empire' --- data/economy_profiles/empire 1970-01-01 00:00:00 + +++ data/economy_profiles/empire 2019-05-04 19:31:09 + @@ -0,0 +1,85 @@ +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug) + +[Efficiency] +armor="1" +armor_chain="1" +armor_gilded="1" +armor_helmet="30" +basket="1" +beer="1" +empire_bread="20" +bread_paddle="0" +cloth="15" +coal="5" +felling_ax="0" +fire_tongs="1" +fishing_rod="0" +flour="20" +gold="1" +gold_ore="1" +granite="10" +hammer="0" +hunting_spear="0" +iron="5" +iron_ore="3" +kitchen_tools="0" +marble="30" +marble_column="10" +meal="5" +meat="20" +pick="1" +planks="1" +ration="20" +saw="0" +scythe="0" +shovel="0" +spear="1" +spear_advanced="1" +spear_heavy="1" +spear_war="1" +spear_wooden
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4886. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/528245915. Appveyor build 4667. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4667. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4901. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/528550026. Appveyor build 4682. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4682. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Work in progress => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands. Commit message: Users can define and save their own profiles of economy target quantities. Redesigned the economy options menu. WaresDisplays and will relayout themselves dynamically on fullscreen switch. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1827696 in widelands: "Allow users to define their own economy default settings" https://bugs.launchpad.net/widelands/+bug/1827696 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366987 For each tribe, I added two profiles: "Efficiency" is equal to the changes proposed previously, and "Stockpile" is for people who, well, like to stockpile stuff. Additionally there is an unchangeable "Default" pseudo-profile that resets items to the default settings. To apply a profile, select the items you wish to change and choose the profile from the dropdown. Use "Save" to save your current settings as a profile. The save window also allows you to delete profiles. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands. === added file 'data/images/ui_basic/scrollbar_down_fast.png' Binary files data/images/ui_basic/scrollbar_down_fast.png 1970-01-01 00:00:00 + and data/images/ui_basic/scrollbar_down_fast.png 2019-05-06 13:53:16 + differ === added file 'data/images/ui_basic/scrollbar_up_fast.png' Binary files data/images/ui_basic/scrollbar_up_fast.png 1970-01-01 00:00:00 + and data/images/ui_basic/scrollbar_up_fast.png 2019-05-06 13:53:16 + differ === added directory 'data/tribes/economy_profiles' === added file 'data/tribes/economy_profiles/atlanteans' --- data/tribes/economy_profiles/atlanteans 1970-01-01 00:00:00 + +++ data/tribes/economy_profiles/atlanteans 2019-05-06 13:53:16 + @@ -0,0 +1,89 @@ +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug) + +[Default] +0=_"Efficiency" +1=_"Stockpile" + +[0] +blackroot_flour="1" +atlanteans_bread="20" +bread_paddle="0" +buckets="0" +coal="5" +cornmeal="3" +diamond="3" +fire_tongs="1" +fishing_net="2" +gold="1" +gold_ore="1" +gold_thread="0" +granite="10" +hammer="0" +hook_pole="0" +hunting_bow="1" +iron="5" +iron_ore="1" +milking_tongs="0" +pick="1" +planks="1" +quartz="3" +saw="0" +scythe="0" +shield_advanced="0" +shield_steel="0" +shovel="0" +smoked_fish="5" +smoked_meat="3" +spidercloth="5" +spider_silk="5" +tabard="1" +tabard_golden="0" +trident_double="0" +trident_heavy_double="0" +trident_light="1" +trident_long="0" +trident_steel="0" +atlanteans_horse="1" +atlanteans_soldier="10" + +[1] +blackroot_flour="20" +atlanteans_bread="30" +bread_paddle="1" +buckets="2" +coal="25" +cornmeal="20" +diamond="10" +fire_tongs="1" +fishing_net="2" +gold="20" +gold_ore="15" +gold_thread="5" +granite="30" +hammer="2" +hook_pole="1" +hunting_bow="1" +iron="25" +iron_ore="20" +milking_tongs="1" +pick="3" +planks="40" +quartz="10" +saw="2" +scythe="1" +shield_advanced="1" +shield_steel="1" +shovel="2" +smoked_fish="40" +smoked_meat="25" +spidercloth="20" +spider_silk="15" +tabard="30" +tabard_golden="1" +trident_double="1" +trident_heavy_double="1" +trident_light="30" +trident_long="1" +trident_steel="1" +atlanteans_horse="20" +atlanteans_soldier="20" === added file 'data/tribes/economy_profiles/barbarians' --- data/tribes/economy_profiles/barbarians 1970-01-01 00:00:00 + +++ data/tribes/economy_profiles/barbarians 2019-05-06 13:53:16 + @@ -0,0 +1,81 @@ +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug) + +[Default] +0=_"Efficiency" +1=_"Stockpile" + +[0] +ax="1" +ax_battle="0" +ax_broad="0" +ax_bronze="0" +ax_sharp="0" +ax_warriors="0" +beer="0" +beer_strong="1" +blackwood="40" +barbarians_bread="5" +bread_paddle="0" +cloth="10" +coal="20" +felling_ax="0" +fire_tongs="1" +fishing_rod="0" +gold="1" +gold_ore="1" +granite="10" +grout="1" +hammer="1" +helmet="0" +helmet_mask="0" +helmet_warhelm="0" +hunting_spear="0" +iron="5" +iron_ore="5" +kitchen_tools="0" +meal="5" +pick="1" +ration="20" +scythe="0" +shovel="0" +snack="0" +barbarians_ox="1" +barbarians_soldier="10" + +[1] +ax="30" +ax_battle="1" +ax_broad="1" +ax_bronze="1" +ax_sharp="1" +ax_warriors="1" +beer="15" +beer_strong="20" +blackwood="45" +barbarians_bread="25" +bread_paddle="1" +cloth="10" +coal="25" +felling_ax="5" +fire_tongs="1" +fishing_rod="1" +gold="20" +gold_ore="15" +granite="30" +grout="20" +hammer="2" +helmet="1" +helmet_mask="1" +helmet_warhelm="1" +hunting_spear="1" +iron="25" +iron_ore="20" +kitchen_tools="1" +meal="15" +pick="2" +ration="30" +scythe="1" +shovel="1" +snack="20" +barbarians_ox="20" +barbarians_soldier="20" === added file 'data/tribes/economy_profiles/empire' --- data/tribes/economy_profiles/empire 1970-01-01 00:00:00 + +++ data/tribes/economy_profiles/empire
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4904. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/528827570. Appveyor build 4685. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4685. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366987 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4911. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/529823274. Appveyor build 4692. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4692. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4930. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/530872873. Appveyor build 4711. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4711. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4940. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/531134022. Appveyor build 4721. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4721. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 4972. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/532432343. Appveyor build 4753. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4753. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5002. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/533445474. Appveyor build 4783. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4783. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5080. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/538595965. Appveyor build 4860. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4860. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 5080. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/538595965. -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Needs review => Merged For more details, see: 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Benedikt Straub has proposed merging
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
Commit message:
Fix economy targets profile application when no items are selected
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1827696 in widelands: "Allow users to define their own economy default
settings"
https://bugs.launchpad.net/widelands/+bug/1827696
Bug #1831196 in widelands: "Economy profile selection doesn't work"
https://bugs.launchpad.net/widelands/+bug/1831196
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
=== modified file 'src/wui/economy_options_window.cc'
--- src/wui/economy_options_window.cc 2019-05-29 16:59:16 +
+++ src/wui/economy_options_window.cc 2019-05-31 09:22:25 +
@@ -307,29 +307,25 @@
const PredefinedTargets settings = economy_options_window_->get_selected_target();
bool anything_selected = false;
- bool second_phase = false;
-
- do {
- 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)));
-}
+ for (const Widelands::DescriptionIndex& index : items) {
+ if (display_.ware_selected(index)) {
+ anything_selected = true;
+ break;
+ }
+ }
+ for (const Widelands::DescriptionIndex& index : items) {
+ if (display_.ware_selected(index) || (!anything_selected && !display_.is_ware_hidden(index))) {
+ 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 (anything_selected) {
- return;
- }
- // Nothing was selected, now go through the loop again and change everything
- second_phase = true;
- } while (!second_phase);
+ }
}
constexpr unsigned kThinkInterval = 200;
___
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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5103. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/539624587. Appveyor build 4884. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4884. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5181. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/544759045. Appveyor build 4961. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4961. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5207. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/548539125. Appveyor build 4986. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-4986. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 5207. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/548539125. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Benedikt Straub has proposed merging
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
Commit message:
Forbid changing economy targets by spectators
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1827696 in widelands: "Allow users to define their own economy default
settings"
https://bugs.launchpad.net/widelands/+bug/1827696
Bug #1831196 in widelands: "Economy profile selection doesn't work"
https://bugs.launchpad.net/widelands/+bug/1831196
Bug #1839948 in widelands: "Missing can_act check in EconomyOptionsWindow"
https://bugs.launchpad.net/widelands/+bug/1839948
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251
Added some missing checks for can_act and got rid of an unused variable.
It is still allowed to save another player´s targets to disk though as this
does not affect the game.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
=== modified file 'src/wui/economy_options_window.cc'
--- src/wui/economy_options_window.cc 2019-08-10 16:38:15 +
+++ src/wui/economy_options_window.cc 2019-08-13 11:25:16 +
@@ -76,6 +76,7 @@
b->sigclicked.connect([this] { change_target(-10); });
buttons->add(b);
b->set_repeating(true);
+ b->set_enabled(can_act);
buttons->add_space(8);
b = new UI::Button(buttons, "decrease_target", 0, 0, 44, 28, UI::ButtonStyle::kWuiSecondary,
g_gr->images().get("images/ui_basic/scrollbar_down.png"),
@@ -83,6 +84,7 @@
b->sigclicked.connect([this] { change_target(-1); });
buttons->add(b);
b->set_repeating(true);
+ b->set_enabled(can_act);
buttons->add_space(24);
b = new UI::Button(buttons, "increase_target", 0, 0, 44, 28, UI::ButtonStyle::kWuiSecondary,
@@ -90,6 +92,7 @@
b->sigclicked.connect([this] { change_target(1); });
buttons->add(b);
b->set_repeating(true);
+ b->set_enabled(can_act);
buttons->add_space(8);
b = new UI::Button(buttons, "increase_target_fast", 0, 0, 44, 28, UI::ButtonStyle::kWuiSecondary,
g_gr->images().get("images/ui_basic/scrollbar_up_fast.png"),
@@ -97,11 +100,16 @@
b->sigclicked.connect([this] { change_target(10); });
buttons->add(b);
b->set_repeating(true);
+ b->set_enabled(can_act);
dropdown_.set_tooltip(_("Profile to apply to the selected items"));
dropdown_box_.set_size(40, 20); // Prevent assert failures
dropdown_box_.add(&dropdown_, UI::Box::Resizing::kFullSize);
- dropdown_.selected.connect([this] { reset_target(); });
+ if (can_act) {
+ dropdown_.selected.connect([this] { reset_target(); });
+ } else {
+ dropdown_.set_enabled(false);
+ }
b = new UI::Button(&dropdown_box_, "save_targets", 0, 0, 34, 34, UI::ButtonStyle::kWuiMenu,
g_gr->images().get("images/wui/menus/save_game.png"),
@@ -239,16 +247,11 @@
serial_(serial),
player_(player),
type_(type),
- can_act_(can_act),
- display_(this, 0, 0, serial_, player_, type_, can_act_),
+ display_(this, 0, 0, serial_, player_, type_, can_act),
economy_options_window_(eco_window) {
add(&display_, UI::Box::Resizing::kFullSize);
display_.set_hgap(AbstractWaresDisplay::calc_hgap(display_.get_extent().w, min_w));
-
- if (!can_act_) {
- return;
- }
}
void EconomyOptionsWindow::EconomyOptionsPanel::set_economy(Widelands::Serial serial) {
=== modified file 'src/wui/economy_options_window.h'
--- src/wui/economy_options_window.h 2019-05-14 18:08:22 +
+++ src/wui/economy_options_window.h 2019-08-13 11:25:16 +
@@ -117,7 +117,6 @@
Widelands::Serial serial_;
Widelands::Player* player_;
Widelands::WareWorker type_;
- bool can_act_;
TargetWaresDisplay display_;
EconomyOptionsWindow* economy_options_window_;
};
___
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/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5320. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/571293137. Appveyor build 5092. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-5092. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251 -- 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Benedikt Straub has proposed merging
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
Commit message:
Move the economy options window to top when clicking Configure Economy
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1827696 in widelands: "Allow users to define their own economy default
settings"
https://bugs.launchpad.net/widelands/+bug/1827696
Bug #1831196 in widelands: "Economy profile selection doesn't work"
https://bugs.launchpad.net/widelands/+bug/1831196
Bug #1839948 in widelands: "Missing can_act check in EconomyOptionsWindow"
https://bugs.launchpad.net/widelands/+bug/1839948
Bug #1842335 in widelands: "Economy settings don't move to foreground on
fieldaction"
https://bugs.launchpad.net/widelands/+bug/1842335
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.
=== modified file 'src/economy/economy.cc'
--- src/economy/economy.cc 2019-06-23 11:41:17 +
+++ src/economy/economy.cc 2019-09-07 16:28:50 +
@@ -52,7 +52,7 @@
}
Economy::Economy(Player& player, Serial init_serial)
- : serial_(init_serial), owner_(player), request_timerid_(0), has_window_(false) {
+ : serial_(init_serial), owner_(player), request_timerid_(0), options_window_(nullptr) {
last_economy_serial_ = std::max(last_economy_serial_, serial_ + 1);
const TribeDescr& tribe = player.tribe();
DescriptionIndex const nr_wares = player.egbase().tribes().nrwares();
@@ -535,7 +535,7 @@
// If the options window for e is open, but not the one for this, the user
// should still have an options window after the merge.
- if (e.has_window() && !has_window()) {
+ if (e.get_options_window() && !get_options_window()) {
Notifications::publish(NoteEconomy{e.serial(), serial_, NoteEconomy::Action::kMerged});
}
=== modified file 'src/economy/economy.h'
--- src/economy/economy.h 2019-02-23 11:00:49 +
+++ src/economy/economy.h 2019-09-07 16:28:50 +
@@ -196,11 +196,11 @@
return worker_target_quantities_[i];
}
- bool has_window() const {
- return has_window_;
+ void* get_options_window() const {
+ return options_window_;
}
- void set_has_window(bool yes) {
- has_window_ = yes;
+ void set_options_window(void* window) {
+ options_window_ = window;
}
const WareList& get_wares() const {
@@ -289,7 +289,11 @@
uint32_t request_timerid_;
static std::unique_ptr soldier_prototype_;
- bool has_window_;
+
+ // This is always an EconomyOptionsWindow* (or nullptr) but I don't want a wui dependency here.
+ // We cannot use UniqueWindow to make sure an economy never has two windows because the serial
+ // may change when merging while the window is open, so we have to keep track of it here.
+ void* options_window_;
// 'list' of unique providers
std::map available_supplies_;
=== modified file 'src/wui/economy_options_window.cc'
--- src/wui/economy_options_window.cc 2019-08-25 14:50:16 +
+++ src/wui/economy_options_window.cc 2019-09-07 16:28:50 +
@@ -124,7 +124,7 @@
main_box_.add_space(8);
main_box_.add(&dropdown_box_, UI::Box::Resizing::kAlign, UI::Align::kCenter);
- economy->set_has_window(true);
+ economy->set_options_window(static_cast(this));
economynotes_subscriber_ = Notifications::subscribe(
[this](const Widelands::NoteEconomy& note) { on_economy_note(note); });
profilenotes_subscriber_ =
@@ -145,7 +145,7 @@
EconomyOptionsWindow::~EconomyOptionsWindow() {
Widelands::Economy* economy = player_->get_economy(serial_);
if (economy != nullptr) {
- economy->set_has_window(false);
+ economy->set_options_window(nullptr);
}
if (save_profile_dialog_) {
save_profile_dialog_->unset_parent();
@@ -162,7 +162,7 @@
die();
return;
}
- economy->set_has_window(true);
+ economy->set_options_window(static_cast(this));
ware_panel_->set_economy(note.new_economy);
worker_panel_->set_economy(note.new_economy);
move_to_top();
=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc 2019-09-05 19:57:55 +
+++ src/wui/fieldaction.cc 2019-09-07 16:28:50 +
@@ -632,7 +632,9 @@
void FieldActionWindow::act_configure_economy() {
if (upcast(const Widelands::Flag, flag, node_.field->get_immovable())) {
Widelands::Economy* economy = flag->get_economy();
- if (!economy->has_window()) {
+ if (economy->get_options_window()) {
+ static_cast(economy->get_options_window())->move_to_top();
+ } else {
bool can_act =
dynamic_cast(ibase()).can_act(economy->owner().player_number());
new EconomyOptionsWindow(dynamic_cast(&ibase()), economy, can_act);
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
Mor
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Continuous integration builds have changed state: Travis build 5411. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/582115732. Appveyor build 5181. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_economy_target_profiles-5181. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
2 Comments. Will do some testing.
Diff comments:
> === added directory 'data/economy_profiles'
> === added file 'data/economy_profiles/atlanteans'
> --- data/economy_profiles/atlanteans 1970-01-01 00:00:00 +
> +++ data/economy_profiles/atlanteans 2019-05-05 05:49:29 +
> @@ -0,0 +1,85 @@
> +# Automatically created by Widelands bzr9094[economy-target-profiles] (Debug)
> +
> +[Efficiency]
These names need to be translatable. I also think that the files should live
under "tribes".
> +blackroot_flour="1"
> +atlanteans_bread="20"
> +bread_paddle="0"
> +buckets="0"
> +coal="5"
> +cornmeal="3"
> +diamond="3"
> +fire_tongs="1"
> +fishing_net="2"
> +gold="1"
> +gold_ore="1"
> +gold_thread="0"
> +granite="10"
> +hammer="0"
> +hook_pole="0"
> +hunting_bow="1"
> +iron="5"
> +iron_ore="1"
> +milking_tongs="0"
> +pick="1"
> +planks="1"
> +quartz="3"
> +saw="0"
> +scythe="0"
> +shield_advanced="0"
> +shield_steel="0"
> +shovel="0"
> +smoked_fish="5"
> +smoked_meat="3"
> +spidercloth="5"
> +spider_silk="5"
> +tabard="1"
> +tabard_golden="0"
> +trident_double="0"
> +trident_heavy_double="0"
> +trident_light="1"
> +trident_long="0"
> +trident_steel="0"
> +atlanteans_horse="1"
> +atlanteans_soldier="10"
> +
> +[Stockpile]
> +blackroot_flour="20"
> +atlanteans_bread="30"
> +bread_paddle="1"
> +buckets="2"
> +coal="25"
> +cornmeal="20"
> +diamond="10"
> +fire_tongs="1"
> +fishing_net="2"
> +gold="20"
> +gold_ore="15"
> +gold_thread="5"
> +granite="30"
> +hammer="2"
> +hook_pole="1"
> +hunting_bow="1"
> +iron="25"
> +iron_ore="20"
> +milking_tongs="1"
> +pick="3"
> +planks="40"
> +quartz="10"
> +saw="2"
> +scythe="1"
> +shield_advanced="1"
> +shield_steel="1"
> +shovel="2"
> +smoked_fish="40"
> +smoked_meat="25"
> +spidercloth="20"
> +spider_silk="15"
> +tabard="30"
> +tabard_golden="1"
> +trident_double="1"
> +trident_heavy_double="1"
> +trident_light="30"
> +trident_long="1"
> +trident_steel="1"
> +atlanteans_horse="20"
> +atlanteans_soldier="20"
>
> === modified file 'src/wui/economy_options_window.cc'
> --- src/wui/economy_options_window.cc 2019-02-23 11:00:49 +
> +++ src/wui/economy_options_window.cc 2019-05-05 05:49:29 +
> @@ -186,35 +247,264 @@
> 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(tq.permanent)) {
> - if (is_wares) {
> - game.send_player_command(*new
> Widelands::CmdSetWareTargetQuantity(
> -game.get_gametime(),
> player_->player_number(), serial_, index,
> -tq.permanent + amount));
> - } else {
> - game.send_player_command(*new
> Widelands::CmdSetWorkerTargetQuantity(
> -game.get_gametime(),
> player_->player_number(), serial_, index,
> -tq.permanent + amount));
> + // Don't allow negative new amount
> + const int old_amount = static_cast(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(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();
> + for (const Widelands::DescriptionIndex& index : items) {
> + if (display_.ware_selected(index)) {
> + if (is_wares) {
> + game.send_player_command(*new
> Widelands::CmdSetWareTargetQuantity(
> + game.get_gametime(),
> player_->player_number(), serial_, index, settings.wa
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Addressed the reviews: The panels now set hgaps to fill the entire available space, and the profiles are stored in tribes/economy_profiles. I added translation markup to the predefined profiles. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
New code LGTM. I am wondering whether we want to save this as Lua tables? I already have the code finished in the spritesheets branch and could pull it out into a separate branch, since spritesheets aren't ready yet. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Is there a reason why you prefer LuaTables over profile? Personally I find Profile much easier to use for configs that the user has no reason to manually edit. Regarding the suggestion in the bug report – A spinbox would make sense, but what value should it display when several wares with different settings are selected? -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
All the tribe's configuration is in LuaTables, so I guess mainly for consistency - I don't feel strongly about this though. Good point about the value in the spinbox - I still think we should have the possibility of having steps of 10 though. Maybe fake it with 4 buttons and make them look like the spinbox buttons? -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366952 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Implemented the comments from the bug report -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/366987 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Review: Approve testing Really nice, thanks :-) It's good that changing a value will show no text in the editbox, so one can immediately see that there is no profile saved for the chosen value(s). After playing around with the tabs and switching between wares/workers, i found that loading a profile will only apply to the items in the actual tab (wares/workers). Is this correct? We may need an entry in the help for this window. But this can be done in a separate branch. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Yes, all changes are only ever applied to the active tab. The inactive tab will never change. A helpful explanation should be added to the economy tutorial IMHO. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Review: Needs Fixing One major problem. Pressing the save button pauses the game without actually showing, that the game is paused. That might tolerable in single player, but in multiplayer it pauses the game for all participants. This should not happen. Also get this warning during compiling: src/wui/economy_options_window.h:40:2: warning: '~EconomyOptionsWindow' overrides a destructor but is not marked 'override' -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Figured it is worse. The game stalls, it is not paused. The game stalls as long as the save window is open, when closing the windows it fast forwards the time. e.g. Opening the window at 1.30 and leaving it open for 1 minute, means after closing it, the time will fast-forward to 2.30. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
I made the save dialog modal, my mistake. This was supposed to ensure that only one save window is open at the time and the parent window won´t disappear while it´s open. Now I ensure this another way; the game progresses normally and input isn´t blocked. The last revision now has a strange behaviour though that the save window doesn´t close by itself if you destroy the last flag of the economy, which I don´t understand yet… -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
If you want to ensure that there is onyl 1 save dialog, you can make it a UniqueWindow. The economy options should control the lifetime of the save window. This is a bit tricky when you merge 2 economies. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Thanks for the suggestion, I´ll use UniqueWindow then :) Currently, I am closing the save dialog from the EconomyOptionsWindow destructor. In both cases – closing by right-click and closing after economy destruction – die() is called on the EconomyOptionsWindow, I don´t understand why it makes a difference… -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Have a look at EconomyOptionsWindow::on_economy_note - maybe you will need to do some changes there too -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
I decided against UniqueWindow after all, it seems a bit overkill. All the remaining problems should be fixed now. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Very good so far, two nits though. Stockpile and Efficiency Profiles should be changeable but not deletable. Also, when deleting the active profile it should switch back to default. Otherwise, the deleted one is still shown as active and the drop-down list has an empty entry. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
BTW while we're on it. Should we consider presetting all Profiles with sane values? -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
> Stockpile and Efficiency Profiles should be changeable but not deletable Okay, I´ll add a "undeletable" tag to these profiles > when deleting the active profile it should switch back to default. Otherwise, > the deleted one is still shown as active I thought we had agreed to not show "Default" if the settings are not equal to the default values? Sounds like the bug is that deleted profiles need to be removed from the dropdown and the dropdown set to "" then. > Should we consider presetting all Profiles with sane values? I preset "Stockpile" with my own personal preferences, and "Efficiency" is a copy of the old proposal. There is room for discussion of better presets… -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
The predefined profiles are now undeletable. But I can´t reproduce this: > Otherwise, the deleted one is still shown as active and the drop-down list > has an empty entry. It works fine for me: When deleting the active profile, the dropdown immediately switches to "". -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Have a look at the picture. I deleted the profile test. https://fosuta.org/pics/empty_profile.png -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
I still can´t reproduce, it works fine for me: https://launchpadlibrarian.net/423268219/wl-economy-profiles.png (screenshot taken directly after I deleted the profile "Zero") -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Maybe I just failed to explain it. Have a look at the video. https://fosuta.org/pics/profile.mov -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Either I´m blind or I don´t see what I am doing wrong… https://bugs.launchpad.net/widelands/+bug/1827696/+attachment/5263113/+files/vokoscreen-2019-05-11_16-27-25.mkv -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
In your video, I don't see you accessing the dropdown in the economy window after the deletion. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Updated video: https://bugs.launchpad.net/widelands/+bug/1827696/+attachment/5263625/+files/vokoscreen-2019-05-14_14-29-18.mkv I still get a different result by the same steps and have no idea why :( -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Diff comments:
>
> === modified file 'src/wui/economy_options_window.cc'
> --- src/wui/economy_options_window.cc 2019-02-23 11:00:49 +
> +++ src/wui/economy_options_window.cc 2019-05-11 13:37:58 +
> @@ -186,35 +281,399 @@
> 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(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(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(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;
> + }
> +}
> +
> +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.en
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Hmm... it seems like the dropdown´s entries and selection are not updated correctly for some reason which is beyond my understanding. The "Not Selected" message is only the consequence of an upstream failure which I suspect in update_profiles(). I have rewritten it, can you try if the problem still exists? -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Still shows "Not Selected" instead of an empty entry. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
I am at a loss. I have now pushed a revision that adds lots of logging output. When I perform these steps: – Open the options window from a flag – Change some values – Click the save button – Save the profile as "test" – (now the dropdown shows "test") – close the options window – re-open it (the dropdown shows "test") – Click the save button – select profile "test" – click delete – (the dropdown now shows "") – close the save dialog – open the dropdown (still showing "") – close the options window – re-open it (the dropdown shows "") I attached the log output I received to the bug. Please post the log output you receive so we can compare… https://launchpadlibrarian.net/423973416/eco-options-profiles-log -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
crash: NOCOM: EconomyOptionsWindow::update_profiles_select(Default) Planning to select »Default« Did not select it because it is already selected NOCOM: EconomyOptionsWindow::update_profiles, applicable »« Desired and actual state of the empty »« profile don´t match NOCOM: EconomyOptionsWindow::update_profiles_needed() Added Default Added Efficiency Added Stockpile Added the empty profile NOCOM: EconomyOptionsWindow::update_profiles_select() Planning to select »Project-Id-Version: Widelands Report-Msgid-Bugs-To: https://wl.widelands.org/wiki/ReportingBugs/ POT-Creation-Date: 2019-04-22 05:17+ PO-Revision-Date: 2019-03-03 08:20+ Last-Translator: GunChleoc Language-Team: English (United States) (http://www.transifex.com/widelands/widelands/language/en_US/) Language: en_US MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plural-Forms: nplurals=2; plural=(n != 1); « Selected it! Assertion failed: (dropdown_.has_selection()), function update_profiles_select, file /Users/toni/Launchpad/widelands-repo/working_tree/src/wui/economy_options_window.cc, line 427. Abort trap: 6 -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Ah yes, it seems like the translation for the empty string is not as accurate
as I expected it...
The dropdown contains translated entries, so I tell it to select the translated
name of the applicable profile. I don´t know what the cause of this weird
translation is, perhaps your i18n library dislikes _("") expressions? Please
retry with the last revision…
--
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
r9110 fixed it. Thanks allot. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Yep, translating the empty string is a bad idea.
Dropdowns, table and listselect entries have 2 data elements - one is the
actual data (Entry value), the other one is a translatable label (const
std::string& name). You need to compare the actual data, not the translatable
label.
void add(const std::string& name,
Entry value,
const Image* pic = nullptr,
const bool select_this = false,
const std::string& tooltip_text = std::string()) {
entry_cache_.push_back(std::unique_ptr(new
Entry(value)));
BaseDropdown::add(name, size(), pic, select_this, tooltip_text);
}
--
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Review: Approve I think I forgot to approve this :) -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
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 +
> +++ src/wui/economy_options_window.cc 2019-05-16 17:28:26 +
> @@ -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(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(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(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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
I have just tested the relayouting when fullscreen switching. It works well for warehouses and the stock statistics. In the ware statistics, the bottom slider and text are cut off. In the economy window, we get wide gaps between the columns and some extra space on the left and right of the button rows. -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Also: ==6158==ERROR: AddressSanitizer: heap-use-after-free on address 0x6183eda0 at pc 0x55fbbb8f1299 bp 0x7ffc346f2020 sp 0x7ffc346f2010 READ of size 8 at 0x6183eda0 thread T0 #0 0x55fbbb8f1298 in std::_Deque_iterator::_Deque_iterator(std::_Deque_iterator const&) /usr/include/c++/7/bits/stl_deque.h:152 #1 0x55fbbb8f1232 in std::deque >::begin() /usr/include/c++/7/bits/stl_deque.h:1167 #2 0x55fbbb8ea1a3 in UI::BaseListselect::clear() ../src/ui_basic/listselect.cc:95 #3 0x55fbbb8cf6fc in UI::BaseDropdown::~BaseDropdown() ../src/ui_basic/dropdown.cc:151 #4 0x55fbbb52cb26 in UI::Dropdown, std::allocator > >::~Dropdown() ../src/ui_basic/dropdown.h:224 #5 0x55fbbc30e0ec in EconomyOptionsWindow::~EconomyOptionsWindow() ../src/wui/economy_options_window.cc:126 #6 0x55fbbc30e17f in EconomyOptionsWindow::~EconomyOptionsWindow() ../src/wui/economy_options_window.cc:134 #7 0x55fbbb910d1d in UI::Panel::free_children() ../src/ui_basic/panel.cc:128 #8 0x55fbbb9108ce in UI::Panel::~Panel() ../src/ui_basic/panel.cc:100 #9 0x55fbbba82826 in InteractiveBase::~InteractiveBase() ../src/wui/interactive_base.cc:189 #10 0x55fbbbae54c1 in InteractiveGameBase::~InteractiveGameBase() ../src/wui/interactive_gamebase.h:58 #11 0x55fbbbaea715 in InteractivePlayer::~InteractivePlayer() ../src/wui/interactive_player.h:38 #12 0x55fbbbaea73d in InteractivePlayer::~InteractivePlayer() ../src/wui/interactive_player.h:38 #13 0x55fbbb6a9884 in std::default_delete::operator()(InteractiveBase*) const (economy-target-profiles/widelands+0x1150884) #14 0x55fbbb6a817e in std::unique_ptr >::reset(InteractiveBase*) /usr/include/c++/7/bits/unique_ptr.h:376 #15 0x55fbbb69d8f8 in Widelands::EditorGameBase::set_ibase(InteractiveBase*) ../src/logic/editor_game_base.cc:222 #16 0x55fbbb6b7242 in Widelands::Game::run(UI::ProgressWindow*, Widelands::Game::StartGameType, std::__cxx11::basic_string, std::allocator > const&, bool, std::__cxx11::basic_string, std::allocator > const&) ../src/logic/game.cc:576 #17 0x55fbbb39b1a0 in WLApplication::new_game() ../src/wlapplication.cc:1350 #18 0x55fbbb399280 in WLApplication::mainmenu_singleplayer() ../src/wlapplication.cc:1204 #19 0x55fbbb3983cb in WLApplication::mainmenu() ../src/wlapplication.cc:1110 #20 0x55fbbb38f3b1 in WLApplication::run() ../src/wlapplication.cc:466 #21 0x55fbbb38b14e in main ../src/main.cc:44 #22 0x7f3374bb0b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) #23 0x55fbbb38afc9 in _start (economy-target-profiles/widelands+0xe31fc9) -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Implemented or replied to the diff comments.
The ASan error is in trunk and can happen everywhere there are dropdowns. For
example, when I´m thrown back to the main menu by an error from a fsmenu screen
that has one I already got this crash. It can appear whenever destroying a
window that contains a dropdown on a lower level. Happens very rarely though,
so I did not report it…
> In the ware statistics, the bottom slider and text are cut off.
> In the economy window, we get wide gaps between the columns and some extra
> space on the left and right of the button rows.
They probably do not layout properly yet… will look into it
Diff comments:
>
> === modified file 'src/wui/economy_options_window.cc'
> --- src/wui/economy_options_window.cc 2019-02-23 11:00:49 +
> +++ src/wui/economy_options_window.cc 2019-05-16 17:28:26 +
> @@ -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(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(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(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;
> + }
> +}
> +
> +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()) {
> +
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Layouting fixed. Ware statistics now sets a hgap as well -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
You can now get rid of the second_phase variable entirely. Just check for
!anything_selected at the end of the loop, then you can also get rid of
if (anything_selected) {
return;
}
Confirmed that the layouting is fixed. The ware statistics doesn't fit yet at
800x600 resolution, can you give it 1 less row for that?
Also added 1 comment.
Diff comments:
>
> === modified file 'src/wui/economy_options_window.cc'
> --- src/wui/economy_options_window.cc 2019-02-23 11:00:49 +
> +++ src/wui/economy_options_window.cc 2019-05-16 17:28:26 +
> @@ -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(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(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(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;
> + }
> +}
> +
> +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) {
> + m
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Ware stats menu fixed. It is already too large in trunk, but I made it shorter
now.
We still need knowledge of the phase we´re in during the loop so we can know
whether to change the target for all wares or only for the selected. Since
anything_selected may change during the loop, we need two variables here.
The Dropdown implementation specifies:
template class Dropdown : public BaseDropdown {
void add(const std::string& name,
Entry value, …);
void select(const Entry& entry);
const Entry& get_selected() const;
…
}
(With Entry = std::string here.) Since Entry is the descname (right?), this
looks as if the selection is handled by descname rather than internal name…
--
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
> Since Entry is the descname (right?), Wrong. The dropdown also comes with documentation: /// Add an element to the list /// \param name the display name of the entry /// \param valuethe index of the entry /// \param pic an image to illustrate the entry. Can be nullptr for textual dropdowns. /// \param select_this whether this element should be selected /// \param tooltip_text a tooltip for this entry /// /// Text conventions: Title Case for the 'name', Sentence case for the 'tooltip_text' void add(const std::string& name, uint32_t value, const Image* pic = nullptr, const bool select_this = false, const std::string& tooltip_text = std::string()); So, the translatable element is: /// \param name the display name of the entry -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
That was the BaseDropdown. The Dropdown template looks like this:
template class Dropdown : public BaseDropdown {
...
/// Add an element to the list
/// \param name the display name of the entry
/// \param valuethe value for the entry
/// \param pic an image to illustrate the entry. Can be
nullptr in textual dropdowns
/// only.
/// \param select_this whether this element should be selected
/// \param tooltip_text a tooltip for this entry
void add(const std::string& name,
Entry value,
const Image* pic = nullptr,
const bool select_this = false,
const std::string& tooltip_text = std::string()) {
"Entry value" can be any data type you like, "const std::string& name" is the
translatable name.
--
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Okay, my mistake :) Double-checked my dropdown usage, should all be fine then… -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Review: Approve I have merged trunk and pushed some changes. Feel free to merge this branch if you agree with them :) -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Thanks everyone for the reviews, and thanks for the fixes :) @bunnybot merge -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Inputqueues again @bunnybot merge force -- 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Selecting the profile is working now, but it shows up as "-" in the dropdown afterwards, although all wares have been adjusted, including the selected ones. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
I cannot reproduce that... When nothing or everything is selected, all wares get changed and the dropdown shows the chosen profile. When some wares are selected, only those are changed and "-" appears as expected. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Maybe I did not describe it well enough - I have recorded a video: https://bugs.launchpad.net/widelands/+bug/1831196/comments/1 I also got a heap-use-after-free, but that's probably already fixed via one of my branches https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
But this is the correct behaviour :) The dropdown always shows the profile that applies to the entire tab. IMHO this is more intuitive than making the dropdown label dependent only on the selected items… -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
When I load a profile, I expect that profile to be applied and overwrite everything, no matter if I have currently selected something or not. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
When you want to apply a profile to everything, you can select either nothing or everything. The dropdown tooltip says "Profile to apply to the selected items". All other buttons affect only the selected items, so it´s consistent that the dropdown should affect only the selection as well. Changing all wares when only some are selected would be an annoying and unexpected behaviour IMHO. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Review: Approve Ah, now I get it. I' still not sure that this is what it should do. but the behavior is as described now, so let's merge this branch. Thanks for fixing! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
three inputqueue fails in one build :P @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/368181 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
LGTM :) Do we want to disable the save button too? -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
It cannot affect the game, so this isn´t necessary. But perhaps one might want to save some clever economy settings of another player for future use in own games, so why not allow it? -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles 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/economy-target-profiles into lp:widelands
Review: Approve OK, let's have it then :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/371251 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Review: Approve I can't think of a cleaner solution either at this point, so code LGTM :) -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
This does not work for minimized windows, they still stay minimzed. -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Fixed as well now -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands
Fix confirmed :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447 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
