Index: client/climisc.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/climisc.c,v
retrieving revision 1.121
diff -u -u -r1.121 climisc.c
--- client/climisc.c	2003/10/21 21:49:55	1.121
+++ client/climisc.c	2003/11/10 17:23:20
@@ -1170,3 +1170,8 @@
     append_output_window(buf);
   }
 }
+
+void upgrade_unit(struct unit *punit, Unit_Type_id to_unit)
+{
+  die("The client don't upgrade units.");
+}
Index: client/gui-gtk/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
retrieving revision 1.173
diff -u -u -r1.173 citydlg.c
--- client/gui-gtk/citydlg.c	2003/11/01 00:53:05	1.173
+++ client/gui-gtk/citydlg.c	2003/11/10 17:23:23
@@ -2688,14 +2688,13 @@
 *****************************************************************/
 static void unit_upgrade_callback(gpointer data)
 {
-  struct unit *punit;
+  struct unit *punit =
+      player_find_unit_by_id(game.player_ptr, (size_t) data);
   char buf[512];
-  int ut1, ut2;
-  int value;
 
-  if ((punit = player_find_unit_by_id(game.player_ptr, (size_t) data))) {
-    ut1 = punit->type;
-    ut2 = can_upgrade_unittype(game.player_ptr, ut1);
+  if (punit) {
+    Unit_Type_id ut1 = punit->type;
+    Unit_Type_id ut2 = can_upgrade_unittype(game.player_ptr, ut1);
 
     if (ut2 == -1) {
       /* this shouldn't generally happen, but it is conceivable */
@@ -2703,12 +2702,23 @@
 		  _("Sorry: cannot upgrade %s."), unit_types[ut1].name);
       popup_message_dialog(top_vbox,
 			   _("Upgrade Unit!"), buf,
-			   dummy_close_callback, NULL, 
+			   dummy_close_callback, NULL,
 			   _("Darn"), NULL, 0, NULL);
     } else {
-      value = unit_upgrade_price(game.player_ptr, ut1, ut2);
+      enum unit_upgrade_result result =
+	  unit_upgrade(punit, ut2, TRUE, TRUE, TRUE);
+      int value = unit_upgrade_price(game.player_ptr, ut1, ut2);
 
-      if (game.player_ptr->economic.gold >= value) {
+      if (result == UR_NO_MONEY) {
+	my_snprintf(buf, sizeof(buf),
+		    _("Upgrading %s to %s costs %d gold.\n"
+		      "Treasury contains %d gold."), unit_types[ut1].name,
+		    unit_types[ut2].name, value,
+		    game.player_ptr->economic.gold);
+	popup_message_dialog(top_vbox, _("Upgrade Unit!"), buf,
+			     dummy_close_callback, NULL,
+			     _("Darn"), NULL, 0, NULL);
+      } else if (result == UR_OK) {
 	my_snprintf(buf, sizeof(buf), _("Upgrade %s to %s for %d gold?\n"
 					"Treasury contains %d gold."),
 		    unit_types[ut1].name, unit_types[ut2].name,
@@ -2721,12 +2731,10 @@
 			     NULL, 0, NULL);
       } else {
 	my_snprintf(buf, sizeof(buf),
-		    _("Upgrading %s to %s costs %d gold.\n"
-		      "Treasury contains %d gold."), unit_types[ut1].name,
-		    unit_types[ut2].name, value,
-		    game.player_ptr->economic.gold);
-	popup_message_dialog(top_vbox,_("Upgrade Unit!"),buf,
-			     dummy_close_callback, NULL, 
+		    _("Sorry: cannot upgrade %s."), unit_types[ut1].name);
+	popup_message_dialog(top_vbox,
+			     _("Upgrade Unit!"), buf,
+			     dummy_close_callback, NULL,
 			     _("Darn"), NULL, 0, NULL);
       }
     }
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.187
diff -u -u -r1.187 unit.c
--- common/unit.c	2003/10/29 08:00:39	1.187
+++ common/unit.c	2003/11/10 17:23:24
@@ -1512,3 +1512,111 @@
     punit->pgr = NULL;
   }
 }
+
+/****************************************************************************
+  Expensive function to check how many units are in the transport.
+****************************************************************************/
+int get_transporter_occupancy(struct unit *ptrans)
+{
+  int occupied = 0;
+
+  unit_list_iterate(map_get_tile(ptrans->x, ptrans->y)->units, pcargo) {
+    if (pcargo->transported_by == ptrans->id) {
+      occupied++;
+    }
+  } unit_list_iterate_end;
+
+  return occupied;
+}
+
+/***************************************************************************
+ Return 1 if upgrading this unit could cause passengers to
+ get stranded at sea, due to transport capacity changes
+ caused by the upgrade.
+ Return 0 if ok to upgrade (as far as stranding goes).
+***************************************************************************/
+static bool upgrade_would_strand(struct unit *punit, Unit_Type_id upgrade_type)
+{
+  int old_cap, new_cap, tile_cap, tile_ncargo;
+  
+  if (!is_sailing_unit(punit))
+    return FALSE;
+  if (!is_ocean(map_get_terrain(punit->x, punit->y))) {
+    return FALSE;
+  }
+
+  /* With weird non-standard unit types, upgrading these could
+     cause air units to run out of fuel; too bad. */
+  if (unit_flag(punit, F_CARRIER)
+      || unit_flag(punit, F_MISSILE_CARRIER))
+    return FALSE;
+
+  old_cap = get_transporter_capacity(punit);
+  new_cap = unit_types[upgrade_type].transport_capacity;
+
+  if (new_cap >= old_cap)
+    return FALSE;
+
+  tile_cap = 0;
+  tile_ncargo = 0;
+  unit_list_iterate(map_get_tile(punit->x, punit->y)->units, punit2) {
+    if (punit2->owner == punit->owner
+        || pplayers_allied(unit_owner(punit2), unit_owner(punit))) {
+      if (is_sailing_unit(punit2) && is_ground_units_transport(punit2)) { 
+	tile_cap += get_transporter_capacity(punit2);
+      } else if (is_ground_unit(punit2)) {
+	tile_ncargo++;
+      }
+    }
+  }
+  unit_list_iterate_end;
+
+  if (tile_ncargo <= tile_cap - old_cap + new_cap)
+    return FALSE;
+
+  freelog(LOG_VERBOSE, "Can't upgrade %s at (%d,%d)"
+	  " because would strand passenger(s)",
+	  unit_type(punit)->name, punit->x, punit->y);
+  return TRUE;
+}
+
+/***************************************************************************
+ Return 1 if upgrading this unit could cause passengers to
+ get stranded at sea, due to transport capacity changes
+ caused by the upgrade.
+ Return 0 if ok to upgrade (as far as stranding goes).
+***************************************************************************/
+enum unit_upgrade_result unit_upgrade(struct unit *punit,
+				      Unit_Type_id to_unittype,
+				      bool test_only, bool need_money,
+				      bool need_city)
+{
+  struct player *pplayer = unit_owner(punit);
+  struct city *pcity = map_get_city(punit->x, punit->y);
+  const int cost = unit_upgrade_price(pplayer, punit->type, to_unittype);
+
+  if (need_money && pplayer->economic.gold < cost) {
+    return UR_NO_MONEY;
+  }
+
+  if (need_city && !pcity) {
+    return UR_NOT_IN_CITY;
+  }
+  if (need_city && pcity && city_owner(pcity) != pplayer) {
+    return UR_NOT_CITY_OWNER;
+  }
+  if (get_transporter_occupancy(punit) >
+      unit_types[to_unittype].transport_capacity
+      || upgrade_would_strand(punit, to_unittype)) {
+    return UR_NOT_ENOUGH_ROOM;
+  }
+
+  if (test_only) {
+    return UR_OK;
+  }
+  upgrade_unit(punit, to_unittype);
+  if (need_money) {
+    pplayer->economic.gold -= cost;
+  }
+  return UR_OK;
+}
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.103
diff -u -u -r1.103 unit.h
--- common/unit.h	2003/10/29 08:00:39	1.103
+++ common/unit.h	2003/11/10 17:23:25
@@ -95,6 +95,14 @@
 				   sewer but city has no sewer */
 };
 
