← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/ai_flag_warehouse_distance into lp:widelands

 

Code review done. Mostly small tweaks to names and some efficiency stuff, and 1 real question.

Diff comments:

> 
> === modified file 'src/ai/ai_help_structs.cc'
> --- src/ai/ai_help_structs.cc	2019-04-09 16:43:49 +0000
> +++ src/ai/ai_help_structs.cc	2019-07-24 19:32:22 +0000
> @@ -1417,4 +1301,190 @@
>  	return update_time;
>  }
>  
> +FlagWarehouseDistances::FlagInfo::FlagInfo(const uint32_t gametime,
> +                                           const uint16_t dist,
> +                                           const uint32_t wh) {
> +	expiry_time = gametime + kFlagDistanceExpirationPeriod;
> +	soft_expiry_time = gametime + kFlagDistanceExpirationPeriod / 2;
> +	distance = dist;
> +	nearest_warehouse = wh;
> +	new_road_prohibited_till = 0;
> +}
> +FlagWarehouseDistances::FlagInfo::FlagInfo() {
> +	expiry_time = 0;
> +	distance = 1000;
> +	new_road_prohibited_till = 0;
> +}
> +
> +// We are updating the distance info, but not all the time.
> +// Always if after soft expiration period, but
> +// if below expiration period only when the new value is lower than current on

on -> one

