[Freeciv-Dev] Re: (PR#18380) [Patch] Rewrite wipe_unit()

["Marko Lindqvist" <cazfi74@xxxxxxxxx>]

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

On 7/7/06, Marko Lindqvist <cazfi74@xxxxxxxxx> wrote:
>
>   #13384 requires changes to wipe_unit() logic. This patch implements those.
>   Now we iterate through all drowning units and search for transport for
> each. If free transport is found, put unit in to the transport. If not,
> unit is destroyed.
>

 - "Loaded" status of saved units was not updated to client.

 While comparing results to old behavior, I noticed that this patch is
useful in itself (and not only as stepping stone to #13384). Old code
allowed cargo of remaining transport to drown when cargo of wiped
transport is saved.


 - ML

diff -Nurd -X.diff_ignore freeciv/common/movement.c freeciv/common/movement.c
--- freeciv/common/movement.c   2006-07-11 13:54:37.484375000 +0300
+++ freeciv/common/movement.c   2006-07-11 14:21:14.000000000 +0300
@@ -26,6 +26,7 @@
 #include "map.h"
 #include "movement.h"
 #include "unit.h"
+#include "unitlist.h"
 #include "unittype.h"
 #include "terrain.h"
 
@@ -486,3 +487,20 @@
   }
   return MOVETYPE_LAST;
 }
+
+/**************************************************************************
+  Search transport suitable for given unit from tile. It has to have
+  free space in it.
+**************************************************************************/
+struct unit *find_transport_from_tile(struct unit *punit, struct tile *ptile)
+{
+  unit_list_iterate(ptile->units, ptransport) {
+    if (get_transporter_capacity(ptransport) > 
get_transporter_occupancy(ptransport)
+        && can_unit_transport(ptransport, punit)
+        && is_native_terrain(unit_type(ptransport), ptile->terrain)) {
+      return ptransport;
+    }
+  } unit_list_iterate_end;
+
+  return NULL;
+}
diff -Nurd -X.diff_ignore freeciv/common/movement.h freeciv/common/movement.h
--- freeciv/common/movement.h   2006-07-11 13:54:37.515625000 +0300
+++ freeciv/common/movement.h   2006-07-11 14:21:14.031250000 +0300
@@ -49,6 +49,7 @@
                                             const struct tile *dst_tile,
                                             bool igzoc);
 bool can_unit_transport(const struct unit *transporter, const struct unit 
*transported);
+struct unit *find_transport_from_tile(struct unit *punit, struct tile *ptile);
 
 enum unit_move_type move_type_from_str(const char *s);
 
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c  2006-07-11 13:57:16.828125000 +0300
+++ freeciv/server/unittools.c  2006-07-11 14:31:11.390625000 +0300
@@ -1416,110 +1416,118 @@
 }
 
 /**************************************************************************
+  Handle units destroyed when their transport is destroyed
+**************************************************************************/
+static void unit_lost_with_transport(const struct player *pplayer,
+                                     struct unit *pcargo,
+                                     const struct unit_type *ptransport)
+{
+  notify_player(pplayer, pcargo->tile, E_UNIT_LOST,
+                _("%s lost when %s was lost."),
+                unit_type(pcargo)->name,
+                ptransport->name);
+  server_remove_unit(pcargo);
+}
+
+/**************************************************************************
   Remove the unit, and passengers if it is a carrying any. Remove the 
   _minimum_ number, eg there could be another boat on the square.
 **************************************************************************/
 void wipe_unit(struct unit *punit)
 {
-  bool wipe_cargo = TRUE; /* This used to be a function parameter. */
   struct tile *ptile = punit->tile;
   struct player *pplayer = unit_owner(punit);
   struct unit_type *ptype = unit_type(punit);
+  int drowning = 0;
 
   /* First pull all units off of the transporter. */
   if (get_transporter_capacity(punit) > 0) {
-    /* FIXME: there's no send_unit_info call below so these units aren't
-     * updated at the client.  I guess this works because the unit info
-     * will be sent eventually anyway, but it's surely a bug. */
     unit_list_iterate(ptile->units, pcargo) {
       if (pcargo->transported_by == punit->id) {
        /* Could use unload_unit_from_transporter here, but that would
         * call send_unit_info for the transporter unnecessarily. */
        pull_unit_from_transporter(pcargo, punit);
+        if (!can_unit_exist_at_tile(pcargo, ptile)) {
+          drowning++;
+        }
        if (pcargo->activity == ACTIVITY_SENTRY) {
          /* Activate sentried units - like planes on a disbanded carrier.
           * Note this will activate ground units even if they just change
           * transporter. */
          set_unit_activity(pcargo, ACTIVITY_IDLE);
        }
-       send_unit_info(NULL, pcargo);
-      } 
+        if (!can_unit_exist_at_tile(pcargo, ptile)) {
+          drowning++;
+          /* No need for send_unit_info() here. Unit info will be sent
+           * when it is assigned to a new transport or it will be removed. */
+        } else {
+          send_unit_info(NULL, pcargo);
+        }
+      }
     } unit_list_iterate_end;
   }
 
