widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

How about naming them "weak", "normal" and "strong"? I think that is pretty standard.

Some comments in the diff.

Diff comments:

> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2015-11-22 19:26:11 +0000
> +++ src/ai/defaultai.cc	2015-11-24 21:27:31 +0000
> @@ -556,6 +557,22 @@
>  			bo.plants_trees_ = false;
>  		}
>  
> +	// Is total count of this building limited by AI mode?
> +	if (type_ == DEFENSIVE && bh.get_weak_ai_limit() >= 0) {

I would rename DEFENSIVE etc to "kWeak" etc. to match. Can they be an enum class too?

Also, instead of initializing with -1, how about INVALID_INDEX?

> +		bo.cnt_limit_by_aimode_ = bh.get_weak_ai_limit();
> +		log (" %d: AI defensive mode: applying limit %d building(s) for %s\n",

AI weak mode

> +		player_number(),
> +		bo.cnt_limit_by_aimode_,
> +		bo.name);
> +	}
> +	if (type_ == NORMAL && bh.get_normal_ai_limit() >= 0) {
> +		bo.cnt_limit_by_aimode_ = bh.get_normal_ai_limit();
> +		log (" %d: AI normal mode: applying limit %d building(s) for %s\n",
> +		player_number(),
> +		bo.cnt_limit_by_aimode_,
> +		bo.name);
> +	}
> +
>  		// Read all interesting data from ware producing buildings
>  		if (bld.type() == MapObjectType::PRODUCTIONSITE) {
>  			const ProductionSiteDescr& prod = dynamic_cast<const ProductionSiteDescr&>(bld);
> @@ -3119,19 +3155,15 @@
>  
>  					// forcing first upgrade
>  					if (en_bo.total_count() == 0) {
> -						enbld = enhancement;
> -						bestbld = &en_bo;
> +						doing_upgrade = true;
>  					}
>  
>  					// if the decision was not made yet, consider normal upgrade
> -					if (enbld == INVALID_INDEX) {
> +					if (!doing_upgrade) {
>  						// compare the performance %
>  						if (static_cast<int32_t>(en_bo.current_stats_) -
> -						       static_cast<int32_t>(site.bo->current_stats_) >
> -						    20) {
> -
> -							enbld = enhancement;
> -							bestbld = &en_bo;
> +						    static_cast<int32_t>(site.bo->current_stats_) > 20) {

Why do you need to cast this? If the compiler complains about this line, I think it's safer to cast the 20.

> +								doing_upgrade = true;
>  						}
>  
>  						if ((static_cast<int32_t>(en_bo.current_stats_) > 85 &&
> @@ -3760,6 +3799,13 @@
>  										const PerfEvaluation purpose,
>  										const uint32_t gametime) {
>  
> +	// Very first we finds if AI is allowed to build such building due to its mode
> +	if (purpose == PerfEvaluation::kForConstruction
> +		&&
> +		bo.total_count() - bo.unconnected_count_ >= bo.cnt_limit_by_aimode_) {

We have this comparison a lot. Create a boolean function for it?

> +			return BuildingNecessity::kForbidden;
> +		}
> +
>  	// First we iterate over outputs of building, count warehoused stock
>  	// and deciding if we have enough on stock (in warehouses)
>  	bo.max_preciousness_ = 0;


-- 
https://code.launchpad.net/~widelands-dev/widelands/ai_differentiation/+merge/278517
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_differentiation into lp:widelands.