[freeciv-ai] Re: [Freeciv-Dev] (PR#6199) huge AI ferry cleanup

["Per I. Mathisen" <per@xxxxxxxxxxx>]

Here is the target part of the cleanup ripped out and isolated. It is
still big, but hopefully Greg won't mind committing it now.

I did some testing, and it looked like it worked without the new beachhead
code (tried to retrofit the old one). (Most of my civworld testing
savegames are broken by overzealous transported_by asserts, though.)

I also added more comments.

  - Per

Index: ai/advmilitary.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
retrieving revision 1.153
diff -u -r1.153 advmilitary.c
--- ai/advmilitary.c    16 Sep 2003 18:04:47 -0000      1.153
+++ ai/advmilitary.c    22 Sep 2003 14:13:02 -0000
@@ -753,10 +753,11 @@
 **************************************************************************/
 static void process_attacker_want(struct city *pcity,
                                   int value, Unit_Type_id victim_unit_type,
-                                  bool veteran, int x, int y,
+                                  bool veteran, struct target *ptarget,
                                   struct ai_choice *best_choice,
                                   struct unit *boat, Unit_Type_id boattype)
 {
+  int x = ptarget->x, y = ptarget->y;
   struct player *pplayer = city_owner(pcity);
   /* The enemy city.  acity == NULL means stray enemy unit */
   struct city *acity = map_get_city(x, y);
@@ -938,30 +939,16 @@
 static void kill_something_with(struct player *pplayer, struct city *pcity, 
                                struct unit *myunit, struct ai_choice *choice)
 {
-  struct ai_data *ai = ai_data_get(pplayer);
-  /* Our attack rating (with reinforcements) */
-  int attack;
-  /* Benefit from fighting the target */
-  int benefit;
-  /* Enemy defender type */
-  Unit_Type_id def_type;
-  /* Target coordinates */
-  int x, y;
-  /* Our transport */
   struct unit *ferryboat = NULL;
-  /* Out target */
-  struct city *acity;
-  /* Defender of the target city/tile */
-  struct unit *pdef; 
-  /* Coordinates of the boat */
-  int bx = 0, by = 0;
-  /* Type of the boat (real or a future one) */
-  Unit_Type_id boattype = U_LAST;
-  int boatspeed;
-  bool go_by_boat;
-  /* Is the defender veteran? */
-  bool def_vet;
+  int attack; /* Our attack rating (with reinforcements) */
+  int benefit; /* Benefit from fighting the target */
+  Unit_Type_id def_type; /* Enemy defender type */
+  Unit_Type_id boattype = U_LAST; /* Type of the boat (real or a future one) */
+  int boatspeed = 1;
+  bool def_vet; /* Is the defender veteran? */
   struct ai_choice best_choice;
+  struct target mytarget;
+  bool virtualferry = FALSE;
 
   init_choice(&best_choice);
   best_choice.choice = myunit->type;
@@ -981,42 +968,49 @@
     return;
   }
 
-  if (is_ground_unit(myunit)) {
+  if (is_ground_unit(myunit)) { /* find or fake ferry */
+    int bx = 0, by = 0; /* Coordinates of the boat */
     int boatid = find_boat(pplayer, &bx, &by, 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);
+    if (boatid != 0) {
+      ferryboat = player_find_unit_by_id(pplayer, boatid);
+    } else if (myunit->id == 0 && is_at_coast(pcity->x, pcity->y)) {
+      /* has no boat, found no boat, so make virtual boat */
+      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);
+      }
+      if (boattype != U_LAST) { /* we really have any boats in this ruleset */
+        ferryboat = create_unit_virtual(pplayer, pcity, boattype, FALSE);
+        virtualferry = TRUE;
+      }
+    }
+    if (ferryboat) {
+      boattype = ferryboat->type;
+      boatspeed = unit_types[boattype].move_rate;
+      myunit->ai.ferryboat = ferryboat->id;
     }
   }
-  boatspeed = unit_types[boattype].move_rate;
 
-  best_choice.want = find_something_to_kill(pplayer, myunit, &x, &y);
+  target_init(myunit, &mytarget);
 
-  acity = map_get_city(x, y);
+  find_something_to_kill(pplayer, &mytarget);
+  best_choice.want = mytarget.want;
 
   if (myunit->id != 0) {
-    freelog(LOG_ERROR, "ERROR: Non-virtual unit in kill_something_with!");
-    return;
+    die("ERROR: Non-virtual unit in kill_something_with!");
   }
-  
+
   attack = unit_att_rating(myunit);
-  if (acity) {
-    attack += acity->ai.attack;
+  if (mytarget.city) {
+    attack += mytarget.city->ai.attack;
   }
   attack *= attack;
-  
-  if (acity) {
+
+  if (mytarget.city) {
     /* Our move rate */
     int move_rate = unit_types[myunit->type].move_rate;
-    /* Distance to target (in turns) */
-    int move_time;
     /* Rating of enemy defender */
     int vuln;
 
@@ -1024,22 +1018,12 @@
       /* See comment in unit_move_turns */
       move_rate *= 3;
     }
-    
-    if (!HOSTILE_PLAYER(pplayer, ai, city_owner(acity))) {
-      /* Not a valid target */
-      return;
-    }
 
-    go_by_boat = !(goto_is_sane(myunit, acity->x, acity->y, TRUE) 
-                  && WARMAP_COST(x, y) <= (MIN(6, move_rate) * THRESHOLD));
-    move_time = turns_to_enemy_city(myunit->type, acity, move_rate, 
-                                    go_by_boat, ferryboat, boattype);
-
-    def_type = ai_choose_defender_versus(acity, myunit->type);
-    if (move_time > 1) {
-      def_vet = do_make_unit_veteran(acity, def_type);
+    def_type = ai_choose_defender_versus(mytarget.city, myunit->type);
+    if (mytarget.ETA > 1) {
+      def_vet = do_make_unit_veteran(mytarget.city, def_type);
       vuln = unittype_def_rating_sq(myunit->type, def_type,
-                                    x, y, FALSE, def_vet);
+                                    mytarget.x, mytarget.y, FALSE, def_vet);
       benefit = unit_types[def_type].build_cost;
     } else {
       vuln = 0;
@@ -1047,56 +1031,59 @@
       def_vet = FALSE;
     }
 
-    pdef = get_defender(myunit, x, y);
-    if (pdef) {
-      int m = unittype_def_rating_sq(myunit->type, pdef->type,
-                                     x, y, FALSE, pdef->veteran);
+    if (mytarget.defender) {
+      int m = unittype_def_rating_sq(myunit->type, mytarget.defender->type,
+                                     mytarget.x, mytarget.y, FALSE, 
+                                     mytarget.defender->veteran);
       if (vuln < m) {
         vuln = m;
-        benefit = unit_type(pdef)->build_cost;
-        def_vet = pdef->veteran;
-        def_type = pdef->type; 
+        benefit = unit_type(mytarget.defender)->build_cost;
+        def_vet = mytarget.defender->veteran;
+        def_type = mytarget.defender->type; 
       }
     }
-    if (COULD_OCCUPY(myunit) || TEST_BIT(acity->ai.invasion, 0)) {
+    if (COULD_OCCUPY(myunit) || TEST_BIT(mytarget.city->ai.invasion, 0)) {
       /* bonus for getting the city */
       benefit += 40;
     }
 
     /* end dealing with cities */
   } else {
-
-    pdef = get_defender(myunit, x, y);
-    if (!pdef) {
+    if (!mytarget.defender) {
       /* Nobody to attack! */
-      return;
+      goto cleanup;
     }
 
-    benefit = unit_type(pdef)->build_cost;
-    go_by_boat = FALSE;
+    benefit = unit_type(mytarget.defender)->build_cost;
 
-    def_type = pdef->type;
-    def_vet = pdef->veteran;
+    def_type = mytarget.defender->type;
+    def_vet = mytarget.defender->veteran;
     /* end dealing with units */
   }
   
-  if (!go_by_boat) {
-    process_attacker_want(pcity, benefit, def_type, def_vet, x, y, 
+  if (!mytarget.ferry) {
+    process_attacker_want(pcity, benefit, def_type, def_vet, &mytarget,
                           &best_choice, NULL, U_LAST);
   } else { 
-    process_attacker_want(pcity, benefit, def_type, def_vet, x, y, 
+    process_attacker_want(pcity, benefit, def_type, def_vet, &mytarget, 
                           &best_choice, ferryboat, boattype);
   }
 
   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_ferryboat(pplayer, pcity, choice);
+    if (mytarget.ferry && !ferryboat) {
+      ai_choose_ferryboat(pplayer, pcity, choice); /* build ferry */
     }
     freelog(LOG_DEBUG, "%s has chosen attacker, %s, want=%d",
             pcity->name, unit_types[choice->choice].name, choice->want);
-  } 
+  }
+
+  cleanup:
+  if (virtualferry) {
+    destroy_unit_virtual(ferryboat);
+  }
+  target_done(&mytarget);
 }
 
 /**********************************************************************
Index: ai/aiunit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
retrieving revision 1.289
diff -u -r1.289 aiunit.c
--- ai/aiunit.c 21 Sep 2003 09:06:43 -0000      1.289
+++ ai/aiunit.c 22 Sep 2003 14:13:02 -0000
@@ -1199,8 +1199,6 @@
   int ok, best = 0;
   enum tile_terrain_type t;
 
-  CHECK_UNIT(punit);
-
   adjc_iterate(dest_x, dest_y, x1, y1) {
     ok = 0;
     t = map_get_terrain(x1, y1);
@@ -1846,44 +1844,95 @@
 }
 
 /*************************************************************************
+  Initialize targeting engine with ptarget. Call this function once 
+  before you try to find or follow targets with a unit. You need to 
+  have found a potential ferry to the unit beforehand, since we will
+  use this ferry for all our calculation needs.
+
+  We set up path-finding for punit and its ferry here.
+**************************************************************************/
+void target_init(struct unit *punit, struct target *ptarget)
+{
+  ptarget->hunter = punit;
+  ptarget->x = ptarget->hunter->x;
+  ptarget->y = ptarget->hunter->y;
+  ptarget->ferry = find_unit_by_id(punit->ai.ferryboat);
+  ptarget->want = -1;
+  ptarget->beachx = -1;
+  ptarget->beachy = -1;
+  ptarget->city = NULL;
+  ptarget->defender = NULL;
+  ptarget->ETA = FC_INFINITY;
+
+  generate_warmap(map_get_city(ptarget->x, ptarget->y), punit);
+  if (ptarget->ferry) { /* temporary until pf */
+    really_generate_warmap(map_get_city(ptarget->ferry->x, ptarget->ferry->y),
+                           ptarget->ferry, SEA_MOVING);
+  }
+}
+
+/*************************************************************************
+  Set target with minimum options, and assure the rest is reset. Always
+  set the other options AFTER calling this function.
+**************************************************************************/
+void target_set(struct target *ptarget, int want, int x, int y,
+                struct unit *pdefender, int ETA)
+{
+  ptarget->x = x;
+  ptarget->y = y;
+  ptarget->ferry = NULL;
+  ptarget->want = want;
+  ptarget->beachx = -1;
+  ptarget->beachy = -1;
+  ptarget->city = NULL;
+  ptarget->defender = pdefender;
+  ptarget->ETA = ETA;
+}
+
+/*************************************************************************
+  Clean up.
+**************************************************************************/
+void target_done(struct target *ptarget)
+{
+  ptarget->hunter = NULL;
+  ptarget->x = -1;
+  ptarget->y = -1;
+  ptarget->beachx = -1;
+  ptarget->beachy = -1;
+  ptarget->want = -1;
+  ptarget->ETA = FC_INFINITY;
+  ptarget->city = NULL;
+  ptarget->defender = NULL;
+  ptarget->ferry = NULL;
+}
+
+/*************************************************************************
   Find something to kill! This function is called for units to find 
   targets to destroy and for cities that want to know if they should
-  build offensive units. Target location returned in (x, y), want as
-  function return value.
+  build offensive units.
 
   punit->id == 0 means that the unit is virtual (considered to be built).
+
+  Ferry must already by arranged before this function is called.
 **************************************************************************/
-int find_something_to_kill(struct player *pplayer, struct unit *punit, 
-                           int *x, int *y)
+void find_something_to_kill(struct player *pplayer, struct target *ptarget)
+#define punit ptarget->hunter
 {
+  struct unit *ferryboat = ptarget->ferry;
   struct ai_data *ai = ai_data_get(pplayer);
-  /* basic attack */
-  int attack_value = unit_att_rating(punit);
-  /* Enemy defence rating */
-  int vuln;
-  /* Benefit from killing the target */
-  int benefit;
-  /* Number of enemies there */
-  int victim_count;
-  /* Want (amortized) of the operaton */
-  int want;
-  /* Best of all wants */
-  int best = 0;
-  /* Our total attack value with reinforcements */
-  int attack;
+  int attack_value = unit_att_rating(punit); /* basic attack */
+  int vuln; /* Enemy defence rating */
+  int benefit; /* Benefit from killing the target */
+  int victim_count; /* Number of enemies there */
+  int want; /* Want (amortized) of the operaton */
+  int attack; /* Our total attack value with reinforcements */
   int move_time, move_rate;
   int con = map_get_continent(punit->x, punit->y);
   struct unit *pdef;
-  int maxd, needferry;
-  /* Do we have access to sea? */
-  bool harbor = FALSE;
-  int bx = 0, by = 0;
-  /* Build cost of the attacker (+adjustments) */
-  int bcost, bcost_bal;
+  int maxd;
+  int bcost, bcost_bal; /* Build cost of the attacker (+adjustments) */
   bool handicap = ai_handicap(pplayer, H_TARGETS);
-  struct unit *ferryboat = NULL;
-  /* Type of our boat (a future one if ferryboat == NULL) */
-  Unit_Type_id boattype = U_LAST;
+  Unit_Type_id boattype = U_LAST; /* Type of our ferry */
   bool unhap = FALSE;
   struct city *pcity;
   /* this is a kluge, because if we don't set x and y with !punit->id,
@@ -1893,17 +1942,13 @@
   int bk = 0; 
 
   /*** Very preliminary checks */
-  *x = punit->x; 
-  *y = punit->y;
-
   if (!is_ground_unit(punit) && !is_sailing_unit(punit)) {
     /* Don't know what to do with them! */
-    return 0;
+    return;
   }
-
   if (attack_value == 0) {
     /* A very poor attacker... */
-    return 0;
+    return;
   }
 
   /*** Part 1: Calculate targets ***/
@@ -1977,29 +2022,8 @@
   bcost = unit_type(punit)->build_cost;
   bcost_bal = build_cost_balanced(punit->type);
 
-  /* most flexible but costs milliseconds */
-  generate_warmap(map_get_city(*x, *y), punit);
-
-  if (is_ground_unit(punit)) {
-    int boatid = find_boat(pplayer, &bx, &by, 2);
-    ferryboat = player_find_unit_by_id(pplayer, boatid);
-  }
-
   if (ferryboat) {
     boattype = ferryboat->type;
-    really_generate_warmap(map_get_city(ferryboat->x, ferryboat->y),
-                           ferryboat, SEA_MOVING);
-  } 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);
-    }
-  }
-
-  if (is_ground_unit(punit) && punit->id == 0 
-      && is_ocean_near_tile(punit->x, punit->y)) {
-    harbor = TRUE;
   }
 
   players_iterate(aplayer) {
@@ -2009,9 +2033,8 @@
     }
 
     city_list_iterate(aplayer->cities, acity) {
-      bool go_by_boat = (is_ground_unit(punit)
-                         && !(goto_is_sane(punit, acity->x, acity->y, TRUE) 
-                              && WARMAP_COST(acity->x, acity->y) < maxd));
+      bool go_by_boat = FALSE;
+      int beachx = -1, beachy = -1;
 
       if (!is_ocean(map_get_terrain(acity->x, acity->y))
           && unit_flag(punit, F_NO_LAND_ATTACK)) {
@@ -2024,13 +2047,14 @@
         continue;
       }
 
-      if (go_by_boat 
-          && (!(ferryboat || harbor)
-              || WARMAP_SEACOST(acity->x, acity->y) > 6 * THRESHOLD)) {
-        /* Too far or impossible to go by boat */
-        continue;
+      /* Go by boat if we have a ferry (nominally) and cannot walk.
+       * FIXME: Check if ferry is faster! */
+      if (is_ground_unit(punit) && ferryboat
+          && !goto_is_sane(punit, acity->x, acity->y, TRUE)
+          && find_beachhead(punit, acity->x, acity->y, &beachx, &beachy)) {
+        go_by_boat = TRUE;
       }
-      
+
       if (is_sailing_unit(punit) 
           && WARMAP_SEACOST(acity->x, acity->y) >= maxd) {
         /* Too far to sail */
@@ -2104,58 +2128,42 @@
       }
       want -= move_time * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING 
                            : SHIELD_WEIGHTING);
-      /* build_cost of ferry */
-      needferry = (go_by_boat && !ferryboat ? unit_value(boattype) : 0);
       /* FIXME: add time to build the ferry? */
       want = military_amortize(pplayer, find_city_by_id(punit->homecity),
-                               want, MAX(1, move_time), bcost_bal + needferry);
+                               want, MAX(1, move_time), bcost_bal);
 
       /* BEGIN STEAM-ENGINES-ARE-OUR-FRIENDS KLUGE */
-      if (want <= 0 && punit->id == 0 && best == 0) {
+      if (want <= 0 && punit->id == 0 && ptarget->want == 0) {
         int bk_e = military_amortize(pplayer, find_city_by_id(punit->homecity),
                                      benefit * SHIELD_WEIGHTING, 
-                                     MAX(1, move_time), bcost_bal + needferry);
+                                     MAX(1, move_time), bcost_bal);
 
         if (bk_e > bk) {
-          *x = acity->x;
-          *y = acity->y;
+          ptarget->x = acity->x;
+          ptarget->y = acity->y;
           bk = bk_e;
         }
       }
       /* END STEAM-ENGINES KLUGE */
-      
-      if (punit->id != 0 && ferryboat && is_ground_unit(punit)) {
-        freelog(LOG_DEBUG, "%s@(%d, %d) -> %s@(%d, %d) -> %s@(%d, %d)"
-                " (go_by_boat=%d, move_time=%d, want=%d, best=%d)",
-                unit_type(punit)->name, punit->x, punit->y,
-                unit_type(ferryboat)->name, bx, by,
-                acity->name, acity->x, acity->y, 
-                go_by_boat, move_time, want, best);
+
+#ifdef DEBUG_FSTK
+      if (punit->id != 0 && go_by_boat) {
+        UNIT_LOG(LOG_NORMAL, punit, "go to %s(%d, %d) by way of %s@(%d, %d)"
+                 "->(%d, %d), time %d, want %d, best %d?", acity->name, 
+                 acity->x, acity->y, unit_type(ferryboat)->name, ferryboat->x, 
+                 ferryboat->y, beachx, beachy, move_time, want, ptarget->want);
       }
+#endif
       
-      if (want > best && ai_fuzzy(pplayer, TRUE)) {
+      if (want > ptarget->want && ai_fuzzy(pplayer, TRUE)) {
         /* Yes, we like this target */
-        if (punit->id != 0 && is_ground_unit(punit) 
-            && !unit_flag(punit, F_MARINES)
-            && map_get_continent(acity->x, acity->y) != con) {
-          /* a non-virtual ground unit is trying to attack something on 
-           * another continent.  Need a beachhead which is adjacent 
-           * to the city and an available ocean tile */
-          int xx, yy;
-
-          if (find_beachhead(punit, acity->x, acity->y, &xx, &yy)) { 
-            best = want;
-            *x = acity->x;
-            *y = acity->y;
-            /* the ferryboat needs to target the beachhead, but the unit 
-             * needs to target the city itself.  This is a little weird, 
-             * but it's the best we can do. -- Syela */
-          } /* else do nothing, since we have no beachhead */
-        } else {
-          best = want;
-          *x = acity->x;
-          *y = acity->y;
-        } /* end need-beachhead else */
+        target_set(ptarget, want, acity->x, acity->y, pdef, move_time);
+        if (go_by_boat) {
+          ptarget->ferry = ferryboat;
+        }
+        ptarget->beachx = beachx;
+        ptarget->beachy = beachy;
+        ptarget->city = acity;
       }
     } city_list_iterate_end;
 
@@ -2164,6 +2172,8 @@
      * I am deliberately not adding ferryboat code to the unit_list_iterate. 
      * -- Syela */
     unit_list_iterate(aplayer->units, aunit) {
+      struct unit *pdefender;
+
       if (map_get_city(aunit->x, aunit->y)) {
         /* already dealt with it */
         continue;
@@ -2183,8 +2193,9 @@
       /* We have to assume the attack is diplomatically ok.
        * We cannot use can_player_attack_tile, because we might not
        * be at war with aplayer yet */
+      pdefender = get_defender(punit, aunit->x, aunit->y);
       if (!can_unit_attack_all_at_tile(punit, aunit->x, aunit->y)
-          || !(aunit == get_defender(punit, aunit->x, aunit->y))) {
+          || aunit != pdefender) {
         /* We cannot attack it, or it is not the main defender. */
         continue;
       }
@@ -2227,16 +2238,13 @@
       }
       want = military_amortize(pplayer, find_city_by_id(punit->homecity),
                                want, MAX(1, move_time), bcost_bal);
-      if (want > best && ai_fuzzy(pplayer, TRUE)) {
-        best = want;
-        *x = aunit->x;
-        *y = aunit->y;
+      if (want > ptarget->want && ai_fuzzy(pplayer, TRUE)) {
+        target_set(ptarget, want, aunit->x, aunit->y, pdefender, move_time);
       }
     } unit_list_iterate_end;
   } players_iterate_end;
-
-  return(best);
 }
+#undef punit
 
 /**********************************************************************
   Find safe city to recover in. An allied player's city is just as 
@@ -2294,10 +2302,12 @@
 **************************************************************************/
 static void ai_military_attack(struct player *pplayer, struct unit *punit)
 {
-  int dest_x, dest_y; 
   int id = punit->id;
   int ct = 10;
   struct city *pcity = NULL;
+  struct target mytarget;
+
+  target_init(punit, &mytarget);
 
   CHECK_UNIT(punit);
 
@@ -2306,41 +2316,42 @@
     /* First find easy nearby enemies, anything better than pillage goes */
     if (!ai_military_rampage(punit, RAMPAGE_ANYTHING, 
                              RAMPAGE_ANYTHING)) {
-      return; /* we died */
+      goto cleanup; /* we died */
     }
 
     if (stay_and_defend(punit)) {
       /* This city needs defending, don't go outside! */
       UNIT_LOG(LOG_DEBUG, punit, "stayed to defend %s", 
                map_get_city(punit->x, punit->y)->name);
-      return;
+      goto cleanup;
     }
 
     /* Then find enemies the hard way */
-    find_something_to_kill(pplayer, punit, &dest_x, &dest_y);
-    if (!same_pos(punit->x, punit->y, dest_x, dest_y)) {
+    find_something_to_kill(pplayer, &mytarget);
+    if (!same_pos(punit->x, punit->y, mytarget.x, mytarget.y)) {
      int repeat;
 
      for(repeat = 0; repeat < 2; repeat++) {
 
-      if (!is_tiles_adjacent(punit->x, punit->y, dest_x, dest_y)
-          || !can_unit_attack_tile(punit, dest_x, dest_y)
-          || (could_unit_move_to_tile(punit, dest_x, dest_y) == 0)) {
+      if (!is_tiles_adjacent(punit->x, punit->y, mytarget.x, mytarget.y)
+          || !can_unit_attack_tile(punit, mytarget.x, mytarget.y)
+          || (could_unit_move_to_tile(punit, mytarget.x, mytarget.y) == 0)) {
         /* Can't attack or move usually means we are adjacent but
          * on a ferry. This fixes the problem (usually). */
         UNIT_LOG(LOG_DEBUG, punit, "mil att gothere -> %d, %d", 
-                 dest_x, dest_y);
-        if (ai_military_gothere(pplayer, punit, dest_x, dest_y) <= 0) {
+                 mytarget.x, mytarget.y);
+        if (ai_military_gothere(pplayer, punit, mytarget.x, mytarget.y) <= 0) {
           /* Died or got stuck */
-          return;
+          goto cleanup;
         }
       } else {
         /* Close combat. fstk sometimes want us to attack an adjacent
          * enemy that rampage wouldn't */
-        UNIT_LOG(LOG_DEBUG, punit, "mil att bash -> %d, %d", dest_x, dest_y);
-        if (!ai_unit_attack(punit, dest_x, dest_y)) {
+        UNIT_LOG(LOG_DEBUG, punit, "mil att bash -> %d, %d", 
+                 mytarget.x, mytarget.y);
+        if (!ai_unit_attack(punit, mytarget.x, mytarget.y)) {
           /* Died */
-          return;
+          goto cleanup;
         }
       }
 
@@ -2357,7 +2368,7 @@
 
   /* Cleanup phase */
   if (punit->moves_left == 0) {
-    return;
+    goto cleanup;
   }
   pcity = find_nearest_safe_city(punit);
   if (is_sailing_unit(punit) && pcity) {
@@ -2376,7 +2387,6 @@
         UNIT_LOG(LOG_DEBUG, punit, "Barbarian marching to conquer %s", 
pc->name);
         ai_military_gothere(pplayer, punit, pc->x, pc->y);
       } else {
-        /* sometimes find_beachhead is not enough */
         if (!find_beachhead(punit, pc->x, pc->y, &fx, &fy)) {
           find_city_beach(pc, punit, &fx, &fy);
         }
@@ -2399,6 +2409,8 @@
       ai_military_gohome(pplayer, punit);
     }
   }
+  cleanup:
+  target_done(&mytarget);
 }
 
 /*************************************************************************
Index: ai/aiunit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.h,v
retrieving revision 1.47
diff -u -r1.47 aiunit.h
--- ai/aiunit.h 26 Aug 2003 11:12:51 -0000      1.47
+++ ai/aiunit.h 22 Sep 2003 14:13:02 -0000
@@ -16,6 +16,15 @@
 #include "combat.h"
 #include "unittype.h"
 
+struct target {
+  struct unit *hunter;
+  struct unit *ferry;
+  int x, y, want, beachx, beachy;
+  struct city *city;
+  struct unit *defender;
+  int ETA; /* Estimated Time to Arrival */
+};
+
 /*
  * To prevent integer overflows the product "power * hp * firepower"
  * is divided by POWER_DIVIDER.
@@ -57,10 +66,14 @@
                         struct unit *boat, Unit_Type_id boattype);
 int turns_to_enemy_unit(Unit_Type_id our_type, int speed, int x, int y, 
                         Unit_Type_id enemy_type);
-int find_something_to_kill(struct player *pplayer, struct unit *punit, 
-                            int *x, int *y);
+void find_something_to_kill(struct player *pplayer, struct target *ptarget);
 bool find_beachhead(struct unit *punit, int dest_x, int dest_y, 
                     int *x, int *y);
+
+void target_init(struct unit *punit, struct target *ptarget);
+void target_set(struct target *ptarget, int want, int x, int y,
+                struct unit *pdefender, int ETA);
+void target_done(struct target *ptarget);
 
 int build_cost_balanced(Unit_Type_id type);
 int unittype_att_rating(Unit_Type_id type, bool veteran,