[Freeciv-Dev] Re: (PR#6408) Remove unused parameter to wipe_unit_safe*

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Mon, 6 Oct 2003, Jason Short wrote:
> There are a lot of functions in the server for removing units:
> wipe_unit, wipe_unit_safe, wipe_unit_spec_safe, server_remove_unit().
> Can we rewrite these to just be one (safe) function, possibly with a
> (local) helper function?

We can at least remove one of them. Done in this expanded version of the
patch. Three unnecessary functions removed.

  - Per

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.145
diff -u -r1.145 aicity.c
--- ai/aicity.c 22 Sep 2003 14:18:46 -0000      1.145
+++ ai/aicity.c 6 Oct 2003 15:12:59 -0000
@@ -696,8 +696,7 @@
 
       UNIT_LOG(LOG_EMERGENCY, punit, "is causing unrest, disbanded");
       pack.unit_id = punit->id;
-      /* in rare cases the _safe might be needed? --dwp */
-      handle_unit_disband_safe(pplayer, &pack, &myiter);
+      handle_unit_disband(pplayer, &pack);
       city_refresh(pcity);
     }
   } unit_list_iterate_end;
Index: server/barbarian.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/barbarian.c,v
retrieving revision 1.69
diff -u -r1.69 barbarian.c
--- server/barbarian.c  2 Oct 2003 17:54:58 -0000       1.69
+++ server/barbarian.c  6 Oct 2003 15:13:00 -0000
@@ -196,7 +196,7 @@
 
   if (game.barbarianrate == 0 || (game.year < game.onsetbarbarian)) {
     unit_list_iterate(map_get_tile(x, y)->units, punit) {
-      wipe_unit_safe(punit, &myiter);
+      wipe_unit(punit);
     } unit_list_iterate_end;
     return FALSE;
   }
@@ -258,7 +258,7 @@
     } else {             /* The village is surrounded! Kill the explorer. */
       unit_list_iterate(map_get_tile(x, y)->units, punit2) {
         if (punit2->owner != me) {
-          wipe_unit_safe(punit2, &myiter);
+          wipe_unit(punit2);
           alive = FALSE;
         } else {
           send_unit_info(NULL, punit2);
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.235
diff -u -r1.235 citytools.c
--- server/citytools.c  27 Sep 2003 11:27:01 -0000      1.235
+++ server/citytools.c  6 Oct 2003 15:13:00 -0000
@@ -642,7 +642,7 @@
       /* Don't transfer units already owned by new city-owner --wegge */ 
       if (unit_owner(vunit) == pvictim) {
        transfer_unit(vunit, pcity, verbose);
-       wipe_unit_safe(vunit, &myiter);
+       wipe_unit(vunit);
        unit_list_unlink(units, vunit);
       } else if (!pplayers_allied(pplayer, unit_owner(vunit))) {
         /* the owner of vunit is allied to pvictim but not to pplayer */
@@ -1109,7 +1109,7 @@
                       punit->moves_left, punit->hp);
     }
 
-    wipe_unit_safe(punit, &myiter);
+    wipe_unit(punit);
   } unit_list_iterate_end;
 
   x = pcity->x;
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.226
diff -u -r1.226 cityturn.c
--- server/cityturn.c   1 Oct 2003 14:48:54 -0000       1.226
+++ server/cityturn.c   6 Oct 2003 15:13:00 -0000
@@ -536,7 +536,7 @@
       if (unit_type(punit)->food_cost > 0 
           && !unit_flag(punit, F_UNDISBANDABLE)) {
        char *utname = unit_type(punit)->name;
-       wipe_unit_safe(punit, &myiter);
+       wipe_unit(punit);
        notify_player_ex(city_owner(pcity), pcity->x, pcity->y, E_UNIT_LOST,
                         _("Game: Famine feared in %s, %s lost!"), 
                         pcity->name, utname);
Index: server/diplomats.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/diplomats.c,v
retrieving revision 1.42
diff -u -r1.42 diplomats.c
--- server/diplomats.c  9 Sep 2003 20:10:28 -0000       1.42
+++ server/diplomats.c  6 Oct 2003 15:13:00 -0000
@@ -1126,7 +1126,7 @@
                         _("Game: An enemy %s has been eliminated defending"
                           " %s."), unit_name(punit->type), pcity->name);
 
-       wipe_unit_safe(punit, &myiter);
+       wipe_unit(punit);
         pdiplomat->moves_left = MAX(0, pdiplomat->moves_left - SINGLE_MOVE);
         return FALSE;
       } else {
@@ -1143,7 +1143,7 @@
                         get_nation_name(pplayer->nation),
                         unit_name(pdiplomat->type), pcity->name);
 
