[Freeciv-Dev] Re: (PR#10459) bug: pplayer_can_ally assert fail

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

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

Found it. We did not properly check that the players we sanity checked
were alive. So there was no actual bug, since pplayers_can_ally() would
not be called by dead players, except by sanity_check().

I changed a few other things, too, that could be bug prone.

  - Per

Index: common/player.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.c,v
retrieving revision 1.158
diff -u -r1.158 player.c
--- common/player.c     8 Oct 2004 05:11:51 -0000       1.158
+++ common/player.c     18 Oct 2004 14:26:28 -0000
@@ -44,6 +44,9 @@
 ***************************************************************/
 bool pplayer_can_ally(struct player *p1, struct player *p2)
 {
+  if (!p1->is_alive || !p2->is_alive) {
+    return TRUE;
+  }
   players_iterate(pplayer) {
     enum diplstate_type ds = pplayer_get_diplstate(p1, pplayer)->type;
     if (pplayer != p1
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.331
diff -u -r1.331 plrhand.c
--- server/plrhand.c    12 Oct 2004 16:35:26 -0000      1.331
+++ server/plrhand.c    18 Oct 2004 14:26:30 -0000
@@ -1109,7 +1112,6 @@
 
   /* else, breaking a treaty */
 
-repeat_break_treaty:
   /* check what the new status will be, and what will happen to our
      reputation */
   switch(old_type) {
@@ -1151,14 +1153,6 @@
     resolve_unit_stacks(pplayer, pplayer2, TRUE);
   }
 
-  /* We want to go all the way to war, whatever the cost! 
-   * This is only used by the AI. */
-  if (clause == CLAUSE_LAST && new_type != DS_WAR) {
-    repeat = TRUE;
-    old_type = new_type;
-    goto repeat_break_treaty;
-  }
-
   /* if there's a reason to cancel the pact, do it without penalty */
   if (pplayer->diplstates[pplayer2->player_no].has_reason_to_cancel > 0) {
     pplayer->diplstates[pplayer2->player_no].has_reason_to_cancel = 0;
@@ -1191,7 +1185,6 @@
   send_player_info(pplayer, NULL);
   send_player_info(pplayer2, NULL);
 
-
   if (old_type == DS_ALLIANCE) {
     /* Inform clients about units that have been hidden.  Units in cities
      * and transporters are visible to allies but not visible once the
@@ -1203,7 +1196,6 @@
     remove_allied_visibility(pplayer2, pplayer);
   }
 
-
   /* 
    * Refresh all cities which have a unit of the other side within
    * city range. 
@@ -1225,7 +1217,7 @@
                   get_nation_name_plural(pplayer->nation),
                   diplstate_text(new_type));
 
-  /* Check fall-out of a war declaration. */
+  /* Check fall-out of a war declaration.  This may involve recursion. */
   players_iterate(other) {
     if (other->is_alive && other != pplayer && other != pplayer2
         && new_type == DS_WAR && pplayers_allied(pplayer2, other)
Index: ai/advdiplomacy.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/advdiplomacy.c,v
retrieving revision 1.47
diff -u -r1.47 advdiplomacy.c
--- ai/advdiplomacy.c   29 Sep 2004 02:24:18 -0000      1.47
+++ ai/advdiplomacy.c   18 Oct 2004 14:30:47 -0000
@@ -912,8 +912,10 @@
     remove_shared_vision(pplayer, target);
   }
 
-  /* will take us straight to war */
-  handle_diplomacy_cancel_pact(pplayer, target->player_no, CLAUSE_LAST);
+  /* Will take us straight to war */
+  while (!pplayers_at_war(pplayer, target)) {
+    handle_diplomacy_cancel_pact(pplayer, target->player_no, CLAUSE_LAST);
+  }
 
   /* Continue war at least in this arbitrary number of turns to show 
    * some spine */
@@ -1162,7 +1164,9 @@
       clause.type = CLAUSE_ALLIANCE;
       if (ai_goldequiv_clause(pplayer, aplayer, &clause, ai, FALSE) < 0
           || (adip->asked_about_alliance > 0 && !aplayer->ai.control)
-          || !target) {
+          || !target
+          || !pplayer_can_ally(pplayer, aplayer)
+          || !pplayer_can_ally(aplayer, pplayer)) {
         /* Note that we don't ever ask for alliance unless we have a target */
         break; 
       }