> +// In both cases new expiration times are calculated
> +bool FlagWarehouseDistances::FlagInfo::update(const uint32_t gametime,
> +                                              const uint16_t new_distance,
> +                                              const uint32_t nearest_wh) {
> +	const uint32_t new_expiry_time = gametime + kFlagDistanceExpirationPeriod;
> +
> +	if (gametime > soft_expiry_time) {
> +		distance = new_distance;
> +		expiry_time = new_expiry_time;
> +		soft_expiry_time = gametime + kFlagDistanceExpirationPeriod / 2;
> +		nearest_warehouse = nearest_wh;
> +		return true;
> +	} else if (new_distance < distance || (new_distance == distance &&
> +	                                       expiry_time < gametime + kFlagDistanceExpirationPeriod)) {

expiry_time < new_expiry_time

> +		distance = new_distance;
> +		expiry_time = gametime + kFlagDistanceExpirationPeriod;

= new_expiry_time

> +		nearest_warehouse = nearest_wh;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +uint16_t FlagWarehouseDistances::FlagInfo::get(const uint32_t gametime, uint32_t* nw) const {
> +	*nw = nearest_warehouse;
> +	if (gametime <= expiry_time) {
> +		return distance;
> +	}
> +	return kFarButReachable;
> +}
> +
> +void FlagWarehouseDistances::FlagInfo::road_built(const uint32_t gametime) {

Name it set_road_built

> +	// Prohibiting for next 60 seconds
> +	new_road_prohibited_till = gametime + 60 * 1000;
> +}
> +
> +bool FlagWarehouseDistances::FlagInfo::road_prohibited(const uint32_t gametime) const {

name it is_road_prohibited

> +	return new_road_prohibited_till > gametime;
> +}
> +
> +bool FlagWarehouseDistances::set_distance(const uint32_t flag_coords,
> +                                          const uint16_t distance,
> +                                          uint32_t const gametime,
> +                                          uint32_t const nearest_warehouse) {
> +	if (flags_map.count(flag_coords) == 0) {
> +		flags_map[flag_coords] =
> +		   FlagWarehouseDistances::FlagInfo(gametime, distance, nearest_warehouse);
> +		return true;
> +	}
> +	return flags_map[flag_coords].update(gametime, distance, nearest_warehouse);
> +}
> +
> +uint16_t FlagWarehouseDistances::count() const {
> +	return flags_map.size();
> +}
> +
> +int16_t
> +FlagWarehouseDistances::get_distance(const uint32_t flag_coords, uint32_t gametime, uint32_t* nw) {

Make this function const

> +	if (flags_map.count(flag_coords) == 0) {
> +		*nw = 0;
> +		return kFarButReachable;  // this is to discourage to build second road from brand new flag...
> +	} else {
> +		return flags_map[flag_coords].get(gametime, nw);
> +	}
> +}
> +
> +void FlagWarehouseDistances::set_road_built(const uint32_t coords_hash, const uint32_t gametime) {
> +	if (flags_map.count(coords_hash) == 1) {
> +		flags_map[coords_hash].road_built(gametime);
> +	}
> +}
> +
> +bool FlagWarehouseDistances::get_road_prohibited(const uint32_t coords_hash,

Naming it is_road_prohibited will make it clearer that it's a boolean.

> +                                                 const uint32_t gametime) {
> +	if (flags_map.count(coords_hash) == 1) {
> +		return flags_map[coords_hash].road_prohibited(gametime);
> +	}
> +	return false;
> +}
> +
> +bool FlagWarehouseDistances::remove_old_flag(uint32_t gametime) {
> +	for (std::map<uint32_t, FlagWarehouseDistances::FlagInfo>::iterator it = flags_map.begin();
> +	     it != flags_map.end(); it++) {
> +		if (it->second.expiry_time + kOldFlagRemoveTime < gametime) {
> +			it = flags_map.erase(it);
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +// Returns pointer to winning road, provided that the treshold is exceeded

treshold -> threshold

> +FlagCandidates::Candidate* FlagCandidates::get_winner(const int16_t treshold) {
> +	if (flags_.empty()) {
> +		return nullptr;
> +	}
> +	sort();
> +	if (flags_[0].score() < treshold) {
> +		return nullptr;
> +	}
> +	if (!flags_[0].is_buildable()) {
> +		return nullptr;
> +	}
> +	return &flags_[0];
> +}
> +
> +FlagCandidates::Candidate::Candidate(
> +   const uint32_t ch, bool de, uint16_t start_f_dist, uint16_t act_dist_to_wh, uint16_t air_dst) {

I think the following names would be easier to understand:

const uint32_t ch -> c_hash

bool de -> different_eco

> +	coords_hash = ch;
> +	different_economy = de;
> +	start_flag_dist_to_wh = start_f_dist;
> +	flag_to_flag_road_distance = 0;
> +	possible_road_distance = kFarButReachable;
> +	cand_flag_distance_to_wh = act_dist_to_wh;
> +	air_distance = air_dst;
> +}
> +
> +int16_t FlagCandidates::Candidate::score() const {
> +	return different_economy * 2000 + (start_flag_dist_to_wh - cand_flag_distance_to_wh) +
> +	       (flag_to_flag_road_distance - 2 * possible_road_distance);
> +}
> +
> +bool FlagCandidates::set_road_possible(const uint32_t coords_hash, const uint16_t steps) {
> +	for (auto& item : flags_) {
> +		if (item.coords_hash == coords_hash) {
> +			item.possible_road_distance = steps;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +bool FlagCandidates::set_cur_road_distance(const uint32_t coords, uint16_t dist) {
> +	for (auto& item : flags_) {
> +		if (item.coords_hash == coords) {
> +			item.flag_to_flag_road_distance = dist;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +void FlagCandidates::sort() {
> +	std::sort(flags_.begin(), flags_.end());
> +}
> +
> +void FlagCandidates::sort_by_air_distance() {
> +	std::sort(flags_.begin(), flags_.end(),
> +	          [](const FlagCandidates::Candidate& lf, const FlagCandidates::Candidate& rf) {
> +		          return lf.air_distance < rf.air_distance;
> +	          });
> +}
> +
> +void FlagCandidates::add_flag(const uint32_t coords,
> +                              const bool different_economy,
> +                              const uint16_t act_dist_to_wh,
> +                              const uint16_t air_distance) {
> +	flags_.push_back(
> +	   Candidate(coords, different_economy, start_flag_dist_to_wh, act_dist_to_wh, air_distance));
> +}
> +
> +bool FlagCandidates::has_candidate(const uint32_t coords_hash) {

Make this function const

> +	for (auto& item : flags_) {
> +		if (item.coords_hash == coords_hash) {
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
>  }  // namespace Widelands
> 
> === modified file 'src/ai/ai_help_structs.h'
> --- src/ai/ai_help_structs.h	2019-05-25 08:20:22 +0000
> +++ src/ai/ai_help_structs.h	2019-07-24 19:32:22 +0000
> @@ -891,6 +859,97 @@
>  	PlayerNumber this_player_number;
>  	PlayerNumber this_player_team;
>  };
> +
> +// This is a wrapper around map of <Flag coords hash:distance from flag to nearest warehouse>
> +// Flags does not have observers so this stuct server like "lazy" substitution for this, where
> +// keys are hash of flags positions
> +// This "lives" during entire "session", and is not saved, but is easily recalulated
> +struct FlagWarehouseDistances {
> +private:
> +	struct FlagInfo {
> +		FlagInfo();
> +		FlagInfo(uint32_t, uint16_t, uint32_t);
> +		// Distance to a nearest warehouse expires, but in two stages
> +		// This is complete expiration and such flag is treated as without distance to WH
> +		uint32_t expiry_time;
> +		// When recalculating new distance, if the current information is younger than
> +		// soft_expiry_time it is only decreased if new value is smaller After soft_expiry_time is
> +		// updated in any case
> +		uint32_t soft_expiry_time;
> +		uint16_t distance;
> +		// This is coords hash of nearest warehouse, not used by now
> +		uint32_t nearest_warehouse;
> +		// after building of new road, the distance information is updated not immediately so
> +		// we prohibit current flag from another road building...
> +		uint32_t new_road_prohibited_till;
> +
> +		bool update(uint32_t, uint16_t, uint32_t);
> +		// Saying the road was built and when
> +		void road_built(uint32_t);
> +		// Asking if road can be built from this flag (providing current gametime)
> +		bool road_prohibited(uint32_t) const;
> +		// get current distance (providing current gametime)
> +		uint16_t get(uint32_t, uint32_t*) const;
> +	};
> +	std::map<uint32_t, FlagInfo> flags_map;
> +
> +public:
> +	// All these function uses lookup in flags_map so first argument is usually flag coords hash

If you add the variable names here, it will be easier to read

> +	bool set_distance(uint32_t, uint16_t, uint32_t, uint32_t);
> +	int16_t get_distance(uint32_t, uint32_t, uint32_t*);
> +	void set_road_built(uint32_t, uint32_t);
> +	bool get_road_prohibited(uint32_t, uint32_t);
> +	uint16_t count() const;
> +	bool remove_old_flag(uint32_t);
> +};
> +
> +// This is one-time structure - initiated and filled up when investigating possible roads to be
> +// build to a flag At the end the flags are scored based on gained info), ordered and if treshold is

build to a flag  -> built to a flag.

> +// achieved the road is to be built
> +struct FlagCandidates {
> +
> +	explicit FlagCandidates(uint16_t wd) : start_flag_dist_to_wh(wd) {
> +	}
> +
> +	struct Candidate {
> +		Candidate() = delete;
> +		Candidate(uint32_t, bool, uint16_t, uint16_t, uint16_t);
> +		uint16_t air_distance;
> +		uint32_t coords_hash;
> +		bool different_economy;
> +		uint16_t start_flag_dist_to_wh;
> +		uint16_t flag_to_flag_road_distance;
> +		uint16_t possible_road_distance;
> +		uint16_t cand_flag_distance_to_wh;
> +		// Scoring is considering multiple things
> +		int16_t score() const;
> +		bool is_buildable() {
> +			return possible_road_distance > 0;
> +		}
> +		bool operator<(const Candidate& other) const {
> +			return score() > other.score();
> +		}
> +	};
> +
> +private:
> +	std::vector<Candidate> flags_;
> +	uint16_t start_flag_dist_to_wh;
> +
> +public:
> +	const std::vector<Candidate>& flags() const {
> +		return flags_;
> +	}
> +	bool has_candidate(uint32_t);
> +	void add_flag(uint32_t, bool, uint16_t, uint16_t);
> +	bool set_cur_road_distance(uint32_t, uint16_t);
> +	bool set_road_possible(uint32_t, uint16_t);
> +	void sort();
> +	void sort_by_air_distance();
> +	uint32_t count() {
> +		return flags_.size();
> +	}
> +	FlagCandidates::Candidate* get_winner(const int16_t = 0);
> +};
>  }  // namespace Widelands
>  
>  #endif  // end of include guard: WL_AI_AI_HELP_STRUCTS_H
> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2019-05-26 11:39:41 +0000
> +++ src/ai/defaultai.cc	2019-07-24 19:32:22 +0000
> @@ -3756,318 +3813,341 @@
>  // connection is not known
>  bool DefaultAI::create_shortcut_road(const Flag& flag,
>                                       uint16_t checkradius,
> -                                     int16_t min_reduction,
>                                       uint32_t gametime) {
>  
> -	// Increasing the failed_connection_tries counter
> -	// At the same time it indicates a time an economy is without a warehouse
> -	EconomyObserver* eco = get_economy_observer(flag.economy());
> -	// if we passed grace time this will be last attempt and if it fails
> -	// building is destroyes
> -	bool last_attempt_ = false;
> -
> -	// this should not happen, but if the economy has a warehouse and a dismantle
> -	// grace time set, we must 'zero' the dismantle grace time
> -	if (!flag.get_economy()->warehouses().empty() &&
> -	    eco->dismantle_grace_time != std::numeric_limits<uint32_t>::max()) {
> -		eco->dismantle_grace_time = std::numeric_limits<uint32_t>::max();
> -	}
> -
> -	// first we deal with situations when this is economy with no warehouses
> -	// and this is a flag belonging to a building/constructionsite
> -	// such economy must get dismantle grace time (if not set yet)
> -	// end sometimes extended checkradius
> -	if (flag.get_economy()->warehouses().empty() && flag.get_building()) {
> -
> -		// occupied military buildings get special treatment
> -		// (extended grace time + longer radius)
> -		bool occupied_military_ = false;
> -		Building* b = flag.get_building();
> -		if (upcast(MilitarySite, militb, b)) {
> -			if (militb->soldier_control()->stationed_soldiers().size() > 0) {
> -				occupied_military_ = true;
> -			}
> -		}
> -
> -		// check if we are within grace time, if not or gracetime unset we need to do something
> -		// if we are within gracetime we do nothing (this loop is skipped)
> -
> -		// if grace time is not set, this is probably first time without a warehouse and we must set
> -		// it
> -		if (eco->dismantle_grace_time == std::numeric_limits<uint32_t>::max()) {
> -
> -			// constructionsites
> -			if (upcast(ConstructionSite const, constructionsite, flag.get_building())) {
> -				BuildingObserver& bo =
> -				   get_building_observer(constructionsite->building().name().c_str());
> -				// first very special case - a port (in the phase of constructionsite)
> -				// this might be a new colonization port
> -				if (bo.is(BuildingAttribute::kPort)) {
> -					eco->dismantle_grace_time = gametime + 60 * 60 * 1000;  // one hour should be enough
> -				} else {  // other constructionsites, usually new (standalone) constructionsites
> -					eco->dismantle_grace_time =
> -					   gametime + 30 * 1000 +            // very shot time is enough
> -					   (eco->flags.size() * 30 * 1000);  // + 30 seconds for every flag in economy
> -				}
> -
> -				// buildings
> -			} else {
> -
> -				if (occupied_military_) {
> -					eco->dismantle_grace_time =
> -					   (gametime + 90 * 60 * 1000) + (eco->flags.size() * 20 * 1000);
> -
> -				} else {  // for other normal buildings
> -					eco->dismantle_grace_time =
> -					   gametime + (45 * 60 * 1000) + (eco->flags.size() * 20 * 1000);
> -				}
> -			}
> -
> -			// we have passed grace_time - it is time to dismantle
> -		} else if (eco->dismantle_grace_time <= gametime) {
> -			last_attempt_ = true;
> -			// we increase a check radius in last attempt
> -			checkradius += 2;
> -		}
> -
> -		// and bonus for occupied military buildings:
> -		if (occupied_military_) {
> -			checkradius += 4;
> -		}
> -
> -		// and generally increase radius for unconnected buildings
> -		checkradius += 2;
> -	}
> -
> -	// Now own roadfinding stuff
> -	const Map& map = game().map();
> -
> -	// initializing new object of FlagsForRoads, we will push there all candidate flags
> -	// First we dont even know if a road can be built there (from current flag)
> -	Widelands::FlagsForRoads RoadCandidates(min_reduction);
> -
> -	FindNodeWithFlagOrRoad functor;
> -	CheckStepRoadAI check(player_, MOVECAPS_WALK, true);
> -
> -	// get all flags within radius
> -	std::vector<Coords> reachable;
> -	map.find_reachable_fields(
> -	   Area<FCoords>(map.get_fcoords(flag.get_position()), checkradius), &reachable, check, functor);
> -
> -	for (const Coords& reachable_coords : reachable) {
> -
> -		// ignore starting flag, of course
> -		if (reachable_coords == flag.get_position()) {
> -			continue;
> -		}
> -
> -		// first make sure there is an immovable (should be, but still)
> -		if (upcast(PlayerImmovable const, player_immovable, map[reachable_coords].get_immovable())) {
> -
> -			// if it is the road, make a flag there
> -			if (dynamic_cast<const Road*>(map[reachable_coords].get_immovable())) {
> -				game().send_player_build_flag(player_number(), reachable_coords);
> -			}
> -
> -			// do not go on if it is not a flag
> -			if (!dynamic_cast<const Flag*>(map[reachable_coords].get_immovable())) {
> -				continue;
> -			}
> -
> -			// testing if a flag/road's economy has a warehouse, if not we are not
> -			// interested to connect to it
> -			if (player_immovable->economy().warehouses().size() == 0) {
> -				continue;
> -			}
> -
> -			// This is a candidate, sending all necessary info to RoadCandidates
> -			const bool different_economy = (player_immovable->get_economy() != flag.get_economy());
> -			const int32_t air_distance = map.calc_distance(flag.get_position(), reachable_coords);
> -			if (!RoadCandidates.has_candidate(reachable_coords.hash())) {
> -				RoadCandidates.add_flag(reachable_coords, air_distance, different_economy);
> -			}
> -		}
> -	}
> -
> -	// now we walk over roads and if field is reachable by roads, we change the distance assigned
> -	// above
> -	std::map<uint32_t, NearFlag> nearflags;  // only used to collect flags reachable walk over roads
> -	nearflags[flag.get_position().hash()] = NearFlag(&flag, 0);
> -
> -	// Algorithm to walk on roads
> -	// All nodes are marked as to_be_checked == true first and once the node is checked it is changed
> -	// to false. Under some conditions, the same node can be checked twice, the to_be_checked can
> -	// be set back to true. Because less hoops (fewer flag-to-flag roads) does not always mean
> -	// shortest
> -	// road.
> -	for (;;) {
> -		// looking for a node with shortest existing road distance from starting flag and one that has
> -		// to be checked
> -		uint32_t start_field = std::numeric_limits<uint32_t>::max();
> -		uint32_t nearest_distance = 10000;
> -		for (auto item : nearflags) {
> -			if (item.second.current_road_distance < nearest_distance && item.second.to_be_checked) {
> -				nearest_distance = item.second.current_road_distance;
> -				start_field = item.first;
> -			}
> -		}
> -		// OK, we failed to find a NearFlag where to_be_checked == true, so quitting the loop now
> -		if (start_field == std::numeric_limits<uint32_t>::max()) {
> -			break;
> -		}
> -
> -		nearflags[start_field].to_be_checked = false;
> -
> -		// Now going over roads leading from this flag
> -		for (uint8_t i = 1; i <= 6; ++i) {
> -			Road* const road = nearflags[start_field].flag->get_road(i);
> -
> -			if (!road) {
> -				continue;
> -			}
> -
> -			Flag* endflag = &road->get_flag(Road::FlagStart);
> -
> -			if (endflag == nearflags[start_field].flag) {
> -				endflag = &road->get_flag(Road::FlagEnd);
> -			}
> -
> -			const uint32_t endflag_hash = endflag->get_position().hash();
> -
> -			const int32_t dist = map.calc_distance(flag.get_position(), endflag->get_position());
> -
> -			if (dist > checkradius + 2) {  //  Testing bigger vicinity then checkradius....
> -				continue;
> -			}
> -
> -			// There is few scenarios for this neighbour flag
> -			if (nearflags.count(endflag_hash) == 0) {
> -				// This is brand new flag
> -				nearflags[endflag_hash] =
> -				   NearFlag(endflag, nearflags[start_field].current_road_distance +
> -				                        road->get_path().get_nsteps());
> -			} else {
> -				// We know about this flag already
> -				if (nearflags[endflag_hash].current_road_distance >
> -				    nearflags[start_field].current_road_distance + road->get_path().get_nsteps()) {
> -					// ..but this current connection is shorter than one found before
> -					nearflags[endflag_hash].current_road_distance =
> -					   nearflags[start_field].current_road_distance + road->get_path().get_nsteps();
> -					// So let re-check neighbours once more
> -					nearflags[endflag_hash].to_be_checked = true;
> -				}
> -			}
> -		}
> -	}
> -
> -	// Sending calculated walking costs from nearflags to RoadCandidates to update info on
> -	// Candidate flags/roads
> -	for (auto& nf_walk : nearflags) {
> -		// NearFlag contains also flags beyond check radius, these are not relevent for us
> -		if (RoadCandidates.has_candidate(nf_walk.second.flag->get_position().hash())) {
> -			RoadCandidates.set_cur_road_distance(
> -			   nf_walk.second.flag->get_position(), nf_walk.second.current_road_distance);
> -		}
> -	}
> -
> -	// Here we must consider how much are buildable fields lacking
> -	// the number will be transformed to a weight passed to findpath function
> -	int32_t fields_necessity = 0;
> -	if (spots_ < kSpotsTooLittle) {
> -		fields_necessity += 10;
> -	}
> -	if (map_allows_seafaring_ && num_ports == 0) {
> -		fields_necessity += 10;
> -	}
> -	if (num_ports < 4) {
> -		fields_necessity += 5;
> -	}
> -	if (spots_ < kSpotsEnough) {
> -		fields_necessity += 5;
> -	}
> -	fields_necessity *= std::abs(management_data.get_military_number_at(64)) * 5;
> -
> -	// We need to sort these flags somehow, because we are not going to investigate all of them
> -	// so sorting first by current road length (that might contain fake values for flags that were
> -	// not reached over roads) and secondary by air_distance (nearer flags first)
> -	std::sort(std::begin(RoadCandidates.flags_queue), std::end(RoadCandidates.flags_queue),
> -	          [](const FlagsForRoads::Candidate& a, const FlagsForRoads::Candidate& b) {
> -		          // Here we are doing a kind of bucketing
> -		          const int32_t a_length = a.current_road_length / 50;
> -		          const int32_t b_length = b.current_road_length / 50;
> -		          return std::tie(b_length, a.air_distance) < std::tie(a_length, b.air_distance);
> -	          });
> -
> -	// Now we calculate roads from here to few best looking RoadCandidates....
> -	uint32_t possible_roads_count = 0;
> -	uint32_t current = 0;  // hash of flag that we are going to calculate in the iteration
> -	while (possible_roads_count < 5 && RoadCandidates.get_best_uncalculated(&current)) {
> -		const Widelands::Coords coords = Coords::unhash(current);
> -		Path path;
> -
> -		// value of pathcost is not important, it just indicates, that the path can be built
> -		// We send this information to RoadCandidates, with length of possible road if applicable
> -		const int32_t pathcost =
> -		   map.findpath(flag.get_position(), coords, 0, path, check, 0, fields_necessity);
> -		if (pathcost >= 0) {
> -			RoadCandidates.road_possible(coords, path.get_nsteps());
> -			possible_roads_count += 1;
> -		}
> -	}
> -
> -	// re-sorting again, now by reduction score (custom operator specified in .h file)
> -	std::sort(std::begin(RoadCandidates.flags_queue), std::end(RoadCandidates.flags_queue));
> -
> -	// Well and finally building the winning road (if any)
> -	uint32_t winner_hash = 0;
> -	if (RoadCandidates.get_winner(&winner_hash)) {
> -		const Widelands::Coords target_coords = Coords::unhash(winner_hash);
> -		Path& path = *new Path();
> +    // Increasing the failed_connection_tries counter
> +    // At the same time it indicates a time an economy is without a warehouse
> +    EconomyObserver* eco = get_economy_observer(flag.economy());
> +    // if we passed grace time this will be last attempt and if it fails
> +    // building is destroyes

destroyes -> destroyed

> +    bool last_attempt_ = false;
> +
> +    // this should not happen, but if the economy has a warehouse and a dismantle
> +    // grace time set, we must 'zero' the dismantle grace time
> +    if (!flag.get_economy()->warehouses().empty() &&
> +        eco->dismantle_grace_time != kNever) {
> +        eco->dismantle_grace_time = kNever;
> +    }
> +
> +    // first we deal with situations when this is economy with no warehouses
> +    // and this is a flag belonging to a building/constructionsite
> +    // such economy must get dismantle grace time (if not set yet)
> +    // end sometimes extended checkradius
> +    if (flag.get_economy()->warehouses().empty() && flag.get_building()) {
> +
> +        // occupied military buildings get special treatment
> +        // (extended grace time + longer radius)
> +        bool occupied_military_ = false;
> +        Building* b = flag.get_building();
> +        if (upcast(MilitarySite, militb, b)) {
> +            if (militb->soldier_control()->stationed_soldiers().size() > 0) {
> +                occupied_military_ = true;
> +            }
> +        }
> +
> +        // check if we are within grace time, if not or gracetime unset we need to do something
> +        // if we are within gracetime we do nothing (this loop is skipped)
> +
> +        // if grace time is not set, this is probably first time without a warehouse and we must set
> +        // it
> +        if (eco->dismantle_grace_time == kNever) {
> +
> +            // constructionsites
> +            if (upcast(ConstructionSite const, constructionsite, flag.get_building())) {
> +                BuildingObserver& bo =
> +                   get_building_observer(constructionsite->building().name().c_str());
> +                // first very special case - a port (in the phase of constructionsite)
> +                // this might be a new colonization port
> +                if (bo.is(BuildingAttribute::kPort)) {
> +                    eco->dismantle_grace_time = gametime + 60 * 60 * 1000;  // one hour should be enough
> +                } else {  // other constructionsites, usually new (standalone) constructionsites
> +                    eco->dismantle_grace_time =
> +                       gametime + 30 * 1000 +            // very shot time is enough
> +                       (eco->flags.size() * 30 * 1000);  // + 30 seconds for every flag in economy
> +                }
> +
> +                // buildings
> +            } else {
> +
> +                if (occupied_military_) {
> +                    eco->dismantle_grace_time =
> +                       (gametime + 90 * 60 * 1000) + (eco->flags.size() * 20 * 1000);
> +
> +                } else {  // for other normal buildings
> +                    eco->dismantle_grace_time =
> +                       gametime + (45 * 60 * 1000) + (eco->flags.size() * 20 * 1000);
> +                }
> +            }
> +
> +            // we have passed grace_time - it is time to dismantle
> +        } else if (eco->dismantle_grace_time <= gametime) {
> +            last_attempt_ = true;
> +            // we increase a check radius in last attempt
> +            checkradius += 2;
> +        }
> +
> +        // and bonus for occupied military buildings:
> +        if (occupied_military_) {
> +            checkradius += 4;
> +        }
> +
> +        // and generally increase radius for unconnected buildings
> +        checkradius += 2;
> +    }
> +
> +    // Now own roadfinding stuff
> +    const Map& map = game().map();
> +
> +    // Initializing new object of FlagsForRoads, we will push there all candidate flags
> +    // First we dont even know if a road can be built there (from current flag)
> +    // Adding also distance of this flag to nearest wh
> +    uint32_t tmp_wh; // This information is not used, but we need it
> +    const uint32_t current_flag_dist_to_wh = flag_warehouse_distance.
> +            get_distance(flag.get_position().hash(), gametime, &tmp_wh);
> +
> +    FlagCandidates flag_candidates(current_flag_dist_to_wh);
> +
> +    FindNodeWithFlagOrRoad functor;
> +    CheckStepRoadAI check(player_, MOVECAPS_WALK, true);
> +
> +    // get all flags within radius
> +    std::vector<Coords> reachable;
> +    map.find_reachable_fields(
> +       Area<FCoords>(map.get_fcoords(flag.get_position()), checkradius), &reachable, check, functor);
> +
> +    for (const Coords& reachable_coords : reachable) {
> +
> +        // ignore starting flag, of course
> +        if (reachable_coords == flag.get_position()) {
> +            continue;
> +        }
> +
> +        const uint32_t reachable_coords_hash = reachable_coords.hash();
> +
> +        // first make sure there is an immovable (should be, but still)
> +        if (upcast(PlayerImmovable const, player_immovable, map[reachable_coords].get_immovable())) {

You could get map[reachable_coords].get_immovable() from the map only once and store it in a variable

> +
> +            // if it is the road, make a flag there
> +            if (dynamic_cast<const Road*>(map[reachable_coords].get_immovable())) {

map[reachable_coords].get_immovable()->descr().type == MapObjectType::ROAD will be more efficient. Same for flag below.

> +                game().send_player_build_flag(player_number(), reachable_coords);
> +            }
> +
> +            // do not go on if it is not a flag
> +            if (!dynamic_cast<const Flag*>(map[reachable_coords].get_immovable())) {
> +                continue;
> +            }
> +
> +            // testing if a flag/road's economy has a warehouse, if not we are not
> +            // interested to connect to it
> +            if (player_immovable->economy().warehouses().size() == 0) {
> +                continue;
> +            }
> +
> +            // This is a candidate, sending all necessary info to RoadCandidates
> +            const bool different_economy = (player_immovable->get_economy() != flag.get_economy());

different_economy -> is_different_economy

> +			const uint16_t air_distance = map.calc_distance(flag.get_position(), reachable_coords);
> +
> +            if (!flag_candidates.has_candidate(reachable_coords_hash) && !flag_warehouse_distance.get_road_prohibited(reachable_coords_hash, gametime)) {
> +                flag_candidates.add_flag(reachable_coords_hash, different_economy,
> +                flag_warehouse_distance.get_distance(reachable_coords_hash, gametime, &tmp_wh), air_distance);
> +            }
> +        }
> +    }
> +
> +    // now we walk over roads and if field is reachable by roads, we change the distance assigned
> +    // above
> +    std::map<uint32_t, NearFlag> nearflags;  // only used to collect flags reachable walk over roads
> +    nearflags[flag.get_position().hash()] = NearFlag(&flag, 0);
> +
> +    collect_nearflags(nearflags, flag, checkradius);
> +
> +

1 empty line will be enough here

> +
> +    // Sending calculated walking costs from nearflags to RoadCandidates to update info on
> +    // Candidate flags/roads
> +    for (auto& nf_walk : nearflags) {
> +		const uint32_t nf_hash = nf_walk.second.flag->get_position().hash();
> +        // NearFlag contains also flags beyond check radius, these are not relevent for us

relevent -> relevant

> +        if (flag_candidates.has_candidate(nf_hash)) {
> +            flag_candidates.set_cur_road_distance(
> +               nf_hash, nf_walk.second.current_road_distance);
> +        }
> +    }
> +
> +    // Here we must consider how much are buildable fields lacking
> +    // the number will be transformed to a weight passed to findpath function
> +    int32_t fields_necessity = 0;
> +    if (spots_ < kSpotsTooLittle) {
> +        fields_necessity += 10;
> +    }
> +    if (map_allows_seafaring_ && num_ports == 0) {
> +        fields_necessity += 10;
> +    }
> +    if (num_ports < 4) {
> +        fields_necessity += 5;
> +    }
> +    if (spots_ < kSpotsEnough) {
> +        fields_necessity += 5;
> +    }
> +
> +    fields_necessity *= std::abs(management_data.get_military_number_at(64)) * 5;
> +
> +    // Now we calculate roads from here to few best looking RoadCandidates....
> +    flag_candidates.sort_by_air_distance();
> +    uint32_t possible_roads_count = 0;
> +    for (const auto &flag_candidate : flag_candidates.flags()) {
> +        if (possible_roads_count > 10) {break;}
> +        const Widelands::Coords coords = Coords::unhash(flag_candidate.coords_hash);
> +        Path path;
> +
> +        // value of pathcost is not important, it just indicates, that the path can be built
> +        // We send this information to RoadCandidates, with length of possible road if applicable
> +        const int32_t pathcost =
> +           map.findpath(flag.get_position(), coords, 0, path, check, 0, fields_necessity);
> +        if (pathcost >= 0) {
> +            flag_candidates.set_road_possible(flag_candidate.coords_hash, path.get_nsteps());
> +            possible_roads_count += 1;

++possible_roads_count;

> +        }
> +    }
> +
> +    // re-sorting again now by default by a score
> +    flag_candidates.sort();
> +
> +
> +    // Well and finally building the winning road (if any)
> +    const int32_t winner_min_score = (spots_ < kSpotsTooLittle) ? 50 : 25;
> +
> +    FlagCandidates::Candidate* winner = flag_candidates.get_winner(winner_min_score);
> +    if (winner) {
> +        const Widelands::Coords target_coords = Coords::unhash(winner->coords_hash);
> +
> +		// This is to prohibit the flag for some time but with exemption of warehouse
> +		if (flag_warehouse_distance.get_distance(winner->coords_hash, gametime, &tmp_wh) > 0) {
> +			flag_warehouse_distance.set_road_built(winner->coords_hash, gametime);
> +		}
> +		// and we stright away set distance of future flag

stright -> straight

> +		flag_warehouse_distance.set_distance(flag.get_position().hash(), winner->start_flag_dist_to_wh + winner->possible_road_distance,
> +		gametime, 0); // faking the warehouse
> +
> +        Path& path = *new Path();
>  #ifndef NDEBUG
> -		const int32_t pathcost =
> -		   map.findpath(flag.get_position(), target_coords, 0, path, check, 0, fields_necessity);
> -		assert(pathcost >= 0);
> +        const int32_t pathcost =
> +           map.findpath(flag.get_position(), target_coords, 0, path, check, 0, fields_necessity);
> +        assert(pathcost >= 0);
>  #else
> -		map.findpath(flag.get_position(), target_coords, 0, path, check, 0, fields_necessity);
> +        map.findpath(flag.get_position(), target_coords, 0, path, check, 0, fields_necessity);
>  #endif
> -		game().send_player_build_road(player_number(), path);
> -		return true;
> -	}
> -	// We can't build a road so let's block the vicinity as an indication this area is not
> -	// connectible
> -	// Usually we block for 2 minutes, but if it is a last attempt we block for 10 minutes
> -	// Note: we block the vicinity only if this economy (usually a sole flag with a building) is not
> -	// connected to a warehouse
> -	if (flag.get_economy()->warehouses().empty()) {
> -
> -		// blocking only if latest block was less then 60 seconds ago or it is last attempt
> -		if (eco->fields_block_last_time + 60000 < gametime || last_attempt_) {
> -			eco->fields_block_last_time = gametime;
> -
> -			const uint32_t block_time = last_attempt_ ? 10 * 60 * 1000 : 2 * 60 * 1000;
> -
> -			FindNodeAcceptAll buildable_functor;
> -			CheckStepOwnTerritory check_own(player_, MOVECAPS_WALK, true);
> -
> -			// get all flags within radius
> -			std::vector<Coords> reachable_to_block;
> -			map.find_reachable_fields(Area<FCoords>(map.get_fcoords(flag.get_position()), checkradius),
> -			                          &reachable_to_block, check_own, buildable_functor);
> -
> -			for (auto coords : reachable_to_block) {
> -				blocked_fields.add(coords, game().get_gametime() + block_time);
> -			}
> -		}
> -
> -		// If it last attempt we also destroy the flag (with a building if any attached)
> -		if (last_attempt_) {
> -			remove_from_dqueue<Widelands::Flag>(eco->flags, &flag);
> -			game().send_player_bulldoze(*const_cast<Flag*>(&flag));
> -			dead_ends_check_ = true;
> -			return true;
> -		}
> -	}
> -	return false;
> +        game().send_player_build_road(player_number(), path);
> +        return true;
> +    }
> +    // We can't build a road so let's block the vicinity as an indication this area is not
> +    // connectible
> +    // Usually we block for 2 minutes, but if it is a last attempt we block for 10 minutes
> +    // Note: we block the vicinity only if this economy (usually a sole flag with a building) is not
> +    // connected to a warehouse
> +    if (flag.get_economy()->warehouses().empty()) {
> +
> +        // blocking only if latest block was less then 60 seconds ago or it is last attempt
> +        if (eco->fields_block_last_time + kOneMinute < gametime || last_attempt_) {
> +            eco->fields_block_last_time = gametime;
> +
> +            const uint32_t block_time = last_attempt_ ? 10 * kOneMinute : 2 * kOneMinute;
> +
> +            FindNodeAcceptAll buildable_functor;
> +            CheckStepOwnTerritory check_own(player_, MOVECAPS_WALK, true);
> +
> +            // get all flags within radius
> +            std::vector<Coords> reachable_to_block;
> +            map.find_reachable_fields(Area<FCoords>(map.get_fcoords(flag.get_position()), checkradius),
> +                                      &reachable_to_block, check_own, buildable_functor);
> +
> +            for (auto coords : reachable_to_block) {
> +                blocked_fields.add(coords, game().get_gametime() + block_time);
> +            }
> +        }
> +
> +        // If it last attempt we also destroy the flag (with a building if any attached)
> +        if (last_attempt_) {
> +            remove_from_dqueue<Widelands::Flag>(eco->flags, &flag);
> +            game().send_player_bulldoze(*const_cast<Flag*>(&flag));
> +            dead_ends_check_ = true;
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +void DefaultAI::collect_nearflags(std::map<uint32_t, NearFlag> &nearflags, const Flag& flag, const uint16_t checkradius) {
> +    // Algorithm to walk on roads
> +    // All nodes are marked as to_be_checked == true first and once the node is checked it is changed
> +    // to false. Under some conditions, the same node can be checked twice, the to_be_checked can
> +    // be set back to true. Because less hoops (fewer flag-to-flag roads) does not always mean
> +    // shortest
> +    // road.
> +
> +	const Map& map = game().map();
> +
> +    for (;;) {
> +        // looking for a node with shortest existing road distance from starting flag and one that has
> +        // to be checked
> +        uint32_t start_field = kNoField;
> +        uint32_t nearest_distance = 10000;
> +        for (auto item : nearflags) {
> +            if (item.second.current_road_distance < nearest_distance && item.second.to_be_checked) {
> +                nearest_distance = item.second.current_road_distance;
> +                start_field = item.first;
> +            }
> +        }
> +        // OK, we failed to find a NearFlag where to_be_checked == true, so quitting the loop now
> +        if (start_field == kNoField) {
> +            break;

This seems to be the only condition to exit the loop. Is this on purpose?

> +        }
> +
> +        nearflags[start_field].to_be_checked = false;
> +
> +        // Now going over roads leading from this flag
> +        for (uint8_t i = 1; i <= 6; ++i) {
> +            Road* const road = nearflags[start_field].flag->get_road(i);
> +
> +            if (!road) {
> +                continue;
> +            }
> +
> +            Flag* endflag = &road->get_flag(Road::FlagStart);
> +
> +            if (endflag == nearflags[start_field].flag) {
> +                endflag = &road->get_flag(Road::FlagEnd);
> +            }
> +
> +            const uint32_t endflag_hash = endflag->get_position().hash();
> +
> +            const int32_t dist = map.calc_distance(flag.get_position(), endflag->get_position());
> +
> +            if (dist > checkradius + 2) {  //  Testing bigger vicinity then checkradius....
> +                continue;
> +            }
> +
> +            // There is few scenarios for this neighbour flag
> +            if (nearflags.count(endflag_hash) == 0) {
> +                // This is brand new flag
> +                // calculating diff how much closer we will get to the flag
> +                nearflags[endflag_hash] =
> +                   NearFlag(endflag, nearflags[start_field].current_road_distance +
> +                                        road->get_path().get_nsteps());
> +            } else {
> +                // We know about this flag already
> +                if (nearflags[endflag_hash].current_road_distance >
> +                    nearflags[start_field].current_road_distance + road->get_path().get_nsteps()) {
> +                    // ..but this current connection is shorter than one found before
> +                    nearflags[endflag_hash].current_road_distance =
> +                       nearflags[start_field].current_road_distance + road->get_path().get_nsteps();
> +                    // So let re-check neighbours once more
> +                    nearflags[endflag_hash].to_be_checked = true;
> +                }
> +            }
> +        }
> +    }
> +
>  }
>  
>  /**


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


References