+enum unit_upgrade_result {
+  UR_OK,
+  UR_NO_MONEY,
+  UR_NOT_IN_CITY,
+  UR_NOT_CITY_OWNER,
+  UR_NOT_ENOUGH_ROOM
+};
+    
 struct unit_ai {
   bool control; /* 0: not automated    1: automated */
   enum ai_unit_task ai_role;
@@ -296,5 +304,15 @@
                                  Unit_Type_id type, bool make_veteran);
 void destroy_unit_virtual(struct unit *punit);
 void free_unit_goto_route(struct unit *punit);
+
+int get_transporter_occupancy(struct unit *ptrans);
+
+/* Implemented in server/unittools.c and client/climisc.c */
+void upgrade_unit(struct unit *punit, Unit_Type_id to_unit);
+
+enum unit_upgrade_result unit_upgrade(struct unit *punit,
+				      Unit_Type_id to_unittype,
+				      bool test_only, bool need_money,
+				      bool need_city);
 
 #endif  /* FC__UNIT_H */
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.277
diff -u -u -r1.277 unithand.c
--- server/unithand.c	2003/10/29 08:00:39	1.277
+++ server/unithand.c	2003/11/10 17:23:27
@@ -153,55 +153,49 @@
 /**************************************************************************
  Upgrade all units of a given type.
 **************************************************************************/
