[Freeciv-Dev] (PR#9886) better AI unit assert

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

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

This patch adds new function ai_assert_unit() which replaces assert(). It
improves on the latter by outputting more data before aborting. In
particular, if DEBUG is compiled in, we remember which function and what
line we set the unit's role, so that we can see where role errors start.

The patch is quite useful in tracking AI role errors, but it is horribly
ugly. So I am not sure if I dare put it in cvs... :)

  - Per

Index: ai/aidiplomat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidiplomat.c,v
retrieving revision 1.38
diff -u -r1.38 aidiplomat.c
--- ai/aidiplomat.c     29 Aug 2004 17:54:13 -0000      1.38
+++ ai/aidiplomat.c     31 Aug 2004 17:34:53 -0000
@@ -580,8 +580,8 @@
       || punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
     bool failure = FALSE;
 
+    ai_assert_unit(punit, is_goto_dest_set(punit));
     ctarget = map_get_city(goto_dest_x(punit), goto_dest_y(punit));
-    assert(is_goto_dest_set(punit));
     if (pf_get_position(map, goto_dest_x(punit), goto_dest_y(punit), &pos)
         && ctarget) {
       if (same_pos(ctarget->x, ctarget->y, punit->x, punit->y)) {
@@ -649,7 +649,9 @@
   }
 
   CHECK_UNIT(punit);
-  assert(punit->moves_left > 0 && ctarget && punit->ai.ai_role != AIUNIT_NONE);
+  ai_assert_unit(punit, punit->moves_left > 0 
+                        && ctarget 
+                        && punit->ai.ai_role != AIUNIT_NONE);
 
   /* GOTO unless we want to stay */
   if (!same_pos(punit->x, punit->y, ctarget->x, ctarget->y)) {
Index: ai/ailog.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/ailog.c,v
retrieving revision 1.16
diff -u -r1.16 ailog.c
--- ai/ailog.c  30 Jul 2004 20:40:49 -0000      1.16
+++ ai/ailog.c  31 Aug 2004 17:34:53 -0000
@@ -32,6 +32,23 @@
 /* General AI logging functions */
 
 /**************************************************************************
+  Assert something related to AI units, especially AI roles.
+**************************************************************************/
+void real_ai_assert_unit(struct unit *punit, bool condition, char *expr,
+                         char *file, int line)
+{
+  if (!condition) {
+    freelog(LOG_FATAL, "error in %s line %d evaluating %s", file, line, expr);
+#ifdef DEBUG
+    UNIT_LOG(LOG_FATAL, punit, "role is %d set by %s line %d",
+             punit->ai.ai_role, punit->ai.ai_role_file,
+             punit->ai.ai_role_line);
+#endif
+    abort();
+  }
+}
+
+/**************************************************************************
   Log player messages, they will appear like this
     2: perrin [ti12 co6 lo5 e]  Increased love for a (now 9)
   where ti is timer, co countdown and lo love for target, who is e.
Index: ai/ailog.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/ailog.h,v
retrieving revision 1.7
diff -u -r1.7 ailog.h
--- ai/ailog.h  29 Apr 2004 19:59:21 -0000      1.7
+++ ai/ailog.h  31 Aug 2004 17:34:53 -0000
@@ -30,6 +30,16 @@
 #define LOGLEVEL_BUILD LOG_DEBUG
 #define LOGLEVEL_HUNT LOG_DEBUG
 
+#ifndef NDEBUG
+#define ai_assert_unit(punit, condition) \
+  real_ai_assert_unit(punit, condition, # condition, __FILE__, __LINE__)
+#else
+#define ai_assert_unit(punit, condition) assert(condition)
+#endif
+
+void real_ai_assert_unit(struct unit *punit, bool condition, char *expr,
+                         char *file, int line);
+
 void PLAYER_LOG(int level, struct player *pplayer, struct ai_data *ai,
                 const char *msg, ...);
 void CITY_LOG(int level, struct city *pcity, const char *msg, ...);
Index: ai/aitools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.c,v
retrieving revision 1.121
diff -u -r1.121 aitools.c
--- ai/aitools.c        31 Aug 2004 15:35:31 -0000      1.121
+++ ai/aitools.c        31 Aug 2004 17:34:53 -0000
@@ -27,6 +27,7 @@
 #include "packets.h"
 #include "player.h"
 #include "shared.h"
+#include "support.h"
 #include "unit.h"
 
 #include "citymap.h"
@@ -303,7 +304,13 @@
   reserve its target, and try to load it with cruise missiles or nukes
   to bring along.
 **************************************************************************/
-void ai_unit_new_role(struct unit *punit, enum ai_unit_task task, int x, int y)
+#ifdef DEBUG
+void real_ai_unit_new_role(struct unit *punit, enum ai_unit_task task, 
+                           int x, int y, char *file, int line)
+#else
+void real_ai_unit_new_role(struct unit *punit, enum ai_unit_task task, 
+                           int x, int y)
+#endif
 {
   struct unit *charge = find_unit_by_id(punit->ai.charge);
   struct unit *bodyguard = find_unit_by_id(punit->ai.bodyguard);
@@ -347,6 +354,10 @@
   punit->ai.charge = BODYGUARD_NONE;
 
   punit->ai.ai_role = task;
+#ifdef DEBUG
+  punit->ai.ai_role_line = line;
+  sz_strlcpy(punit->ai.ai_role_file, file);
+#endif
 
   /* Verify and set the goto destination.  Eventually this can be a lot more
    * stringent, but for now we don't want to break things too badly. */
@@ -490,7 +501,7 @@
   assert(is_tiles_adjacent(punit->x, punit->y, x, y));
 
   handle_unit_activity_request(punit, ACTIVITY_IDLE);
-  handle_unit_move(unit_owner(punit), punit->id,x,y);
+  (void) handle_unit_move_request(punit, x, y, FALSE, FALSE);
   alive = (find_unit_by_id(sanity) != NULL);
 
   if (alive && same_pos(x, y, punit->x, punit->y)
@@ -555,7 +566,7 @@
 
   /* go */
   handle_unit_activity_request(punit, ACTIVITY_IDLE);
-  handle_unit_move(unit_owner(punit), punit->id, x, y);
+  (void) handle_unit_move_request(punit, x, y, FALSE, TRUE);
 
   /* handle the results */
   if (find_unit_by_id(sanity) && same_pos(x, y, punit->x, punit->y)) {
Index: ai/aitools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.h,v
retrieving revision 1.45
diff -u -r1.45 aitools.h
--- ai/aitools.h        5 Aug 2004 11:34:18 -0000       1.45
+++ ai/aitools.h        31 Aug 2004 17:34:53 -0000
@@ -47,8 +47,17 @@
                 int dest_x, int dest_y);
 bool ai_unit_goto(struct unit *punit, int x, int y);
 
-void ai_unit_new_role(struct unit *punit, enum ai_unit_task task, 
-                      int x, int y);
+#ifdef DEBUG
+#define ai_unit_new_role(punit, task, x, y) \
+  real_ai_unit_new_role(punit, task, x, y, __FILE__, __LINE__)
+void real_ai_unit_new_role(struct unit *punit, enum ai_unit_task task, 
+                           int x, int y, char *file, int line);
+#else
+#define ai_unit_new_role(punit, task, x, y) \
+  real_ai_unit_new_role(punit, task, x, y)
+void real_ai_unit_new_role(struct unit *punit, enum ai_unit_task task, 
+                           int x, int y);
+#endif
 bool ai_unit_make_homecity(struct unit *punit, struct city *pcity);
 bool ai_unit_attack(struct unit *punit, int x, int y);
 bool ai_unit_move(struct unit *punit, int x, int y);
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.216
diff -u -r1.216 unit.c
--- common/unit.c       31 Aug 2004 04:40:50 -0000      1.216
+++ common/unit.c       31 Aug 2004 17:34:57 -0000
@@ -1755,6 +1755,10 @@
   punit->ai.hunted = 0;
   punit->ai.control = FALSE;
   punit->ai.ai_role = AIUNIT_NONE;
+#ifdef DEBUG
+  sz_strlcpy(punit->ai.ai_role_file, "none");
+  punit->ai.ai_role_line = 0;
+#endif
   punit->ai.ferryboat = 0;
   punit->ai.passenger = 0;
   punit->ai.bodyguard = 0;
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.122
diff -u -r1.122 unit.h
--- common/unit.h       26 Aug 2004 18:37:52 -0000      1.122
+++ common/unit.h       31 Aug 2004 17:34:58 -0000
@@ -116,6 +116,10 @@
 struct unit_ai {
   bool control; /* 0: not automated    1: automated */
   enum ai_unit_task ai_role;
+#ifdef DEBUG
+  char ai_role_file[MAX_LEN_NAME];
+  int ai_role_line;
+#endif
   /* The following are all unit ids */
   int ferryboat; /* the ferryboat assigned to us */
   int passenger; /* the unit assigned to this ferryboat */