widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

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.