-       wipe_unit_safe(pdiplomat, &myiter);
+       wipe_unit(pdiplomat);
        return FALSE;
       }
     }
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.274
diff -u -r1.274 unithand.c
--- server/unithand.c   22 Sep 2003 16:54:09 -0000      1.274
+++ server/unithand.c   6 Oct 2003 15:13:01 -0000
@@ -395,11 +395,18 @@
 /**************************************************************************
   Disband a unit.  If its in a city, add 1/2 of the worth of the unit
   to the city's shield stock for the current production.
-  "iter" may be NULL, see wipe_unit_safe for details.
 **************************************************************************/
-static void do_unit_disband_safe(struct city *pcity, struct unit *punit,
-                                struct genlist_iterator *iter)
+void handle_unit_disband(struct player *pplayer, 
+                        struct packet_unit_request *req)
 {
+  struct unit *punit = player_find_unit_by_id(pplayer, req->unit_id);
+  struct city *pcity;
+
+  if (!punit) {
+    return;
+  }
+  pcity = map_get_city(punit->x, punit->y);
+
   if (!unit_flag(punit, F_UNDISBANDABLE)) { /* refuse to kill ourselves */
     if (pcity) {
       pcity->shield_stock += (unit_type(punit)->build_cost/2);
@@ -411,37 +418,12 @@
        * That's why we must use city_owner instead of pplayer -- Zamar */
       send_city_info(city_owner(pcity), pcity);
     }
-    wipe_unit_safe(punit, iter);
+    wipe_unit(punit);
   } else {
     notify_player_ex(unit_owner(punit), punit->x, punit->y, E_NOEVENT,
               _("Game: %s refuses to disband!"), unit_name(punit->type));
     return;
   }
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-void handle_unit_disband_safe(struct player *pplayer, 
-                             struct packet_unit_request *req,
-                             struct genlist_iterator *iter)
-{
-  struct unit *punit = player_find_unit_by_id(pplayer, req->unit_id);
-
-  if (!punit) {
-    return;
-  }
-
-  do_unit_disband_safe(map_get_city(punit->x, punit->y), punit, iter);
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-void handle_unit_disband(struct player *pplayer, 
-                        struct packet_unit_request *req)
-{
-  handle_unit_disband_safe(pplayer, req, NULL);
 }
 
 /**************************************************************************
Index: server/unithand.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.h,v
retrieving revision 1.34
diff -u -r1.34 unithand.h
--- server/unithand.h   2 Jan 2003 03:02:16 -0000       1.34
+++ server/unithand.h   6 Oct 2003 15:13:01 -0000
@@ -32,9 +32,6 @@
                                 struct packet_unit_request *req);
 void handle_unit_disband(struct player *pplayer, 
                         struct packet_unit_request *req);
-void handle_unit_disband_safe(struct player *pplayer, 
-                             struct packet_unit_request *req,
-                             struct genlist_iterator *iter);
 void handle_unit_build_city(struct player *pplayer, 
                            struct packet_unit_request *req);
 void handle_unit_info(struct player *pplayer, struct packet_unit_info *pinfo);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.256
diff -u -r1.256 unittools.c
--- server/unittools.c  29 Sep 2003 21:47:58 -0000      1.256
+++ server/unittools.c  6 Oct 2003 15:13:02 -0000
@@ -304,7 +304,7 @@
       notify_player_ex(pplayer, -1, -1, E_UNIT_LOST,
                       _("Not enough gold. %s disbanded"),
                       unit_type(punit)->name);
-      wipe_unit_safe(punit, &myiter);
+      wipe_unit(punit);
     } else {
       /* Gold can get negative here as city improvements will be sold
        * afterwards to balance our budget. FIXME: Should units with gold 
@@ -432,7 +432,7 @@
       gamelog(GAMELOG_UNITF, _("%s lose a %s (out of hp)"),
              get_nation_name_plural(pplayer->nation),
              unit_name(punit->type));
-      wipe_unit_safe(punit, &myiter);
+      wipe_unit(punit);
       continue; /* Continue iterating... */
     }
 
@@ -444,7 +444,7 @@
                       unit_name(punit->type));
       gamelog(GAMELOG_UNITTRI, _("%s Trireme lost at sea"),
              get_nation_name_plural(pplayer->nation));
