[Freeciv-Dev] Re: (PR#12781) Create AI guard API

["Benedict Adamson" <badamson@xxxxxxxxxxx>]

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

Here is the third version of my patch. It fixes PR#12788 (Does a Unit 
Ever Know its Body Guard?) by enforcing correct cross references between 
guards and charges.

diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/Makefile.am freeciv.PR12781/ai/Makefile.am
--- vendor.freeciv.current/ai/Makefile.am       2005-01-24 20:43:05.000000000 
+0000
+++ freeciv.PR12781/ai/Makefile.am      2005-04-22 00:49:16.000000000 +0100
@@ -25,6 +25,8 @@
                aiexplorer.h    \
                aiferry.c       \
                aiferry.h       \
+               aiguard.c       \
+               aiguard.h       \
                aihand.c        \
                aihand.h        \
                aihunt.c        \
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aidiplomat.c freeciv.PR12781/ai/aidiplomat.c
--- vendor.freeciv.current/ai/aidiplomat.c      2005-03-23 23:06:42.000000000 
+0000
+++ freeciv.PR12781/ai/aidiplomat.c     2005-04-22 00:49:16.000000000 +0100
@@ -45,6 +45,7 @@
 #include "advmilitary.h"
 #include "aicity.h"
 #include "aidata.h"
+#include "aiguard.h"
 #include "aihand.h"
 #include "ailog.h"
 #include "aitools.h"
