(PR#10567) pubserver crash: ai-controlled unit has no goto destination

"Jason Short" <[email protected]>
Newsgroups gmane.games.freeciv.ai
Message-ID <[email protected]>
<URL: http://rt.freeciv.org/Ticket/Display.html?id=10567 >

As Per suggested, this patch makes the error non-fatal.  Instead a
LOG_ERROR is called and the AI role is reset.  This should probably be
applied to both branches.  Getting more crashes here isn't going to help
us find the bug.  To find the bug we need to audit the places where the
goto destination and the AI role are changed.

(Apply the patch then do "cvs diff -b" to see the short version.)

jason
diplomat_goto_tile.diff (text/x-patch, 2.4 KB)
Index: ai/aidiplomat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidiplomat.c,v
retrieving revision 1.41
diff -u -r1.41 aidiplomat.c
--- ai/aidiplomat.c	18 Oct 2004 23:28:12 -0000	1.41
+++ ai/aidiplomat.c	23 Oct 2004 21:00:57 -0000
@@ -572,30 +572,37 @@
   /* Check if existing target still makes sense */
   if (punit->ai.ai_role == AIUNIT_ATTACK
       || punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
-    bool failure = FALSE;
+    if (!punit->goto_tile) {
+      /* This has been known to happen for unknown reason.  See PR#10567. */
+      UNIT_LOG(LOG_ERROR, punit,
+	       "Diplomatic unit on a mission has no goto tile!");
+      ai_unit_new_role(punit, AIUNIT_NONE, NULL);
+    } else {
+      bool failure = FALSE;
 
-    ctarget = map_get_city(punit->goto_tile);
-    if (pf_get_position(map, punit->goto_tile, &pos)
-        && ctarget) {
-      if (same_pos(ctarget->tile, punit->tile)) {
-        failure = TRUE;
-      } else if (pplayers_allied(pplayer, city_owner(ctarget))
-          && punit->ai.ai_role == AIUNIT_ATTACK
-          && player_has_embassy(pplayer, city_owner(ctarget))) {
-        /* We probably incited this city with another diplomat */
-        failure = TRUE;
-      } else if (!pplayers_allied(pplayer, city_owner(ctarget))
-                 && punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
-        /* We probably lost the city */
-        failure = TRUE;
+      ctarget = map_get_city(punit->goto_tile);
+      if (pf_get_position(map, punit->goto_tile, &pos)
+	  && ctarget) {
+	if (same_pos(ctarget->tile, punit->tile)) {
+	  failure = TRUE;
+	} else if (pplayers_allied(pplayer, city_owner(ctarget))
+		   && punit->ai.ai_role == AIUNIT_ATTACK
+		   && player_has_embassy(pplayer, city_owner(ctarget))) {
+	  /* We probably incited this city with another diplomat */
+	  failure = TRUE;
+	} else if (!pplayers_allied(pplayer, city_owner(ctarget))
+		   && punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
+	  /* We probably lost the city */
+	  failure = TRUE;
+	}
+      } else {
+	/* City vanished! */
+	failure = TRUE;
+      }
+      if (failure) {
+	UNIT_LOG(LOG_DIPLOMAT, punit, "mission aborted");
+	ai_unit_new_role(punit, AIUNIT_NONE, NULL);
       }
-    } else {
-      /* City vanished! */
-      failure = TRUE;
-    }
-    if (failure) {
-      UNIT_LOG(LOG_DIPLOMAT, punit, "mission aborted");
-      ai_unit_new_role(punit, AIUNIT_NONE, NULL);
     }
   }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.