[freeciv-ai] Re: (PR#10216) AI Builds Too Many Transports

["Benedict Adamson" <badamson@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10216 >

Gregory Berkolaiko wrote:
...
> We should manage the boat building on 
> the centralized level.  This will allow k_s_w not care about actually 
> finding a boat, if it really wants a boat, one would be found/built for 
> it.  It would take take some time to make this system robust, but it is 
> the way forward I feel.

I tend to agree.

FWIW, I've attached my alterations as two patches. The first is for 
alterations to advmilitary.c and fixes the bug, at the cost of extra CPU 
time because of the increase in PF use. The second is commentary and 
assertions I added in various places while trying to understand what was 
happening.

These patches are for the 3 October CVS snapshot.


--- vendor.freeciv.current/ai/advmilitary.c     2004-10-03 11:40:07.000000000 
+0100
+++ freeciv.PR10216/ai/advmilitary.c    2004-10-06 00:45:47.000000000 +0100
@@ -33,6 +33,7 @@
 #include "aicity.h"
 #include "aidata.h"
 #include "aidiplomat.h"
+#include "aiferry.h"
 #include "aihand.h"
 #include "aihunt.h"
 #include "ailog.h"
@@ -961,6 +962,8 @@
 3. calculates the relevant stats of the victim.
 4. finds the best attacker for this type of victim (in process_attacker_want)
 5. if we still want to attack, records the best attacker in choice.
+If the target is overseas, the function might suggest building a ferry
+to carry a land attack unit, instead of the land attack unit itself.
 **************************************************************************/
 static void kill_something_with(struct player *pplayer, struct city *pcity, 
                                struct unit *myunit, struct ai_choice *choice)
@@ -980,11 +983,9 @@
   struct city *acity;
   /* Defender of the target city/tile */
   struct unit *pdef; 
-  /* Coordinates of the boat */
-  struct tile *boat_tile = NULL;
   /* Type of the boat (real or a future one) */
   Unit_Type_id boattype = U_LAST;
-  bool go_by_boat;
+  bool go_by_boat = FALSE;
   /* Is the defender veteran? */
   int def_vet;
   struct ai_choice best_choice;
@@ -1007,21 +1008,6 @@
     return;
   }
 
-  if (is_ground_unit(myunit)) {
-    int boatid = find_boat(pplayer, &boat_tile, 2);
-    ferryboat = player_find_unit_by_id(pplayer, boatid);
-  }
-
-  if (ferryboat) {
-    boattype = ferryboat->type;
-  } else {
-    boattype = best_role_unit_for_player(pplayer, L_FERRYBOAT);
-    if (boattype == U_LAST) {
-      /* We pretend that we can have the simplest boat -- to stimulate tech */
-      boattype = get_role_unit(L_FERRYBOAT, 0);
-    }
-  }
-
   best_choice.want = find_something_to_kill(pplayer, myunit, &ptile);
 
   acity = map_get_city(ptile);
@@ -1055,8 +1041,26 @@
       return;
     }
 
-    go_by_boat = !(goto_is_sane(myunit, acity->tile, TRUE) 
-                  && WARMAP_COST(ptile) <= (MIN(6, move_rate) * THRESHOLD));
+    if (is_ground_unit(myunit)) {
+      int boatid = aiferry_find_boat(myunit, 1, NULL);
+      ferryboat = find_unit_by_id(boatid);
+
+      if (ferryboat) {
+        boattype = ferryboat->type;
+      } else {
+        boattype = best_role_unit_for_player(pplayer, L_FERRYBOAT);
+        if (boattype == U_LAST) {
+          /* We pretend that we can have the simplest boat --
+             to stimulate tech */
+          boattype = get_role_unit(L_FERRYBOAT, 0);
+        }
+      }
+      assert(SEA_MOVING == unit_types[boattype].move_type);
+
+      go_by_boat = !(WARMAP_COST(ptile) <= (MIN(6, move_rate) * THRESHOLD)
+                     && goto_is_sane(myunit, acity->tile, TRUE));
+    }/*else never needs a boat*/
+
     move_time = turns_to_enemy_city(myunit->type, acity, move_rate, 
                                     go_by_boat, ferryboat, boattype);
 
