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

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

[jdorje - Thu Dec 19 18:58:36 2002]: 
> 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 changed this. New patch. 
 
 
> Another small style issue: I agree with Greg about the function 
> wrapping.  I would never use 
>  
> void handle_upgrade_unittype_request 
> (struct player * const pplayer, const Unit_Type_id from_unittype); 
>  
> (but this is trivial since it is easy for the patch committer to 
> change it.) 
 
I changed this too. But normally I would never use a declaration like 
 
void handle_upgrade_unittype_request(struct player * const pplayer, 
                                     const struct packet_unittype_info 
* const 
                                     packet) 
 
Refusing to linewrap between identifier and argument list only gets 
uglier the longer the function name is. And with calls (as opposed to 
declarations) it gets even worse, for example: 
 
/* indentation */ the_first_function(the_second_function(an_argument, 
                                                         another_arg, 
                                                         yet_another            
                                              
                                                         [index]), 
                                     last_argument_to_first_function); 
 
 
I consider this to be much nicer: 
 
/* indentation */ the_first_function 
                    (the_second_function 
                       (an_argument, another_arg, yet_another[index]), 
                     last_argument_to_first_function); 
 
 
The root of the problem is that when refusing to linewrap between 
identifier and argument list the column number increases with the 
lengt of the identifier for each nesting level instead of with a small 
constant amount. 

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-19 21:07:09.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)
-{
-  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);
+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);
+  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-19 21:05:36.000000000 +0100
@@ -23,6 +23,7 @@
 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 struct packet_unittype_info * const
+                                     packet);
 void handle_unit_upgrade_request(struct player *pplayer,
                                 struct packet_unit_request *packet);