@@ -629,7 +630,7 @@
 
     if (ctarget) {
       task = AIUNIT_ATTACK;
-      punit->ai.bodyguard = -1; /* want one */
+      aiguard_request_guard(punit);
       UNIT_LOG(LOG_DIPLOMAT, punit, "going on attack");
     } else if ((ctarget = ai_diplomat_defend(pplayer, punit,
                                              punit->type, map)) != NULL) {
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aiferry.c freeciv.PR12781/ai/aiferry.c
--- vendor.freeciv.current/ai/aiferry.c 2005-04-15 19:20:43.000000000 +0100
+++ freeciv.PR12781/ai/aiferry.c        2005-04-22 00:49:16.000000000 +0100
@@ -27,6 +27,7 @@
 #include "unittools.h"
 
 #include "aidata.h"
+#include "aiguard.h"
 #include "aiexplorer.h"
 #include "ailog.h"
 #include "aitools.h"
@@ -603,7 +604,7 @@
 
     /* Check if we are the passenger-in-charge */
     if (is_boat_free(ferryboat, punit, 0)) {
-      struct unit *bodyguard = find_unit_by_id(punit->ai.bodyguard);
+      struct unit *bodyguard = aiguard_guard_of(punit);
 
       UNIT_LOG(LOGLEVEL_GOBYBOAT, punit, 
               "got boat[%d](moves left: %d), going (%d,%d)",
@@ -617,7 +618,7 @@
         if (!goto_is_sane(bodyguard, punit->tile, TRUE)
             || !ai_unit_goto(bodyguard, punit->tile)) {
           /* Bodyguard can't get there or died en route */
-          punit->ai.bodyguard = BODYGUARD_WANTED;
+          aiguard_request_guard(punit);
           bodyguard = NULL;
         } else if (bodyguard->moves_left <= 0) {
           /* Wait for me, I'm cooooming!! */
@@ -627,7 +628,7 @@
         } else {
           /* Crap bodyguard. Got stuck somewhere. Ditch it! */
           UNIT_LOG(LOGLEVEL_GOBYBOAT, punit, "ditching useless bodyguard");
-          punit->ai.bodyguard = BODYGUARD_WANTED;
+          aiguard_request_guard(punit);
           ai_unit_new_role(bodyguard, AIUNIT_NONE, NULL);
           bodyguard = NULL;
         }
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aiguard.c freeciv.PR12781/ai/aiguard.c
--- vendor.freeciv.current/ai/aiguard.c 1970-01-01 01:00:00.000000000 +0100
+++ freeciv.PR12781/ai/aiguard.c        2005-04-22 00:49:16.000000000 +0100
@@ -0,0 +1,289 @@
+/********************************************************************** 
+ Freeciv - Copyright (C) 2004 - The Freeciv Project
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+***********************************************************************/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "log.h"
+#include "unit.h"
+
+#include "ailog.h"
+#include "aitools.h"
+
+#include "aiguard.h"
+
+
+
+enum bodyguard_enum {
+  BODYGUARD_WANTED = -1,
+  BODYGUARD_NONE
+};
+
+
+
+/**************************************************************************
+  Do sanity checks on a guard, reporting error messages to the log
+  if necessary.
+**************************************************************************/
+void aiguard_check_guard(const struct unit *guard)
+{
+  const struct unit *charge_unit = find_unit_by_id(guard->ai.charge);
+  const struct city *charge_city = find_city_by_id(guard->ai.charge);
+  const struct player *guard_owner = unit_owner(guard);
+  const struct player *charge_owner = NULL;
+
+  assert(BODYGUARD_NONE <= guard->ai.charge);
+  assert(charge_unit == NULL || charge_city == NULL); /* IDs always distinct */
+
+  if (charge_unit) {
+    charge_owner = unit_owner(charge_unit);
+  } else if (charge_city) {
+    charge_owner = city_owner(charge_city);
+  }
+
+  if (charge_unit && charge_unit->ai.bodyguard != guard->id) {
+    BODYGUARD_LOG(LOG_ERROR, guard, "inconsistent guard references");
+  } else if (!charge_unit && !charge_city && 0 < guard->ai.charge) {
+    BODYGUARD_LOG(LOG_ERROR, guard, "dangling guard reference");
+  }
+  if (charge_owner && pplayers_at_war(charge_owner, guard_owner)) {
+    BODYGUARD_LOG(LOG_ERROR, guard, "enemy charge");
+  } else if (charge_owner && charge_owner != guard_owner) {
+    /* peculiar, but not always an error */
+    BODYGUARD_LOG(LOG_DEBUG, guard, "foreign charge");
+  }
+}
+
+/**************************************************************************
+  Do sanity checks on a charge, reporting error messages to the log
+  if necessary.
+**************************************************************************/
+void aiguard_check_charge_unit(const struct unit *charge)
+{
+  const struct player *charge_owner = unit_owner(charge);
+  struct unit *guard = find_unit_by_id(charge->ai.bodyguard);
+  assert(guard == NULL || BODYGUARD_WANTED <= guard->ai.bodyguard);
+ 
+ if (guard && guard->ai.charge != charge->id) {
+    /* FIXME: UNIT_LOG should take a const struct * */
+    UNIT_LOG(LOG_ERROR, (struct unit *)charge,
+             "inconsistent guard references");
+  } else if (guard && unit_owner(guard) != charge_owner) {
+    /* FIXME: UNIT_LOG should take a const struct * */
+    UNIT_LOG(LOG_ERROR, (struct unit *)charge, "foreign guard");
+  }
+}
+
+/**************************************************************************
+  Remove the assignment of a charge to a guard.
+
+  Assumes that a unit can have at most one guard.
+**************************************************************************/
+void aiguard_clear_charge(struct unit *guard)
+{
+  struct unit *charge_unit = find_unit_by_id(guard->ai.charge);
+  struct city *charge_city = find_city_by_id(guard->ai.charge);
+  assert(charge_unit == NULL || charge_city == NULL); /* IDs always distinct */
+  if (charge_unit) {
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "unassigned");
+    charge_unit->ai.bodyguard = BODYGUARD_NONE;
+  } else if (charge_city) {
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "unassigned");
+  }
+  /* else not assigned or charge was destroyed */
+  guard->ai.charge = BODYGUARD_NONE;
+
+  CHECK_GUARD(guard);
+}
+
+/**************************************************************************
+  Remove assignment of bodyguard for a unit.
+
+  Assumes that a unit can have at most one guard.
+  There is no analogous function for cities, because cities can have many
+  guards: instead use aiguard_clear_charge for each city guard.
+**************************************************************************/
+void aiguard_clear_guard(struct unit *charge)
+{
+  if (0 < charge->ai.bodyguard) {
+    struct unit *guard = find_unit_by_id(charge->ai.bodyguard);
+
+    if (guard && guard->ai.charge == charge->id) {
+      /* charge doesn't want us anymore */
+      guard->ai.charge = BODYGUARD_NONE;
+    }
+  }
+
+  charge->ai.bodyguard = BODYGUARD_NONE;
+
+  CHECK_CHARGE_UNIT(charge);
+}
+
+/**************************************************************************
+  Assign a bodyguard to a unit.
+
+  Assumes that a unit can have at most one guard.
+**************************************************************************/
+void aiguard_assign_guard_unit(struct unit *charge, struct unit *guard)
+{
+  assert(charge != NULL);
+  assert(guard != NULL);
+  assert(guard != charge);
+  assert(unit_owner(charge) == unit_owner(guard));
+
+  /* Remove previous assignment: */
+  aiguard_clear_charge(guard);
+  aiguard_clear_guard(charge);
+
+  guard->ai.charge = charge->id;
+  charge->ai.bodyguard = guard->id;
+
+  BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "assigned charge");
+  CHECK_GUARD(guard);
+  CHECK_CHARGE_UNIT(charge);
+}
+
+/**************************************************************************
+  Assign a guard to a city.
+**************************************************************************/
+void aiguard_assign_guard_city(struct city *charge, struct unit *guard)
+{
+  assert(charge != NULL);
+  assert(guard != NULL);
+  /*
+   * Usually, but not always, city_owner(charge) == unit_owner(guard).
+   */
+
+  if (0 < guard->ai.charge && guard->ai.charge != charge->id) {
+    /* Remove previous assignment: */
+    aiguard_clear_charge(guard);
+  }
+
+  guard->ai.charge = charge->id;
+  if (city_owner(charge) != unit_owner(guard)) {
+    /* Peculiar, but not always an error */
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "assigned foreign charge");
+  } else {
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "assigned charge");
+  }
+
+  CHECK_GUARD(guard);
+}
+
+/**************************************************************************
+  Request a (new) bodyguard for the unit.
+**************************************************************************/
+void aiguard_request_guard(struct unit *punit)
+{
+  /* Remove previous assignment */
+  aiguard_clear_guard(punit);
+
+  UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "requests a guard");
+  punit->ai.bodyguard = BODYGUARD_WANTED;
+
+  CHECK_CHARGE_UNIT(punit);
+}
+
+/**************************************************************************
+  Has a unit requested a guard and not (yet) been provided with one?
+**************************************************************************/
+bool aiguard_wanted(struct unit *charge)
+{
+  CHECK_CHARGE_UNIT(charge);
+  return charge->ai.bodyguard == BODYGUARD_WANTED;
+}
+
+/**************************************************************************
+  Has a charge unit been assigned to a guard?
+**************************************************************************/
+bool aiguard_has_charge(struct unit *guard)
+{
+  CHECK_GUARD(guard);
+  return guard->ai.charge != BODYGUARD_NONE;
+}
+
+/**************************************************************************
+  Has a guard been assigned to a charge?
+**************************************************************************/
+bool aiguard_has_guard(struct unit *charge)
+{
+  CHECK_CHARGE_UNIT(charge);
+  return 0 < charge->ai.bodyguard;
+}
+
+/**************************************************************************
+  Which unit, if any, is the body guard of a unit?
+  Returns NULL if the unit has not been assigned a guard.
+**************************************************************************/
+struct unit *aiguard_guard_of(struct unit *charge)
+{
+  CHECK_CHARGE_UNIT(charge);
+  return find_unit_by_id(charge->ai.bodyguard);
+}
+
+/**************************************************************************
+  Which unit (if any) has a guard been assigned to?
+  Returns NULL if the unit is not the guard for a unit.
+**************************************************************************/
+struct unit *aiguard_charge_unit(struct unit *guard)
+{
+  CHECK_GUARD(guard);
+  return find_unit_by_id(guard->ai.charge);
+}
+
+/**************************************************************************
+  Which city (if any) has a guard been assigned to?
+  Returns NULL if the unit is not a guard for a city.
+**************************************************************************/
+struct city *aiguard_charge_city(struct unit *guard)
+{
+  CHECK_GUARD(guard);
+  return find_city_by_id(guard->ai.charge);
+}
+
+/**************************************************************************
+  Check whether the assignment of a guard is still sane, and fix and problems.
+  It was once sane, but might have been destroyed or become an enemy since.
+**************************************************************************/
+void aiguard_update_charge(struct unit *guard)
+{
+  const struct unit *charge_unit = find_unit_by_id(guard->ai.charge);
+  const struct city *charge_city = find_city_by_id(guard->ai.charge);
+  const struct player *guard_owner = unit_owner(guard);
+  const struct player *charge_owner = NULL;
+
+  assert(BODYGUARD_NONE <= guard->ai.charge);
+  assert(charge_unit == NULL || charge_city == NULL); /* IDs always distinct */
+
+  if (charge_unit) {
+    charge_owner = unit_owner(charge_unit);
+  } else if (charge_city) {
+    charge_owner = city_owner(charge_city);
+  }
+
+  if (!charge_unit && !charge_city && 0 < guard->ai.charge) {
+    guard->ai.charge = BODYGUARD_NONE;
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "charge was destroyed");
+  }
+  if (charge_owner && pplayers_at_war(charge_owner, guard_owner)) {
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "charge became an enemy");
+    aiguard_clear_charge(guard);
+  } else if (charge_owner && charge_owner != guard_owner
+             && !pplayers_allied(charge_owner, guard_owner)) {
+    BODYGUARD_LOG(LOGLEVEL_BODYGUARD, guard, "charge broke alliance");
+    aiguard_clear_charge(guard);
+  }
+
+  CHECK_GUARD(guard);
+}
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aiguard.h freeciv.PR12781/ai/aiguard.h
--- vendor.freeciv.current/ai/aiguard.h 1970-01-01 01:00:00.000000000 +0100
+++ freeciv.PR12781/ai/aiguard.h        2005-04-22 00:49:16.000000000 +0100
@@ -0,0 +1,56 @@
+/********************************************************************** 
+ Freeciv - Copyright (C) 2002 - The Freeciv Project
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+***********************************************************************/
+#ifndef FC__AIGUARD_H
+#define FC__AIGUARD_H
+
+#include "shared.h"            /* bool type */
+
+#include "fc_types.h"
+
+#ifndef NDEBUG
+#define CHECK_GUARD(guard) aiguard_check_guard(guard)
+#define CHECK_CHARGE_UNIT(charge) aiguard_check_charge_unit(charge)
+#else
+#define CHECK_GUARD(guard) (void)0
+#define CHECK_CHARGE_UNIT(charge) (void)0
+#endif
+
+void aiguard_check_guard(const struct unit *guard);
+
+void aiguard_check_charge_unit(const struct unit *charge);
+
+void aiguard_clear_charge(struct unit *guard);
+
+void aiguard_clear_guard(struct unit *charge);
+
+void aiguard_assign_guard_unit(struct unit *charge, struct unit *guard);
+
+void aiguard_assign_guard_city(struct city *charge, struct unit *guard);
+
+void aiguard_request_guard(struct unit *punit);
+
+bool aiguard_wanted(struct unit *charge);
+
+bool aiguard_has_charge(struct unit *charge);
+
+bool aiguard_has_guard(struct unit *charge);
+
+struct unit *aiguard_guard_of(struct unit *charge);
+
+struct unit *aiguard_charge_unit(struct unit *guard);
+
+struct city *aiguard_charge_city(struct unit *guard);
+
+void aiguard_update_charge(struct unit *guard);
+
+#endif /* FC__AIGUARD_H */
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/ailog.c freeciv.PR12781/ai/ailog.c
--- vendor.freeciv.current/ai/ailog.c   2005-04-12 20:48:41.000000000 +0100
+++ freeciv.PR12781/ai/ailog.c  2005-04-22 00:49:16.000000000 +0100
@@ -214,14 +214,16 @@
     2: ai4's bodyguard Mech. Inf.[485] (38,22){Riflemen:574@37,23} was ...
   note that these messages are likely to wrap if long.
 **************************************************************************/
