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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
