[Widelands-dev] [Merge] lp:~widelands-dev/widelands/failed_skipped_10s into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/failed_skipped_10s into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
the inputqueues again @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 5095. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/538979803. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
The 10s increase in the test fixed it. But we only need this as long as the unify_sleep_time isn't merged into branch. We can revert these changes there because sleeps aren't executed before a possible fail anymore. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
This will run the whole test suite - he will only want to run the casern test when fixing. https://wl.widelands.org/wiki/RegressionTests/ -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Running regression tests locally: ./regression_test.py -b ./widelands -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
This branch fails: test/maps/plain.wmf/scripting/test_casern.lua How do I run the test locally? -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Okay, I'm going to set a new branch up that contains the changes to the lua files. Base will be this branch. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Did some thinking :D > It is not necessary for buildings with leading sleep times of less then 10 > secs as these would been masked by the code now. > Furthermore it is not necessary for buildings with only one output as the > effect of wasted time if undersupplied would been averaged over time. But now in all programs that have a sleep before consume and/or skipped it will always be 10s plus sleep. E.g. the barbarian's baker has a sleep of 30 seconds before consume. Every fail would now result in 10s + 30s before for the next execution. Even if the time would be 1 second it would now always be 10s + 1s, the first sleep never gets masked but added. That's why I would like to have skipped the first, consume the second and sleep the third call in every program. Generally, having a sleep before consume would be a time penalty, regardless of the length. Having every program like this would also have the benefit, that the program flow itself would be unified. A program skipped or failed means, it starts 10 second later before it would fail or skip again. Otherwise, it would be 10s plus whatever sleep time we have before consume/skip. The sleep times then would be part of a completed cycle and not a failed or skipped. Does this sound reasonable? > That was the basis for my suggestion above. > > I'd like to keep the sleep times around at least of one third to one half of > the total time as it looks more natural if a building isn't busy all the time. > Having only working animations would make the screen a bit unquiet I am > afraid. Sounds sensible. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Unless you find others for which the criteria apply, but we could change that anytime in a new branch as well. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Okay, so basically only the buildings you listed above? -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Barbarians mines are not affected I think. Basically the change of order is definitely necessary for all buildings with multiple outputs and different inputs needed for the outputs (best example atlantean or frisian toolsmithy). It can be adopted (which I prefer) for buildings with multiple outputs and high sleep times (e.g. crystal mine). It is not necessary for buildings with leading sleep times of less then 10 secs as these would been masked by the code now. Furthermore it is not necessary for buildings with only one output as the effect of wasted time if undersupplied would been averaged over time. That was the basis for my suggestion above. I'd like to keep the sleep times around at least of one third to one half of the total time as it looks more natural if a building isn't busy all the time. Having only working animations would make the screen a bit unquiet I am afraid. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
It basically affects all buildings. Most buildings have a sleep before skipped and/or consume. So we need to move them as well. But the question is, do we want to move them or add them to the animation cycle? Or at least move some seconds to the animation cycle? -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Hmm, there are many more. The question is. Shall we move the sleep time just behind consume, or remove sleep and add the time to the animation time, were applicable? -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Found some more. Barbarian's mines. Going to add the changes to this branch, since it won't be merged because the casern_test failed. I guess we need to adapt this test as well... -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Oops found some other buildings which need to be adopted to this scheme then atlanteans crystal mine empire marble mine, marble mine deep frisians tailors shop, aqua farm, recycling center and eventually reindeer farm -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Review: Approve review looks good to me. However there might be some complaints about having the sound as some people (we all know) do find this annoying. However I like it. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Review: Approve LGTM :) I removed the gettext function from the pure placeholder, because it does not need translation. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Continuous integration builds have changed state: Travis build 5079. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/538470132. Appveyor build 4859. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_failed_skipped_10s-4859. -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/failed_skipped_10s into lp:widelands has been updated. Commit message changed to: add failed programs to the skipped stack don't show % for trend move sleep behind consume in metal_workshop & toolsmithy added sound to helmsmithy For more details, see: https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/failed_skipped_10s into lp:widelands
Toni Förster has proposed merging
lp:~widelands-dev/widelands/failed_skipped_10s into lp:widelands.
Commit message:
add failed programs to the skipped stack
don't show % for trend
Requested reviews:
hessenfarmer (stephan-lutz)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014
--
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/failed_skipped_10s.
=== modified file 'src/logic/map_objects/tribes/productionsite.cc'
--- src/logic/map_objects/tribes/productionsite.cc 2019-05-26 17:21:15 +
+++ src/logic/map_objects/tribes/productionsite.cc 2019-05-28 21:06:25 +
@@ -428,7 +428,7 @@
}
const std::string trend_str =
- g_gr->styles().color_tag((boost::format(_("%i%%")) % trend).str(), color);
+ g_gr->styles().color_tag((boost::format(_("%i")) % trend).str(), color);
// TODO(GunChleoc): We might need to reverse the order here for RTL languages
statistics_string_on_changed_statistics_ =
@@ -924,8 +924,8 @@
program_timer_ = true;
uint32_t tdelta = 10;
- SkippedPrograms::const_iterator i = skipped_programs_.find(program_name);
- if (i != skipped_programs_.end()) {
+ FailedSkippedPrograms::const_iterator i = failed_skipped_programs_.find(program_name);
+ if (i != failed_skipped_programs_.end()) {
uint32_t const gametime = game.get_gametime();
uint32_t const earliest_allowed_start_time = i->second + 1;
if (gametime + tdelta < earliest_allowed_start_time)
@@ -956,13 +956,14 @@
switch (result) {
case ProgramResult::kFailed:
+ failed_skipped_programs_[program_name] = game.get_gametime();
statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
statistics_.push_back(false);
calc_statistics();
update_crude_statistics(current_duration, false);
break;
case ProgramResult::kCompleted:
- skipped_programs_.erase(program_name);
+ failed_skipped_programs_.erase(program_name);
statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
statistics_.push_back(true);
train_workers(game);
@@ -970,11 +971,11 @@
calc_statistics();
break;
case ProgramResult::kSkipped:
- skipped_programs_[program_name] = game.get_gametime();
+ failed_skipped_programs_[program_name] = game.get_gametime();
update_crude_statistics(current_duration, false);
break;
case ProgramResult::kNone:
- skipped_programs_.erase(program_name);
+ failed_skipped_programs_.erase(program_name);
break;
}
=== modified file 'src/logic/map_objects/tribes/productionsite.h'
--- src/logic/map_objects/tribes/productionsite.h 2019-05-22 15:46:08 +
+++ src/logic/map_objects/tribes/productionsite.h 2019-05-28 21:06:25 +
@@ -307,13 +307,13 @@
int32_t fetchfromflag_; ///< Number of wares to fetch from flag
- /// If a program has ended with the result Skipped, that program may not
+ /// If a program has ended with the result Failed or Skipped, that program may not
/// start again until a certain time has passed. This is a map from program
- /// name to game time. When a program ends with the result Skipped, its name
+ /// name to game time. When a program ends with the result Failed or Skipped, its name
/// is added to this map, with the current game time. (When the program ends
/// with any other result, its name is removed from the map.)
- using SkippedPrograms = std::map;
- SkippedPrograms skipped_programs_;
+ using FailedSkippedPrograms = std::map;
+ FailedSkippedPrograms failed_skipped_programs_;
using Stack = std::vector;
Stack stack_; ///< program stack
=== modified file 'src/map_io/map_buildingdata_packet.cc'
--- src/map_io/map_buildingdata_packet.cc 2019-05-18 11:58:43 +
+++ src/map_io/map_buildingdata_packet.cc 2019-05-28 21:06:25 +
@@ -646,13 +646,13 @@
if (pr_descr.programs().count(program_name)) {
uint32_t const skip_time = fr.unsigned_32();
if (gametime < skip_time)
- throw GameDataError("program %s was skipped at time %u, but time is only "
+ throw GameDataError("program %s failed/was skipped at time %u, but time is only "
"%u",
program_name, skip_time, gametime);
- productionsite.skipped_programs_[program_name] = skip_time;
+ productionsite.failed_skipped_programs_[program_name] = skip_time;
} else {
fr.unsigned_32(); // eat skip time
- log("WARNING: productionsite has skipped program \"%s\", which "
+ log("WARNING: productionsite has failed/skipped program \"%s\", which "
"does not exist\n",
program_name);
}
@@ -1129,10 +1129,10 @@
fw.signed_32(productionsite.fetchfromflag_);
// skipped programs
- assert(productionsite.skipped_programs_.size() <= std::numeric_limits::max());
- fw.unsigned_8(productionsite.skipped_programs_.size());
+ assert(productionsite.failed_skipped_programs_.size() <= std::numeric_limits::max());
+