-  /* No need to wipe the cargo unless it's a ground transporter. */
-  wipe_cargo &= is_ground_units_transport(punit);
-
   /* Now remove the unit. */
   server_remove_unit(punit);
 
-  /* Finally reassign, bounce, or destroy all ground units at this location.
-   * There's no need to worry about air units; they can fly away. */
-  if (wipe_cargo
-      && is_ocean(tile_get_terrain(ptile))
-      && !tile_get_city(ptile)) {
+  /* Finally reassign, bounce, or destroy all units that cannot exist at this
+   * location without transport. */
+  if (drowning) {
     struct city *pcity = NULL;
-    int capacity = ground_unit_transporter_capacity(ptile, pplayer);
-
-    /* Get rid of excess standard units. */
-    if (capacity < 0) {
-      unit_list_iterate_safe(ptile->units, pcargo) {
-       if (is_ground_unit(pcargo)
-           && pcargo->transported_by == -1
-           && !unit_flag(pcargo, F_UNDISBANDABLE)
-           && !unit_flag(pcargo, F_GAMELOSS)) {
-         server_remove_unit(pcargo);
-         if (++capacity >= 0) {
-           break;
-         }
-       }
-      } unit_list_iterate_safe_end;
-    }
 
-    /* Get rid of excess undisbandable/gameloss units. */
-    if (capacity < 0) {
-      unit_list_iterate_safe(ptile->units, pcargo) {
-       if (is_ground_unit(pcargo) && pcargo->transported_by == -1) {
+    /* First save undisbandable and gameloss units */
+    unit_list_iterate_safe(ptile->units, pcargo) {
+      if (pcargo->transported_by == -1
+          && !can_unit_exist_at_tile(pcargo, ptile)
+          && (unit_flag(pcargo, F_UNDISBANDABLE) || unit_flag(pcargo, 
F_GAMELOSS))) {
+        struct unit *ptransport = find_transport_from_tile(pcargo, ptile);
+        if (ptransport != NULL) {
+          put_unit_onto_transporter(pcargo, ptransport);
+          send_unit_info(NULL, pcargo);
+        } else {
          if (unit_flag(pcargo, F_UNDISBANDABLE)) {
            pcity = find_closest_owned_city(unit_owner(pcargo),
                                            pcargo->tile, TRUE, NULL);
            if (pcity && teleport_unit_to_city(pcargo, pcity, 0, FALSE)) {
              notify_player(pplayer, ptile, E_UNIT_RELOCATED,
-                              _("%s escaped the destruction of %s, and "
-                                "fled to %s."), unit_type(pcargo)->name,
-                              ptype->name, pcity->name);
+                            _("%s escaped the destruction of %s, and "
+                              "fled to %s."), unit_type(pcargo)->name,
+                            ptype->name, pcity->name);
            }
-         }
-         if (!unit_flag(pcargo, F_UNDISBANDABLE) || !pcity) {
-           notify_player(pplayer, ptile, E_UNIT_LOST,
-                            _("%s lost when %s was lost."),
-                            unit_type(pcargo)->name,
-                            ptype->name);
-           server_remove_unit(pcargo);
-         }
-         if (++capacity >= 0) {
-           break;
-         }
-       }
-      } unit_list_iterate_safe_end;
-    }
+          }
+          if (!unit_flag(pcargo, F_UNDISBANDABLE) || !pcity) {
+            unit_lost_with_transport(pplayer, pcargo, ptype);
+          }
+        }
 
-    /* Reassign existing units.  This is an O(n^2) operation as written. */
-    unit_list_iterate(ptile->units, ptrans) {
-      if (is_ground_units_transport(ptrans)) {
-       int occupancy = get_transporter_occupancy(ptrans);
+        drowning--;
+        if (!drowning) {
+          break;
+        }
+      }
+    } unit_list_iterate_safe_end;
+  }
 
-       unit_list_iterate(ptile->units, pcargo) {
-         if (occupancy >= get_transporter_capacity(ptrans)) {
-           break;
-         }
-         if (is_ground_unit(pcargo) && pcargo->transported_by == -1) {
-           put_unit_onto_transporter(pcargo, ptrans);
-           occupancy++;
-         }
-       } unit_list_iterate_end;
+  /* Then other units */
+  if (drowning) {
+    unit_list_iterate_safe(ptile->units, pcargo) {
+      if (pcargo->transported_by == -1
+          && !can_unit_exist_at_tile(pcargo, ptile)) {
+        struct unit *ptransport = find_transport_from_tile(pcargo, ptile);
+
+        if (ptransport != NULL) {
+          put_unit_onto_transporter(pcargo, ptransport);
+          send_unit_info(NULL, pcargo);
+        } else {
+          unit_lost_with_transport(pplayer, pcargo, ptype);
+        }
+
+        drowning--;
+        if (!drowning) {
+          break;
+        }
       }
-    } unit_list_iterate_end;
+    } unit_list_iterate_safe_end;
   }
 }