widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

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

Requested reviews:
  Widelands Developers (widelands-dev)

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

The allows_seafaring rework some time ago damaged the functionality of AI seafaring. One of basic issue is that AI expected that if map is not seafaring (and brief time after game start it is by default so), it disabled all seafaring activities forever. So AI can have shipyard building ships forever and never stop it and never start preparation for expedition.
Entire concept of switching seafaring map ON/OFF in AI is bit fragile...
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_seafaring_fix into lp:widelands.

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2017-12-10 21:19:02 +0000
+++ src/ai/defaultai.cc	2018-01-03 20:28:36 +0000
@@ -381,13 +381,20 @@
 			}
 			break;
 		case SchedulerTaskId::kCheckShips:
-			set_taskpool_task_time(gametime + 3 * kShipCheckInterval, SchedulerTaskId::kCheckShips);
-			check_ships(gametime);
+			// if function returns false, we can postpone next call
+			{
+				const uint8_t wait_multiplier = (check_ships(gametime)) ? 1 : 5;
+				set_taskpool_task_time(gametime + wait_multiplier * kShipCheckInterval,
+										SchedulerTaskId::kCheckShips);
+			}
 			break;
 		case SchedulerTaskId::KMarineDecisions:
-			set_taskpool_task_time(
-			   gametime + kMarineDecisionInterval, SchedulerTaskId::KMarineDecisions);
-			marine_main_decisions();
+			// if function returns false, we can postpone for next call
+			{
+				const uint8_t wait_multiplier = (marine_main_decisions()) ? 1 : 5;
+				set_taskpool_task_time(gametime + wait_multiplier * kMarineDecisionInterval,
+				                       SchedulerTaskId::KMarineDecisions);
+			}
 			break;
 		case SchedulerTaskId::kCheckMines:
 			if (check_economies()) {  // economies must be consistent

=== modified file 'src/ai/defaultai_seafaring.cc'
--- src/ai/defaultai_seafaring.cc	2017-12-10 18:11:29 +0000
+++ src/ai/defaultai_seafaring.cc	2018-01-03 20:28:36 +0000
@@ -104,8 +104,8 @@
 // - build a ship
 // - start preparation for expedition
 bool DefaultAI::marine_main_decisions() {
-	if (!map_allows_seafaring_) {
-		set_taskpool_task_time(kNever, SchedulerTaskId::KMarineDecisions);
+	if (!map_allows_seafaring_ &&
+	    count_buildings_with_attribute(BuildingAttribute::kShipyard) == 0 && allships.empty()) {
 		return false;
 	}
 
@@ -134,6 +134,14 @@
 		if (ps_obs.bo->is(BuildingAttribute::kShipyard)) {
 			shipyards_count += 1;
 
+			// In very rare situation, we might have non-seafaring map but the shipyard is working
+			if (!map_allows_seafaring_ && !ps_obs.site->is_stopped()) {
+				log("  %1d: we have working shipyard in a non seafaring ecoomy, stopping it...\n",
+				    player_number());
+				game().send_player_start_stop_building(*ps_obs.site);
+				return false;
+			}
+
 			// counting stocks
 			uint8_t stocked_wares = 0;
 			std::vector<InputQueue*> const inputqueues = ps_obs.site->inputqueues();
@@ -148,6 +156,11 @@
 		}
 	}
 
+	// If non-seafaring economy, no sense to go on with this function
+	if (!map_allows_seafaring_) {
+		return false;
+	}
+
 	// and now over ships
 	for (std::deque<ShipObserver>::iterator sp_iter = allships.begin(); sp_iter != allships.end();
 	     ++sp_iter) {
@@ -215,6 +228,8 @@
 		// we need to find a port
 		for (const WarehouseSiteObserver& wh_obs : warehousesites) {
 			if (wh_obs.bo->is(BuildingAttribute::kPort)) {
+				log("  %1d: Starting preparation for expedition in port at %3dx%3d\n", player_number(),
+				    wh_obs.site->get_position().x, wh_obs.site->get_position().y);
 				game().send_player_start_or_cancel_expedition(*wh_obs.site);
 				return true;
 			}
@@ -225,13 +240,13 @@
 
 // This identifies ships that are waiting for command
 bool DefaultAI::check_ships(uint32_t const gametime) {
-	if (!map_allows_seafaring_) {
-		set_taskpool_task_time(kNever, SchedulerTaskId::kCheckShips);
+	// There is possibility that the map is not seafaring but we still have ships and/or shipyards
+	if (!map_allows_seafaring_ &&
+	    count_buildings_with_attribute(BuildingAttribute::kShipyard) == 0 && allships.empty()) {
+		// False indicates that we can postpone next call of this function
 		return false;
 	}
 
-	bool action_taken = false;
-
 	if (!allships.empty()) {
 		// iterating over ships and doing what is needed
 		for (ShipObserver& so : allships) {
@@ -276,7 +291,6 @@
 			// if ship is waiting for command
 			if (so.waiting_for_command_) {
 				expedition_management(so);
-				action_taken = true;
 			}
 
 			// Checking utilization
@@ -326,10 +340,11 @@
 		marine_task_queue.pop_back();
 	}
 
-	if (action_taken) {
-		set_taskpool_task_time(gametime + kShipCheckInterval, SchedulerTaskId::kCheckShips);
+	if (map_allows_seafaring_) {
+		// here we indicate that normal frequency check makes sense
+		return true;
 	}
-	return true;
+	return false;
 }
 
 /**