← Back to team overview

widelands-dev team mailing list archive

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

 

Looks like a good solution to me :)

I have added a bunch of small nits with the comments function.

Diff comments:

> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2015-02-16 20:23:15 +0000
> +++ src/ai/defaultai.cc	2015-03-04 20:14:13 +0000
> @@ -50,12 +50,6 @@
>  #include "logic/world/world.h"
>  #include "profile/profile.h"
>  
> -// Building of new military buildings can be restricted
> -constexpr int kPushExpansion = 1;
> -constexpr int kResourcesOrDefense = 2;
> -constexpr int kDefenseOnly = 3;
> -constexpr int kNoNewMilitary = 4;
> -
>  // following is in miliseconds (widelands counts time in ms)
>  constexpr int kFieldUpdateInterval = 2000;
>  constexpr int kIdleMineUpdateInterval = 22000;
> @@ -65,7 +59,7 @@
>  constexpr int kMinBFCheckInterval = 5 * 1000;
>  constexpr int kShipCheckInterval = 5 * 1000;
>  constexpr int kMarineDecisionInterval = 20 * 1000;
> -constexpr int kTrainingSitesCheckInterval = 30 * 1000;
> +constexpr int kTrainingSitesCheckInterval = 5 * 60 * 1000;
>  
>  // this is intended for map developers, by default should be off
>  constexpr bool kPrintStats = false;
> @@ -85,28 +79,16 @@
>  DefaultAI::DefaultAI(Game& ggame, PlayerNumber const pid, uint8_t const t)
>     : ComputerPlayer(ggame, pid),
>       type_(t),
> -     m_buildable_changed(true),
> -     m_mineable_changed(true),
> +     // m_buildable_changed(true),
> +     // m_mineable_changed(true),
>       player_(nullptr),

Remove

