[Freeciv-Dev] (PR#2566) PATCH: clean up handle_upgrade_unittype_request

["Guest via RT" <rt@xxxxxxxxxxxxxx>]

[rfalke - Fri Dec 20 12:19:38 2002]: 
> On Thu, Dec 19, 2002 at 10:58:37AM -0800, Jason Short via RT wrote: 
> > Upon further review, I don't think it's appropriate to pass the 
> > unittype directly.  This function should take the packet struct, 
> > just like all the other handle_xxx functions do.  In general, this 
> > is because if the packet changes more data will be needed by the 
> > function. 
>  
> I disagree. Especial in the light of the comming direct send/receive 
> as implmented in the delta patches. 
 
Here is a version of the patch with the argument changed again. Now I 
also added the empty lines requested by kauf. 

diff -rXi -U2 freeciv/server/srv_main.c 
freeciv-cleanup_handle_upgrade_unittype_request/server/srv_main.c
--- freeciv/server/srv_main.c   2002-12-20 16:04:58.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/srv_main.c   
2002-12-20 16:05:36.000000000 +0100
@@ -851,5 +851,5 @@
 
   case PACKET_UNITTYPE_UPGRADE:
-    handle_upgrade_unittype_request(pplayer, (struct packet_unittype_info 
*)packet);
+    handle_upgrade_unittype_request(pplayer, ((struct packet_unittype_info 
*)packet)->type);
     break;
 
diff -rXi -U2 freeciv/server/unithand.c 
freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.c
--- freeciv/server/unithand.c   2002-12-19 09:59:53.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.c   
2002-12-20 16:47:57.000000000 +0100
@@ -157,38 +157,45 @@
  Upgrade all units of a given type.
 **************************************************************************/
-void handle_upgrade_unittype_request(struct player *pplayer, 
-                                    struct packet_unittype_info *packet)
+void handle_upgrade_unittype_request(struct player * const pplayer,
+                                     const Unit_Type_id from_unittype)
 {
-  int cost;
-  int to_unit;
-  int upgraded = 0;
-  if ((to_unit =can_upgrade_unittype(pplayer, packet->type)) == -1) {
-    notify_player(pplayer, _("Game: Illegal packet, can't upgrade %s (yet)."), 
-                 unit_types[packet->type].name);
-    return;
-  }
-  cost = unit_upgrade_price(pplayer, packet->type, to_unit);
-  conn_list_do_buffer(&pplayer->connections);
-  unit_list_iterate(pplayer->units, punit) {
-    struct city *pcity;
-    if (cost > pplayer->economic.gold)
-      break;
-    pcity = map_get_city(punit->x, punit->y);
-    if (punit->type == packet->type && pcity && pcity->owner == 
pplayer->player_no) {
-      pplayer->economic.gold -= cost;
-      
-      upgrade_unit(punit, to_unit);
-      upgraded++;
-    }
-  }
-  unit_list_iterate_end;
-  conn_list_do_unbuffer(&pplayer->connections);
-  if (upgraded > 0) {
-    notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."), 
-                 upgraded, unit_types[packet->type].name, 
-                 unit_types[to_unit].name, cost * upgraded);
-    send_player_info(pplayer, pplayer);
+  const Unit_Type_id to_unittype =
+    can_upgrade_unittype(pplayer, from_unittype);
+
+  if (to_unittype == -1) {
+    notify_player(pplayer, _("Game: Illegal packet, can't upgrade %s (yet)."),
+                  unit_types[from_unittype].name);
   } else {
-    notify_player(pplayer, _("Game: No units could be upgraded."));
+    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;
+
+      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);
+
+          if (pcity && pcity->owner == player_no) {
+            upgrade_unit(punit, to_unittype);
+            ++number_of_upgraded_units;
+            if ((pplayer->economic.gold -= cost) < cost) {
+              break;
+            }
+          }
+        }
+      } unit_list_iterate_end;
+      conn_list_do_unbuffer(&pplayer->connections);
+    }
+    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."));
+    }
   }
 }
diff -rXi -U2 freeciv/server/unithand.h 
freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.h
--- freeciv/server/unithand.h   2002-12-18 19:17:18.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.h   
2002-12-20 16:10:40.000000000 +0100
@@ -23,6 +23,6 @@
 void handle_unit_connect(struct player *pplayer, 
                          struct packet_unit_connect *req);
-void handle_upgrade_unittype_request(struct player *pplayer, 
-                                    struct packet_unittype_info *packet);
+void handle_upgrade_unittype_request(struct player * const pplayer,
+                                     const Unit_Type_id from_unittype);
 void handle_unit_upgrade_request(struct player *pplayer,
                                 struct packet_unit_request *packet);