widelands-dev team mailing list archive

[hessenfarmer <stephan-lutz@xxxxxx>]

see inline comments

Diff comments:

> === modified file 'src/logic/map_objects/tribes/productionsite.cc'
> --- src/logic/map_objects/tribes/productionsite.cc	2019-05-18 11:58:43 +0000
> +++ src/logic/map_objects/tribes/productionsite.cc	2019-05-19 20:55:45 +0000
> @@ -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 + 1000000 * 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 * 10000) / entire_duration;
> +	assert(crude_percent_ <= 10000); //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.