[freeciv-ai] Re: Possible bug in ai_military_gothere

[Jordi Negrevernis i Font <jorneg@xxxxxxxxxxx>]

En/na Per I. Mathisen ha escrit:

> +++ freeciv-cvs-Aug-24-ferryboat/ai/aiunit.c    Thu Sep  4 15:25:08 2003
> @@ -1484,7 +1485,16 @@
>       ferryboat->ai.passenger = punit->id;
>
>       /* Last ingredient: a beachhead. */
> -      if (find_beachhead(punit, dest_x, dest_y, &boat_x, &boat_y)) {
> +      if (!find_beachhead(punit, dest_x, dest_y, &boat_x, &boat_y)) {
> +        /* we try the other function */
> +        if (dcity) {
> +          find_city_beach(dcity, punit, &boat_x, &boat_y);
> +        } else {
> +          /* the previous function failed, so... */
> +          boat_x = punit->x; boat_y = punit->y;
> +        }
> +      }
>
> Better:
>
> if (!find_beachhead(punit, dest_x, dest_y, &boat_x, &boat_y)
>    && dcity) {
>  /* we try the other function */
>  find_city_beach(dcity, punit, &boat_x, &boat_y);
> }
   Ok.

> +      if (!same_pos(punit->x, punit->y, boat_x, boat_y)) {
>
> This is not good. I think the code below should be run in any case, since
> we need to sentry and set variables.
   No. I just split the check for find_beachhead in two paragrafs. So 
that check must exist. We must sentry the units if we find a beach.
> +  if (ferryboat) {
> +    /* we go on a ferry! did we arrive? */
> +    boat_arrived = same_pos(ferryboat->x, ferryboat->y, ferryboat->goto_dest.x, 
> ferryboat->goto_dest.y) ||
> +                   is_tiles_adjacent(ferryboat->x, ferryboat->y, 
> ferryboat->goto_dest.x, ferryboat->goto_dest.y);
>
> Style nitpick: The || should be in front of the second conditional above.
   Ok.

> +  } else {
> +    boat_arrived = FALSE;
>   }
>
> -  if (goto_is_sane(punit, dest_x, dest_y, TRUE) && punit->moves_left > 0
> -      && (!ferryboat
> -          || (real_map_distance(punit->x, punit->y, dest_x, dest_y) < 3
> -              && (punit->ai.bodyguard == BODYGUARD_NONE
> +  if (goto_is_sane(punit, dest_x, dest_y, TRUE) && punit->moves_left > 0
> +      && (!ferryboat
> +          || ( boat_arrived
> +              && (punit->ai.bodyguard == BODYGUARD_NONE
>
> Can you explain this change?
   Yep!
   This is the main change in the patch. This change makes that the 
units get off the boat when they arrive to the beach head. Previously, 
they would not disembark because the objective was more that 3 tiles away.
>     /* if we are on a boat, disembark only if we are within two tiles of
> -     * our target, and either 1) we don't need a bodyguard, 2) we have a
> -     * bodyguard, or 3) we are going to an empty city.  Previously, cannons
> -     * would disembark before the cruisers arrived and die. -- Syela */
> +     * our beach head near the target, and either 1) we don't need a bodyguard,
> +     * 2) we have a bodyguard, or 3) we are going to an empty city. Previously,
> +     * cannons would disembark before the cruisers arrived and die. -- Syela */
>
> If you change comments attributed to an author (in this case syela), then
> remove the attribution. We should never put words in anyone else's mouth.
   ok

> -            /* the ferryboat needs to target the beachhead, but the unit
> -             * needs to target the city itself.  This is a little weird,
> +            /* the ferryboat needs to target the beachhead, but the unit
> +             * needs to target the city itself.  This is a little weird,
>
> Spurious diff? I see no differences.
>
>              * but it's the best we can do. -- Syela */
> -          } /* else do nothing, since we have no beachhead */
> +          } else {
> +            /* try to use find_city_beach */
> +            find_city_beach(acity, punit, &xx, &yy);
> +            if (!same_pos(punit->x, punit->y, xx, yy)) {
> +              /* we find beach head, same comment as above! */
> +              best = want;
> +              *x = acity->x;
> +              *y = acity->y;
> +            }
> +          }
>
> Is the !same_pos() above necessary?
   Yes. This is in the function find_something_to_kill. And we need to 
be sure that there is a beach head.
find_city_beach can fail returning the coords of the unit.

>         if (!find_beachhead(punit, pc->x, pc->y, &fx, &fy)) {
>           find_city_beach(pc, punit, &fx, &fy);
> -          freelog(LOG_DEBUG, "Barbarian sailing to city");
> -          ai_military_gothere(pplayer, punit, fx, fy);
> -       }
> +        }
> +        freelog(LOG_DEBUG, "Barbarian sailing to city");
> +        ai_military_gothere(pplayer, punit, fx, fy);
>       }
>     }
>
> Ouch. This fixes a rather nasty bug with barbarians, I think. My fault, I
> guess.
>
> Lastly, can you please post some savegames to show that it used to do the
> wrong thing but now does the right thing? Civworld is your friend.
   Maybe civworld is our friend, but the current ai is not may friend. 
The units that i place in the savegame ugly fortify when i run it!!
   I've been able to produce a savegame only for the barbarians.

   New patch attached and a savegame.


diff -Nur -Xfreeciv-cvs-Aug-24/diff_ignore freeciv-cvs-Aug-24/ai/aiunit.c 
freeciv-cvs-Aug-24-ferryboat/ai/aiunit.c
--- freeciv-cvs-Aug-24/ai/aiunit.c      Thu Sep  4 11:24:41 2003
+++ freeciv-cvs-Aug-24-ferryboat/ai/aiunit.c    Sat Sep 13 17:23:12 2003
@@ -1440,6 +1440,7 @@
   struct unit *def;
   struct city *dcity = map_get_city(dest_x, dest_y);
   struct tile *ptile;
+  int boat_x = -1, boat_y = -1, boat_arrived = 0;

   CHECK_UNIT(punit);

@@ -1474,19 +1475,25 @@
     ptile = map_get_tile(punit->x, punit->y);
     ferryboat = unit_list_find(&ptile->units, punit->ai.ferryboat);

     if (ferryboat && (ferryboat->ai.passenger == 0
                       || ferryboat->ai.passenger == punit->id)) {
-      int boat_x, boat_y;

       freelog(LOG_DEBUG, "We have FOUND BOAT, %d ABOARD %d@(%d,%d)->(%d, %d).",
               punit->id, ferryboat->id, punit->x, punit->y, dest_x, dest_y);
       handle_unit_activity_request(punit, ACTIVITY_SENTRY);
       ferryboat->ai.passenger = punit->id;

-      /* Last ingredient: a beachhead. */
-      if (find_beachhead(punit, dest_x, dest_y, &boat_x, &boat_y)) {
-       set_goto_dest(ferryboat, boat_x, boat_y);
-       set_goto_dest(punit, dest_x, dest_y);
+      /* Last ingredient: find beachhead. */
+      boat_x = punit->x; boat_y = punit->y;
+      if (!find_beachhead(punit, dest_x, dest_y, &boat_x, &boat_y) && (dcity)) 
{
+        /* we try the other function */
+        find_city_beach(dcity, punit, &boat_x, &boat_y);
+      }
+
+      /* we have a beach head, aboard!! */
+      if (!same_pos(punit->x, punit->y, boat_x, boat_y)) {
+        set_goto_dest(ferryboat, boat_x, boat_y);
+        set_goto_dest(punit, dest_x, dest_y);
         if (ground_unit_transporter_capacity(punit->x, punit->y, pplayer)
             <= 0) {
           /* FIXME: perhaps we should only require only two passengers */
@@ -1501,36 +1508,45 @@
               }
             }
           } unit_list_iterate_end; /* passengers are safely stowed away */
           if (!ai_unit_goto(ferryboat, boat_x, boat_y)) {
             return -1; /* died */
           }
           handle_unit_activity_request(punit, ACTIVITY_IDLE);
         } /* else wait, we can GOTO when more passengers come. */
       }
     }
   }