-void handle_upgrade_unittype_request(struct player * const pplayer,
-			const struct packet_unittype_info * const packet)
+void handle_upgrade_unittype_request(struct player *const pplayer,
+				     const struct packet_unittype_info *const
+				     packet)
 {
   const Unit_Type_id from_unittype = packet->type;
   const Unit_Type_id to_unittype = can_upgrade_unittype(pplayer,
 							from_unittype);
+  int number_of_upgraded_units = 0;
 
   if (to_unittype == -1) {
     notify_player(pplayer,
 		  _("Game: Illegal packet, can't upgrade %s (yet)."),
-                  unit_types[from_unittype].name);
-  } else {
-    const int cost = unit_upgrade_price(pplayer, from_unittype, to_unittype);
-    int number_of_upgraded_units = 0;
-
-    if (pplayer->economic.gold >= cost) {
-      const int player_no = pplayer->player_no;
+		  unit_types[from_unittype].name);
+    return;
+  }
 
-      /* Try to upgrade units. The order we upgrade in is arbitrary (if
-       * the player really cared they should have done it manually). */
-      conn_list_do_buffer(&pplayer->connections);
-      unit_list_iterate(pplayer->units, punit) {
-        if (punit->type == from_unittype) {
-          const struct city * const pcity = map_get_city(punit->x, punit->y);
-
-	  /* Only units in cities can be upgraded. */
-          if (pcity && pcity->owner == player_no) {
-            upgrade_unit(punit, to_unittype);
-	    number_of_upgraded_units++;
-            if ((pplayer->economic.gold -= cost) < cost) {
-	      /* We can't upgrade any more units. */
-              break;
-            }
-          }
-        }
-      } unit_list_iterate_end;
-      conn_list_do_unbuffer(&pplayer->connections);
+  /* Try to upgrade units. The order we upgrade in is arbitrary (if
+   * the player really cared they should have done it manually). */
+  conn_list_do_buffer(&pplayer->connections);
+  unit_list_iterate(pplayer->units, punit) {
+    if (punit->type == from_unittype) {
+      enum unit_upgrade_result result =
+	  unit_upgrade(punit, to_unittype, FALSE, TRUE, TRUE);
+      if (result == UR_OK) {
+	number_of_upgraded_units++;
+      }
+      if (result == UR_NO_MONEY) {
+	break;
+      }
     }
+  } unit_list_iterate_end;
+  conn_list_do_unbuffer(&pplayer->connections);
 
-    /* Alert the player about what happened. */
-    if (number_of_upgraded_units > 0) {
-      notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."),
-                    number_of_upgraded_units, unit_types[from_unittype].name,
-                    unit_types[to_unittype].name,
-                    cost * number_of_upgraded_units);
-      send_player_info(pplayer, pplayer);
-    } else {
-      notify_player(pplayer, _("Game: No units could be upgraded."));
-    }
+  /* Alert the player about what happened. */
+  if (number_of_upgraded_units > 0) {
+    const int cost = unit_upgrade_price(pplayer, from_unittype, to_unittype);
+    notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."),
+		  number_of_upgraded_units, unit_types[from_unittype].name,
+		  unit_types[to_unittype].name,
+		  cost * number_of_upgraded_units);
+    send_player_info(pplayer, pplayer);
+  } else {
+    notify_player(pplayer, _("Game: No units could be upgraded."));
   }
 }
 
