widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

TiborB has proposed merging lp:~widelands-dev/widelands/ai_small_requests into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1722376 in widelands: "AI builds Barbarian Weaving Mill on non-seafaring maps"
  https://bugs.launchpad.net/widelands/+bug/1722376
  Bug #1724073 in widelands: "ai_help_structs: Assertion `all_stats.count(pl2) > 0' failed."
  https://bugs.launchpad.net/widelands/+bug/1724073

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/ai_small_requests/+merge/332519

Changes:
- prohibits seafaring-specific buildings for AI if map does not have at least two ports
- fixes weird bug in AI when number of players was lower than number of game slots
- AI now recognizes changes in teams setup during the game
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_small_requests into lp:widelands.

=== modified file 'src/ai/ai_help_structs.cc'
--- src/ai/ai_help_structs.cc	2017-09-29 16:10:25 +0000
+++ src/ai/ai_help_structs.cc	2017-10-19 19:37:15 +0000
@@ -1037,7 +1037,6 @@
 
 // Constructor to be used
 PlayersStrengths::PlayerStat::PlayerStat(Widelands::TeamNumber tc,
-                                         bool e,
                                          uint32_t pp,
                                          uint32_t op,
                                          uint32_t o60p,
@@ -1046,7 +1045,6 @@
                                          uint32_t oland,
                                          uint32_t o60l)
    : team_number(tc),
-     is_enemy(e),
      players_power(pp),
      old_players_power(op),
      old60_players_power(o60p),
@@ -1079,17 +1077,9 @@
                            uint32_t oland,
                            uint32_t o60l) {
 	if (all_stats.count(opn) == 0) {
-		bool enemy = false;
-		if (pn == opn) {
-			;
-		} else if (pltn == 0 || mytn == 0) {
-			enemy = true;
-		} else if (pltn != mytn) {
-			enemy = true;
-		}
 		this_player_number = pn;
-		all_stats.insert(
-		   std::make_pair(opn, PlayerStat(pltn, enemy, pp, op, o60p, cs, land, oland, o60l)));
+		this_player_team = mytn;
+		all_stats.insert(std::make_pair(opn, PlayerStat(pltn, pp, op, o60p, cs, land, oland, o60l)));
 	} else {
 		all_stats[opn].players_power = pp;
 		all_stats[opn].old_players_power = op;
@@ -1098,6 +1088,25 @@
 		all_stats[opn].players_land = land;
 		all_stats[opn].old_players_land = oland;
 		all_stats[opn].old60_players_land = oland;
+		assert(this_player_number == pn);
+		if (this_player_team != mytn) {
+			log("%2d: Team changed %d -> %d\n", pn, this_player_team, mytn);
+			this_player_team = mytn;
+		};
+		if (all_stats[opn].team_number != pltn) {
+			log("%2d: Team changed for player %d: %d -> %d\n", pn, opn, all_stats[opn].team_number,
+			    pltn);
+			all_stats[opn].team_number = pltn;
+		};
+	}
+}
+
+// Very tiny possibility that player that has a statistics info here
+// does not exist anymore
+void PlayersStrengths::remove_stat(const Widelands::PlayerNumber pn) {
+	if (all_stats.count(pn) > 0) {
+		log("%d: AI: Erasing statistics for player %d\n", this_player_number, pn);
+		all_stats.erase(pn);
 	}
 }
 