-void BODYGUARD_LOG(int level, struct unit *punit, const char *msg)
+void BODYGUARD_LOG(int level, const struct unit *punit, const char *msg)
 {
   char buffer[500];
   int minlevel = MIN(LOGLEVEL_BODYGUARD, level);
-  struct unit *pcharge;
-  struct city *pcity;
+  const struct unit *pcharge;
+  const struct city *pcity;
   int id = -1;
-  struct tile *ptile = NULL;
+  int charge_x = -1;
+  int charge_y = -1;
+  const char *type = "guard";
   const char *s = "none";
 
   if (punit->debug) {
@@ -233,19 +235,25 @@
   pcity = find_city_by_id(punit->ai.charge);
   pcharge = find_unit_by_id(punit->ai.charge);
   if (pcharge) {
-    ptile = pcharge->tile;
+    charge_x = pcharge->tile->x;
+    charge_y = pcharge->tile->y;
     id = pcharge->id;
+    type = "bodyguard";
     s = unit_type(pcharge)->name;
   } else if (pcity) {
-    ptile = pcity->tile;
+    charge_x = pcity->tile->x;
+    charge_y = pcity->tile->y;
     id = pcity->id;
+    type = "cityguard";
     s = pcity->name;
   }
+  /* else perhaps the charge died */
+
   my_snprintf(buffer, sizeof(buffer),
-              "%s's bodyguard %s[%d] (%d,%d){%s:%d@%d,%d} ",
-              unit_owner(punit)->name, unit_type(punit)->name,
+              "%s's %s %s[%d] (%d,%d){%s:%d@%d,%d} ",
+              unit_owner(punit)->name, type, unit_type(punit)->name,
               punit->id, punit->tile->x, punit->tile->y,
-             s, id, ptile->x, ptile->y);
+             s, id, charge_x, charge_y);
   cat_snprintf(buffer, sizeof(buffer), msg);
   if (punit->debug) {
     notify_conn(game.est_connections, buffer);
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/ailog.h freeciv.PR12781/ai/ailog.h
--- vendor.freeciv.current/ai/ailog.h   2005-04-12 20:48:41.000000000 +0100
+++ freeciv.PR12781/ai/ailog.h  2005-04-22 00:49:16.000000000 +0100
@@ -81,7 +81,7 @@
      fc__attribute((format (printf, 3, 4)));
 void UNIT_LOG(int level, struct unit *punit, const char *msg, ...)
      fc__attribute((format (printf, 3, 4)));
-void BODYGUARD_LOG(int level, struct unit *punit, const char *msg);
+void BODYGUARD_LOG(int level, const struct unit *punit, const char *msg);
 void TIMING_LOG(enum ai_timer timer, enum ai_timer_activity activity);
 void TIMING_RESULTS(void);
 
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aitools.c freeciv.PR12781/ai/aitools.c
--- vendor.freeciv.current/ai/aitools.c 2005-04-15 19:20:43.000000000 +0100
+++ freeciv.PR12781/ai/aitools.c        2005-04-22 00:49:16.000000000 +0100
@@ -50,6 +50,7 @@
 #include "aicity.h"
 #include "aidata.h"
 #include "aiferry.h"
+#include "aiguard.h"
 #include "ailog.h"
 #include "aitech.h"
 #include "aiunit.h"
@@ -191,10 +192,11 @@
   unsigned int danger = 0;
   struct city *dcity;
   struct tile *ptile;
+  struct unit *guard = aiguard_guard_of(punit);
   
   if (is_barbarian(unit_owner(punit))) {
     /* barbarians must have more courage (ie less brains) */
-    punit->ai.bodyguard = BODYGUARD_NONE;
+    aiguard_clear_guard(punit);
     return;
   }
 
@@ -214,7 +216,9 @@
   }
   danger *= POWER_DIVIDER;
 
-  /* If we are fast, there is less danger. */
+  /* If we are fast, there is less danger.
+   * FIXME: that assumes that most units have move_rate == SINGLE_MOVE;
+   * not true for all rule-sets */
   danger /= (unit_type(punit)->move_rate / SINGLE_MOVE);
   if (unit_flag(punit, F_IGTER)) {
     danger /= 1.5;
@@ -222,7 +226,7 @@
 
   ptile = punit->tile;
   /* We look for the bodyguard where we stand. */
-  if (!unit_list_find(ptile->units, punit->ai.bodyguard)) {
+  if (guard == NULL || guard->tile != punit->tile) {
     int my_def = (punit->hp 
                   * unit_type(punit)->veteran[punit->veteran].power_fact
                  * unit_type(punit)->defense_strength
@@ -232,9 +236,9 @@
       UNIT_LOG(LOGLEVEL_BODYGUARD, punit, 
                "want bodyguard @(%d, %d) danger=%d, my_def=%d", 
                TILE_XY(dest_tile), danger, my_def);
-      punit->ai.bodyguard = BODYGUARD_WANTED;
+      aiguard_request_guard(punit);
     } else {
-      punit->ai.bodyguard = BODYGUARD_NONE;
+      aiguard_clear_guard(punit);
     }
   }
 
@@ -750,8 +754,7 @@
 void ai_unit_new_role(struct unit *punit, enum ai_unit_task task,
                      struct tile *ptile)
 {
-  struct unit *charge = find_unit_by_id(punit->ai.charge);
-  struct unit *bodyguard = find_unit_by_id(punit->ai.bodyguard);
+  struct unit *bodyguard = aiguard_guard_of(punit);
 
   /* If the unit is under (human) orders we shouldn't control it. */
   assert(!unit_has_orders(punit));
@@ -784,15 +787,10 @@
     }
   }
 
-  if (charge && (charge->ai.bodyguard == punit->id)) {
-    /* ensure we don't let the unit believe we bodyguard it */
-    charge->ai.bodyguard = BODYGUARD_NONE;
-  }
-  punit->ai.charge = BODYGUARD_NONE;
-
+  aiguard_clear_charge(punit);
   /* Record the city to defend; our goto may be to transport. */
   if (task == AIUNIT_DEFEND_HOME && ptile && ptile->city) {
-    punit->ai.charge = ptile->city->id;
+    aiguard_assign_guard_city(ptile->city, punit);
   }
 
   punit->ai.ai_role = task;
@@ -863,24 +861,23 @@
 
 /**************************************************************************
   Move a bodyguard along with another unit. We assume that unit has already
-  been moved to (x, y) which is a valid, safe coordinate, and that our
+  been moved to ptile which is a valid, safe tile, and that our
   bodyguard has not. This is an ai_unit_* auxiliary function, do not use 
   elsewhere.
 **************************************************************************/
-static void ai_unit_bodyguard_move(int unitid, struct tile *ptile)
+static void ai_unit_bodyguard_move(struct unit *bodyguard, struct tile *ptile)
 {
-  struct unit *bodyguard = find_unit_by_id(unitid);
   struct unit *punit;
   struct player *pplayer;
 
   assert(bodyguard != NULL);
   pplayer = unit_owner(bodyguard);
   assert(pplayer != NULL);
-  punit = find_unit_by_id(bodyguard->ai.charge);
+  punit = aiguard_charge_unit(bodyguard);
   assert(punit != NULL);
 
-  assert(punit->ai.bodyguard == bodyguard->id);
-  assert(bodyguard->ai.charge == punit->id);
+  CHECK_GUARD(bodyguard);
+  CHECK_CHARGE_UNIT(punit);
 
   if (!is_tiles_adjacent(ptile, bodyguard->tile)) {
     return;
@@ -897,33 +894,11 @@
 }
 
 /**************************************************************************
-  Check if we have a bodyguard with sanity checking and error recovery.
-  Repair incompletely referenced bodyguards. When the rest of the bodyguard
-  mess is cleaned up, this repairing should be replaced with an assert.
-**************************************************************************/
-static bool has_bodyguard(struct unit *punit)
-{
-  struct unit *guard;
-  if (punit->ai.bodyguard > BODYGUARD_NONE) {
-    if ((guard = find_unit_by_id(punit->ai.bodyguard))) {
-      if (guard->ai.charge != punit->id) {
-        BODYGUARD_LOG(LOG_VERBOSE, guard, "my charge didn't know about me!");
-      }
-      guard->ai.charge = punit->id; /* ensure sanity */
-      return TRUE;
-    } else {
-      punit->ai.bodyguard = BODYGUARD_NONE;
-      UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "bodyguard disappeared!");
-    }
-  }
-  return FALSE;
-}
-
-/**************************************************************************
   Move and attack with an ai unit. We do not wait for server reply.
 **************************************************************************/
 bool ai_unit_attack(struct unit *punit, struct tile *ptile)
 {
+  struct unit *bodyguard = aiguard_guard_of(punit);
   int sanity = punit->id;
   bool alive;
 
@@ -936,8 +911,8 @@
   alive = (find_unit_by_id(sanity) != NULL);
 
   if (alive && same_pos(ptile, punit->tile)
-      && has_bodyguard(punit)) {
-    ai_unit_bodyguard_move(punit->ai.bodyguard, ptile);
+      && bodyguard != NULL) {
+    ai_unit_bodyguard_move(bodyguard, ptile);
     /* Clumsy bodyguard might trigger an auto-attack */
     alive = (find_unit_by_id(sanity) != NULL);
   }
@@ -978,8 +953,8 @@
   }
 
   /* don't leave bodyguard behind */
-  if (is_ai && has_bodyguard(punit)
-      && (bodyguard = find_unit_by_id(punit->ai.bodyguard))
+  if (is_ai
+      && (bodyguard = aiguard_guard_of(punit))
       && same_pos(punit->tile, bodyguard->tile)
       && bodyguard->moves_left == 0) {
     UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "does not want to leave "
@@ -1002,8 +977,9 @@
 
   /* handle the results */
   if (find_unit_by_id(sanity) && same_pos(ptile, punit->tile)) {
-    if (is_ai && has_bodyguard(punit)) {
-      ai_unit_bodyguard_move(punit->ai.bodyguard, ptile);
+    struct unit *bodyguard = aiguard_guard_of(punit);
+    if (is_ai && bodyguard != NULL) {
+      ai_unit_bodyguard_move(bodyguard, ptile);
     }
     return TRUE;
   }
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aitools.h freeciv.PR12781/ai/aitools.h
--- vendor.freeciv.current/ai/aitools.h 2005-04-05 22:40:45.000000000 +0100
+++ freeciv.PR12781/ai/aitools.h        2005-04-22 00:49:16.000000000 +0100
@@ -40,11 +40,6 @@
 #define CHECK_UNIT(punit) assert(TRUE)
 #endif
 
-enum bodyguard_enum {
-  BODYGUARD_WANTED=-1,
-  BODYGUARD_NONE
-};
-
 /*
  * Initialise using ai_avoid_risks()
  */
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aiunit.c freeciv.PR12781/ai/aiunit.c
--- vendor.freeciv.current/ai/aiunit.c  2005-04-20 21:44:31.000000000 +0100
+++ freeciv.PR12781/ai/aiunit.c 2005-04-22 00:49:16.000000000 +0100
@@ -54,6 +54,7 @@
 #include "aidiplomat.h"
 #include "aiexplorer.h"
 #include "aiferry.h"
+#include "aiguard.h"
 #include "aihand.h"
 #include "aihunt.h"
 #include "ailog.h"
@@ -729,14 +730,18 @@
 **************************************************************************/
 static void ai_military_bodyguard(struct player *pplayer, struct unit *punit)
 {
-  struct unit *aunit = player_find_unit_by_id(pplayer, punit->ai.charge);
-  struct city *acity = find_city_by_id(punit->ai.charge);
+  struct unit *aunit = aiguard_charge_unit(punit);
+  struct city *acity = aiguard_charge_city(punit);
   struct tile *ptile;
 
   CHECK_UNIT(punit);
+  CHECK_GUARD(punit);
 
   if (aunit && aunit->owner == punit->owner) {
     /* protect a unit */
+    /* FIXME: different behaviour for sailing units is silly;
+     * should choose behaviour based on relative positions and
+     * movement rates */
     if (is_sailing_unit(aunit)) {
       ptile = aunit->goto_tile;
     } else {
@@ -747,28 +752,22 @@
     ptile = acity->tile;
   } else {
     /* should be impossible */
-    UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "we lost our charge");
+    BODYGUARD_LOG(LOG_DEBUG, punit, "we lost our charge");
     ai_unit_new_role(punit, AIUNIT_NONE, NULL);
     return;
   }
 
-  if (aunit) {
-    UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "to meet charge %s#%d@(%d,%d){%d}",
-             unit_type(aunit)->name, aunit->id, aunit->tile->x,
-            aunit->tile->y,
-             aunit->ai.bodyguard);
-  } else if (acity) {
-    UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "to guard %s", acity->name);
-  }
-
-  if (!same_pos(punit->tile, ptile)) {
+  if (same_pos(punit->tile, ptile)) {
+    BODYGUARD_LOG(LOG_DEBUG, punit, "at RV");
+  } else {
     if (goto_is_sane(punit, ptile, TRUE)) {
+      BODYGUARD_LOG(LOG_DEBUG, punit, "meeting charge");
       if (!ai_gothere(pplayer, punit, ptile)) {
         /* We died */
         return;
       }
     } else {
-      /* can't possibly get there to help */
+      BODYGUARD_LOG(LOG_DEBUG, punit, "can not meet charge");
       ai_unit_new_role(punit, AIUNIT_NONE, NULL);
     }
   }
@@ -869,7 +868,7 @@
 
   /* Unit bodyguard */
   unit_list_iterate(pplayer->units, buddy) {
-    if (buddy->ai.bodyguard != BODYGUARD_WANTED
+    if (!aiguard_wanted(buddy)
         || !goto_is_sane(punit, buddy->tile, TRUE)
         || unit_move_rate(buddy) > unit_move_rate(punit)
         || DEFENCE_POWER(buddy) >= DEFENCE_POWER(punit)
@@ -880,6 +879,8 @@
     }
     if (punit->tile->city
         && punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
+      /* FIXME: Not even if it is an allied city?
+       * And why is this *inside* the loop ?*/
       continue; /* Do not run away from defense duty! */
     }
     dist = unit_move_turns(punit, buddy->tile);
@@ -960,15 +961,19 @@
     return;
   }
 
-  /* I am a bodyguard, check if I do my job! */
-  if (punit->ai.charge != BODYGUARD_NONE
+  /* If I am a bodyguard, check whether I can do my job. */
+  if (punit->ai.ai_role == AIUNIT_ESCORT
+      || punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
+    aiguard_update_charge(punit);
+  }
+  if (aiguard_has_charge(punit)
       && punit->ai.ai_role == AIUNIT_ESCORT) {
-    struct unit *aunit = player_find_unit_by_id(pplayer, punit->ai.charge);
-    struct city *acity = find_city_by_id(punit->ai.charge);
+    struct unit *aunit = aiguard_charge_unit(punit);
+    struct city *acity = aiguard_charge_city(punit);
 
     /* Check if the city we are on our way to rescue is still in danger,
      * or the unit we should protect is still alive... */
-    if ((aunit && aunit->ai.bodyguard != BODYGUARD_NONE 
+    if ((aunit && (aiguard_has_guard(aunit) || aiguard_wanted(aunit))
          && unit_def_rating_basic(punit) > unit_def_rating_basic(aunit)) 
         || (acity && acity->owner == punit->owner && acity->ai.urgency != 0 
             && acity->ai.danger > assess_defense_quadratic(acity))) {
@@ -1003,11 +1008,11 @@
     val = look_for_charge(pplayer, punit, &aunit, &acity);
     if (acity) {
       ai_unit_new_role(punit, AIUNIT_ESCORT, acity->tile);
-      punit->ai.charge = acity->id;
+      aiguard_assign_guard_city(acity, punit);
       BODYGUARD_LOG(LOG_DEBUG, punit, "going to defend city");
     } else if (aunit) {
       ai_unit_new_role(punit, AIUNIT_ESCORT, aunit->tile);
-      punit->ai.charge = aunit->id;
+      aiguard_assign_guard_unit(aunit, punit);
       BODYGUARD_LOG(LOG_DEBUG, punit, "going to defend unit");
     }
   }
@@ -1025,13 +1030,17 @@
 ***********************************************************************/
 static void ai_military_defend(struct player *pplayer,struct unit *punit)
 {
-  struct city *pcity = find_city_by_id(punit->ai.charge);
+  struct city *pcity = aiguard_charge_city(punit);
+  /* FIXME Although pcity was once an allied city or one of our cities,
+   * can we really rely on that? The city might have been lost.
+   */
 
   CHECK_UNIT(punit);
 
   if (!pcity || pcity->owner != pplayer->player_no) {
     pcity = punit->tile->city;
-    punit->ai.charge = 0; /* clear duty */
+    /* Do not stay defending an allied city forever */
+    aiguard_clear_charge(punit);
   }
 
   if (!pcity) {
@@ -2055,7 +2064,7 @@
 **************************************************************************/
 void ai_manage_unit(struct player *pplayer, struct unit *punit)
 {
-  struct unit *bodyguard = find_unit_by_id(punit->ai.bodyguard);
+  struct unit *bodyguard = aiguard_guard_of(punit);
 
   CHECK_UNIT(punit);
 
@@ -2078,9 +2087,9 @@
 
   /* Check if we have lost our bodyguard. If we never had one, all
    * fine. If we had one and lost it, ask for a new one. */
-  if (!bodyguard && punit->ai.bodyguard > BODYGUARD_NONE) {
+  if (!bodyguard && aiguard_has_guard(punit)) {
     UNIT_LOG(LOGLEVEL_BODYGUARD, punit, "lost bodyguard, asking for new");
-    punit->ai.bodyguard = BODYGUARD_WANTED;
+    aiguard_request_guard(punit);
   }  
 
   if (punit->moves_left <= 0) {