@@ -210,38 +204,47 @@
  TODO: should upgrades in allied cities be possible?
 **************************************************************************/
 void handle_unit_upgrade_request(struct player *pplayer,
-                                 struct packet_unit_request *packet)
+				 struct packet_unit_request *packet)
 {
-  int cost;
   int from_unit, to_unit;
   struct unit *punit = player_find_unit_by_id(pplayer, packet->unit_id);
   struct city *pcity = player_find_city_by_id(pplayer, packet->city_id);
-  
+  enum unit_upgrade_result result;
+
   if (!punit || !pcity) {
     return;
   }
 
-  if(!same_pos(punit->x, punit->y, pcity->x, pcity->y))  {
+  if (!same_pos(punit->x, punit->y, pcity->x, pcity->y)) {
     notify_player(pplayer, _("Game: Illegal move, unit not in city!"));
     return;
   }
   from_unit = punit->type;
-  if((to_unit=can_upgrade_unittype(pplayer, punit->type)) == -1) {
-    notify_player(pplayer, _("Game: Illegal package, can't upgrade %s (yet)."), 
+  to_unit = can_upgrade_unittype(pplayer, punit->type);
+  if (to_unit == -1) {
+    notify_player(pplayer,
+		  _("Game: Illegal package, can't upgrade %s (yet)."),
 		  unit_type(punit)->name);
     return;
   }
-  cost = unit_upgrade_price(pplayer, punit->type, to_unit);
-  if(cost > pplayer->economic.gold) {
+
+  result = unit_upgrade(punit, to_unit, FALSE, TRUE, TRUE);
+  if (result == UR_NO_MONEY) {
+    int cost = unit_upgrade_price(pplayer, punit->type, to_unit);
     notify_player(pplayer, _("Game: Insufficient funds, upgrade costs %d."),
 		  cost);
-    return;
+  } else if (result == UR_OK) {
+    int cost = unit_upgrade_price(pplayer, punit->type, to_unit);
+    send_player_info(pplayer, pplayer);
+    notify_player(pplayer, _("Game: %s upgraded to %s for %d gold."),
+		  unit_name(from_unit), unit_name(to_unit), cost);
+  } else if (result == UR_NOT_ENOUGH_ROOM) {
+    notify_player(pplayer, _("Game: Cannot upgrade %s because it is "
+			     "transporting other units."),
+		  unit_type(punit)->name);
+  } else {
+    notify_player(pplayer, _("Game: Upgrading failed."));
   }
-  pplayer->economic.gold -= cost;
-  upgrade_unit(punit, to_unit);
-  send_player_info(pplayer, pplayer);
-  notify_player(pplayer, _("Game: %s upgraded to %s for %d gold."), 
-		unit_name(from_unit), unit_name(to_unit), cost);
 }
 
 
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.263
diff -u -u -r1.263 unittools.c
--- server/unittools.c	2003/11/04 10:13:28	1.263
+++ server/unittools.c	2003/11/10 17:23:29
@@ -59,7 +59,6 @@
 static void unit_restore_movepoints(struct player *pplayer, struct unit *punit);
 static void update_unit_activity(struct unit *punit);
 static void wakeup_neighbor_sentries(struct unit *punit);
-static bool upgrade_would_strand(struct unit *punit, int upgrade_type);
 static void handle_leonardo(struct player *pplayer);
 
 static void sentry_transported_idle_units(struct unit *ptrans);
@@ -158,63 +157,11 @@
 }
 
 /***************************************************************************
- Return 1 if upgrading this unit could cause passengers to
- get stranded at sea, due to transport capacity changes
- caused by the upgrade.
- Return 0 if ok to upgrade (as far as stranding goes).
-***************************************************************************/
-static bool upgrade_would_strand(struct unit *punit, int upgrade_type)
-{
-  int old_cap, new_cap, tile_cap, tile_ncargo;
-  
-  if (!is_sailing_unit(punit))
-    return FALSE;
-  if (!is_ocean(map_get_terrain(punit->x, punit->y))) {
-    return FALSE;
-  }
-
-  /* With weird non-standard unit types, upgrading these could
-     cause air units to run out of fuel; too bad. */
-  if (unit_flag(punit, F_CARRIER)
-      || unit_flag(punit, F_MISSILE_CARRIER))
-    return FALSE;
-
-  old_cap = get_transporter_capacity(punit);
-  new_cap = unit_types[upgrade_type].transport_capacity;
-
-  if (new_cap >= old_cap)
-    return FALSE;
-
-  tile_cap = 0;
-  tile_ncargo = 0;
-  unit_list_iterate(map_get_tile(punit->x, punit->y)->units, punit2) {
-    if (punit2->owner == punit->owner
-        || pplayers_allied(unit_owner(punit2), unit_owner(punit))) {
-      if (is_sailing_unit(punit2) && is_ground_units_transport(punit2)) { 
-	tile_cap += get_transporter_capacity(punit2);
-      } else if (is_ground_unit(punit2)) {
-	tile_ncargo++;
-      }
-    }
-  }
-  unit_list_iterate_end;
-
-  if (tile_ncargo <= tile_cap - old_cap + new_cap)
-    return FALSE;
-
-  freelog(LOG_VERBOSE, "Can't upgrade %s at (%d,%d)"
-	  " because would strand passenger(s)",
-	  unit_type(punit)->name, punit->x, punit->y);
-  return TRUE;
-}
-
-/***************************************************************************
 Do Leonardo's Workshop upgrade(s). Select unit to upgrade by random. --Zamar
 Now be careful not to strand units at sea with the Workshop. --dwp
 ****************************************************************************/
 static void handle_leonardo(struct player *pplayer)
 {
-  int upgrade_type; 
   int leonardo_variant;
 	
   struct unit_list candidates;
@@ -227,9 +174,11 @@
   unit_list_init(&candidates);
 	
   unit_list_iterate(pplayer->units, punit) {
-    if ((upgrade_type=can_upgrade_unittype(pplayer, punit->type))!=-1 &&
-       !upgrade_would_strand(punit, upgrade_type))
-      unit_list_insert(&candidates, punit); /* Potential candidate :) */
+    Unit_Type_id upgrade_type = can_upgrade_unittype(pplayer, punit->type);
+    if (upgrade_type != -1
+	&& unit_upgrade(punit, upgrade_type, TRUE, FALSE, FALSE) == UR_OK) {
+      unit_list_insert(&candidates, punit);	/* Potential candidate :) */
+    }
   } unit_list_iterate_end;
 	
   if (unit_list_size(&candidates) == 0)
@@ -241,7 +190,9 @@
   i=0;	
   unit_list_iterate(candidates, punit) {
     if (leonardo_variant != 0 || i == candidate_to_upgrade) {
-      upgrade_type=can_upgrade_unittype(pplayer, punit->type);
+      enum unit_upgrade_result result;
+      Unit_Type_id upgrade_type = can_upgrade_unittype(pplayer, punit->type);
+
       notify_player(pplayer,
             _("Game: %s has upgraded %s to %s%s."),
             improvement_types[B_LEONARDO].name,
@@ -249,7 +200,8 @@
             get_unit_type(upgrade_type)->name,
             get_location_str_in(pplayer, punit->x, punit->y));
       punit->veteran = FALSE;
-      upgrade_unit(punit, upgrade_type);
+      result = unit_upgrade(punit, upgrade_type, FALSE, FALSE, FALSE);
+      assert(result == UR_OK);
     }
     i++;
   } unit_list_iterate_end;
@@ -1398,22 +1350,6 @@
       cap++;
     return cap>0;
   }
-}
-
-/****************************************************************************
-  Expensive function to check how many units are in the transport.
-****************************************************************************/
-int get_transporter_occupancy(struct unit *ptrans)
-{
-  int occupied = 0;
-
-  unit_list_iterate(map_get_tile(ptrans->x, ptrans->y)->units, pcargo) {
-    if (pcargo->transported_by == ptrans->id) {
-      occupied++;
-    }
-  } unit_list_iterate_end;
-
-  return occupied;
 }
 
 /**************************************************************************
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.57
diff -u -u -r1.57 unittools.h
--- server/unittools.h	2003/10/08 16:56:07	1.57
+++ server/unittools.h	2003/11/10 17:23:29
@@ -29,7 +29,6 @@
 /* move check related */
 bool is_airunit_refuel_point(int x, int y, struct player *pplayer,
 			     Unit_Type_id type, bool unit_is_on_tile);
-int get_transporter_occupancy(struct unit *ptrans);
 
 /* turn update related */
 void player_restore_units(struct player *pplayer);