@@ -1110,6 +1114,7 @@
                           &best_choice, NULL, U_LAST);
   } else { 
     /* Attract a boat to our city or retain the one that's already here */
+    assert(is_ground_unit(myunit));
     best_choice.need_boat = TRUE;
     process_attacker_want(pcity, benefit, def_type, def_vet, ptile, 
                           &best_choice, ferryboat, boattype);
@@ -1118,11 +1123,22 @@
   if (best_choice.want > choice->want) {
     /* We want attacker more that what we have selected before */
     copy_if_better_choice(&best_choice, choice);
-    if (go_by_boat && !ferryboat) {
-      ai_choose_role_unit(pplayer, pcity, choice, L_FERRYBOAT, choice->want);
-    }
     freelog(LOG_DEBUG, "%s has chosen attacker, %s, want=%d",
             pcity->name, unit_types[choice->choice].name, choice->want);
+
+    if (go_by_boat && !ferryboat) {/*need a new ferry*/
+      /* We might need a new boat even if there are boats free,
+       * if they are blockaded or in inland seas*/
+      assert(is_ground_unit(myunit));
+      ai_choose_role_unit(pplayer, pcity, choice, L_FERRYBOAT, choice->want);
+      if (SEA_MOVING == unit_types[choice->choice].move_type) {
+        struct ai_data *ai = ai_data_get(pplayer);
+        freelog(LOG_DEBUG,
+                "%s has chosen attacker ferry, %s, want=%d, %d of %d free",
+                pcity->name, unit_types[choice->choice].name, choice->want,
+                ai->stats.available_boats, ai->stats.boats);
+      }/*else can not build ferries yet*/
+    }
   } 
 }
 
@@ -1370,7 +1386,9 @@
     destroy_unit_virtual(virtualunit);
   }
 
-  /* Consider a land attacker */
+  /* Consider a land attacker or a ferried land attacker
+   * (in which case, we might want a ferry before an attacker)
+   */
   unit_type = ai_choose_attacker(pcity, LAND_MOVING);
   if (unit_type >= 0) {
     virtualunit = create_unit_virtual(pplayer, pcity, unit_type, 1);

diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/ai/advdomestic.c freeciv.PR10216/ai/advdomestic.c
--- vendor.freeciv.current/ai/advdomestic.c     2004-10-03 11:40:07.000000000 
+0100
+++ freeciv.PR10216/ai/advdomestic.c    2004-10-06 21:09:08.000000000 +0100
@@ -194,8 +194,12 @@
       
     } else if (want < -choice->want) {
       /* We need boats to colonize! */
+      /* We might need boats even if there are boats free,
+       * if they are blockaded or in inland seas*/
+      struct ai_data *ai = ai_data_get(pplayer);
       CITY_LOG(LOG_DEBUG, pcity, "desires founders with passion %d and asks"
-              " for a boat", want);
+              " for a new boat (%d of %d free)",
+              want, ai->stats.available_boats, ai->stats.boats);
       choice->want = 0 - want;
       choice->type = CT_NONMIL;
       choice->choice = unit_type; /* default */
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/ai/aiferry.c freeciv.PR10216/ai/aiferry.c
--- vendor.freeciv.current/ai/aiferry.c 2004-10-03 11:40:07.000000000 +0100
+++ freeciv.PR10216/ai/aiferry.c        2004-10-06 21:09:08.000000000 +0100
@@ -44,7 +44,7 @@
  * The below is used only by passengers in ai.ferryboat field 
  */ 
 #define FERRY_WANTED      -1      /* Needs a boat */
-#define FERRY_NONE         0      /* Has no boa and doesn't need one */
+#define FERRY_NONE         0      /* Has no boat and doesn't need one */
 
 
 /* =================== group log levels, debug options ================= */
