[Freeciv-Dev] (PR#12931) autoexplorers crash in development code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12931 >

The attached patch improves the logging here.

The problem is with unit 342.  It does a goto to tile (24,20).  However
this tile is on another island!  Somehow the autoexplore PF found this
tile by walking across to the adjacent tile along a ferryboat that
happened to be in the channel.  Not sure if this is a feature or a bug,
but in ai_unit_goto this goto is deemed to be impossible.  The unit's
activity is set to IDLE inside the goto.  Not sure if this is a feature
or a bug, but it causes the aiexplorer code to conclude the exploration
is complete (seems right).  It then returns to the parent server code
which triggers the assertion.

You'd think the assertion is simply spurious.  However without it the
unit gets stuck under AI control.  Units of a human player under AI
control that are IDLE do nothing at all (which is a separate bug and
should be fixed).  So the horsemen just get stuck in place.  The second
patch fixes this (but it's a hack).  It should probably be committed to
2.0 also.

-jason

Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.342
diff -u -r1.342 unittools.c
--- server/unittools.c  29 Apr 2005 17:39:29 -0000      1.342
+++ server/unittools.c  30 Apr 2005 03:21:21 -0000
@@ -648,9 +648,15 @@
       return;
     }
 
-    assert(punit->activity == ACTIVITY_EXPLORE); 
-    if (!more_to_explore) {
+    /* ai_manage_explorer isn't supposed to change the activity but we
+     * don't count on this. */
+    if (punit->activity != ACTIVITY_EXPLORE || !more_to_explore) {
       handle_unit_activity_request(punit, ACTIVITY_IDLE);
+
+      /* FIXME: When the ai_manage_explorer call changes the activity from
+       * EXPLORE to IDLE, then for some reason the ai.control value gets left
+       * set.  We reset it here.  See PR#12931. */
+      punit->ai.control = FALSE;
     }
     send_unit_info(NULL, punit);
     return;

? vgcore.pid17532
Index: ai/aiexplorer.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiexplorer.c,v
retrieving revision 1.12
diff -u -r1.12 aiexplorer.c
--- ai/aiexplorer.c     23 Apr 2005 17:40:21 -0000      1.12
+++ ai/aiexplorer.c     30 Apr 2005 03:09:35 -0000
@@ -273,7 +273,10 @@
   double logDF = log(DIST_FACTOR);
   double logBPS = log(BEST_POSSIBLE_SCORE);
 
+  UNIT_LOG(LOG_DEBUG, punit, "auto-exploring.");
+
   if (pplayer->ai.control && unit_flag(punit, F_GAMELOSS)) {
+    UNIT_LOG(LOG_DEBUG, punit, "exploration too dangerous!");
     return FALSE; /* too dangerous */
   }
 
@@ -355,6 +358,7 @@
       /* Died?  Strange... */
       return FALSE;
     }
+    UNIT_LOG(LOG_DEBUG, punit, "exploration GOTO succeeded");
     if (punit->moves_left > 0) {
       /* We can still move on... */
       if (punit->moves_left < init_moves) {
@@ -362,6 +366,7 @@
          * Let's do more exploring. 
          * (Checking only whether our position changed is unsafe: can allow
          * yoyoing on a RR) */
+       UNIT_LOG(LOG_DEBUG, punit, "recursively exploring...");
        return ai_manage_explorer(punit);          
       } else {
        /* Something went wrong. What to do but return?
@@ -374,11 +379,14 @@
             && (punit->moves_left != unit_move_rate(punit))) {
           /* we're a trireme with non-full complement of movement points,
            * so wait until next turn. */
+         UNIT_LOG(LOG_DEBUG, punit, "done exploring (had to hold)...");
           return TRUE;
         }
+       UNIT_LOG(LOG_DEBUG, punit, "done exploring (all finished)...");
        return FALSE;
       }
     }
+    UNIT_LOG(LOG_DEBUG, punit, "done exploring (but more go go)...");
     return TRUE;
   } else {
     /* Didn't find anything. */
Index: ai/ailog.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/ailog.c,v
retrieving revision 1.24
diff -u -r1.24 ailog.c
--- ai/ailog.c  23 Apr 2005 17:40:21 -0000      1.24
+++ ai/ailog.c  30 Apr 2005 03:09:35 -0000
@@ -192,9 +192,12 @@
     gx = gy = -1;
   }
   
-  my_snprintf(buffer, sizeof(buffer), "%s's %s[%d] (%d,%d)->(%d,%d){%d,%d} ",
+  my_snprintf(buffer, sizeof(buffer),
+             "%s's %s[%d] (%s) (%d,%d)->(%d,%d){%d,%d} ",
               unit_owner(punit)->name, unit_type(punit)->name,
-              punit->id, punit->tile->x, punit->tile->y,
+              punit->id,
+             get_activity_text(punit->activity),
+             punit->tile->x, punit->tile->y,
              gx, gy,
               punit->ai.bodyguard, punit->ai.ferryboat);
 
Index: ai/aitools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.c,v
retrieving revision 1.147
diff -u -r1.147 aitools.c
--- ai/aitools.c        23 Apr 2005 17:40:21 -0000      1.147
+++ ai/aitools.c        30 Apr 2005 03:09:36 -0000
@@ -389,18 +389,27 @@
   struct pf_map *map = NULL;
   struct pf_path *path = NULL;
 
+  UNIT_LOG(LOG_DEBUG, punit, "constrained goto to %d,%d",
+          ptile->x, ptile->y);
+
   ptile = immediate_destination(punit, ptile);
 
+  UNIT_LOG(LOG_DEBUG, punit, "constrained goto: let's go to %d,%d",
+          ptile->x, ptile->y);
+
   if (same_pos(punit->tile, ptile)) {
     /* Not an error; sometimes immediate_destination instructs the unit
      * to stay here. For example, to refuel.*/
+    UNIT_LOG(LOG_DEBUG, punit, "constrained goto: already there!");
     send_unit_info(NULL, punit);
     return TRUE;
   } else if (!goto_is_sane(punit, ptile, FALSE)) {
+    UNIT_LOG(LOG_DEBUG, punit, "constrained goto: 'insane' goto!");
     punit->activity = ACTIVITY_IDLE;
     send_unit_info(NULL, punit);
     return TRUE;
   } else if(punit->moves_left == 0) {
+    UNIT_LOG(LOG_DEBUG, punit, "constrained goto: no moves left!");
     send_unit_info(NULL, punit);
     return TRUE;
   }
@@ -410,6 +419,7 @@
 
   if (path) {
     ai_log_path(punit, path, parameter);
+    UNIT_LOG(LOG_DEBUG, punit, "constrained goto: following path.");
     alive = ai_follow_path(punit, path, ptile);
   } else {
     UNIT_LOG(LOG_DEBUG, punit, "no path to destination");
@@ -739,6 +749,7 @@
   struct pf_parameter parameter;
   struct ai_risk_cost risk_cost;
 
+  UNIT_LOG(LOG_DEBUG, punit, "ai_unit_goto to %d,%d", ptile->x, ptile->y);
   ai_fill_unit_param(&parameter, &risk_cost, punit, ptile);
   return ai_unit_goto_constrained(punit, ptile, &parameter);
 }