-      wipe_unit_safe(punit, &myiter);
+      wipe_unit(punit);
       continue; /* Continue iterating... */
     }
 
@@ -500,7 +500,7 @@
       gamelog(GAMELOG_UNITF, _("%s lose a %s (fuel)"),
              get_nation_name_plural(pplayer->nation),
              unit_name(punit->type));
-      wipe_unit_safe(punit, &myiter);
+      wipe_unit(punit);
     } 
   } unit_list_iterate_end;
 }
@@ -976,7 +976,7 @@
                         _("Game: Disbanded your %s due to changing"
                           " land to sea at (%d, %d)."),
                         unit_name(punit2->type), punit2->x, punit2->y);
-       wipe_unit_spec_safe(punit2, NULL, FALSE);
+       wipe_unit_spec_safe(punit2, FALSE);
        goto START;
       }
     } unit_list_iterate_end;
@@ -1038,7 +1038,7 @@
                         _("Game: Disbanded your %s due to changing"
                           " sea to land at (%d, %d)."),
                         unit_name(punit2->type), punit2->x, punit2->y);
-       wipe_unit_spec_safe(punit2, NULL, FALSE);
+       wipe_unit_spec_safe(punit2, FALSE);
        goto START;
       }
     } unit_list_iterate_end;
@@ -1627,17 +1627,10 @@
 }
 
 /**************************************************************************
-this is a highlevel routine
-Remove the unit, and passengers if it is a carrying any.
-Remove the _minimum_ number, eg there could be another boat on the square.
-Parameter iter, if non-NULL, should be an iterator for a unit list,
-and if it points to a unit which we wipe, we advance it first to
-avoid dangling pointers.
-NOTE: iter should not be an iterator for the map units list, but
-city supported, or player units, is ok.
+  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_spec_safe(struct unit *punit, struct genlist_iterator *iter,
-                        bool wipe_cargo)
+void wipe_unit_spec_safe(struct unit *punit, bool wipe_cargo)
 {
   int x = punit->x;
   int y = punit->y;
@@ -1733,17 +1726,9 @@
 /**************************************************************************
 ...
 **************************************************************************/
-
-void wipe_unit_safe(struct unit *punit, struct genlist_iterator *iter){
-  wipe_unit_spec_safe(punit, iter, TRUE);
-}
-
-/**************************************************************************
-...
-**************************************************************************/
 void wipe_unit(struct unit *punit)
 {
-  wipe_unit_safe(punit, NULL);
+  wipe_unit_spec_safe(punit, TRUE);
 }
 
 /**************************************************************************
@@ -1833,7 +1818,7 @@
                get_nation_name_plural(unit_owner(punit2)->nation),
                unit_type(punit2)->name,
                get_nation_name_plural(destroyer->nation));
-       wipe_unit_spec_safe(punit2, NULL, FALSE);
+       wipe_unit_spec_safe(punit2, FALSE);
       }
     }
     unit_list_iterate_end;
@@ -2059,7 +2044,7 @@
                       unit_owner(punit)->name,
                       unit_name(punit->type));
     }
-    wipe_unit_spec_safe(punit, NULL, FALSE);
+    wipe_unit_spec_safe(punit, FALSE);
   } unit_list_iterate_end;
 
   if (pcity) {
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.56
diff -u -r1.56 unittools.h
--- server/unittools.h  22 Sep 2003 16:54:09 -0000      1.56
+++ server/unittools.h  6 Oct 2003 15:13:02 -0000
@@ -58,9 +58,7 @@
                              Unit_Type_id type, bool make_veteran, int 
homecity_id,
                              int moves_left, int hp_left);
 void wipe_unit(struct unit *punit);
-void wipe_unit_safe(struct unit *punit, struct genlist_iterator *iter);
-void wipe_unit_spec_safe(struct unit *punit, struct genlist_iterator *iter,
-                        bool wipe_cargo);
+void wipe_unit_spec_safe(struct unit *punit, bool wipe_cargo);
 void kill_unit(struct unit *pkiller, struct unit *punit);
 
 /* sending to client */