@@ -281,6 +281,7 @@
 /****************************************************************************
   Proper and real PF function for finding a boat.  If you don't require
   the path to the ferry, pass path=NULL.
+  Return the unit ID of the boat; punit is the passenger.
 
   WARNING: Due to the nature of this function and PF (see the comment of 
   combined_land_sea_move), the path won't lead onto the boat itself.
@@ -293,12 +294,12 @@
   int best_id = 0;
   struct pf_parameter param;
   struct pf_map *search_map;
-
-
+  int ferryboat = punit->ai.ferryboat; /*currently assigned ferry*/
+  assert(0 < ferryboat || FERRY_NONE == ferryboat || FERRY_WANTED == 
ferryboat);
   UNIT_LOG(LOGLEVEL_FINDFERRY, punit, "asked find_ferry for a boat");
 
   if (aiferry_avail_boats(unit_owner(punit)) <= 0 
-      && punit->ai.ferryboat <= 0) {
+      && ferryboat <= 0) {
     /* No boats to be found (the second check is to ensure that we are not 
      * the ones keeping the last boat busy) */
     return 0;
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/ai/aisettler.c freeciv.PR10216/ai/aisettler.c
--- vendor.freeciv.current/ai/aisettler.c       2004-10-03 11:40:07.000000000 
+0100
+++ freeciv.PR10216/ai/aisettler.c      2004-10-06 21:09:08.000000000 +0100
@@ -522,17 +522,20 @@
 
   pf_destroy_map(map);
 
+  assert(!found || 0 <= best->result);
   return found;
 }
 
 /**************************************************************************
   Find nearest and best city placement or (TODO) a city to immigrate to.
 
-  Option look_for_boat forces to find a boat before cosidering going 
+  Option look_for_boat forces to find a (real) boat before cosidering going 
   overseas.  Option use_virt_boat allows to use virtual boat but only
   if punit is in a coastal city right now (should only be used by 
   virtual units).  I guess it won't hurt to remove this condition, PF 
   will just give no positions.
+  If (!look_for_boat && !use_virt_boat), will not consider placements
+  overseas.
 **************************************************************************/
 void find_best_city_placement(struct unit *punit, struct cityresult *best,
                              bool look_for_boat, bool use_virt_boat)
@@ -542,6 +545,8 @@
   struct unit *ferry = NULL;
 
   assert(pplayer->ai.control);
+  /*Only virtual units may use virtual boats:*/
+  assert(0 == punit->id || !use_virt_boat);
 
   best->tile = NULL;
   best->result = 0;
@@ -586,6 +591,7 @@
       ferry->tile = punit->tile;
     }
 
+    assert(SEA_MOVING == unit_type(ferry)->move_type);
     pft_fill_unit_overlap_param(&parameter, ferry);
     parameter.get_TB = no_fights_or_unknown;
 
@@ -599,4 +605,7 @@
       best->virt_boat = (ferry->id == 0);
     }
   }
+  /*If we use a virtual boat, we must have permission and be emigrating:*/
+  assert(!best->virt_boat || use_virt_boat);
+  assert(!best->virt_boat || best->overseas);
 }
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/common/aicore/pf_tools.c 
freeciv.PR10216/common/aicore/pf_tools.c
--- vendor.freeciv.current/common/aicore/pf_tools.c     2004-10-03 
11:40:06.000000000 +0100
+++ freeciv.PR10216/common/aicore/pf_tools.c    2004-10-06 21:09:07.000000000 
+0100
@@ -75,7 +75,7 @@
 
 /************************************************************
   A cost function for a sea unit which allows going one step 
-  into the land (shore bombardment).
+  into the land (for shore bombardment and ferries).
   Things to remember: we should prevent going from land to
   anywhere, unless we are leaving a friendly city, in which
   case we can move into the ocean but not into the land.
@@ -470,8 +470,8 @@
 }
 
 /**********************************************************************
-  Switch on one tile overlapping into the sea/land 
-  ("sea/land bombardment")
+  Switch on one tile overlapping into the sea/land.
+  For sea/land bombardment and for ferries.
 **********************************************************************/
 void pft_fill_unit_overlap_param(struct pf_parameter *parameter,
                                 struct unit *punit)
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/common/city.h freeciv.PR10216/common/city.h
--- vendor.freeciv.current/common/city.h        2004-10-03 11:40:06.000000000 
+0100
+++ freeciv.PR10216/common/city.h       2004-10-06 21:09:07.000000000 +0100
@@ -201,7 +201,10 @@
   signed short int tile_value[CITY_MAP_SIZE][CITY_MAP_SIZE];
 
   /* so we can contemplate with warmap fresh and decide later */
