[Freeciv-Dev] (PR#2721) ferries and breaking of alliances

["Per I. Mathisen via RT" <rt@xxxxxxxxxxxxxx>]

These are two patches to fixes problems that happen when you break
alliances and you have units in enemy ferries, or you transport enemy
units in your ferries.

Problem #1: AI code wasn't careful so it snatched allied units onto its
ferries.

Problem #2: Existing code to teleport away allied units from our ferries
wasn't good enough. Now it should be.

Cleaned up some rather ugly code with the safe unit iterator.

Patches taken from AI diplomacy patch v6 (coming soon).

  - Per

Index: ai/aiunit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
retrieving revision 1.248
diff -u -r1.248 aiunit.c
--- ai/aiunit.c 2003/01/02 11:59:29     1.248
+++ ai/aiunit.c 2003/01/04 16:11:02
@@ -1220,7 +1220,8 @@
            freelog(LOG_DEBUG, "All aboard!");
            /* perhaps this should only require two passengers */
             unit_list_iterate(ptile->units, mypass) {
-              if (mypass->ai.ferryboat == ferryboat->id) {
+              if (mypass->ai.ferryboat == ferryboat->id
+                  && punit->owner == mypass->owner) {
                 handle_unit_activity_request(mypass, ACTIVITY_SENTRY);
                 def = unit_list_find(&ptile->units, mypass->ai.bodyguard);
                 if (def) {
@@ -1257,7 +1258,8 @@
       if (punit->ai.bodyguard < BODYGUARD_NONE) { 
        adjc_iterate(punit->x, punit->y, i, j) {
          unit_list_iterate(map_get_tile(i, j)->units, aunit) {
-           if (aunit->ai.charge == punit->id) {
+           if (aunit->ai.charge == punit->id
+                && punit->owner == aunit->owner) {
              freelog(LOG_DEBUG,
                      "Bodyguard at (%d, %d) is adjacent to (%d, %d)",
                      i, j, punit->x, punit->y);
@@ -1396,8 +1398,10 @@
 /* def is the defense of the city without punit */
         if (unit_flag(punit, F_FIELDUNIT)) val = -1;
         unit_list_iterate(map_get_tile(pcity->x, pcity->y)->units, pdef)
-          if (is_military_unit(pdef) && pdef != punit &&
-              !unit_flag(pdef, F_FIELDUNIT)) {
+          if (is_military_unit(pdef) 
+              && pdef != punit 
+              && !unit_flag(pdef, F_FIELDUNIT)
+              && pdef->owner == punit->owner) {
             if (assess_defense_unit(pcity, pdef, FALSE) >= val) val = 0;
           }
         unit_list_iterate_end; /* was getting confused without the is_military 
part in */
@@ -2270,6 +2274,9 @@
     punit->ai.passenger = 0;
 
   unit_list_iterate(map_get_tile(punit->x, punit->y)->units, aunit)
+    if (punit->owner != aunit->owner) {
+      return;
+    }
     if (aunit->ai.ferryboat == punit->id) {
       if (punit->ai.passenger == 0) punit->ai.passenger = aunit->id; /* oops */
       p++;

Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.202
diff -u -r1.202 unittools.c
--- server/unittools.c  2002/12/18 17:36:20     1.202
+++ server/unittools.c  2003/01/04 17:34:15
@@ -1549,29 +1549,21 @@
     }
   } /* end while */
 
-  /* There is only one allied units left on this square.  If there is not 
-     enough transporter capacity left send surplus to the closest friendly 
-     city. */
-  punit = unit_list_get(&(ptile->units), 0);
+  /* There are only allied units left on this square.  If there is not enough 
+     transporter capacity left, send surplus to the closest friendly city. */
+  unit_list_iterate_safe(ptile->units, aunit) {
+    if (ground_unit_transporter_capacity(x, y, unit_owner(aunit)) < 0
+        && is_ground_unit(aunit)) {
+      struct city *acity =
+                find_closest_owned_city(unit_owner(aunit), x, y, FALSE, NULL);
 
-  if (ptile->terrain == T_OCEAN) {
-  START:
-    unit_list_iterate(ptile->units, vunit) {
-      if (ground_unit_transporter_capacity(x, y, unit_owner(punit)) < 0) {
-       unit_list_iterate(ptile->units, wunit) {
-         if (is_ground_unit(wunit) && wunit->owner == vunit->owner) {
-           struct city *wcity =
-               find_closest_owned_city(unit_owner(wunit), x, y, FALSE, NULL);
-           if (wcity)
-             (void) teleport_unit_to_city(wunit, wcity, 0, verbose);
-           else
-             disband_stack_conflict_unit(wunit, verbose);
-           goto START;
-         }
-       } unit_list_iterate_end; /* End of find a unit from that player to 
disband*/
+      if (acity) {
+        (void) teleport_unit_to_city(aunit, acity, 0, verbose);
+      } else {
+        disband_stack_conflict_unit(aunit, verbose);
       }
-    } unit_list_iterate_end; /* End of find a player */
-  }
+    }
+  } unit_list_iterate_safe_end;
 }
 
 /**************************************************************************
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.253
diff -u -r1.253 plrhand.c
--- server/plrhand.c    2002/12/18 17:36:20     1.253
+++ server/plrhand.c    2003/01/04 17:34:15
@@ -882,39 +882,13 @@
   send_player_info(pplayer, NULL);
   send_player_info(pplayer2, NULL);
 
-  /* If the old state was alliance the players' units can share tiles
+  /* If the old state was alliance, the players' units can share tiles
      illegally, and we need to call resolve_unit_stack() on all the players'
-     units. We can't just iterate through the unitlist as it could get
-     corrupted when resolve_unit_stack() deletes units. */
+     potentially shared unit positions. */
   if (old_type == DS_ALLIANCE) {
-    int *resolve_list = NULL;
-    int tot_units = unit_list_size(&(pplayer->units))
-      + unit_list_size(&(pplayer2->units));
-    int no_units = unit_list_size(&(pplayer->units));
-    int i;
-
-    if (tot_units > 0)
-      resolve_list = fc_malloc(tot_units * 2 * sizeof(int));
-    i = 0;
-    unit_list_iterate(pplayer->units, punit) {
-      resolve_list[i]   = punit->x;
-      resolve_list[i+1] = punit->y;
-      i += 2;
-    } unit_list_iterate_end;
-
-    no_units = unit_list_size(&(pplayer2->units));
-    unit_list_iterate(pplayer2->units, punit) {
-      resolve_list[i]   = punit->x;
-      resolve_list[i+1] = punit->y;
-      i += 2;
-    } unit_list_iterate_end;
-
-    if (resolve_list) {
-      for (i = 0; i < tot_units * 2; i += 2)
-       resolve_unit_stack(resolve_list[i], resolve_list[i+1], TRUE);
-      free(resolve_list);
-      resolve_list = NULL;
-    }
+    unit_list_iterate_safe(pplayer->units, punit) {
+      resolve_unit_stack(punit->x, punit->y, TRUE);
+    } unit_list_iterate_safe_end;
   }
 
   /* 
@@ -1602,7 +1577,7 @@
   for(i = 0; i<game.num_tech_types ; i++)
     cplayer->research.inventions[i] = pplayer->research.inventions[i];
   cplayer->turn_done = TRUE; /* Have other things to think about - paralysis*/
-  cplayer->embassy = 0;   /* all embassys destroyed */
+  cplayer->embassy = 0;   /* all embassies destroyed */
 
   /* Do the ai */