Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/ai_productionsites_statistics into lp:widelands
I think skipped should be ignored in the UI, but the AI should be able to know whether a program has been skipped a lot recently, because that means that a new building is not needed. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
transient failure on travis @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
Have done some playtesting 4 AI players on full moon. found no obvious anomalies. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
For AI this is the best I believe... -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
ok you are right especially for the AI cause the AI might want to build another idle building then. However we should perhaps have different rating for failed and skipped. If you like we could deliver only half of the duration time to the update fuctionality in case we skipped so it will fall more slowly. If you don't agree feel free to merge. I believe you tested this as I didn't -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
Low utilization can mean one of two shortage of inputs or that the output is not needed. So by my opinion, skipped programs should be included in statistics... Because utilization close to 100 while the building being idle most of time can confuse players in believing that new building is needed... -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
OK, comments implemented. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
by the way I now see another option. - this change plus exposing the new crude Statistics to the UI ;-) -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_productionsites_statistics. ___ 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/ai_productionsites_statistics into lp:widelands
Review: Approve
Got the concept. I think we should have this after you perhaps might fix the
small nits from the below diff comments
Diff comments:
> === modified file 'src/logic/map_objects/tribes/productionsite.cc'
> --- src/logic/map_objects/tribes/productionsite.cc2019-05-18 11:58:43
> +
> +++ src/logic/map_objects/tribes/productionsite.cc2019-05-19 20:55:45
> +
> @@ -1032,4 +1037,17 @@
>
> default_anim_ = anim;
> }
> +
> +void ProductionSite::update_crude_statistics(uint32_t duration, const bool
> produced) {
> + static const uint32_t duration_cap = 90 * 1000; //This is highest
> allowed program duration
yes I think to cover for weird things 3 minutes might be ok.
> + // just for case something went very wrong...
> + static const uint32_t entire_duration = 5 * 60 *1000;
> + if (duration > duration_cap) {
> + duration = duration_cap;
> + };
> + const uint32_t old_duration = entire_duration - duration;
I would name this past_duration as this is what it is. I first thought this
should be the duration of the previous cycle.
> + crude_percent_ = (crude_percent_ * old_duration + produced * duration *
> 1) / entire_duration;
> + assert(crude_percent_ <= 1); //be sure we do not go above 100 %
> + }
> +
> } // namespace Widelands
--
https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ai_productionsites_statistics.
___
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/ai_productionsites_statistics into lp:widelands
Hm I made a excel now. reported a bug to upload it and linked it to this branch. It seems your formula is delivering weird results. I don't know why or if I made a mistake. I think I don't have understood this in total but at least the old crude_statistics wasn't that far from UI but more volatile. Please have a look: if you don't like excel I can try to export in odt as well. We could try to have a attempts in last minutes solution though. That would be probably the best -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
I completely understand current code. If it was "attempts in last x minutes" - it would be fine for me. But 'last 20 attempts' - this I understand from coding perspective - but dont like it. 'official statistics' to me is like actual_statistics^2. 100% and 0% are right, but in between it is undervalued... But I am willing to accept removal of this statistics and modify AI code and retrain it if there is consensus about it.. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
UI stats are calcualted over the last 20 real attempts. Skipped does not count as attempt. It is build like a shifting register with 20 positions containing true or false. it is falling faster for most buildings due to the fail time is shorter than working time. if fail time is really short it falls down very fast in result (atlantean toolsmithy e.g) interesting part is we calculate 2 values: the percentage (number of true / 20) and the trend (number of true in last 10 attempts / 10). >From my perspective the only way to control a buildings productivity is the >supply of wares. In fact we have the time of one work cycle to supply the >wares for the next cycle. To cope with economy shortfalls we have a buffer of >normally around 1 cycle in the input queues. So for me the attempt based >statistics is a good indication whether we supply a building properly. An >additional building is only reasonable if the current buildings are supplied >properly. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
I see the opinions differ too much. I see these options: - no change - this change - no change, but expose official statistics to the AI's genetic algorithm and retrain - this change, but expose official statistics to the AI's genetic algorithm and retrain - remove this crude_statistics, and expose official statistics to AI and retrain -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
I dont say that official statistics is completely wrong. But rather it looks at the thing from other angle... Also you said it will rise up slowly, but will fall down much faster, correct? -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
skips take 10 seconds if second time skipped this is hardcoded in productionsite.cc fails do not need any time (except some milliseconds) for e.g. atlantean toolsmithy If we use UI stats we will not go to 100% suddenly cause we take the last 20 task into account. so if we have currently failed every third attempt and now we manage constantly to be successful we will slightly raise productivity to 100% until we have finished 20 tasks without failing one. As aid there was a fix for the stats of productionsites last year. now we have somewhat reliable task based statistics in the UI I believe. So my suggestion is to use the Stats for the players for the AI as well. I think the resolution of 5% is good enough to base Ai decisions on. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
But we are still talking only about info for AI not players If we cannot produce something due to missing inputs and suddenly we will have those inputs, the productionsite will have to reduce production of the ware that is being produced 90% of time now. Might not be bad, but we will be suddenly on 100 % and new building might be needed definitely If I remember properly skips and failures took about 5 seconds so are not that short... -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
do we need another building also is task based from my perspective not time based. if we fail every 3rd attempt we don't need another building as we can't provide enough inputs for the current one. Fails are rather short now for a lot of buildings, as we have updated the working programs of them to fail immediately if not enough wares are present. means we are not sleeping useless time to just fail, but continue with other programs, or recheck if we can do it now. So I really would vote for a task based utilization scheme rather than having a time based. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
The question to answer here is - do we need another building of the type. If time utilization is 35 % - we dont need. If 97% - we definitely need. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
Correct. I think the question is what is the relevant Information. I believe the information necessary for management is how good is a building satisfying its intended task. For me in this case it is more relevant the relation between success and failure rather than the relation active to not active. in your example with 2 successes and 1 fail the activity would be 97% while still failing every third attempt to produce something. This might even be worse if it has more than one outputs . It might be that it fails completely to produce one ware_type while having stats of 97%. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
But if failure (whatever) takes 5 seconds and success 90, then after 2 failures and 1 success, UI statistics will be 35%, but the productionsite was actually working 90% of time -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
Just had a second look. Now we do count a skipped program as a fail which hasn't be the case in the previous version. >From what I can see in the old version the algorithm was: if failed --> scale the old stats to 80% (resulting in asymptotically falling values if fail everytime) if skipped --> scale old percentage to 98% (which is basically stable for a lot of time) if success --> scale to 80 % of previous and add 20 % (resulting in asymptotically rising values) the UI percent is not really a percentage of successful time but a percentage of successful attempts. Which mean it does not reflect different length of programs. The UI also provides a trending. So for me it might be worth a try to use the UI percentage, cause it only updates if needed (failed or complete) and we removed the effect of skipping on stats with one of the balancing branches thanks to stonerl inventing the state no stats. So for me the UI stats are pretty reliable now in terms of successful attempts. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
AI need % of building utilization. And I think showed % (in UI) is not reliable enough... By old approach it just presumed that successful program took 5x more than unsuccessful program - this is example. With exact timing the number would be more exact. Also we have 'skipped' and 'failed' programs that takes various time I think. This change guarantee that whatever changes to productions programs this code will not need an update... -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
I guess the question here is - what are you using this for? Do you need the exact duration of a specific run of the production program, or do you need to know how long it usually takes? -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
@hessenfarmer - see my responses
Diff comments:
> === modified file 'src/logic/map_objects/tribes/productionsite.cc'
> --- src/logic/map_objects/tribes/productionsite.cc2019-05-18 11:58:43
> +
> +++ src/logic/map_objects/tribes/productionsite.cc2019-05-19 20:55:45
> +
> @@ -954,24 +955,28 @@
> top_state().phase = result;
> }
>
> + const uint32_t current_duration = game.get_gametime() -
> last_program_end_time;
> + assert(game.get_gametime() >= last_program_end_time);
yes, it would be more logical, but at the end this does not matter
> + last_program_end_time = game.get_gametime();
> +
> switch (result) {
> case ProgramResult::kFailed:
> statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
> statistics_.push_back(false);
> calc_statistics();
> - crude_percent_ = crude_percent_ * 8 / 10;
> + update_crude_statistics(current_duration, false);
> break;
> case ProgramResult::kCompleted:
> skipped_programs_.erase(program_name);
> statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
> statistics_.push_back(true);
> train_workers(game);
> - crude_percent_ = crude_percent_ * 8 / 10 + 100 * 2 / 10;
> + update_crude_statistics(current_duration, true);
> calc_statistics();
> break;
> case ProgramResult::kSkipped:
> skipped_programs_[program_name] = game.get_gametime();
> - crude_percent_ = crude_percent_ * 98 / 100;
> + update_crude_statistics(current_duration, false);
> break;
> case ProgramResult::kNone:
> skipped_programs_.erase(program_name);
> @@ -1032,4 +1037,17 @@
>
> default_anim_ = anim;
> }
> +
> +void ProductionSite::update_crude_statistics(uint32_t duration, const bool
> produced) {
> + static const uint32_t duration_cap = 90 * 1000; //This is highest
> allowed program duration
OK, I will extend to what 3 minutes? I have printf to see the values and did
not see any wild numbers, so this is really only insurance for first program
run and some corner situations...
> + // just for case something went very wrong...
> + static const uint32_t entire_duration = 5 * 60 *1000;
> + if (duration > duration_cap) {
> + duration = duration_cap;
> + };
> + const uint32_t old_duration = entire_duration - duration;
> + crude_percent_ = (crude_percent_ * old_duration + produced * duration *
> 1) / entire_duration;
> + assert(crude_percent_ <= 1); //be sure we do not go above 100 %
> + }
> +
> } // namespace Widelands
--
https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
What I need here is % of time utilization, something more accurate that what is now calculated. I know that times are deducted from LUA files, but it seems easier to add one variable here. Also some production programs can vary in duration, correct? -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
see inline comments
Diff comments:
> === modified file 'src/logic/map_objects/tribes/productionsite.cc'
> --- src/logic/map_objects/tribes/productionsite.cc2019-05-18 11:58:43
> +
> +++ src/logic/map_objects/tribes/productionsite.cc2019-05-19 20:55:45
> +
> @@ -954,24 +955,28 @@
> top_state().phase = result;
> }
>
> + const uint32_t current_duration = game.get_gametime() -
> last_program_end_time;
> + assert(game.get_gametime() >= last_program_end_time);
shouldn't we assert this before the above calculation?
> + last_program_end_time = game.get_gametime();
> +
> switch (result) {
> case ProgramResult::kFailed:
> statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
> statistics_.push_back(false);
> calc_statistics();
> - crude_percent_ = crude_percent_ * 8 / 10;
> + update_crude_statistics(current_duration, false);
> break;
> case ProgramResult::kCompleted:
> skipped_programs_.erase(program_name);
> statistics_.erase(statistics_.begin(), statistics_.begin() + 1);
> statistics_.push_back(true);
> train_workers(game);
> - crude_percent_ = crude_percent_ * 8 / 10 + 100 * 2 / 10;
> + update_crude_statistics(current_duration, true);
> calc_statistics();
> break;
> case ProgramResult::kSkipped:
> skipped_programs_[program_name] = game.get_gametime();
> - crude_percent_ = crude_percent_ * 98 / 100;
> + update_crude_statistics(current_duration, false);
> break;
> case ProgramResult::kNone:
> skipped_programs_.erase(program_name);
> @@ -1032,4 +1037,17 @@
>
> default_anim_ = anim;
> }
> +
> +void ProductionSite::update_crude_statistics(uint32_t duration, const bool
> produced) {
> + static const uint32_t duration_cap = 90 * 1000; //This is highest
> allowed program duration
Where did you get this value from. There are programs that last much longer.
See Frisian hunter for example
> + // just for case something went very wrong...
> + static const uint32_t entire_duration = 5 * 60 *1000;
> + if (duration > duration_cap) {
> + duration = duration_cap;
> + };
> + const uint32_t old_duration = entire_duration - duration;
shouldn't we do this after crude percent is calculated, else it is not what it
says.
> + crude_percent_ = (crude_percent_ * old_duration + produced * duration *
> 1) / entire_duration;
> + assert(crude_percent_ <= 1); //be sure we do not go above 100 %
> + }
> +
> } // namespace Widelands
--
https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ai_productionsites_statistics 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/ai_productionsites_statistics into lp:widelands
As I wrote on the forums, I think that this should be calculated when parsing the production programs. This way, we get more accurate statistics, and we can use it in the encyclopedia. This has been in my head for a while and I plan to start working on this once the 2 prerequisite branches are in, so I'd rather not add new code that we'll then need to remove anyway. -- https://code.launchpad.net/~widelands-dev/widelands/ai_productionsites_statistics/+merge/367613 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_productionsites_statistics 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