@@ -1118,7 +1127,7 @@
 // This just goes over information about all enemies and where they were seen the last time
 bool PlayersStrengths::any_enemy_seen_lately(const uint32_t gametime) {
 	for (auto& item : all_stats) {
-		if (item.second.is_enemy && player_seen_lately(item.first, gametime)) {
+		if (get_is_enemy(item.first) && player_seen_lately(item.first, gametime)) {
 			return true;
 		}
 	}
@@ -1129,7 +1138,7 @@
 uint8_t PlayersStrengths::enemies_seen_lately_count(const uint32_t gametime) {
 	uint8_t count = 0;
 	for (auto& item : all_stats) {
-		if (item.second.is_enemy && player_seen_lately(item.first, gametime)) {
+		if (get_is_enemy(item.first) && player_seen_lately(item.first, gametime)) {
 			count += 1;
 		}
 	}
@@ -1144,13 +1153,23 @@
 	all_stats[pn].last_time_seen = seentime;
 }
 
-bool PlayersStrengths::get_is_enemy(Widelands::PlayerNumber pn) {
-	if (all_stats.count(pn) == 0) {
+bool PlayersStrengths::get_is_enemy(Widelands::PlayerNumber other_player_number) {
+	// So this is me
+	if (other_player_number == this_player_number) {
+		return false;
+	}
+	// If we do not belong to any team, all others are our enemies
+	if (this_player_team == 0) {
+		return true;
+	}
+	if (all_stats.count(other_player_number) == 0) {
 		// Should happen only rarely so we print a warning here
-		log("%d: WARNING: player has no statistics yet\n", this_player_number);
+		log("%d: WARNING: player has no statistics yet for player %d\n", this_player_number,
+		    other_player_number);
 		return false;
 	}
-	return all_stats[pn].is_enemy;
+	// finally we compare my team number of the other player team number
+	return all_stats[other_player_number].team_number != this_player_team;
 }
 
 // Was the player seen less then 2 minutes ago

=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h	2017-09-29 16:10:25 +0000
+++ src/ai/ai_help_structs.h	2017-10-19 19:37:15 +0000
@@ -78,6 +78,7 @@
 	kUpgradeExtends,
 	kLogRefiner,
 	kIronMine,
+	kNeedsSeafaring,
 };
 
 enum class AiType : uint8_t { kVeryWeak, kWeak, kNormal };
@@ -780,7 +781,6 @@
 	struct PlayerStat {
 		PlayerStat() = default;
 		PlayerStat(Widelands::TeamNumber tc,
-		           bool en,
 		           uint32_t pp,
 		           uint32_t op,
 		           uint32_t o60p,
@@ -790,7 +790,6 @@
 		           uint32_t o60l);
 
 		Widelands::TeamNumber team_number = 0U;
-		bool is_enemy = false;
 		uint32_t players_power = 0U;
 		uint32_t old_players_power = 0U;
 		uint32_t old60_players_power = 0U;
@@ -815,6 +814,7 @@
 	         uint32_t land,
 	         uint32_t oland,
 	         uint32_t o60l);
+	void remove_stat(Widelands::PlayerNumber pn);
 	void recalculate_team_power();
 
 	// This is strength of player plus third of strength of other members of his team
@@ -838,7 +838,6 @@
 	bool get_is_enemy(Widelands::PlayerNumber);
 	uint8_t enemies_seen_lately_count(uint32_t);
 	bool any_enemy_seen_lately(uint32_t);
-	PlayerNumber this_player_number;
 	void set_update_time(uint32_t);
 	uint32_t get_update_time();
 
@@ -850,6 +849,9 @@
 	std::map<Widelands::TeamNumber, uint32_t> team_powers;
 
 	uint32_t update_time;
+	PlayerNumber this_player_number;
+	PlayerNumber this_player_team;
+
 };
 }  // namespace Widelands
 

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2017-09-29 16:10:25 +0000
+++ src/ai/defaultai.cc	2017-10-19 19:37:15 +0000
@@ -628,6 +628,9 @@
 		if (bld.is_buildable()) {
 			bo.set_is(BuildingAttribute::kBuildable);
 		}
+		if (bld.needs_seafaring()) {
+			bo.set_is(BuildingAttribute::kNeedsSeafaring);
+		}
 		if (bh.is_logproducer()) {
 			bo.set_is(BuildingAttribute::kLumberjack);
 		}
@@ -952,10 +955,6 @@
 		} while (mr_nw.advance(map));
 	}
 
