[Freeciv-Dev] (PR#4176) FIX is_stack_vulnerable

["Olivier DAVY" <olivier.davy@xxxxxxx>]

Olivier DAVY
FRANCE

Here is a patch that cleans some code about is_stack_vulnerable, according to 
Gregory ideas.
As it is the second time I write a patch, fell free to send me comments about 
it.

Olive

Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx> : 
""""""""
I would recommend the following patch: do use is_stack_vulnerable like you 
did in your patch.  Move is_stack_vulnerable all the way to common/ (I 
think combat.c is the best place).  Use is_stack_vulnerable in the server 
as well (there is one place somewhere in unittools or unithand, do a 
grep).  And make no other changes in this patch.  Such a compact patch 
with clear-cut scope will be reviewed and commited much faster.

Best wishes,
G.
""""""""""

---------------------------------------
 Olivier DAVY
 ENSIMAG engineer - HEC alumnus
 E-mail : olivier.davy@xxxxxxx
 Phone  : +33/(0)1.42.67.19.85
---------------------------------------

diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/ai/advmilitary.c 
freeciv/ai/advmilitary.c
--- freeciv-cvs-May-01/ai/advmilitary.c 2003-04-30 06:58:07.000000000 +0200
+++ freeciv/ai/advmilitary.c    2003-05-05 23:39:41.000000000 +0200
@@ -745,9 +745,9 @@
     needferry = unit_types[boattype].build_cost;
   }
   
-  if (acity) {
-    /* If it is a city, we may have to whack it many times */
-    /* FIXME: Also valid for fortresses! */
+  if (!is_stack_vulnerable(x,y)) {
+    /* If it is a city, a fortress or an air base,
+     * we may have to whack it many times */
     victim_count += unit_list_size(&(map_get_tile(x, y)->units));
   }
 
diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/ai/aitools.c 
freeciv/ai/aitools.c
--- freeciv-cvs-May-01/ai/aitools.c     2003-04-21 06:56:58.000000000 +0200
+++ freeciv/ai/aitools.c        2003-05-05 23:42:47.000000000 +0200
@@ -469,16 +469,6 @@
   return(pc);
 }
 
-/**************************************************************************
-  Is it a city/fortress or will the whole stack die in an attack
-  TODO: use new killstack thing
-**************************************************************************/
-static bool is_stack_vulnerable(int x, int y)
-{
-  return !(map_get_city(x, y) != NULL ||
-          map_has_special(x, y, S_FORTRESS) ||
-          map_has_special(x, y, S_AIRBASE) );
-}
 
 /**************************************************************************
   Calculate the value of the target unit including the other units which
diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/common/combat.c 
freeciv/common/combat.c
--- freeciv-cvs-May-01/common/combat.c  2003-01-09 06:55:56.000000000 +0100
+++ freeciv/common/combat.c     2003-05-06 00:07:35.000000000 +0200
@@ -545,3 +545,14 @@
 
   return bestatt;
 }
+
+/**************************************************************************
+  Is it a city/fortress/air base or will the whole stack die in an attack
+  TODO: use new killstack thing
+**************************************************************************/
+bool is_stack_vulnerable(int x, int y)
+{
+  return !(map_get_city(x, y) != NULL ||
+          map_has_special(x, y, S_FORTRESS) ||
+          map_has_special(x, y, S_AIRBASE) );
+}
diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/common/combat.h 
freeciv/common/combat.h
--- freeciv-cvs-May-01/common/combat.h  2002-12-19 06:55:11.000000000 +0100
+++ freeciv/common/combat.h     2003-05-05 23:45:31.000000000 +0200
@@ -52,4 +52,5 @@
 struct unit *get_defender(struct unit *attacker, int x, int y);
 struct unit *get_attacker(struct unit *defender, int x, int y);
 
+bool is_stack_vulnerable(int x, int y);
 #endif /* FC__COMBAT_H */
diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/server/unithand.c 
freeciv/server/unithand.c
--- freeciv-cvs-May-01/server/unithand.c        2003-04-05 07:56:53.000000000 
+0200
+++ freeciv/server/unithand.c   2003-05-06 00:01:35.000000000 +0200
@@ -840,8 +840,9 @@
   }
 
   /* If attacker wins, and occupychance > 0, it might move in.  Don't move in
-     if there are enemy units in the tile (a fortress or city with multiple
-     defenders and unstacked combat).  Note that this could mean capturing (or
+     if there are enemy units in the tile (a fortress or city or an air base
+     with multiple defenders and unstacked combat).
+     Note that this could mean capturing (or
      destroying) a city. -GJW */
 
   if (pwinner == punit && myrand(100) < game.occupychance &&
diff -ruN -X freeciv/diff_ignore freeciv-cvs-May-01/server/unittools.c 
freeciv/server/unittools.c
--- freeciv-cvs-May-01/server/unittools.c       2003-04-18 06:57:06.000000000 
+0200
+++ freeciv/server/unittools.c  2003-05-06 00:16:31.000000000 +0200
@@ -1800,7 +1800,6 @@
 **************************************************************************/
 void kill_unit(struct unit *pkiller, struct unit *punit)
 {
-  struct city   *incity    = map_get_city(punit->x, punit->y);
   struct player *pplayer   = unit_owner(punit);
   struct player *destroyer = unit_owner(pkiller);
   char *loc_str = get_location_str_in(pplayer, punit->x, punit->y);
@@ -1828,10 +1827,7 @@
     unit_list_iterate_end;
   }
 
-  if( (incity) ||
-      map_has_special(punit->x, punit->y, S_FORTRESS) ||
-      map_has_special(punit->x, punit->y, S_AIRBASE) ||
-      unitcount == 1) {
+  if (!is_stack_vulnerable(punit->x,punit->y) || unitcount == 1) {
     notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_LOST,
                     _("Game: %s lost to an attack by %s's %s%s."),
                     unit_type(punit)->name, destroyer->name,