>       tribe_(nullptr),
>       num_constructionsites_(0),
>       num_milit_constructionsites(0),
>       num_prod_constructionsites(0),
>       num_ports(0),
> -     next_road_due_(2000),
> -     next_stats_update_due_(30000),
> -     next_construction_due_(1000),
> +     next_ai_think_(0),
>       next_mine_construction_due_(0),
> -     next_productionsite_check_due_(0),
> -     next_mine_check_due_(0),
> -     next_militarysite_check_due_(0),
> -     next_ship_check_due(30 * 1000),
> -     next_marine_decisions_due(30 * 1000),
> -     next_attack_consideration_due_(300000),
> -     next_trainingsites_check_due_(15 * 60 * 1000),
> -     next_bf_check_due_(1000),
> -     next_wares_review_due_(15 * 60 * 1000),
> -     next_statistics_report_(30 * 60 * 1000),
>       inhibit_road_building_(0),
>       time_of_last_construction_(0),
>       enemy_last_seen_(-2 * 60 * 1000),
> @@ -202,7 +184,7 @@
>  				   }
>  			   }
>  			   break;
> -		   	default:
> +		   default:
>  				;
>  		   }
>  		});
> @@ -238,111 +220,79 @@
>  
>  	const int32_t gametime = game().get_gametime();
>  
> -	if (m_buildable_changed || next_bf_check_due_ < gametime) {
> -		// update statistics about buildable fields
> +	if (next_ai_think_ > gametime) {
> +		return;
> +	}
> +
> +	// AI now thinks twice in a seccond, if the game engine allows this
> +	// if too busy, the period can be many seconds.
> +	next_ai_think_ = gametime + 500;
> +	ScheduleTasks DueTask = ScheduleTasks::kIdle;
> +	DueTask = get_oldest_task(gametime);
> +	schedStat[static_cast<uint32_t>(DueTask)] += 1;
> +
> +	// now AI runs a job selected above to be performed in this turn
> +	// (only one but some of them needs to run check_economies() to
> +	// guarantee consistency)
> +	// job names are selfexplanatory
> +	if (DueTask == ScheduleTasks::kBFCheck) {
>  		update_all_buildable_fields(gametime);
> -		next_bf_check_due_ = gametime + kMinBFCheckInterval;
> -	}
> -
> -	m_buildable_changed = false;
> -
> -	// perpetually tries to improve roads
> -	if (next_road_due_ <= gametime) {
> -		next_road_due_ = gametime + 1000;
> -
> -		if (improve_roads(gametime)) {
> -			m_buildable_changed = true;
> +		taskDue[ScheduleTasks::kBFCheck] = gametime + kMinBFCheckInterval;
> +	} else if (DueTask == ScheduleTasks::kRoadCheck) {

The code would be easier to read if you renamed BF to BuildableFields

> +		if (check_economies()) {  // is a must
>  			return;
> -		}
> -	} else {
> -		// only go on, after defaultAI tried to improve roads.
> -		return;
> -	}
> -
> -	// NOTE Because of the check above, the following parts of think() are used
> -	// NOTE only once every second at maximum. This increases performance and as
> -	// NOTE human player_s can not even react that fast, it should not be a
> -	// NOTE disadvantage for the defaultAI.
> -	// This must be checked every time as changes of bobs in AI area aren't
> -	// handled by the AI itself.
> -	update_all_not_buildable_fields();
> -
> -	// considering attack
> -	if (next_attack_consideration_due_ <= gametime) {
> +		};
> +		taskDue[ScheduleTasks::kRoadCheck] = gametime + 400;
> +		improve_roads(gametime);
> +	} else if (DueTask == ScheduleTasks::kUnbuildableFCheck) {
> +		taskDue[ScheduleTasks::kUnbuildableFCheck] = gametime + 4000;
> +		update_all_not_buildable_fields();
> +	} else if (DueTask == ScheduleTasks::kConsiderAttack) {
>  		consider_attack(gametime);
> -	}
> -
> -	// check if anything in the economies changed.
> -	// This needs to be done before new buildings are placed, to ensure that no
> -	// empty economy is left.
> -	if (check_economies()) {
> -		return;
> -	}
> -
> -	// Before thinking about a new construction, update current stats, to have
> -	// a better view on current economy.
> -	if (next_stats_update_due_ <= gametime) {
> +	} else if (DueTask == ScheduleTasks::kCheckEconomies) {
> +		check_economies();
> +		taskDue[ScheduleTasks::kCheckEconomies] = gametime + 8000;
> +	} else if (DueTask == ScheduleTasks::kProductionsitesStats) {
>  		update_productionsite_stats(gametime);
> -	}
> -
> -	// Now try to build something if possible
> -	if (next_construction_due_ <= gametime) {
> -		next_construction_due_ = gametime + 2000;
> -
> +	} else if (DueTask == ScheduleTasks::kConstructBuilding) {
> +		if (check_economies()) {  // economies must be consistent
> +			return;
> +		}
> +		taskDue[ScheduleTasks::kConstructBuilding] = gametime + 6000;
>  		if (construct_building(gametime)) {
>  			time_of_last_construction_ = gametime;
> -			m_buildable_changed = true;
> -			return;
> -		}
> -	}
> -
> -	// verify that our production sites are doing well
> -	if (check_productionsites(gametime)) {
> -		return;
> -	}
> -
> -	if (check_ships(gametime)) {
> -		return;
> -	}
> -
> -	if (marine_main_decisions(gametime)) {
> -		return;
> -	}
> -
> -	// Check the mines and consider upgrading or destroying one
> -	if (check_mines_(gametime)) {
> -		return;
> -	}
> -
> -	// consider whether a change of the soldier capacity of some militarysites
> -	// would make sense.
> -	if (check_militarysites(gametime)) {
> -		return;
> -	}
> -
> -	if (check_trainingsites(gametime)) {
> -		return;
> -	}
> -
> -	// improve existing roads!
> -	// main part of this improvment is creation 'shortcut roads'
> -	// this includes also connection of new buildings
> -	if (improve_roads(gametime)) {
> -		m_buildable_changed = true;
> -		m_mineable_changed = true;
> -		return;
> -	}
> -
> -	// once in 15 minutes we increase(or decrease) targets for wares
> -	if (next_wares_review_due_ <= gametime) {
> -		next_wares_review_due_ = gametime + 15 * 60 * 1000;
> +		}
> +	} else if (DueTask == ScheduleTasks::kCheckProductionsites) {
> +		if (check_economies()) {  // economies must be consistent
> +			return;
> +		}
> +		check_productionsites(gametime);
> +		taskDue[ScheduleTasks::kCheckProductionsites] = gametime + 5000;
> +	} else if (DueTask == ScheduleTasks::kCheckShips) {
> +		check_ships(gametime);
> +	} else if (DueTask == ScheduleTasks::KMarineDecisions) {
> +		marine_main_decisions(gametime);
> +	} else if (DueTask == ScheduleTasks::kCheckMines) {
> +		if (check_economies()) {  // economies must be consistent
> +			return;
> +		}
> +		taskDue[ScheduleTasks::kCheckMines] = gametime + 7000;  // 7 seconds is enough
> +		check_mines_(gametime);
> +	} else if (DueTask == ScheduleTasks::kCheckMilitarysites) {
> +		check_militarysites(gametime);
> +	} else if (DueTask == ScheduleTasks::kCheckTrainingsites) {
> +		check_trainingsites(gametime);
> +	} else if (DueTask == ScheduleTasks::kWareReview) {
> +		if (check_economies()) {  // economies must be consistent
> +			return;
> +		}
> +		taskDue[ScheduleTasks::kWareReview] = gametime + 15 * 60 * 1000;
>  		review_wares_targets(gametime);
> -	}
> -
> -	// print statistics
> -	if (kPrintStats && next_statistics_report_ <= gametime) {
> -		print_stats();
> -		next_statistics_report_ += 60 * 60 * 1000;
> +	} else if (DueTask == ScheduleTasks::kprintStats) {
> +		if (check_economies()) {  // economies must be consistent
> +			return;
> +		}
> +		print_stats(gametime);
>  	}
>  }
>  
> @@ -500,13 +450,6 @@
>  		}
>  	}
>  
> -	num_constructionsites_ = 0;
> -	num_milit_constructionsites = 0;
> -	num_prod_constructionsites = 0;
> -	next_construction_due_ = 0;
> -	next_road_due_ = 1000;
> -	next_productionsite_check_due_ = 0;
> -	inhibit_road_building_ = 0;
>  	// atlanteans they consider water as a resource
>  	// (together with mines, stones and wood)
>  	if (tribe_->name() == "atlanteans") {
> @@ -524,7 +467,7 @@
>  				port_reserved_coords.insert(hash);
>  		} while (mr.advance(map));
>  
> -		//the same for NW neighbour of a field
> +		// the same for NW neighbour of a field
>  		Coords c_nw;
>  		map.get_tln(c, &c_nw);
>  		MapRegion<Area<FCoords>> mr_nw(map, Area<FCoords>(map.get_fcoords(c_nw), 3));
> @@ -593,6 +536,22 @@
>  			} while (mr.advance(map));
>  		}
>  	}
> +
> +	taskDue[ScheduleTasks::kConstructBuilding] = 0;
> +	taskDue[ScheduleTasks::kRoadCheck] = 1000;
> +	taskDue[ScheduleTasks::kCheckProductionsites] = 15 * 1000;
> +	taskDue[ScheduleTasks::kProductionsitesStats] = 30000;
> +	taskDue[ScheduleTasks::kCheckMines] = 30 * 1000;
> +	taskDue[ScheduleTasks::kCheckMilitarysites] = 0;
> +	taskDue[ScheduleTasks::kCheckShips] = 30 * 1000;
> +	taskDue[ScheduleTasks::kCheckEconomies] = 1000;
> +	taskDue[ScheduleTasks::KMarineDecisions] = 30 * 1000;
> +	taskDue[ScheduleTasks::kConsiderAttack] = 300000;
> +	taskDue[ScheduleTasks::kCheckTrainingsites] = 15 * 60 * 1000;
> +	taskDue[ScheduleTasks::kBFCheck] = 1000;
> +	taskDue[ScheduleTasks::kUnbuildableFCheck] = 1000;
> +	taskDue[ScheduleTasks::kWareReview] = 15 * 60 * 1000;
> +	taskDue[ScheduleTasks::kprintStats] = 30 * 60 * 1000;
>  }

