I have gone through the code and added my usual nitpicking. I haven't gotten
around to any testing yet.
Diff comments:
>
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc 2015-05-07 20:46:32 +0000
> +++ src/ai/defaultai.cc 2015-06-29 19:22:37 +0000
> @@ -253,8 +268,17 @@
> if (check_economies()) { // is a must
> return;
> };
> - taskDue[ScheduleTasks::kRoadCheck] = gametime + 400;
> - improve_roads(gametime);
> + taskDue[ScheduleTasks::kRoadCheck] = gametime + 1000;
> + // testing 5 roads
> + {
> + int32_t roads_to_check = (roads.size() + 1 < 5)
> ? roads.size() + 1 : 5;
> + for (int i = 0; i < roads_to_check; i += 1) {
> + if (improve_roads(gametime)) {
> + // if significant change takes
> place do not go on
> + break;
> + };
> + }
> + }
> break;
I don't understand this code. How does checking the same thing 5 times improve
things? Same for the production sites etc. below. Please add a comment to the
code.
> case ScheduleTasks::kUnbuildableFCheck :
> taskDue[ScheduleTasks::kUnbuildableFCheck] = gametime +
> 4000;
> @@ -1192,26 +1291,24 @@
> new_buildings_stop_ = true;
> }
> // BUT if enemy is nearby, we cancel above stop
> - if (new_buildings_stop_ && enemy_last_seen_ + 2 * 60 * 1000 > gametime)
> {
> + if (new_buildings_stop_ && enemy_last_seen_ + 10 * 60 * 1000 >
> gametime) {
> new_buildings_stop_ = false;
> }
>
> - // sometimes there is too many military buildings in construction, so
> we must
> - // prevent initialization of further buildings start
> - const int32_t vacant_plus_in_construction_minus_prod =
> - vacant_mil_positions_ + 2 * num_milit_constructionsites -
> productionsites.size() / 7;
> - if (vacant_plus_in_construction_minus_prod > 20) {
> - expansion_mode = MilitaryStrategy::kNoNewMilitary;
> - } else if (vacant_plus_in_construction_minus_prod > 13) {
> - expansion_mode = MilitaryStrategy::kDefenseOnly;
> - } else if (vacant_plus_in_construction_minus_prod > 6) {
> - expansion_mode = MilitaryStrategy::kResourcesOrDefense;
> + // we must calculate wood policy
> + const WareIndex wood_index = tribe_->safe_ware_index("log");
> + // the name of variable is not 100% proper
> + const int32_t stocked_wood = get_warehoused_stock(wood_index) -
I do not understand this comment. If you don't like the variable name, how
about "available_wood"?
> + productionsites.size() * 2 -
> + num_prod_constructionsites;
> + if (stocked_wood > 80) {
> + wood_policy_ = WoodPolicy::kDismantleRangers;
> + } else if (stocked_wood > 25) {
> + wood_policy_ = WoodPolicy::kStopRangers;
> + } else if (stocked_wood > 10) {
> + wood_policy_ = WoodPolicy::kStartRangers;
> } else {
> - if (unstationed_milit_buildings_ + num_milit_constructionsites
> >= 1) {
> - expansion_mode = MilitaryStrategy::kExpansion;
> - } else {
> - expansion_mode = MilitaryStrategy::kPushExpansion;
> - }
> + wood_policy_ = WoodPolicy::kBuildRangers;
> }
>
> // we must consider need for mines
> @@ -1243,6 +1340,27 @@
> }
> }
>
> + // this controls a speed and willingness to expand the teritorry
> + const int32_t vacant_plus_in_construction_minus_prod =
> + vacant_mil_positions_ + 2 * num_milit_constructionsites -
> productionsites.size() / 7;
> + if (vacant_plus_in_construction_minus_prod > 20) {
> + expansion_mode = MilitaryStrategy::kNoNewMilitary;
> + } else if (vacant_plus_in_construction_minus_prod > 13) {
> + expansion_mode = MilitaryStrategy::kDefenseOnly;
> + } else if (vacant_plus_in_construction_minus_prod > 6) {
> + expansion_mode = MilitaryStrategy::kResourcesOrDefense;
> + } else {
> + // this is intended for initial phase of game when the player
> has enough soldiers yet
> + // but we still want to force it to follow resources instead
> for plain expansion
A tiny nit: when the player has enough soldiers yet => when the player still
has enough soldiers
> + if (virtual_mines <= 2 && (unstationed_milit_buildings_ +
> num_milit_constructionsites) > 2) {
> + expansion_mode = MilitaryStrategy::kResourcesOrDefense;
> + } else if (unstationed_milit_buildings_ +
> num_milit_constructionsites >= 1) {
> + expansion_mode = MilitaryStrategy::kExpansion;
> + } else {
> + expansion_mode = MilitaryStrategy::kPushExpansion;
> + }
> + }
> +
> BuildingObserver* best_building = nullptr;
> int32_t proposed_priority = 0;
> Coords proposed_coords;
> @@ -1572,9 +1684,10 @@
> continue;
> }
> // we can go above target if
> there is shortage of logs on stock
> - else if (bo.total_count() >=
> bo.cnt_target_ &&
> - bo.stocklevel_ > 40 +
> productionsites.size() * 2) {
> + else if (bo.total_count() >=
> bo.cnt_target_) {
> + if (wood_policy_ !=
> WoodPolicy::kBuildRangers) {
> continue;
> + }
Indent please :)
> }
>
> // considering near trees and
> producers
> @@ -1625,15 +1738,29 @@
> // this will depend on number of mines_
> and productionsites
> if
> (static_cast<int32_t>((productionsites.size() + mines_.size()) / 30) >
> bo.total_count() &&
> - bo.cnt_under_construction_ == 0)
> - prio = 4 + kDefaultPrioBoost;
> + (bo.cnt_under_construction_ +
> bo.unoccupied_) == 0 &&
> + //but only if current buildings are
> utilized enouth
> + (bo.total_count() == 0 ||
> bo.current_stats_ > 60)) {
// but only if current buildings are utilized enough
> + prio = 4 +
> kDefaultPrioBoost;
> + }
> } else { // finally normal productionsites
> if (bo.production_hint_ >= 0) {
> continue;
> }
>
> - if ((bo.cnt_under_construction_ +
> bo.unoccupied_) > 0) {
> - continue;
> + // generally we allow 1 building in
> construction, but if
> + // preciousness of missing ware is >=10
> and it is farm-like building
> + // we allow 2 in construction
> + if (max_needed_preciousness >= 10
> + && bo.inputs_.empty()
> + && gametime > 30 * 60 * 1000) {
> + if ((bo.cnt_under_construction_
> + bo.unoccupied_) > 1) {
> + continue;
> + }
> + } else {
> + if ((bo.cnt_under_construction_
> + bo.unoccupied_) > 0) {
> + continue;
> + }
> }
>
> if (bo.forced_after_ < gametime &&
> (bo.total_count() - bo.unconnected_) == 0) {
> @@ -1724,6 +1856,21 @@
> prio += 1;
> }
> }
> +
> + //consider borders (for medium + big buildings
> and ones with input)
> + //=>decreasing the score
> + //but only if we have enough free spots to
> built on
> + //otherwise it will slow down the expansion -
> small buildings would be preffered
> + if (spots_avail.at(BUILDCAPS_MEDIUM) > 40//HERE
> TRANSFER
Please precede and follow // with a blank space, so it is easier to read.
preffered => preferred
> + &&
> + spots_avail.at(BUILDCAPS_BIG) > 20
> + &&
> + (bo.desc->get_size() == 2 ||
> + bo.desc->get_size() == 3 ||
> + !bo.inputs_.empty())) {
> + prio =
> recalc_with_border_range(*bf, prio);
> + }
> +
> } // production sites done
> else if (bo.type == BuildingObserver::MILITARYSITE) {
>
> @@ -3595,12 +3726,76 @@
> }
> }
>
> - trainingsites.push_back(trainingsites.front());
> - trainingsites.pop_front();
> -
> - // changing capacity
> - if (tso.site->soldier_capacity() != 2) {
> - game().send_player_change_soldier_capacity(*ts, 2 -
> tso.site->soldier_capacity());
> + // changing capacity to 0 - this will happen only once.....
> + if (tso.site->soldier_capacity() > 1) {
> + game().send_player_change_soldier_capacity(*ts, -
> tso.site->soldier_capacity());
> + return true;
> + }
> +
> + // reducing ware queues
> + // - for armours and weapons to 1
> + // - for others to 6
> + std::vector<WaresQueue*> const warequeues1 = tso.site->warequeues();
> + size_t nr_warequeues = warequeues1.size();
> + for (size_t i = 0; i < nr_warequeues; ++i) {
> +
> + // if it was decreased yet
> + if (warequeues1[i]->get_max_fill() <= 1) {
> + continue;}
> +
> + // now modifying max_fill of armors and weapons
> + for (std::string pattern : armors_and_weapons) {
> +
> + if
> (tribe_->get_ware_descr(warequeues1[i]->get_ware())->name().find(pattern) !=
> std::string::npos) {
> + if (warequeues1[i]->get_max_fill() > 1) {
> +
> game().send_player_set_ware_max_fill(*ts, warequeues1[i]->get_ware(), 1);
> + continue;
> + }
> + }
> + }
> + }
> +
> + // changing priority if basic
> + if (tso.bo->trainingsite_type_ == TrainingSiteType::kBasic) {
> + for (uint32_t k = 0; k < tso.bo->inputs_.size(); ++k) {
> + game().send_player_set_ware_priority(
> + *ts, wwWARE, tso.bo->inputs_.at(k), HIGH_PRIORITY);
> + }
> + }
> +
> + // if soldier capacity is set to 0, we need to find out if the site is
> + // supplied enough to incrase the capacity to 1
> + if (tso.site->soldier_capacity() == 0) {
> +
> + // First subsitute wares
> + int32_t filled = 0;
> + bool supplied_enough = true;
> + std::vector<WaresQueue*> const warequeues2 =
> tso.site->warequeues();
> + nr_warequeues = warequeues2.size();
> + for (size_t i = 0; i < nr_warequeues; ++i) {
> + if
> (tso.bo->substitute_inputs_.count(warequeues2[i]->get_ware()) > 0) {
> + filled += warequeues2[i]->get_filled();
> + }
> + }
> + if (filled < 5) {
> + supplied_enough = false;
> + }
> +
> + // checking non subsitutes
> + for (size_t i = 0; i < nr_warequeues; ++i) {
> + if
> (tso.bo->substitute_inputs_.count(warequeues2[i]->get_ware()) == 0) {
> + const uint32_t required_amount
> + =
> + (warequeues2[i]->get_max_fill()<5) ?
> warequeues2[i]->get_max_fill() : 5;
get_max_fill()<5 => get_max_fill() < 5
> + if (warequeues2[i]->get_filled() <
> required_amount) {
> + supplied_enough = false;
> + }
> + }
> + }
> +
> + if (supplied_enough) {
> + game().send_player_change_soldier_capacity(*ts, 1);
> + }
> }
>
> ts_without_trainers_ = 0; // zeroing
> @@ -3753,8 +3951,8 @@
>
> /**
> * This function takes care about the unowned and opposing territory and
> - * recalculates the priority for none military buildings depending on the
> - * initialisation type of a defaultAI
> + * recalculates the priority for non-military buildings
> + * The goal is to minimine losses when teritory is lost
minimine => minimize
> *
> * \arg bf = BuildableField to be checked
> * \arg prio = priority until now.
> @@ -3762,18 +3960,26 @@
> * \returns the recalculated priority
> */
> int32_t DefaultAI::recalc_with_border_range(const BuildableField& bf,
> int32_t prio) {
> - // Prefer building space in the inner land.
> -
> +
> + //no change when priority is not positive number
Blank space after // please.
> + if (prio <= 0) {
> + return prio;
> + }
> +
> + //in uýnowned teritory, decreasing to 2/3
uýnowned => unowned
> if (bf.unowned_land_nearby_ > 15) {
> - prio -= (bf.unowned_land_nearby_ - 15);
> - }
> -
> - // Especially places near the frontier to the enemies are unlikely
> - // NOTE take care about the type of computer player_. The more
> - // NOTE aggressive a computer player_ is, the more important is
> - // NOTE this check. So we add \var type as bonus.
> - if (bf.enemy_nearby_ && prio > 0) {
> - prio /= (3 + type_);
> + prio *= 2;
> + prio /= 3;
> + }
> +
> + //to preserve positive score
Blank space after // please.
> + if (prio == 0) {
> + prio = 1;
> + }
> +
> + //Further decrease the score if enemy nearby
Blank space after // please.
> + if (bf.enemy_nearby_) {
> + prio /= 2;
> }
>
> return prio;
> @@ -3952,6 +4158,23 @@
> }
> }
>
> +// This is called when soldier left the trainingsite
> +// the purpose is to set soldier capacity to 0
> +// (AI will then wait till training site is stocked)
> +void DefaultAI::soldier_trained(const TrainingSite& site) {
> +
> + // we must identify particular training site
> + for (std::list<TrainingSiteObserver>::iterator i =
> trainingsites.begin(); i != trainingsites.end(); ++i)
Can you turn this into a range-based for loop? Iterators are hard to read.
Something like this:
for (const TrainingSiteObserver& trainingsite_obs : trainingsites ) {
> + if (i->site == &site) {
> + if (i->site->soldier_capacity() > 0) {
> +
> game().send_player_change_soldier_capacity(*i->site, -
> i->site->soldier_capacity());
> + }
> + return;
> + }
> + log (" %d: Computer player error - trainingsite not found\n",
> + player_number());
> +}
> +
> // walk and search for teritorry controlled by other player
> // usually scanning radius is enough but sometimes we must walk to
> // verify that an enemy teritory is really accessible by land
> @@ -3993,10 +4216,35 @@
> // a port location), but when testing (starting from) own
> military building
> // we must ignore own teritory, of course
> if (f->get_owned_by() > 0) {
> - if (type == WalkSearch::kAnyPlayer ||
> - (type == WalkSearch::kOtherPlayers &&
> f->get_owned_by() != pn)) {
> - *tested_fields = done.size();
> - return true;
> +
> + // if field is owned by anybody
> + if (type == WalkSearch::kAnyPlayer) {
> + *tested_fields = done.size();
> + return true;
> + }
Unindent please.
> +
> + // if anybody but not me
> + if (type == WalkSearch::kOtherPlayers &&
> f->get_owned_by() != pn) {
> + *tested_fields = done.size();
> + return true;
> + }
Unindent please.
> +
> + // if owned by enemy
> + if (type == WalkSearch::kEnemy && f->get_owned_by() !=
> pn) {
> + // for case I am not member of a team
> + if (player_->team_number() == 0) {
> + *tested_fields = done.size();
> + return true;
> + }
> + // if I am in team, testing if the same team
> + if (player_->team_number() > 0
Can this bit be replaced by else if?
else if (player_->team_number() ...
> + &&
> + player_->team_number()
> + !=
> +
> game().get_player(f->get_owned_by())->team_number()) {
> + *tested_fields = done.size();
> + return true;
> + }
> }
> }
>
> @@ -4421,6 +4669,65 @@
> return supplied == bo.inputs_.size();
> }
>
> +// This calculates strength of vector of soldiers, f.e. soldiers in a
> building or
> +// ones ready to attack
> +int32_t DefaultAI::calculate_strength(const std::vector<Widelands::Soldier*>
> soldiers) {
> +
> + if (soldiers.empty()) {
> + return 0;
> + }
> +
> + enum {BARBARIANS, ATLANTEANS, EMPIRE};
How about:
enum class Tribes {kNone, kBarbarians, kAtlanteans, kEmpire};
Tribes tribe = kNone;
if (soldiers.at(0)->get_owner()->tribe().name() == "atlanteans") {
tribe = Tribes::kAtlanteans;
}
...
switch (tribe) {
case (Tribes::kAtlanteans):
...
> + uint8_t tribe = std::numeric_limits<uint8_t>::max();
> +
> + if (soldiers.at(0)->get_owner()->tribe().name() == "atlanteans") {
> + tribe = ATLANTEANS;
> + } else if (soldiers.at(0)->get_owner()->tribe().name() == "barbarians")
> {
> + tribe = BARBARIANS;
> + } else if (soldiers.at(0)->get_owner()->tribe().name() == "empire") {
> + tribe = EMPIRE;
> + } else {
> + throw wexception("AI warning: Unable to calculate strenght for
> player of tribe %s",
> + soldiers.at(0)->get_owner()->tribe().name().c_str());
> + }
> +
> + float hp = 0;
> + float al = 0;
> + float dl = 0;
> + float el = 0;
> + float final = 0;
> +
> + for (Soldier * soldier : soldiers) {
> + switch (tribe) {
> + case (ATLANTEANS):
> + hp = 135 + 40 * soldier->get_hp_level();
> + al = 14 + 8 * soldier->get_attack_level();
> + dl = static_cast<float>(94 - 8 *
> soldier->get_defense_level()) / 100;
> + el = static_cast<float>(70 - 17 *
> soldier->get_evade_level()) / 100;
> + break;
> + case (BARBARIANS):
> + hp += 130 + 28 * soldier->get_hp_level();
> + al += 14 + 7 * soldier->get_attack_level();
> + dl += static_cast<float>(97 - 8 *
> soldier->get_defense_level()) / 100;
> + el += static_cast<float>(75 - 15 *
> soldier->get_evade_level()) / 100;
> + break;
> + case (EMPIRE):
> + hp += 130 + 21 * soldier->get_hp_level();
> + al += 14 + 8 * soldier->get_attack_level();
> + dl += static_cast<float>(95 - 8 *
> soldier->get_defense_level()) / 100;
> + el += static_cast<float>(70 - 16 *
> soldier->get_evade_level()) / 100;
> + break;
> + default:
> + assert (false);
> + }
> +
> + final += (al * hp) / (dl * el);
> + }
> +
> + // 2500 is aproximate strength of one unpromoted soldier
> + return static_cast<int32_t>(final / 2500);
> +}
> +
> bool DefaultAI::check_enemy_sites(uint32_t const gametime) {
>
> Map& map = game().map();
> @@ -4588,31 +4947,42 @@
>
> // can we attack:
> if (is_attackable) {
> - site->second.attack_soldiers =
> player_->find_attack_soldiers(*flag);
> + std::vector<Soldier *> attackers;
> + player_->find_attack_soldiers(*flag,
> &attackers);
> + int32_t strength =
> calculate_strength(attackers);
> +
> + site->second.attack_soldiers_strength =
> strength;
> } else {
> - site->second.attack_soldiers = 0;
> + site->second.attack_soldiers_strength = 0;
> }
>
> - site->second.defenders = defenders;
> -
> - if (site->second.attack_soldiers > 0) {
> - site->second.score =
> site->second.attack_soldiers - site->second.defenders / 2;
> -
> - if (!is_warehouse)
> - site->second.score -= 1;
> + site->second.defenders_strength = defenders_strength;
> +
> + if (site->second.attack_soldiers_strength > 0
> + &&
> + player_attackable[onwer_number - 1]) {
onwer => owner
> + site->second.score =
> site->second.attack_soldiers_strength - site->second.defenders_strength / 2;
> +
> + if (is_warehouse) {
> + site->second.score += 2;
> + } else {
> + site->second.score -= 2;
> + }
>
> // here is some differentiation based on
> "character" of a player
> if (type_ == NORMAL) {
> - site->second.score -= 1;
> - site->second.score -=
> vacant_mil_positions_ / 10;
> + site->second.score -= 3;
> + site->second.score -=
> vacant_mil_positions_ / 8;
> } else if (type_ == DEFENSIVE) {
> - site->second.score -= 2;
> - site->second.score -=
> vacant_mil_positions_ / 5;
> + site->second.score -= 6;
> + site->second.score -=
> vacant_mil_positions_ / 4;
> } else { //=AGRESSIVE
> - site->second.score -=
> vacant_mil_positions_ / 15;
> + site->second.score -=
> vacant_mil_positions_ / 16;
> }
> if (site->second.mines_nearby ==
> ExtendedBool::kFalse) {
> site->second.score -= 1;
> + } else {
> + site->second.score += 1;
> }
> // we dont want to attack multiple players at
> the same time too eagerly
> if (onwer_number != last_attacked_player_) {
--
https://code.launchpad.net/~widelands-dev/widelands/trainingsites_and_teams/+merge/260517
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/trainingsites_and_teams into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
