widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Code LGTM - just some small nits and suggestions.

I haven't tested it.

Diff comments:

> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2015-12-06 19:57:49 +0000
> +++ src/ai/defaultai.cc	2015-12-12 20:55:58 +0000
> @@ -1520,6 +1522,11 @@
>  	} else {
>  		needed_spots = 1300 + (productionsites.size() - 200) * 20;
>  	}
> +	const bool has_enough_space = (spots_ > needed_spots);
> +
> +	//This is a replacement for simple count of mines

Blank space after //

> +	const int32_t virtual_mines =
> +	   mines_.size() + mineable_fields.size() / 25;
>  
>  	// *_military_scores are used as minimal score for a new military building
>  	// to be built. As AI does not traverse all building fields at once, these thresholds
> @@ -1533,7 +1540,18 @@
>  	// least_military_score_ is allowed to get bellow 100 only if there is no military site in construction
>  	// right now in order to (try to) avoid expansion lockup
>  
> -	// this is just helpers to improve readability of code
> +	// Bools below are helpers to improve readability of code
> +
> +	// Military sites have generally higher scores so this is a helper to boost economy
> +	bool needs_boost_economy = false;
> +	if (highest_nonmil_prio_ > 10
> +		&& has_enough_space
> +		&& virtual_mines >= 5){
> +			needs_boost_economy = true;
> +		}
> +	//resetting highest_nonmil_prio_ so it can be recalculated anew

Blank space after //

> +	highest_nonmil_prio_ = 0;
> +
>  	const bool too_many_ms_constructionsites =
>  		(pow(msites_in_constr(), 2) > militarysites.size());
>  	const bool too_many_vacant_mil =
> @@ -1674,9 +1691,15 @@
>  		// checking we have enough critical material on stock
>  		for (uint32_t m = 0; m < bo.critical_built_mat_.size(); ++m) {
>  			DescriptionIndex wt(static_cast<size_t>(bo.critical_built_mat_.at(m)));
> -			// shortage = less then 3 items in warehouses
> -			if (get_warehoused_stock(wt) < 3) {
> +			uint32_t treshold = 3;
> +			//generally trainingsites are more important

Blank space after //

> +			if (bo.type == BuildingObserver::TRAININGSITE) {
> +				treshold = 2;
> +			}
> +
> +			if (get_warehoused_stock(wt) < treshold) {
>  				bo.build_material_shortage_ = true;
> +				break;
>  			}
>  		}
>  	}
> @@ -2280,53 +2311,45 @@
>  
>  			} else if (bo.type == BuildingObserver::TRAININGSITE) {
>  
> -				assert(!bo.build_material_shortage_);
> +				prio = 30;
> +
> +				if (bo.new_building_ == BuildingNecessity::kForced) {
> +					prio += 30;
> +				}
> +
> +				if (bo.trainingsite_type_ == TrainingSiteType::kBasic){

Blank space after )

> +					prio = static_cast<int32_t>(militarysites.size())
> +						- 40 * static_cast<int32_t>(ts_basic_count_);

Can ts_basic_count_ be an int32_t in the first place?

> +				}
> +
> +				if (bo.trainingsite_type_ == TrainingSiteType::kAdvanced) {
> +					prio = static_cast<int32_t>(militarysites.size())
> +						- 50 * static_cast<int32_t>(ts_advanced_count_);

Can ts_advanced_count_ be an int32_t in the first place?

> +				}
>  
>  				// exclude spots on border
>  				if (bf->near_border_) {
> -					continue;
> -				}
> -
> -				// it is a bit difficult to get a new trainer.....
> -				if (ts_without_trainers_) {
> -					continue;
> -				}
> -
> -				// target is only one for both types
> -				if ((ts_basic_const_count_ + ts_advanced_const_count_) > 0) {
> -					continue;
> -				}
> -
> -				// we build one basic training site for 50 militarysites
> -				if (bo.trainingsite_type_ == TrainingSiteType::kBasic &&
> -				    militarysites.size() / 50 < static_cast<int32_t>(ts_basic_count_)) {
> -					continue;
> -				}
> -				// we build one advanced training site for 75 militarysites
> -				if (bo.trainingsite_type_ == TrainingSiteType::kAdvanced &&
> -				    militarysites.size() / 75 < static_cast<int32_t>(ts_advanced_count_)) {
> -					continue;
> -				}
> +					prio -= 5;
> +				}
> +
>  
>  				// for type1 we need 15 productionsties
>  				if (bo.trainingsite_type_ == TrainingSiteType::kBasic && productionsites.size() < 15) {
> -					continue;
> +					prio -= 15;
>  				}
>  
>  				// for type2 we need 4 mines
>  				if (bo.trainingsite_type_ == TrainingSiteType::kAdvanced && virtual_mines < 4) {
> -					continue;
> +					prio -= 15;;
>  				}
>  
> -				prio = 10;
> -
>  				// take care about borders and enemies
>  				if (bf->enemy_nearby_) {
> -					prio /= 2;
> +					prio -= 10;
>  				}
>  
>  				if (bf->unowned_land_nearby_) {
> -					prio -= bf->unowned_land_nearby_ / 10;
> +					prio -= 5;
>  				}
>  			}
>  
> @@ -3156,26 +3187,26 @@
>  						doing_upgrade = true;
>  					}
>  
> -					// if the decision was not made yet, consider normal upgrade
> -					if (!doing_upgrade) {
> -						// compare the performance %
> -						if (en_bo.current_stats_ - site.bo->current_stats_
> -							> static_cast<uint32_t>(20)) {
> -								doing_upgrade = true;
> -						}
> -
> -						if ((static_cast<int32_t>(en_bo.current_stats_) > 85 &&
> -						     en_bo.total_count() * 2 < site.bo->total_count()) ||
> -						    (static_cast<int32_t>(en_bo.current_stats_) > 50 &&
> -						     en_bo.total_count() * 4 < site.bo->total_count())) {
> -
> -								doing_upgrade = true;
> -						}
> -
> -						// Dont forget about limitation of number of buildings
> -						if (en_bo.cnt_limit_by_aimode_ <= en_bo.total_count() - en_bo.unconnected_count_) {
> -							doing_upgrade = false;
> -						}
> +					if (en_bo.total_count() == 1) {
> +						//if the upgrade itself can be upgradeed futher, we are more willing to upgrade 2nd building

Blank space after //

upgradeed => upgraded

> +						if (en_bo.upgrade_extends_ || en_bo.upgrade_substitutes_) {
> +							if (en_bo.current_stats_ > 30) {
> +								doing_upgrade = true;
> +							}
> +						} else if (en_bo.current_stats_ > 50) {
> +							doing_upgrade = true;
> +						}
> +					}
> +
> +					if (en_bo.total_count() > 1) {
> +						if (en_bo.current_stats_ > 80) {
> +								doing_upgrade = true;
> +						}
> +					}
> +
> +					// Dont forget about limitation of number of buildings
> +					if (en_bo.cnt_limit_by_aimode_ <= en_bo.total_count() - en_bo.unconnected_count_) {
> +						doing_upgrade = false;
>  					}
>  				}
>  			}


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1523165/+merge/280397
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1523165.