kPrintStats - capital P

>  
>  /**
> @@ -606,7 +565,7 @@
>  	uint16_t i = 0;
>  
>  	while (!buildable_fields.empty() && buildable_fields.front()->next_update_due_ <= gametime &&
> -	       i < 25) {
> +	       i < 40) {
>  		BuildableField& bf = *buildable_fields.front();
>  
>  		//  check whether we lost ownership of the node
> @@ -753,7 +712,7 @@
>  		}
>  	}
>  
> -	//testing for near porspaces
> +	// testing for near porspaces
>  	if (field.portspace_nearby_ == Widelands::ExtendedBool::kUnset) {
>  		field.portspace_nearby_ = ExtendedBool::kFalse;
>  		MapRegion<Area<FCoords>> mr(map, Area<FCoords>(field.coords, 2));
> @@ -1012,7 +971,7 @@
>  /// Updates the production and MINE sites statistics needed for construction decision.
>  void DefaultAI::update_productionsite_stats(int32_t const gametime) {
>  	// Updating the stats every 10 seconds should be enough
> -	next_stats_update_due_ = gametime + 10000;
> +	taskDue[ScheduleTasks::kProductionsitesStats] = gametime + 10000;
>  	uint16_t fishers_count = 0;  // used for atlanteans only
>  
>  	// Reset statistics for all buildings
> @@ -1115,7 +1074,7 @@
>  
>  	// here we possible stop building of new buildings
>  	new_buildings_stop_ = false;
> -	uint8_t expansion_mode = kResourcesOrDefense;
> +	MilitaryStrategy expansion_mode = MilitaryStrategy::kResourcesOrDefense;
>  
>  	// there are many reasons why to stop building production buildings
>  	// (note there are numerous exceptions)
> @@ -1146,13 +1105,15 @@
>  	const uint32_t treshold = militarysites.size() / 40 + 2;
>  
>  	if (unstationed_milit_buildings_ + num_milit_constructionsites > 3 * treshold) {
> -		expansion_mode = kNoNewMilitary;
> +		expansion_mode = MilitaryStrategy::kNoNewMilitary;
>  	} else if (unstationed_milit_buildings_ + num_milit_constructionsites > 2 * treshold) {
> -		expansion_mode = kDefenseOnly;
> +		expansion_mode = MilitaryStrategy::kDefenseOnly;
> +	} else if (unstationed_milit_buildings_ + num_milit_constructionsites >= treshold - 1) {
> +		expansion_mode = MilitaryStrategy::kResourcesOrDefense;
>  	} else if (unstationed_milit_buildings_ + num_milit_constructionsites >= 1) {
> -		expansion_mode = kResourcesOrDefense;
> +		expansion_mode = MilitaryStrategy::kExpansion;
>  	} else {
> -		expansion_mode = kPushExpansion;
> +		expansion_mode = MilitaryStrategy::kPushExpansion;
>  	}
>  
>  	// we must consider need for mines
> @@ -1163,9 +1124,9 @@
>  	if (virtual_mines <= 7) {
>  		resource_necessity_mines_ = std::numeric_limits<uint8_t>::max();
>  	} else if (virtual_mines > 19) {
> -		resource_necessity_mines_ = 0;
> +		resource_necessity_mines_ = 50;
>  	} else {
> -		const uint32_t tmp = ((18 - virtual_mines) * 255) / 12;
> +		const uint32_t tmp = (24 - virtual_mines) * 10;
>  		resource_necessity_mines_ = tmp;
>  	}
>  
> @@ -1246,7 +1207,7 @@
>  	     ++i) {
>  		BuildableField* const bf = *i;
>  
> -		if (bf->next_update_due_ < gametime - 8000) {
> +		if (bf->next_update_due_ < gametime - 10000) {
>  			continue;
>  		}
>  
> @@ -1518,7 +1479,7 @@
>  						}
>  						// 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() * 5) {
> +						         bo.stocklevel_ > 40 + productionsites.size() * 2) {
>  							continue;
>  						}
>  
> @@ -1679,11 +1640,18 @@
>  					continue;
>  				}
>  
> -				if (expansion_mode == kNoNewMilitary) {
> -					continue;
> -				}
> -
> -				if (expansion_mode == kDefenseOnly && !bf->enemy_nearby_) {
> +				if (expansion_mode == MilitaryStrategy::kNoNewMilitary) {
> +					continue;
> +				}
> +
> +				if (expansion_mode == MilitaryStrategy::kDefenseOnly && !bf->enemy_nearby_) {
> +					continue;
> +				}
> +
> +				if (expansion_mode == MilitaryStrategy::kResourcesOrDefense &&
> +				    !(bf->unowned_mines_pots_nearby_ || bf->stones_nearby_ || bf->water_nearby_ ||
> +				      (bf->distant_water_ && resource_necessity_water_needed_) ||
> +				      bf->enemy_nearby_)) {
>  					continue;
>  				}
>  
> @@ -1694,19 +1662,25 @@
>  				         (bo.mountain_conqueror_ || bo.expansion_type_)) {
>  					;
>  				}  // it is ok, go on
> -				else if (bf->unowned_land_nearby_ && bo.expansion_type_ &&
> -				         num_milit_constructionsites <= 1) {
> -					;  // we allow big buildings now
> -				} else if (bf->unowned_land_nearby_ &&
> -				           bo.expansion_type_) {  // decreasing probability for big buidlings
> +				else if (bo.expansion_type_) {
>  					if (bo.desc->get_size() == 2 && gametime % 2 >= 1) {
>  						continue;
>  					}
>  					if (bo.desc->get_size() == 3 && gametime % 3 >= 1) {
>  						continue;
> -					}
> +					};
>  				}
> -				// it is ok, go on
> +				//;  // we allow big buildings now
> +				//} else if (bf->unowned_land_nearby_ &&
> +				// bo.expansion_type_) {  // decreasing probability for big buidlings
> +				// if (bo.desc->get_size() == 2 && gametime % 2 >= 1) {
> +				// continue;
> +				//}
> +				// if (bo.desc->get_size() == 3 && gametime % 3 >= 1) {
> +				// continue;
> +				//}
> +				//}
> +				//// it is ok, go on
>  				else {

Remove

>  					continue;
>  				}  // the building is not suitable for situation
> @@ -1719,22 +1693,22 @@
>  
>  				// a boost to prevent an expansion halt
>  				int32_t local_boost = 0;
> -				if (expansion_mode == kPushExpansion) {
> +				if (expansion_mode == MilitaryStrategy::kPushExpansion) {
>  					local_boost = 200;
>  				}
>  
> -				prio = (bf->unowned_land_nearby_ * 2 * resource_necessity_territory_ / 255 +
> -				        bf->unowned_mines_pots_nearby_ * resource_necessity_mines_ / 255 +
> +				prio = ((bf->unowned_land_nearby_ * 2 * resource_necessity_territory_) / 255 +
> +				        (bf->unowned_mines_pots_nearby_ * resource_necessity_mines_) / 255 +
>  				        bf->stones_nearby_ / 2 + bf->military_loneliness_ / 10 - 60 + local_boost +
> -				        bf->water_nearby_ * resource_necessity_water_ / 255);
> +				        (bf->water_nearby_ * resource_necessity_water_) / 255);
>  
>  				// special bonus due to remote water for atlanteans
>  				if (resource_necessity_water_needed_)
> -					prio += bf->distant_water_ * resource_necessity_water_ / 255;
> +					prio += (bf->distant_water_ * resource_necessity_water_) / 255;
>  
> -				//special bonus if a portspace is close
> +				// special bonus if a portspace is close
>  				if (bf->portspace_nearby_ == ExtendedBool::kTrue) {
> -					if (num_ports == 0){
> +					if (num_ports == 0) {
>  						prio += 25;
>  					} else {
>  						prio += 5;
> @@ -2044,9 +2018,6 @@
>  	// set the type of update that is needed
>  	if (mine) {
>  		next_mine_construction_due_ = gametime + kBusyMineUpdateInterval;
> -
> -	} else {
> -		m_buildable_changed = true;
>  	}
>  
>  	return true;
> @@ -2093,7 +2064,7 @@
>  		roads.pop_front();
>  
>  		// occasionaly we test if the road can be dismounted
> -		if (gametime % 25 == 0) {
> +		if (gametime % 5 == 0) {
>  			const Road& road = *roads.front();
>  			if (dispensable_road_test(*const_cast<Road*>(&road))) {
>  				game().send_player_bulldoze(*const_cast<Road*>(&road));
> @@ -2134,14 +2105,25 @@
>  		return true;
>  	}
>  
> +	bool is_warehouse = false;
> +	if (Building* b = flag.get_building()) {
> +		BuildingObserver& bo = get_building_observer(b->descr().name().c_str());
> +		if (bo.type == BuildingObserver::WAREHOUSE) {
> +			is_warehouse = true;
> +		}
> +	}
> +
>  	// if this is end flag (or sole building) or just randomly
> -	if (flag.nr_of_roads() <= 1 || gametime % 200 == 0) {
> -		create_shortcut_road(flag, 13, 20, gametime);
> +	if (flag.nr_of_roads() <= 1 || gametime % 10 == 0) {
> +		create_shortcut_road(flag, 11, 20, gametime);
>  		inhibit_road_building_ = gametime + 800;
> -	}
> -	// this is when a flag is full
> -	else if (flag.current_wares() > 6 && gametime % 10 == 0) {
> -		create_shortcut_road(flag, 9, 0, gametime);
> +		// a warehouse with 3 or less roads
> +	} else if (is_warehouse && flag.nr_of_roads() <= 3) {
> +		create_shortcut_road(flag, 9, -1, gametime);
> +		inhibit_road_building_ = gametime + 400;
> +		// and when a flag is full with wares
> +	} else if (flag.current_wares() > 5) {
> +		create_shortcut_road(flag, 9, -2, gametime);
>  		inhibit_road_building_ = gametime + 400;
>  	}
>  
> @@ -2227,7 +2209,7 @@
>  // or other economy
>  bool DefaultAI::create_shortcut_road(const Flag& flag,
>                                       uint16_t checkradius,
> -                                     uint16_t minred,
> +                                     int16_t minred,
>                                       int32_t gametime) {

What does "minred" mean? I don't think you mean the color red?

>  
>  	// Increasing the failed_connection_tries counter
> @@ -2246,15 +2228,27 @@
>  
>  	// 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->stationed_soldiers().size() > 0) {
> +				occupied_military_ = true;
> +			}
> +		}
> +
>  		// if we are within grace time, it is OK, just go on
>  		if (eco->dismantle_grace_time_ > gametime &&
>  		    eco->dismantle_grace_time_ != std::numeric_limits<int32_t>::max()) {
>  			;
>  
> -		// if grace time is not set, this is probably first time without a warehouse and we must
> -		// set it
> +			// if grace time is not set, this is probably first time without a warehouse and we must
> +			// set it
>  		} else if (eco->dismantle_grace_time_ == std::numeric_limits<int32_t>::max()) {
>  
>  			// constructionsites
> @@ -2267,27 +2261,16 @@
>  					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
> +					   gametime + 30 * 1000 +            // very shot time is enough
>  					   (eco->flags.size() * 30 * 1000);  // + 30 seconds for every flag in economy
>  				}
>  
> -			// buildings
> +				// buildings
>  			} else {
>  
> -				//occupied military buildings get special treatment
> -				//(extended grace time)
> -				bool occupied_military_ = false;
> -				Building* b = flag.get_building();
> -				if (upcast(MilitarySite, militb, b)) {
> -					if (militb->stationed_soldiers().size() > 0) {
> -						occupied_military_ = true;
> -					}
> -				}
> -
>  				if (occupied_military_) {
>  					eco->dismantle_grace_time_ =
>  					   (gametime + 20 * 60 * 1000) + (eco->flags.size() * 20 * 1000);
> -					checkradius += 3;
>  
>  				} else {  // for other normal buildings
>  					eco->dismantle_grace_time_ =
> @@ -2298,9 +2281,17 @@
>  			// we have passed grace_time - it is time to dismantle
>  		} else {
>  			last_attempt_ = true;
> -			//we increase a check radius in last attempt
> +			// we increase a check radius in last attempt
>  			checkradius += 2;
>  		}
> +
> +		// and bon us for occupied military buildings:
> +		if (occupied_military_) {

bon us -> bonus

> +			checkradius += 4;
> +		}
> +
> +		// and generally increase radius for unconnected buildings
> +		checkradius += 2;
>  	}
>  
>  	Map& map = game().map();
> @@ -2552,12 +2543,10 @@
>   * \returns true, if something was changed.
>   */
>  bool DefaultAI::check_productionsites(int32_t gametime) {
> -	if ((next_productionsite_check_due_ > gametime) || productionsites.empty()) {
> +	if (productionsites.empty()) {
>  		return false;
>  	}
>  
> -	next_productionsite_check_due_ = gametime + 4000;
> -
>  	bool changed = false;
>  	// Reorder and set new values; - better now because there are multiple returns in the function
>  	productionsites.push_back(productionsites.front());
> @@ -2831,7 +2820,7 @@
>  
>  		// logs can be stored also in productionsites, they are counted as on stock
>  		// but are no available for random production site
> -		int16_t score = site.bo->stocklevel_ - productionsites.size() * 5;
> +		int16_t score = site.bo->stocklevel_ - productionsites.size() * 2;
>  
>  		if (score > 200 && site.bo->cnt_built_ > site.bo->cnt_target_) {
>  
> @@ -2859,16 +2848,18 @@
>  // and makes two decisions:
>  // - build a ship
>  // - start preparation for expedition
> -bool DefaultAI::marine_main_decisions(uint32_t const gametime) {
> -	if (gametime < next_marine_decisions_due) {
> +bool DefaultAI::marine_main_decisions(int32_t const gametime) {
> +	if (gametime < taskDue[ScheduleTasks::KMarineDecisions]) {

Although we generally prefer int over uint, the gametime we get from SDL is a uint32_t. So, we should try to stick with that. I know it is inconsistent across the code, we have a bug open to clean this up.

>  		return false;
>  	}
> -	next_marine_decisions_due = gametime + kMarineDecisionInterval;
>  
>  	if (!seafaring_economy) {
> +		taskDue[ScheduleTasks::KMarineDecisions] = std::numeric_limits<int32_t>::max();
>  		return false;

uint32_t - see comment above

>  	}
>  
> +	taskDue[ScheduleTasks::KMarineDecisions] = gametime + kMarineDecisionInterval;
> +
>  	// getting some base statistics
>  	player_ = game().get_player(player_number());
>  	uint16_t ports_count = 0;
> @@ -2918,7 +2909,7 @@
>  		}
>  	}
>  
> -	enum class FleetStatus : uint8_t {kNeedShip = 0, kEnoughShips = 1, kDoNothing = 2 };
> +	enum class FleetStatus : uint8_t {kNeedShip = 0, kEnoughShips = 1, kDoNothing = 2};
>  
>  	// now we must compare ports vs ships to decide if new ship is needed or new expedition can start
>  	FleetStatus enough_ships = FleetStatus::kDoNothing;
> @@ -2969,17 +2960,18 @@
>  }
>  
>  // This identifies ships that are waiting for command
> -bool DefaultAI::check_ships(uint32_t const gametime) {
> -	if (gametime < next_ship_check_due) {
> +bool DefaultAI::check_ships(int32_t const gametime) {
> +	if (gametime < taskDue[ScheduleTasks::kCheckShips]) {

uint32_t - see comment above

>  		return false;
>  	}
>  
> -	next_ship_check_due = gametime + kShipCheckInterval;
> -
>  	if (!seafaring_economy) {
> +		taskDue[ScheduleTasks::kCheckShips] = std::numeric_limits<int32_t>::max();
>  		return false;

uint32_t - see comment above

>  	}
>  
> +	bool action_taken = false;
> +
>  	if (!allships.empty()) {
>  		// iterating over ships and executing what is needed
>  		for (std::list<ShipObserver>::iterator i = allships.begin(); i != allships.end(); ++i) {
> @@ -3001,6 +2993,7 @@
>  			// if ships is waiting for command
>  			if (i->waiting_for_command_) {
>  				expedition_management(*i);
> +				action_taken = true;
>  			}
>  		}
>  	}
> @@ -3036,6 +3029,12 @@
>  		marineTaskQueue_.pop_back();
>  	}
>  
> +	if (action_taken) {
> +		taskDue[ScheduleTasks::kCheckShips] = gametime + kShipCheckInterval;
> +	} else {
> +		taskDue[ScheduleTasks::kCheckShips] = gametime + 3 * kShipCheckInterval;
> +	}
> +
>  	return true;
>  }
>  
> @@ -3046,10 +3045,10 @@
>   * \returns true, if something was changed.
>   */
>  bool DefaultAI::check_mines_(int32_t const gametime) {
> -	if ((next_mine_check_due_ > gametime) || mines_.empty())
> +	if (mines_.empty()) {
>  		return false;
> +	}
>  
> -	next_mine_check_due_ = gametime + 7000;  // 7 seconds is enough
>  	// Reorder and set new values; - due to returns within the function
>  	mines_.push_back(mines_.front());
>  	mines_.pop_front();
> @@ -3172,6 +3171,18 @@
>  	}
>  }
>  
> +// very primitive function - it sets thisTask to dueTask, if its due time is
> +// older then due time of current dueTask
> +// void DefaultAI::scheduler_review(int32_t* next_check_due_,
> +// int32_t* oldestTaskTime,
> +// ScheduleTasks* dueTask,
> +// ScheduleTasks thisTask) {
> +// if (*next_check_due_ < *oldestTaskTime) {
> +//*oldestTaskTime = *next_check_due_;
> +//*dueTask = thisTask;
> +//}
> +//}
> +

Remove

>  // counts produced output on stock
>  // if multiple outputs, it returns lowest value
>  uint32_t DefaultAI::get_stocklevel(BuildingObserver& bo) {
> @@ -3229,13 +3240,13 @@
>  // this function only manipulates with trainingsites' inputs priority
>  // decreases it when too many unoccupied military buildings
>  bool DefaultAI::check_trainingsites(int32_t gametime) {
> -	if (next_trainingsites_check_due_ > gametime) {
> +	if (taskDue[ScheduleTasks::kCheckTrainingsites] > gametime) {
>  		return false;
>  	}
>  	if (!trainingsites.empty()) {
> -		next_trainingsites_check_due_ = gametime + kTrainingSitesCheckInterval;
> +		taskDue[ScheduleTasks::kCheckTrainingsites] = gametime + kTrainingSitesCheckInterval;
>  	} else {
> -		next_trainingsites_check_due_ = gametime + 3 * kTrainingSitesCheckInterval;
> +		taskDue[ScheduleTasks::kCheckTrainingsites] = gametime + 5 * kTrainingSitesCheckInterval;
>  	}
>  
>  	uint8_t new_priority = DEFAULT_PRIORITY;
> @@ -3265,12 +3276,12 @@
>   * \returns true if something was changed
>   */
>  bool DefaultAI::check_militarysites(int32_t gametime) {
> -	if (next_militarysite_check_due_ > gametime) {
> +	if (taskDue[ScheduleTasks::kCheckMilitarysites] > gametime) {
>  		return false;
>  	}
>  
>  	// just to be sure the value is reset
> -	next_militarysite_check_due_ = gametime + 4 * 1000;  // 4 seconds is really fine
> +	taskDue[ScheduleTasks::kCheckMilitarysites] = gametime + 4 * 1000;  // 4 seconds is really fine
>  	// even if there are no finished & attended military sites, probably there are ones just in
>  	// construction
>  	unstationed_milit_buildings_ = 0;
> @@ -3385,7 +3396,7 @@
>  	// reorder:;
>  	militarysites.push_back(militarysites.front());
>  	militarysites.pop_front();
> -	next_militarysite_check_due_ = gametime + 5 * 1000;  // 10 seconds is really fine
> +	taskDue[ScheduleTasks::kCheckMilitarysites] = gametime + 5 * 1000;  // 10 seconds is really fine
>  	return changed;
>  }
>  
> @@ -3833,7 +3844,7 @@
>  		}
>  
>  		// Let defaultAI try to directly connect the constructionsite
> -		next_road_due_ = game().get_gametime();
> +		taskDue[ScheduleTasks::kRoadCheck] = game().get_gametime();
>  	} else {
>  		++bo.cnt_built_;
>  
> @@ -3989,9 +4000,6 @@
>  			}
>  		}
>  	}
> -
> -	m_buildable_changed = true;
> -	m_mineable_changed = true;
>  }
>  
>  // Checks that supply line exists for given building.
> @@ -4043,7 +4051,7 @@
>  
>  	// Only useable, if it owns at least one militarysite
>  	if (militarysites.empty()) {
> -		next_attack_consideration_due_ = next_attack_waittime_ * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = next_attack_waittime_ * 1000 + gametime;
>  		return false;
>  	}
>  
> @@ -4092,7 +4100,7 @@
>  	if (current_margin < needed_margin) {  // no attacking!
>  		last_attack_target_.x = std::numeric_limits<uint16_t>::max();
>  		last_attack_target_.y = std::numeric_limits<uint16_t>::max();
> -		next_attack_consideration_due_ = next_attack_waittime_ * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = next_attack_waittime_ * 1000 + gametime;
>  		return false;
>  	}
>  
> @@ -4131,7 +4139,7 @@
>  
>  	// if we cannot attack anybody, terminating...
>  	if (!any_attackable) {
> -		next_attack_consideration_due_ = next_attack_waittime_ * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = next_attack_waittime_ * 1000 + gametime;
>  		last_attack_target_.x = std::numeric_limits<uint16_t>::max();
>  		last_attack_target_.y = std::numeric_limits<uint16_t>::max();
>  		return false;
> @@ -4245,7 +4253,7 @@
>  
>  		game().send_player_enemyflagaction(best_wh_target->base_flag(), pn, attackers);
>  		last_attack_target_ = best_wh_target->get_position();
> -		next_attack_consideration_due_ = (gametime % 10 + 10) * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = (gametime % 10 + 10) * 1000 + gametime;
>  		next_attack_waittime_ = 10;
>  		return true;
>  
> @@ -4261,11 +4269,11 @@
>  
>  		game().send_player_enemyflagaction(best_ms_target->base_flag(), pn, attackers);
>  		last_attack_target_ = best_ms_target->get_position();
> -		next_attack_consideration_due_ = (gametime % 10 + 10) * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = (gametime % 10 + 10) * 1000 + gametime;
>  		next_attack_waittime_ = 10;
>  		return true;
>  	} else {
> -		next_attack_consideration_due_ = next_attack_waittime_ * 1000 + gametime;
> +		taskDue[ScheduleTasks::kConsiderAttack] = next_attack_waittime_ * 1000 + gametime;
>  		last_attack_target_.x = std::numeric_limits<uint16_t>::max();
>  		last_attack_target_.y = std::numeric_limits<uint16_t>::max();
>  		return false;
> @@ -4300,13 +4308,36 @@
>  	}
>  }
>  
> +// run over dueTasks map and returns task with lower duetime
> +DefaultAI::ScheduleTasks DefaultAI::get_oldest_task(int32_t const gametime) {
> +
> +	int32_t oldestTaskTime = gametime;             // we are looking for jobs due before now
> +	ScheduleTasks DueTask = ScheduleTasks::kIdle;  // default
> +	taskDue[ScheduleTasks::kIdle] = gametime;
> +
> +	for (std::map<ScheduleTasks, int32_t>::iterator it = taskDue.begin(); it != taskDue.end();
> +	     ++it) {

uint32_t - see comment above. Also, a range-based for loop would be a lot easier to read:

for (std::pair<ScheduleTasks, int32_t> task : taskDue) {

> +		if (it->second < oldestTaskTime) {
> +			oldestTaskTime = it->second;
> +			DueTask = it->first;
> +		}
> +	}
> +	return DueTask;
> +}
> +
>  // This prints some basic statistics during a game to the command line -
>  // missing materials and counts of different types of buildings.
>  // The main purpose of this is when a game creator needs to finetune a map
>  // and needs to know what resourcess are missing for which player and so on.
>  // By default it is off (see kPrintStats)
>  // TODO(tiborb ?): - it would be nice to have this activated by a command line switch
> -void DefaultAI::print_stats() {
> +void DefaultAI::print_stats(int32_t const gametime) {
> +

uint32_t - see comment above.

> +	if (!kPrintStats) {
> +		taskDue[ScheduleTasks::kprintStats] = std::numeric_limits<int32_t>::max();
> +		return;

uint32_t - see comment above.

> +	}
> +	taskDue[ScheduleTasks::kprintStats] = gametime + 30 * 60 * 1000;
>  
>  	PlayerNumber const pn = player_number();
>  
> 
> === modified file 'src/ai/defaultai.h'
> --- src/ai/defaultai.h	2015-02-16 21:19:59 +0000
> +++ src/ai/defaultai.h	2015-03-04 20:14:13 +0000
> @@ -78,8 +78,33 @@
>  		DEFENSIVE = 0,
>  	};
>  
> -	enum class WalkSearch : uint8_t {kAnyPlayer, kOtherPlayers };
> -	enum class NewShip : uint8_t {kBuilt, kFoundOnLoad };
> +	enum class WalkSearch : uint8_t {kAnyPlayer, kOtherPlayers};
> +	enum class NewShip : uint8_t {kBuilt, kFoundOnLoad};
> +	enum class ScheduleTasks : uint8_t {
> +		kBFCheck,
> +		kRoadCheck,
> +		kUnbuildableFCheck,
> +		kConsiderAttack,
> +		kCheckEconomies,
> +		kProductionsitesStats,
> +		kConstructBuilding,
> +		kCheckProductionsites,
> +		kCheckShips,
> +		KMarineDecisions,
> +		kCheckMines,
> +		kWareReview,
> +		kprintStats,
> +		kIdle,
> +		kCheckMilitarysites,
> +		kCheckTrainingsites
> +	};
> +	enum class MilitaryStrategy : uint8_t {
> +		kNoNewMilitary,
> +		kDefenseOnly,
> +		kResourcesOrDefense,
> +		kExpansion,
> +		kPushExpansion
> +	};
>  
>  	/// Implementation for Aggressive
>  	struct AggressiveImpl : public ComputerPlayer::Implementation {
> @@ -136,6 +161,13 @@
>  	                          int16_t* max_preciousness,
>  	                          int16_t* max_needed_preciousness);
>  
> +	// void scheduler_review(int32_t* next_check_due_,
> +	// int32_t* oldestTaskTime,
> +	// ScheduleTasks* DueTask,
> +	// ScheduleTasks thisTask);
> +

Remove

> +	ScheduleTasks get_oldest_task(int32_t);
> +

uint32_t - see comment above.

>  	bool construct_building(int32_t);
>  
>  	uint32_t coords_hash(Widelands::Coords coords) {
> @@ -156,7 +188,7 @@
>  	bool improve_roads(int32_t);
>  	bool create_shortcut_road(const Widelands::Flag&,
>  	                          uint16_t maxcheckradius,
> -	                          uint16_t minred,
> +	                          int16_t minred,
>  	                          const int32_t gametime);
>  	// trying to identify roads that might be removed
>  	bool dispensable_road_test(Widelands::Road&);
> @@ -165,14 +197,13 @@
>  	bool check_trainingsites(int32_t);
>  	bool check_mines_(int32_t);
>  	bool check_militarysites(int32_t);
> -	bool marine_main_decisions(uint32_t);
> -	bool check_ships(uint32_t);
> -	void print_stats();
> +	bool marine_main_decisions(int32_t);
> +	bool check_ships(int32_t);
> +	void print_stats(int32_t);
>  	uint32_t get_stocklevel_by_hint(size_t);

all uint32_t - see comment above.

>  	uint32_t get_stocklevel(BuildingObserver&);
>  	uint32_t get_warehoused_stock(Widelands::WareIndex wt);
>  	uint32_t get_stocklevel(Widelands::WareIndex);  // count all direct outputs_
> -	void check_helpersites(int32_t);
>  	void review_wares_targets(int32_t);
>  
>  	// sometimes scanning an area in radius gives inappropriate results, so this is to verify that
> @@ -213,8 +244,8 @@
>  	// Variables of default AI
>  	uint8_t type_;
>  
> -	bool m_buildable_changed;
> -	bool m_mineable_changed;
> +	// collect statistics on how many times which job was run
> +	uint32_t schedStat[20] = {0};
>  
>  	Widelands::Player* player_;
>  	Widelands::TribeDescr const* tribe_;
> @@ -240,23 +271,12 @@
>  	std::list<WarehouseSiteObserver> warehousesites;
>  	std::list<TrainingSiteObserver> trainingsites;
>  	std::list<ShipObserver> allships;
> +	std::map<ScheduleTasks, int32_t> taskDue;
>  

uint32_t - see comment above.

>  	std::vector<WareObserver> wares;
>  
> -	int32_t next_road_due_;
> -	int32_t next_stats_update_due_;
> -	int32_t next_construction_due_;
> +	int32_t next_ai_think_;
>  	int32_t next_mine_construction_due_;
> -	int32_t next_productionsite_check_due_;
> -	int32_t next_mine_check_due_;
> -	int32_t next_militarysite_check_due_;
> -	uint32_t next_ship_check_due;
> -	uint32_t next_marine_decisions_due;
> -	int32_t next_attack_consideration_due_;
> -	int32_t next_trainingsites_check_due_;
> -	int32_t next_bf_check_due_;
> -	int32_t next_wares_review_due_;
> -	int32_t next_statistics_report_;
>  	int32_t inhibit_road_building_;
>  	int32_t time_of_last_construction_;
>  	int32_t enemy_last_seen_;
> @@ -288,7 +308,7 @@
>  	// it decreases with failed scans
>  	int32_t spots_;  // sum of buildable fields
>  
> -	enum {kReprioritize, kStopShipyard, kStapShipyard };
> +	enum {kReprioritize, kStopShipyard, kStapShipyard};
>  
>  	std::vector<int16_t> marineTaskQueue_;
>  
> 


-- 
https://code.launchpad.net/~widelands-dev/widelands/ai-scheduler/+merge/251327
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai-scheduler into lp:widelands.


References