-	if (!port_reserved_coords.empty()) {
-		seafaring_economy = true;
-	}
-
 	// here we scan entire map for own ships
 	std::set<OPtr<Ship>> found_ships;
 	for (int16_t y = 0; y < map.get_height(); ++y) {
@@ -1073,6 +1072,9 @@
 void DefaultAI::update_all_buildable_fields(const uint32_t gametime) {
 	uint16_t i = 0;
 
+	// To be sure we have some info about enemies we might see
+	update_player_stat(gametime);
+
 	// we test 40 fields that were update more than 1 seconds ago
 	while (!buildable_fields.empty() &&
 	       (buildable_fields.front()->field_info_expiration - kFieldInfoExpiration + 1000) <=
@@ -1958,6 +1960,13 @@
 
 	const Map& map = game().map();
 
+	if (gametime % 5 == 0) {
+		// TODO(unknown): Counting port spaces is very primitive way for this
+		// there should be better alternative f.e. like map::allows_seafaring()
+		// function but simplier
+		seafaring_economy = map.get_port_spaces().size() >= 2;
+	}
+
 	for (int32_t i = 0; i < 4; ++i)
 		spots_avail.at(i) = 0;
 
@@ -2285,7 +2294,8 @@
 		} else if (bo.type == BuildingObserver::Type::kProductionsite ||
 		           bo.type == BuildingObserver::Type::kMine) {
 
-			bo.new_building = check_building_necessity(bo, PerfEvaluation::kForConstruction, gametime);
+			bo.new_building = check_building_necessity(
+			   bo, PerfEvaluation::kForConstruction, gametime, seafaring_economy);
 
 			if (bo.is(BuildingAttribute::kShipyard)) {
 				assert(bo.new_building == BuildingNecessity::kAllowed ||
@@ -2356,7 +2366,8 @@
 		} else if (bo.type == BuildingObserver::Type::kMilitarysite) {
 			bo.new_building = check_building_necessity(bo, gametime);
 		} else if (bo.type == BuildingObserver::Type::kTrainingsite) {
-			bo.new_building = check_building_necessity(bo, PerfEvaluation::kForConstruction, gametime);
+			bo.new_building = check_building_necessity(
+			   bo, PerfEvaluation::kForConstruction, gametime, seafaring_economy);
 		} else if (bo.type == BuildingObserver::Type::kWarehouse) {
 			bo.new_building = check_warehouse_necessity(bo, gametime);
 		} else if (bo.aimode_limit_status() != AiModeBuildings::kAnotherAllowed) {
@@ -4291,7 +4302,8 @@
 // dismantle
 BuildingNecessity DefaultAI::check_building_necessity(BuildingObserver& bo,
                                                       const PerfEvaluation purpose,
-                                                      const uint32_t gametime) {
+                                                      const uint32_t gametime,
+                                                      const bool seafaring_map) {
 
 	bo.primary_priority = 0;
 
@@ -4317,6 +4329,12 @@
 		return BuildingNecessity::kForbidden;
 	}
 
+	// Perhaps buildings are not allowed because the map is no seafaring
+	if (purpose == PerfEvaluation::kForConstruction && !seafaring_map &&
+	    bo.is(BuildingAttribute::kNeedsSeafaring)) {
+		return BuildingNecessity::kForbidden;
+	}
+
 	// First we deal with training sites, they are separate category
 	if (bo.type == BuildingObserver::Type::kTrainingsite) {
 
@@ -5705,7 +5723,6 @@
 			warehousesites.back().bo = &bo;
 			if (bo.is(BuildingAttribute::kPort)) {
 				++num_ports;
-				seafaring_economy = true;
 				// unblock nearby fields, might be used for other buildings...
 				const Map& map = game().map();
 				MapRegion<Area<FCoords>> mr(
@@ -5878,15 +5895,13 @@
 	player_statistics.set_update_time(gametime);
 	Widelands::PlayerNumber const pn = player_number();
 	PlayerNumber const nr_players = game().map().get_nrplayers();
-	uint32_t plr_in_game = 0;
-	iterate_players_existing_novar(p, nr_players, game())++ plr_in_game;
 
 	// receiving games statistics and parsing it (reading latest entry)
 	const Game::GeneralStatsVector& genstats = game().get_general_statistics();
 
 	// Collecting statistics and saving them in player_statistics object
 	const Player* me = game().get_player(pn);
-	for (Widelands::PlayerNumber j = 1; j <= plr_in_game; ++j) {
+	for (Widelands::PlayerNumber j = 1; j <= nr_players; ++j) {
 		const Player* this_player = game().get_player(j);
 		if (this_player) {
 			try {
@@ -5928,6 +5943,10 @@
 				    static_cast<unsigned int>(player_number()),
 				    static_cast<unsigned int>(genstats.size()));
 			}
+		} else {
+			// Well, under some circumstances it is possible we have stat for this player and he does
+			// not exist anymore
+			player_statistics.remove_stat(j);
 		}
 	}
 

=== modified file 'src/ai/defaultai.h'
--- src/ai/defaultai.h	2017-09-29 16:10:25 +0000
+++ src/ai/defaultai.h	2017-10-19 19:37:15 +0000
@@ -168,7 +168,7 @@
 
 	// for production sites
 	Widelands::BuildingNecessity
-	check_building_necessity(Widelands::BuildingObserver& bo, PerfEvaluation purpose, uint32_t);
+	check_building_necessity(Widelands::BuildingObserver& bo, PerfEvaluation purpose, uint32_t, const bool = true);
 	Widelands::BuildingNecessity check_warehouse_necessity(Widelands::BuildingObserver&,
 	                                                       uint32_t gametime);
 	void sort_task_pool();

=== modified file 'src/ai/defaultai_warfare.cc'
--- src/ai/defaultai_warfare.cc	2017-09-29 16:10:25 +0000
+++ src/ai/defaultai_warfare.cc	2017-10-19 19:37:15 +0000
@@ -790,6 +790,9 @@
 		return false;
 	}
 
+	// Make sure we have statistics about our enemies up-to-date
+	update_player_stat(gametime);
+
 	// Make sure we are not above ai type limit
 	assert(mso.bo->total_count() <= mso.bo->cnt_limit_by_aimode);