[Freeciv-Dev] (PR#11572) building targets for is_req_active

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11572 >

Here is a patch that changes the target_building the effect and
requirement code to be a building pointer rather than an integer. 
Again, the advantage of this comes because all the targets are pointers
and there are a lot of them.  With lots of paremeters it's easy to get
them out of order, but with pointers this isn't likely since each
pointer has a different type.

There should be no measurable speed difference although a small number
of get_improvement_type() calls are done.

This is not an attempt to change all functions to use building pointers.
 Although such a thing would probably be a good idea, a general change
will require more discussion.

-jason

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.211
diff -u -r1.211 aicity.c
--- ai/aicity.c 12 Apr 2005 14:44:35 -0000      1.211
+++ ai/aicity.c 15 Apr 2005 06:47:00 -0000
@@ -271,8 +271,7 @@
     struct requirement *mypreq;
     bool useful;
 
-    if (is_effect_disabled(pplayer, pcity,
-                          id, NULL, peffect)) {
+    if (is_effect_disabled(pplayer, pcity, pimpr, NULL, peffect)) {
       CITY_LOG(LOG_DEBUG, pcity, "%s has a disabled effect: %s", 
                get_improvement_name(id), effect_type_name(peffect->type));
       continue;
@@ -288,7 +287,7 @@
        mypreq = preq;
         continue;
       }
-      if (!is_req_active(pplayer, pcity, id, NULL, preq)) {
+      if (!is_req_active(pplayer, pcity, pimpr, NULL, preq)) {
        useful = FALSE;
        break;
       }
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.327
diff -u -r1.327 city.c
--- common/city.c       14 Apr 2005 04:49:13 -0000      1.327
+++ common/city.c       15 Apr 2005 06:47:01 -0000
@@ -673,7 +673,7 @@
     return FALSE;
   }
 
-  return are_reqs_active(city_owner(pcity), pcity, B_LAST, NULL,
+  return are_reqs_active(city_owner(pcity), pcity, NULL, NULL,
                         game.rgame.specialists[type].req, MAX_NUM_REQS);
 }
 
@@ -1251,7 +1251,7 @@
 
   while ((replace = city_styles[prev].replaced_by) != -1) {
     prev = replace;
-    if (are_reqs_active(plr, NULL, B_LAST, NULL,
+    if (are_reqs_active(plr, NULL, NULL, NULL,
                        city_styles[replace].req, MAX_NUM_REQS)) {
       style = replace;
     }
Index: common/effects.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/effects.c,v
retrieving revision 1.29
diff -u -r1.29 effects.c
--- common/effects.c    14 Apr 2005 04:49:13 -0000      1.29
+++ common/effects.c    15 Apr 2005 06:47:01 -0000
@@ -554,7 +554,7 @@
 **************************************************************************/
 bool is_effect_disabled(const struct player *target_player,
                        const struct city *target_city,
-                       Impr_Type_id target_building,
+                       const struct impr_type *target_building,
                        const struct tile *target_tile,
                        const struct effect *peffect)
 {
@@ -574,7 +574,7 @@
 **************************************************************************/
 static bool is_effect_enabled(const struct player *target_player,
                              const struct city *target_city,
-                             Impr_Type_id target_building,
+                             const struct impr_type *target_building,
                              const struct tile *target_tile,
                              const struct effect *peffect)
 {
@@ -596,14 +596,16 @@
   (player,city,building,tile) give the exact target
   peffect gives the exact effect value
 **************************************************************************/
-static bool is_effect_active(const struct player *plr,
-                            const struct city *pcity,
-                            Impr_Type_id building,
-                            const struct tile *ptile,
+static bool is_effect_active(const struct player *target_player,
+                            const struct city *target_city,
+                            const struct impr_type *target_building,
+                            const struct tile *target_tile,
                             const struct effect *peffect)
 {
-  return is_effect_enabled(plr, pcity, building, ptile, peffect)
-    && !is_effect_disabled(plr, pcity, building, ptile, peffect);
+  return is_effect_enabled(target_player, target_city, target_building,
+                          target_tile, peffect)
+    && !is_effect_disabled(target_player, target_city, target_building,
+                          target_tile, peffect);
 }
 
 /**************************************************************************
@@ -620,7 +622,7 @@
 **************************************************************************/
 bool is_effect_useful(const struct player *target_player,
                      const struct city *target_city,
-                     Impr_Type_id target_building,
+                     const struct impr_type *target_building,
                      const struct tile *target_tile,
                      Impr_Type_id source, const struct effect *peffect)
 {
@@ -665,7 +667,7 @@
      * the building is its own target - but whether this is actually
      * checked depends on the range of the effect. */
     if (!is_effect_disabled(city_owner(pcity), pcity,
-                           building, NULL, peffect)) {
+                           get_improvement_type(building), NULL, peffect)) {
       return FALSE;
     }
   } effect_list_iterate_end;
@@ -687,7 +689,7 @@
 static int get_target_bonus_effects(struct effect_list *plist,
                                    const struct player *target_player,
                                    const struct city *target_city,
-                                   Impr_Type_id target_building,
+                                   const struct impr_type *target_building,
                                    const struct tile *target_tile,
                                    enum effect_type effect_type)
 {
@@ -716,7 +718,7 @@
 int get_world_bonus(enum effect_type effect_type)
 {
   return get_target_bonus_effects(NULL,
-                                 NULL, NULL, B_LAST, NULL, effect_type);
+                                 NULL, NULL, NULL, NULL, effect_type);
 }
 
 /**************************************************************************
@@ -727,7 +729,7 @@
 {
   assert(pplayer != NULL);
   return get_target_bonus_effects(NULL,
-                                 pplayer, NULL, B_LAST, NULL,
+                                 pplayer, NULL, NULL, NULL,
                                  effect_type);
 }
 
@@ -738,7 +740,7 @@
 {
   assert(pcity != NULL);
   return get_target_bonus_effects(NULL,
-                                 city_owner(pcity), pcity, B_LAST, NULL,
+                                 city_owner(pcity), pcity, NULL, NULL,
                                  effect_type);
 }
 
@@ -750,7 +752,7 @@
 {
   assert(pcity != NULL && ptile != NULL);
   return get_target_bonus_effects(NULL,
-                                 city_owner(pcity), pcity, B_LAST, ptile,
+                                 city_owner(pcity), pcity, NULL, ptile,
                                  effect_type);
 }
 
@@ -762,7 +764,8 @@
 {
   assert(pcity != NULL && id != B_LAST);
   return get_target_bonus_effects(NULL,
-                                 city_owner(pcity), pcity, id, NULL,
+                                 city_owner(pcity), pcity,
+                                 get_improvement_type(id), NULL,
                                  effect_type);
 }
 
@@ -778,7 +781,7 @@
 {
   assert(pplayer != NULL);
   return get_target_bonus_effects(plist,
-                                 pplayer, NULL, B_LAST, NULL,
+                                 pplayer, NULL, NULL, NULL,
                                  effect_type);
 }
 
@@ -794,7 +797,7 @@
 {
   assert(pcity != NULL);
   return get_target_bonus_effects(plist,
-                                 city_owner(pcity), pcity, B_LAST, NULL,
+                                 city_owner(pcity), pcity, NULL, NULL,
                                  effect_type);
 }
 
@@ -818,12 +821,14 @@
     struct effect_list *plist = get_req_source_effects(&source);
 
     if (plist) {
+      struct impr_type *building = get_improvement_type(id);
+
       effect_list_iterate(plist, peffect) {
        if (peffect->type != effect_type) {
          continue;
        }
        if (is_effect_useful(city_owner(pcity),
-                            pcity, id, NULL, id, peffect)) {
+                            pcity, building, NULL, id, peffect)) {
          power += peffect->value;
        }
       } effect_list_iterate_end;
Index: common/effects.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/effects.h,v
retrieving revision 1.17
diff -u -r1.17 effects.h
--- common/effects.h    10 Apr 2005 00:50:37 -0000      1.17
+++ common/effects.h    15 Apr 2005 06:47:01 -0000
@@ -159,7 +159,7 @@
 
 bool is_effect_useful(const struct player *target_player,
                      const struct city *target_pcity,
-                     Impr_Type_id target_building,
+                     const struct impr_type *target_building,
                      const struct tile *target_tile,
                      Impr_Type_id source, const struct effect *effect);
 
@@ -178,7 +178,7 @@
 struct effect_list *get_req_source_effects(struct req_source *psource);
 bool is_effect_disabled(const struct player *target_player,
                        const struct city *target_city,
-                       Impr_Type_id target_building,
+                       const struct impr_type *target_building,
                        const struct tile *target_tile,
                        const struct effect *peffect);
 
Index: common/fc_types.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/fc_types.h,v
retrieving revision 1.18
diff -u -r1.18 fc_types.h
--- common/fc_types.h   11 Apr 2005 22:42:06 -0000      1.18
+++ common/fc_types.h   15 Apr 2005 06:47:01 -0000
@@ -55,6 +55,7 @@
 struct player;
 struct tile;
 struct unit;
+struct impr_type;
 
 #define SP_MAX 20
 #define MAX_NUM_REQS 2
Index: common/government.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/government.c,v
retrieving revision 1.51
diff -u -r1.51 government.c
--- common/government.c 3 Apr 2005 20:19:52 -0000       1.51
+++ common/government.c 15 Apr 2005 06:47:01 -0000
@@ -205,7 +205,7 @@
     return TRUE;
   }
 
-  return are_reqs_active(pplayer, NULL, B_LAST, NULL,
+  return are_reqs_active(pplayer, NULL, NULL, NULL,
                         gov->req, MAX_NUM_REQS);
 }
 
Index: common/requirements.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/requirements.c,v
retrieving revision 1.12
diff -u -r1.12 requirements.c
--- common/requirements.c       14 Apr 2005 04:49:13 -0000      1.12
+++ common/requirements.c       15 Apr 2005 06:47:01 -0000
@@ -438,11 +438,11 @@
   the number of available sources.  However not all source caches exist: if
   the cache doesn't exist then we return 0.
 ****************************************************************************/
-int count_buildings_in_range(const struct player *target_player,
-                            const struct city *target_city,
-                            Impr_Type_id target_building,
-                            enum req_range range, bool survives,
-                            Impr_Type_id source)
+static int count_buildings_in_range(const struct player *target_player,
+                                   const struct city *target_city,
+                                   const struct impr_type *target_building,
+                                   enum req_range range, bool survives,
+                                   Impr_Type_id source)
 {
   if (improvement_obsolete(target_player, source)) {
     return 0;
@@ -477,7 +477,7 @@
   case REQ_RANGE_CITY:
     return target_city ? num_city_buildings(target_city, source) : 0;
   case REQ_RANGE_LOCAL:
-    if (target_building != B_LAST && target_building == source) {
+    if (target_building && target_building->index == source) {
       return num_city_buildings(target_city, source);
     } else {
       /* TODO: other local targets */
@@ -619,7 +619,7 @@
 ****************************************************************************/
 bool is_req_active(const struct player *target_player,
                   const struct city *target_city,
-                  Impr_Type_id target_building,
+                  const struct impr_type *target_building,
                   const struct tile *target_tile,
                   const struct requirement *req)
 {
@@ -681,7 +681,7 @@
 ****************************************************************************/
 bool are_reqs_active(const struct player *target_player,
                     const struct city *target_city,
-                    Impr_Type_id target_building,
+                    const struct impr_type *target_building,
                     const struct tile *target_tile,
                     const struct requirement *reqs, int num_reqs)
 {
Index: common/requirements.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/requirements.h,v
retrieving revision 1.9
diff -u -r1.9 requirements.h
--- common/requirements.h       14 Apr 2005 04:49:14 -0000      1.9
+++ common/requirements.h       15 Apr 2005 06:47:01 -0000
@@ -96,21 +96,15 @@
 
 bool is_req_active(const struct player *target_player,
                   const struct city *target_city,
-                  Impr_Type_id target_building,
+                  const struct impr_type *target_building,
                   const struct tile *target_tile,
                   const struct requirement *req);
 bool are_reqs_active(const struct player *target_player,
                     const struct city *target_city,
-                    Impr_Type_id target_building,
+                    const struct impr_type *target_building,
                     const struct tile *target_tile,
                     const struct requirement *reqs, int num_reqs);
 
-int count_buildings_in_range(const struct player *target_player,
-                            const struct city *target_city,
-                            Impr_Type_id target_building,
-                            enum req_range range, bool survives,
-                            Impr_Type_id source);
-
 /* Req-source helper functions. */
 bool are_req_sources_equal(const struct req_source *psource1,
                           const struct req_source *psource2);