[Freeciv-Dev] Re: (PR#13402) ai bug

["Benedict Adamson" <badamson@xxxxxxxxxxx>]

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

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

Here is a patch.

* When a unit changes owners (for example, because of civil war), its AI
data should be reinitialised, including is guard/charge data.
* The AI guard code should not complain so loudly about inconsistent
cross references, because we can not guarantee them.
* The AI movement code should likewise not assume correct guard-charge
cross references.

Apologies for the delay. I sent the original to the wrong e-mail address.



diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aiguard.c freeciv.PR13402/ai/aiguard.c
--- vendor.freeciv.current/ai/aiguard.c 2005-07-10 20:13:05.000000000 +0100
+++ freeciv.PR13402/ai/aiguard.c        2005-07-10 21:34:57.000000000 +0100
@@ -32,6 +32,10 @@
 /**************************************************************************
   Do sanity checks on a guard, reporting error messages to the log
   if necessary.
+
+  Inconsistent references do not always indicate an error, because units
+  can change owners (for example, because of civil war) outside the control
+  the the AI code.
 **************************************************************************/
 void aiguard_check_guard(const struct unit *guard)
 {
@@ -50,14 +54,15 @@
   }
 
   if (charge_unit && charge_unit->ai.bodyguard != guard->id) {
-    BODYGUARD_LOG(LOG_ERROR, guard, "inconsistent guard references");
+    BODYGUARD_LOG(LOG_DEBUG, guard, "inconsistent guard references");
   } else if (!charge_unit && !charge_city && 0 < guard->ai.charge) {
-    BODYGUARD_LOG(LOG_ERROR, guard, "dangling guard reference");
+    BODYGUARD_LOG(LOG_DEBUG, guard, "dangling guard reference");
   }
   if (charge_owner && pplayers_at_war(charge_owner, guard_owner)) {
-    BODYGUARD_LOG(LOG_ERROR, guard, "enemy charge");
+    /* Probably due to civil war */
+    BODYGUARD_LOG(LOG_DEBUG, guard, "enemy charge");
   } else if (charge_owner && charge_owner != guard_owner) {
-    /* peculiar, but not always an error */
+    /* Probably sold a city with its supported units. */
     BODYGUARD_LOG(LOG_DEBUG, guard, "foreign charge");
   }
 }
@@ -65,6 +70,10 @@
 /**************************************************************************
   Do sanity checks on a charge, reporting error messages to the log
   if necessary.
+
+  Inconsistent references do not always indicate an error, because units
+  can change owners (for example, because of civil war) outside the control
+  the the AI code.
 **************************************************************************/
 void aiguard_check_charge_unit(const struct unit *charge)
 {
@@ -74,11 +83,11 @@
  
  if (guard && guard->ai.charge != charge->id) {
     /* FIXME: UNIT_LOG should take a const struct * */
-    UNIT_LOG(LOG_ERROR, (struct unit *)charge,
+    UNIT_LOG(LOG_DEBUG, (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");
+    UNIT_LOG(LOG_DEBUG, (struct unit *)charge, "foreign guard");
   }
 }
 
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aitools.c freeciv.PR13402/ai/aitools.c
--- vendor.freeciv.current/ai/aitools.c 2005-07-10 20:13:05.000000000 +0100
+++ freeciv.PR13402/ai/aitools.c        2005-07-10 21:34:57.000000000 +0100
@@ -922,7 +922,7 @@
   alive = (find_unit_by_id(sanity) != NULL);
 
   if (alive && same_pos(ptile, punit->tile)
-      && bodyguard != NULL) {
+      && bodyguard != NULL  && bodyguard->ai.charge == punit->id) {
     ai_unit_bodyguard_move(bodyguard, ptile);
     /* Clumsy bodyguard might trigger an auto-attack */
     alive = (find_unit_by_id(sanity) != NULL);
@@ -989,7 +989,7 @@
   /* handle the results */
   if (find_unit_by_id(sanity) && same_pos(ptile, punit->tile)) {
     struct unit *bodyguard = aiguard_guard_of(punit);
-    if (is_ai && bodyguard != NULL) {
+    if (is_ai && bodyguard != NULL && bodyguard->ai.charge == punit->id) {
       ai_unit_bodyguard_move(bodyguard, ptile);
     }
     return TRUE;
@@ -1234,3 +1234,24 @@
 {
   return ai_data_get(pplayer)->wants_no_science;
 }
+
+/**************************************************************************
+  Clear all the AI information for a unit, placing the unit in a blank
+  state.
+
+  This is a suitable action for when a unit is about to change owners;
+  the new owner can not use (and should not know) what the previous owner was
+  using the unit for.
+**************************************************************************/
+void ai_reinit(struct unit *punit)
+{
+  punit->ai.control = false;
+  punit->ai.ai_role = AIUNIT_NONE;
+  aiguard_clear_charge(punit);
+  aiguard_clear_guard(punit);
+  aiferry_clear_boat(punit);
+  punit->ai.target = 0;
+  punit->ai.hunted = 0;
+  punit->ai.done = FALSE;
+}
+
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/ai/aitools.h freeciv.PR13402/ai/aitools.h
--- vendor.freeciv.current/ai/aitools.h 2005-07-10 20:13:05.000000000 +0100
+++ freeciv.PR13402/ai/aitools.h        2005-07-10 21:34:57.000000000 +0100
@@ -103,4 +103,6 @@
 
 bool is_player_dangerous(struct player *pplayer, struct player *aplayer);
 
+void ai_reinit(struct unit *punit);
+
 #endif  /* FC__AITOOLS_H */
diff -ruN -Xvendor.freeciv.current/diff_ignore 
vendor.freeciv.current/server/unithand.c freeciv.PR13402/server/unithand.c
--- vendor.freeciv.current/server/unithand.c    2005-07-10 20:12:48.000000000 
+0100
+++ freeciv.PR13402/server/unithand.c   2005-07-10 21:34:33.000000000 +0100
@@ -281,6 +281,7 @@
   }
   if (old_owner != new_owner) {
     remove_unit_sight_points(punit);
+    ai_reinit(punit);
 
     unit_list_unlink(old_owner->units, punit);
     unit_list_prepend(new_owner->units, punit);