+
+  if (ferryboat) {
+    /* we go on a ferry! did we arrive? */
+    boat_arrived = same_pos(ferryboat->x, ferryboat->y, 
ferryboat->goto_dest.x, ferryboat->goto_dest.y)
+                   || is_tiles_adjacent(ferryboat->x, ferryboat->y, 
ferryboat->goto_dest.x, ferryboat->goto_dest.y);
+  } else {
+    boat_arrived = FALSE;
+  }

-  if (goto_is_sane(punit, dest_x, dest_y, TRUE) && punit->moves_left > 0
-      && (!ferryboat
-          || (real_map_distance(punit->x, punit->y, dest_x, dest_y) < 3
-              && (punit->ai.bodyguard == BODYGUARD_NONE
+  if (goto_is_sane(punit, dest_x, dest_y, TRUE) && punit->moves_left > 0
+      && (!ferryboat
+          || ( boat_arrived
+              && (punit->ai.bodyguard == BODYGUARD_NONE
                   || unit_list_find(&(map_get_tile(punit->x, punit->y)->units),
                                     punit->ai.bodyguard)
                   || (dcity && !has_defense(dcity)))))) {
     /* if we are on a boat, disembark only if we are within two tiles of
-     * our target, and either 1) we don't need a bodyguard, 2) we have a
-     * bodyguard, or 3) we are going to an empty city.  Previously, cannons
-     * would disembark before the cruisers arrived and die. -- Syela */
+     * our beach head near the target, and either 1) we don't need a bodyguard,
+     * 2) we have a bodyguard, or 3) we are going to an empty city. Previously,
+     * cannons would disembark before the cruisers arrived and die. */

     set_goto_dest(punit, dest_x, dest_y);
 
     /* The following code block is supposed to stop units from running away
      * from their bodyguards, and not interfere with those that don't have
      * bodyguards nearby -- Syela */
     /* The case where the bodyguard has moves left and could meet us en route
      * is not handled properly.  There should be a way to do it with dir_ok
      * but I'm tired now. -- Syela */
     if (punit->ai.bodyguard == BODYGUARD_WANTED) {
       adjc_iterate(punit->x, punit->y, i, j) {
         unit_list_iterate(map_get_tile(i, j)->units, aunit) {
           if (aunit->ai.charge != punit->id || punit->owner != aunit->owner) {
@@ -2230,25 +2246,34 @@
                 acity->name, acity->x, acity->y,
                 go_by_boat, move_time, want, best);
       }
 
       if (want > best && 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 {
+            /* try to use find_city_beach */
+            find_city_beach(acity, punit, &xx, &yy);
+            if (!same_pos(punit->x, punit->y, xx, yy)) {
+              /* we find beach head, same comment as above! */
+              best = want;
+              *x = acity->x;
+              *y = acity->y;
+            }
+          }
         } else {
           best = want;
           *x = acity->x;
@@ -2475,9 +2500,9 @@
         /* sometimes find_beachhead is not enough */
         if (!find_beachhead(punit, pc->x, pc->y, &fx, &fy)) {
           find_city_beach(pc, punit, &fx, &fy);
-          freelog(LOG_DEBUG, "Barbarian sailing to city");
-          ai_military_gothere(pplayer, punit, fx, fy);
-       }
+        }
+        freelog(LOG_DEBUG, "Barbarian sailing to city");
+        ai_military_gothere(pplayer, punit, fx, fy);
       }
     }
   }

prova-ferry.sav.gz
Description: application/compressed