[Freeciv-Dev] (bugfix) Re: (PR#4110) quadruple messages

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

On Tue, 29 Apr 2003, ChrisK@xxxxxxxx wrote:
> On automatic unit relocation/transport, when alliance ends,
> the messages given for each unit appear ~4 times.

This is the fix. I essentially rewrote the entire resolve unit stack code.
It is now much more sane, and much smaller.

The game logic changed slightly as well. Previously you would teleport
some but not all offending units in an illegal stack, using rather complex
and IMHO arbitrary rules I am sure no players is aware of.

Now we bounce away all non-allied units from our cities, then all
offending units are teleported from illegal stacks outside cities. On
ocean tiles, we also teleport away third party units to avoid drowning
(transport) issues, which is very hard to get right. Units that cannot
find a new home are deleted.

  - Per

Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.216
diff -u -r1.216 citytools.c
--- server/citytools.c  29 Apr 2003 18:05:28 -0000      1.216
+++ server/citytools.c  2 May 2003 18:54:47 -0000
@@ -802,8 +802,7 @@
                   int kill_outside, bool transfer_unit_verbose,
                   bool resolve_stack, bool raze)
 {
-  struct map_position *resolve_list = NULL;
-  int i, no_units = 0;
+  int i;
   struct unit_list old_city_units;
   struct player *pgiver = city_owner(pcity);
   int old_trade_routes[NUM_TRADEROUTES];
@@ -850,24 +849,6 @@
   pcity->owner = ptaker->player_no;
   city_list_insert(&ptaker->cities, pcity);
 
-  /* transfer_city_units() destroys the city's units_supported
-     list; we save the list so we can resolve units afterwards. */
-  if (resolve_stack) {
-    no_units = unit_list_size(&old_city_units);
-    if (no_units > 0) {
-      resolve_list = fc_malloc((no_units + 1) * sizeof(struct map_position));
-      i = 0;
-      unit_list_iterate(old_city_units, punit) {
-       resolve_list[i].x = punit->x;
-       resolve_list[i].y = punit->y;
-       i++;
-      } unit_list_iterate_end;
-      resolve_list[i].x = pcity->x;
-      resolve_list[i].y = pcity->y;
-      assert(i == no_units);
-    }
-  }
-
   transfer_city_units(ptaker, pgiver, &old_city_units,
                      pcity, NULL,
                      kill_outside, transfer_unit_verbose);
@@ -875,12 +856,8 @@
   unit_list_unlink_all(&old_city_units);
   reset_move_costs(pcity->x, pcity->y);
 
-  if (resolve_stack && (no_units > 0) && resolve_list) {
-    for (i = 0; i < no_units+1 ; i++)
-      resolve_unit_stack(resolve_list[i].x, resolve_list[i].y,
-                        transfer_unit_verbose);
-    free(resolve_list);
-    resolve_list = NULL;
+  if (resolve_stack && !pplayers_allied(pgiver, ptaker)) {
+    resolve_unit_stacks(pgiver, ptaker, transfer_unit_verbose);
   }
 
   /* Update the city's trade routes. */
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.268
diff -u -r1.268 plrhand.c
--- server/plrhand.c    30 Apr 2003 21:14:35 -0000      1.268
+++ server/plrhand.c    2 May 2003 18:54:47 -0000
@@ -907,12 +907,9 @@
   send_player_info(pplayer2, NULL);
 
   /* 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'
-     potentially shared unit positions. */
+     illegally, and we need to call resolve_unit_stacks() */
   if (old_type == DS_ALLIANCE) {
-    unit_list_iterate_safe(pplayer->units, punit) {
-      resolve_unit_stack(punit->x, punit->y, TRUE);
-    } unit_list_iterate_safe_end;
+    resolve_unit_stacks(pplayer, pplayer2, TRUE);
   }
 
   /* 
@@ -1713,17 +1710,8 @@
   city_list_iterate_end;
 
   i = 0;
-  
-  unit_list_iterate(pplayer->units, punit) 
-    resolve_unit_stack(punit->x, punit->y, FALSE);
-  unit_list_iterate_end;
-  city_list_iterate(pplayer->cities, pcity) {
-    resolve_unit_stack(pcity->x, pcity->y, FALSE);
-  } city_list_iterate_end;
-  city_list_iterate(cplayer->cities, pcity) {
-    resolve_unit_stack(pcity->x, pcity->y, FALSE);
-  } city_list_iterate_end;
 
+  resolve_unit_stacks(pplayer, cplayer, FALSE);
 
   notify_player(NULL,
                _("Game: The capture of %s's capital and the destruction "
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.218
diff -u -r1.218 unittools.c
--- server/unittools.c  17 Apr 2003 20:06:37 -0000      1.218
+++ server/unittools.c  2 May 2003 18:54:47 -0000
@@ -1375,102 +1375,69 @@
 }
 
 /**************************************************************************
-  Resolve unit stack
+  Teleport or remove a unit due to stack conflict.
+**************************************************************************/
+static void bounce_unit(struct unit *punit, bool verbose)
+{
+  struct player *pplayer = unit_owner(punit);
+  struct city *pcity = find_closest_owned_city(pplayer, punit->x, punit->y,
+                                               is_sailing_unit(punit), NULL);
+
+  if (pcity) {
+    (void) teleport_unit_to_city(punit, pcity, 0, verbose);
+  } else {
+    disband_stack_conflict_unit(punit, verbose);
+  }
+}
 
-  When in civil war (or an alliance breaks) there will potentially be units 
+/**************************************************************************
+  When in civil war or an alliance breaks there will potentially be units 
   from both sides coexisting on the same squares.  This routine resolves 
-  this by teleporting the units in multiowner stacks to the closest city.
-
-  That is, if a unit is closer to its city than the coexistent enemy unit,
-  then the enemy unit is teleported to its owner's closest city.
-
-                         - Kris Bubendorfer
+  this by first bouncing off non-allied units from their cities, then by 
+  bouncing both players' units in now illegal multiowner stacks.  To avoid
+  drowning due to removal of transports, we bounce everyone (including
+  third parties' units) from ocean tiles.
 
   If verbose is true, the unit owner gets messages about where each
-  units goes.  --dwp
+  units goes.
 **************************************************************************/
-void resolve_unit_stack(int x, int y, bool verbose)
+void resolve_unit_stacks(struct player *pplayer, struct player *aplayer,
+                         bool verbose)
 {
-  struct unit *punit, *cunit;
-  struct tile *ptile = map_get_tile(x,y);
-
-  /* We start by reducing the unit list until we only have allied units */
-  while(TRUE) {
-    struct city *pcity, *ccity;
-
-    if (unit_list_size(&ptile->units) == 0)
-      return;
-
-    punit = unit_list_get(&(ptile->units), 0);
-    pcity = find_closest_owned_city(unit_owner(punit), x, y,
-                                   is_sailing_unit(punit), NULL);
-
-    /* If punit is in an enemy city we send it to the closest friendly city
-       This is not always caught by the other checks which require that
-       there are units from two nations on the tile */
-    if (ptile->city && !is_allied_city_tile(ptile, unit_owner(punit))) {
-      if (pcity)
-       (void) teleport_unit_to_city(punit, pcity, 0, verbose);
-      else
-       disband_stack_conflict_unit(punit, verbose);
-      continue;
+  /* Throw aplayer's units out of pplayer's cities */
+  city_list_iterate(pplayer->cities, pcity) {
+    struct unit *punit;
+    while ((punit = is_non_allied_unit_tile(map_get_tile(pcity->x, pcity->y),
+                                            pplayer))) {
+      bounce_unit(punit, verbose);
     }
+  } city_list_iterate_end;
 
-    cunit = is_non_allied_unit_tile(ptile, unit_owner(punit));
-    if (!cunit)
-      break;
-
-    ccity = find_closest_owned_city(unit_owner(cunit), x, y,
-                                   is_sailing_unit(cunit), NULL);
-
-    if (pcity && ccity) {
-      /* Both unit owners have cities; teleport unit farthest from its
-        owner's city to that city. This also makes sure we get no loops
-        from when we resolve the stack inside a city. */
-      if (map_distance(x, y, pcity->x, pcity->y) 
-         < map_distance(x, y, ccity->x, ccity->y))
-       (void) teleport_unit_to_city(cunit, ccity, 0, verbose);
-      else
-       (void) teleport_unit_to_city(punit, pcity, 0, verbose);
-    } else {
-      /* At least one of the unit owners doesn't have any cities;
-        if the other owner has any cities we teleport his units to
-        the closest. We take care not to teleport the unit to the
-        original square, as that would cause the while loop in this
-        function to potentially never stop. */
-      if (pcity) {
-       if (same_pos(x, y, pcity->x, pcity->y))
-         disband_stack_conflict_unit(cunit, verbose);
-       else
-         (void) teleport_unit_to_city(punit, pcity, 0, verbose);
-      } else if (ccity) {
-       if (same_pos(x, y, ccity->x, ccity->y))
-         disband_stack_conflict_unit(punit, verbose);
-       else
-         (void) teleport_unit_to_city(cunit, ccity, 0, verbose);
-      } else {
-       /* Neither unit owners have cities;
-          disband both units. */
-       disband_stack_conflict_unit(punit, verbose);
-       disband_stack_conflict_unit(cunit, verbose);
-      }      
+  /* Throw pplayer's units out of aplayer's cities */
+  city_list_iterate(aplayer->cities, pcity) {
+    struct unit *punit;
+    while ((punit = is_non_allied_unit_tile(map_get_tile(pcity->x, pcity->y),
+                                            aplayer))) {
+      bounce_unit(punit, verbose);
     }
-  } /* end while */
+  } city_list_iterate_end;
 
-  /* 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 (acity) {
-        (void) teleport_unit_to_city(aunit, acity, 0, verbose);
-      } else {
-        disband_stack_conflict_unit(aunit, verbose);
-      }
-    }
+  /* Now cities are clean - do non-city stacks. For each unit, check
+   * if we stack illegally, if so, bounce both players' units. If
+   * on ocean tile, bounce everyone to avoid drowning. */
+  unit_list_iterate_safe(pplayer->units, punit) {
+    int x = punit->x, y = punit->y;
+    struct tile *ptile = map_get_tile(x, y);
+
+    if (is_non_allied_unit_tile(ptile, pplayer)) {
+      unit_list_iterate(ptile->units, aunit) {
+        if (unit_owner(aunit) == pplayer
+            || unit_owner(aunit) == aplayer
+            || is_ocean(ptile->terrain)) {
+          bounce_unit(aunit, verbose);
+        }
+      } unit_list_iterate_end;
+    }    
   } unit_list_iterate_safe_end;
 }
 
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.49
diff -u -r1.49 unittools.h
--- server/unittools.h  4 Apr 2003 15:47:50 -0000       1.49
+++ server/unittools.h  2 May 2003 18:54:47 -0000
@@ -44,7 +44,8 @@
 bool enemies_at(struct unit *punit, int x, int y);
 bool teleport_unit_to_city(struct unit *punit, struct city *pcity, int 
move_cost,
                          bool verbose);
-void resolve_unit_stack(int x, int y, bool verbose);
+void resolve_unit_stacks(struct player *pplayer, struct player *aplayer,
+                         bool verbose);
 void disband_stack_conflict_unit(struct unit *punit, bool verbose);
 int get_watchtower_vision(struct unit *punit);
 bool unit_profits_of_watchtower(struct unit *punit);