[Freeciv-Dev] remove_city: bug with cities on the sea (PR#5073)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

remove_city() will move all sea units off of a destroyed land city.  If 
the city is in the sea, we want to instead move all land units off of 
the city.

Untested patch attached.

jason

Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.237
diff -u -r1.237 citytools.c
--- server/citytools.c  2003/10/13 01:33:31     1.237
+++ server/citytools.c  2003/10/27 16:36:26
@@ -1116,44 +1116,45 @@
   x = pcity->x;
   y = pcity->y;
 
-  /* make sure ships are not left on land when city is removed. */
- MOVE_SEA_UNITS:
-  unit_list_iterate(ptile->units, punit) {
-    bool moved;
-    if (!punit
-       || !same_pos(punit->x, punit->y, x, y)
-       || !is_sailing_unit(punit)) {
-      continue;
-    }
+  /* Make sure units are not left on invalid terrain when city is removed.
+   * This means ships on land or land units in the ocean. */
+  unit_list_iterate_safe(ptile->units, punit) {
+    bool oceanic = is_ocean(map_get_terrain(x, y));
+
+    assert(punit != NULL && same_pos(punit->x, punit->y, x, y));
 
-    handle_unit_activity_request(punit, ACTIVITY_IDLE);
-    moved = FALSE;
-    adjc_iterate(x, y, x1, y1) {
-      if (is_ocean(map_get_terrain(x1, y1))) {
+    if ((oceanic && !is_sailing_unit(punit)
+        && ground_unit_transporter_capacity(x, y, unit_owner(punit)) < 0)
+       || (!oceanic && is_sailing_unit(punit))) {
+      bool moved = FALSE;
+
+      handle_unit_activity_request(punit, ACTIVITY_IDLE);
+      adjc_iterate(x, y, x1, y1) {
        if (could_unit_move_to_tile(punit, x1, y1) == 1) {
-         moved = handle_unit_move_request(punit, x1, y1, FALSE, TRUE);
-         if (moved) {
+         if (handle_unit_move_request(punit, x1, y1, FALSE, TRUE)) {
+           /* TRANS: Given for land units in the ocean, and ocean units
+            * in the land. */
            notify_player_ex(unit_owner(punit), -1, -1, E_NOEVENT,
                             _("Game: Moved %s out of disbanded city %s "
-                              "to avoid being landlocked."),
+                              "to avoid being stuck."),
                             unit_type(punit)->name, pcity->name);
-           goto OUT;
+           moved = TRUE;
+           break;
          }
        }
+      } adjc_iterate_end;
+
+      if (!moved) {
+       /* TRANS: Given for land units in the ocean, and ocean units
+        * in the land. */
+       notify_player_ex(unit_owner(punit), -1, -1, E_NOEVENT,
+                        _("Game: When %s was disbanded your %s could not "
+                          "get out, and it was therefore stranded."),
+                        pcity->name, unit_type(punit)->name);
+       wipe_unit(punit);
       }
-    } adjc_iterate_end;
-  OUT:
-    if (!moved) {
-      notify_player_ex(unit_owner(punit), -1, -1, E_NOEVENT,
-                      _("Game: When %s was disbanded your %s could not "
-                        "get out, and it was therefore stranded."),
-                      pcity->name, unit_type(punit)->name);
-      wipe_unit(punit);
     }
-    /* We just messed with the unit list. Avoid trouble by starting over.
-       Note that the problem is reduced by one unit, so no loop trouble. */
-    goto MOVE_SEA_UNITS;
-  } unit_list_iterate_end;
+  } unit_list_iterate_safe_end;
 
   for (o = 0; o < NUM_TRADEROUTES; o++) {
     struct city *pother_city = find_city_by_id(pcity->trade[o]);