-  int settler_want, founder_want; /* for builder (F_SETTLERS) and founder 
(F_CITIES) */
+  /* For builder (F_SETTLERS) and founder (F_CITIES) */
+  /* Negative values indicate that the city needs a boat first;
+     -1*value is the degree of want in that case.*/
+  int settler_want, founder_want;
   bool founder_boat; /* if the city founder will need a boat */
   int invasion; /* who's coming to kill us, for attack co-ordination */
   int attack, bcost; /* This is also for invasion - total power and value of
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/common/unit.h freeciv.PR10216/common/unit.h
--- vendor.freeciv.current/common/unit.h        2004-10-03 11:40:06.000000000 
+0100
+++ freeciv.PR10216/common/unit.h       2004-10-06 21:09:07.000000000 +0100
@@ -107,7 +107,7 @@
 struct unit_ai {
   bool control; /* 0: not automated    1: automated */
   enum ai_unit_task ai_role;
-  /* The following are all unit ids */
+  /* The following are unit ids or special indicator values (<=0)*/
   int ferryboat; /* the ferryboat assigned to us */
   int passenger; /* the unit assigned to this ferryboat */
   int bodyguard; /* the unit bodyguarding us */
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/diff_ignore freeciv.PR10216/diff_ignore
--- vendor.freeciv.current/diff_ignore  2004-09-05 21:01:58.000000000 +0100
+++ freeciv.PR10216/diff_ignore 2004-10-06 21:10:17.000000000 +0100
@@ -17,6 +17,7 @@
 *~
 .#*
 .deps
+.svn
 CVS
 Freeciv.h
 Makefile
diff -Nur -xadvmilitary.c -Xfreeciv.PR10216/diff_ignore 
vendor.freeciv.current/server/settlers.c freeciv.PR10216/server/settlers.c
--- vendor.freeciv.current/server/settlers.c    2004-10-03 11:39:45.000000000 
+0100
+++ freeciv.PR10216/server/settlers.c   2004-10-06 21:08:47.000000000 +0100
@@ -750,7 +750,7 @@
 }
 
 /**************************************************************************
-  Tries to find a boat for our settler. Requires warmap to be initialized
+  Tries to find a boat for our unit. Requires warmap to be initialized
   with respect to x, y. cap is the requested capacity on the transport.
   Note that it may return a transport with less than cap capacity if this
   transport has zero move cost to x, y.
@@ -1126,7 +1126,7 @@
   }
 
   if (unit_flag(punit, F_CITIES) && pplayer->ai.control) {
-    find_best_city_placement(punit, &result, TRUE, FALSE);
+    find_best_city_placement(punit, &result, TRUE, FALSE);/*may use a boat*/
     UNIT_LOG(LOG_SETTLER, punit, "city want %d (impr want %d)", result.result,
              best_impr);
     if (result.result > best_impr) {
@@ -1476,6 +1476,7 @@
     bool is_coastal = is_ocean_near_tile(pcity->tile);
 
     find_best_city_placement(virtualunit, &result, is_coastal, is_coastal);
+    assert(0 <= result.result);
 
     CITY_LOG(LOG_DEBUG, pcity, "want(%d) to establish city at"
             " (%d, %d) and will %s to get there", result.result,