[Freeciv-Dev] (PR#9900) RFC: can_build functions

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=9900 >

Currently there are functions 
[can|could]_(player_)(eventually_)build_[improvement|unit](_direct).

I want to agree on a single naming system for these and impose it across 
all functions.

   city versus player => can_build_xxx versus can_player_build_xxx
   unit versus improvement => can_xxx_build_improvement versus
                              can_player_build_improvement
   now versus direct versus eventually =>
     can_xxx_build_yyy
     can_xxx_build_yyy_direct
     can_xxx_build_yyy_eventually

Note the different uses of these functions.

   now => can we build it...
   direct => this ignores obsoletion and replacement.  Could
     also be called "forced".  Useful in calculating upgrades,
     etc.
   eventually => ignores tech requirements, allowing future
     targets to be shown.  Useful for worklists.

   player => can the player build the target, in general.
     Useful for global worklists and clipboards, calculation
     of upgrades, etc.
   city => can it be built by a specific city.  This
     obviously checks terrain and building requirements.  It
     is the default so can_build_unit means at city range.

   unit => for units, obviously
   improvement => for buildings.  Should perhaps be renamed "building".

The unit functions are pretty consistently named.  But the improvement 
functions are not, they use "could" in some places and there is no 
"direct" variant (not needed currently, but it will be under the effects 
patch).  Also there is an inconsistency in general in that "eventually" 
it put into the middle of the name while "direct" is put onto the end.

I want to make all this consistent.  2 * 3 * 2 = 12 functions.  "direct" 
or "eventually" are tacked onto the end.  "could" is never used, it's 
always "can".

This patch makes the changes for the improvement functions.  There are 6 
of them, although the "direct" ones are not used (yet).  This patch also 
removes some spurious checks (sometimes improvement_exists was called 
more than once).

A second patch will be needed to rename the unit functions only.

Renaming "direct" as "forced" would also be fine with me.  Renaming 
"improvement" as "building" would also be fine with me.  I would like to 
keep the "eventually" at the end though even though it creates some 
noise in the patch.

jason

Index: client/climisc.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/climisc.c,v
retrieving revision 1.137
diff -u -r1.137 climisc.c
--- client/climisc.c    1 Sep 2004 03:16:47 -0000       1.137
+++ client/climisc.c    1 Sep 2004 20:11:36 -0000
@@ -713,13 +713,13 @@
   impr_type_iterate(id) {
     bool can_build = can_player_build_improvement(game.player_ptr, id);
     bool can_eventually_build =
-       could_player_eventually_build_improvement(game.player_ptr, id);
+       can_player_build_improvement_eventually(game.player_ptr, id);
 
     /* If there's a city, can the city build the improvement? */
     if (pcity) {
       can_build = can_build && can_build_improvement(pcity, id);
       can_eventually_build = can_eventually_build &&
-         can_eventually_build_improvement(pcity, id);
+         can_build_improvement_eventually(pcity, id);
     }
 
     if ((advanced_tech && can_eventually_build) ||
Index: client/gui-gtk/wldlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/wldlg.c,v
retrieving revision 1.36
diff -u -r1.36 wldlg.c
--- client/gui-gtk/wldlg.c      8 Apr 2004 18:19:09 -0000       1.36
+++ client/gui-gtk/wldlg.c      1 Sep 2004 20:11:36 -0000
@@ -1064,7 +1064,7 @@
   if (peditor->pcity &&
       ((is_unit && !can_eventually_build_unit(peditor->pcity, target)) ||
        (!is_unit
-       && !can_eventually_build_improvement(peditor->pcity, target)))) {
+       && !can_build_improvement_eventually(peditor->pcity, target)))) {
     /* Nope, this city can't build this target, ever.  Don't put it into
        the worklist. */
     return;
Index: client/gui-mui/worklistclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/worklistclass.c,v
retrieving revision 1.26
diff -u -r1.26 worklistclass.c
--- client/gui-mui/worklistclass.c      25 Feb 2004 20:09:51 -0000      1.26
+++ client/gui-mui/worklistclass.c      1 Sep 2004 20:11:37 -0000
@@ -487,13 +487,13 @@
   impr_type_iterate(i) {
     /* Can the player (eventually) build this improvement? */
     can_build = can_player_build_improvement(pplr,i);
-    can_eventually_build = could_player_eventually_build_improvement(pplr,i);
+    can_eventually_build = can_player_build_improvement_eventually(pplr,i);
 
     /* If there's a city, can the city build the improvement? */
     if (data->pcity) {
       can_build = can_build && can_build_improvement(data->pcity, i);
       can_eventually_build = can_eventually_build &&
-       can_eventually_build_improvement(data->pcity, i);
+       can_build_improvement_eventually(data->pcity, i);
     }
     
     if (( advanced_tech && can_eventually_build) ||
@@ -519,7 +519,7 @@
     if (data->pcity) {
       can_build = can_build && can_build_improvement(data->pcity, i);
       can_eventually_build = can_eventually_build &&
-       can_eventually_build_improvement(data->pcity, i);
+       can_build_improvement_eventually(data->pcity, i);
     }
 
     if (( advanced_tech && can_eventually_build) ||
Index: client/gui-sdl/wldlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-sdl/wldlg.c,v
retrieving revision 1.14
diff -u -r1.14 wldlg.c
--- client/gui-sdl/wldlg.c      25 Feb 2004 20:09:51 -0000      1.14
+++ client/gui-sdl/wldlg.c      1 Sep 2004 20:11:37 -0000
@@ -737,7 +737,7 @@
       if(((pWorkList->wlefs[count] == WEF_UNIT) &&
          !can_eventually_build_unit(pEditor->pCity, pWorkList->wlids[count])) 
||
        ((pWorkList->wlefs[count] == WEF_IMPR) &&
-         !can_eventually_build_improvement(pEditor->pCity, 
pWorkList->wlids[count]))) {
+         !can_build_improvement_eventually(pEditor->pCity, 
pWorkList->wlids[count]))) {
        continue;  
       }
       
@@ -823,7 +823,7 @@
       if(((pWorkList->wlefs[count] == WEF_UNIT) &&
          !can_eventually_build_unit(pEditor->pCity, pWorkList->wlids[count])) 
||
        ((pWorkList->wlefs[count] == WEF_IMPR) &&
-         !can_eventually_build_improvement(pEditor->pCity, 
pWorkList->wlids[count]))) {
+         !can_build_improvement_eventually(pEditor->pCity, 
pWorkList->wlids[count]))) {
        continue;  
       }
       
@@ -1393,13 +1393,13 @@
   impr_type_iterate(imp) {
     can_build = can_player_build_improvement(game.player_ptr, imp);
     can_eventually_build =
-       could_player_eventually_build_improvement(game.player_ptr, imp);
+       can_player_build_improvement_eventually(game.player_ptr, imp);
     
     /* If there's a city, can the city build the improvement? */
     if (pCity) {
       can_build = can_build && can_build_improvement(pCity, imp);
       can_eventually_build = can_eventually_build &&
-         can_eventually_build_improvement(pCity, imp);
+         can_build_improvement_eventually(pCity, imp);
     }
     
     if ((advanced_tech && can_eventually_build) ||
Index: client/gui-win32/wldlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/wldlg.c,v
retrieving revision 1.16
diff -u -r1.16 wldlg.c
--- client/gui-win32/wldlg.c    14 Nov 2002 09:14:59 -0000      1.16
+++ client/gui-win32/wldlg.c    1 Sep 2004 20:11:37 -0000
@@ -785,7 +785,7 @@
   if (peditor->pcity &&
       ((is_unit && !can_eventually_build_unit(peditor->pcity, target)) ||
        (!is_unit
-        && !can_eventually_build_improvement(peditor->pcity, target)))) {
+        && !can_build_improvement_eventually(peditor->pcity, target)))) {
     /* Nope, this city can't build this target, ever.  Don't put it into
        the worklist. */
     return;
Index: client/gui-xaw/wldlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/wldlg.c,v
retrieving revision 1.24
diff -u -r1.24 wldlg.c
--- client/gui-xaw/wldlg.c      25 Feb 2004 20:09:52 -0000      1.24
+++ client/gui-xaw/wldlg.c      1 Sep 2004 20:11:38 -0000
@@ -837,7 +837,7 @@
      worklist may try to insert an odd-ball unit or target. */
   if (pdialog->pcity &&
       ((is_unit  && !can_eventually_build_unit(pdialog->pcity, target)) ||
-       (!is_unit && !can_eventually_build_improvement(pdialog->pcity, 
target))))
+       (!is_unit && !can_build_improvement_eventually(pdialog->pcity, 
target))))
     /* Nope, this city can't build this target, ever.  Don't put it into
        the worklist. */
     return;
@@ -1303,13 +1303,13 @@
   impr_type_iterate(i) {
     /* Can the player (eventually) build this improvement? */
     can_build = can_player_build_improvement(pplr,i);
-    can_eventually_build = could_player_eventually_build_improvement(pplr,i);
+    can_eventually_build = can_player_build_improvement_eventually(pplr,i);
 
     /* If there's a city, can the city build the improvement? */
     if (pdialog->pcity) {
       can_build = can_build && can_build_improvement(pdialog->pcity, i);
       can_eventually_build = can_eventually_build &&
-       can_eventually_build_improvement(pdialog->pcity, i);
+       can_build_improvement_eventually(pdialog->pcity, i);
     }
     
     if (( advanced_tech && can_eventually_build) ||
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.240
diff -u -r1.240 city.c
--- common/city.c       1 Sep 2004 19:45:19 -0000       1.240
+++ common/city.c       1 Sep 2004 20:11:38 -0000
@@ -402,10 +402,11 @@
  Will this city ever be able to build this improvement?
  Doesn't check for building prereqs
 **************************************************************************/
-bool can_eventually_build_improvement(const struct city *pcity, Impr_Type_id 
id)
+bool can_build_improvement_eventually(const struct city *pcity,
+                                     Impr_Type_id id)
 {
   /* also does an improvement_exists() */
-  if (!could_player_eventually_build_improvement(city_owner(pcity),id)) {
+  if (!can_player_build_improvement_eventually(city_owner(pcity),id)) {
     return FALSE;
   }
 
@@ -417,7 +418,20 @@
     return FALSE;
   }
 
-  return !improvement_redundant(city_owner(pcity),pcity, id, TRUE);
+  if (!improvement_redundant(city_owner(pcity),pcity, id, TRUE)) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**************************************************************************
+  Can the city build the given improvement?  This ignores obsoletion and
+  replacement.
+**************************************************************************/
+bool can_build_improvement_direct(const struct city *pcity, Impr_Type_id id)
+{
+  return can_build_improvement(pcity, id);
 }
 
 /**************************************************************************
@@ -428,18 +442,16 @@
   struct player *p = city_owner(pcity);
   struct impr_type *impr = get_improvement_type(id);
 
-  if (!improvement_exists(id)) {
-    return FALSE;
-  }
-  if (!player_knows_improvement_tech(p, id)) {
+  if (!can_build_improvement_eventually(pcity, id)) {
     return FALSE;
   }
 
-  if (!can_eventually_build_improvement(pcity, id)) {
+  /* Check tech requirement (like in can_player_build_improvement). */
+  if (get_invention(p, improvement_types[id].tech_req) != TECH_KNOWN) {
     return FALSE;
   }
 
-  /* The building pre req */
+  /* Check building requirements. */
   if (impr->bldg_req != B_LAST) {
     if (!city_got_building(pcity, impr->bldg_req)) {
       return FALSE;
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.157
diff -u -r1.157 city.h
--- common/city.h       14 Aug 2004 21:29:43 -0000      1.157
+++ common/city.h       1 Sep 2004 20:11:39 -0000
@@ -361,8 +361,12 @@
 
 bool city_has_terr_spec_gate(const struct city *pcity, Impr_Type_id id); 
 int improvement_upkeep(const struct city *pcity, Impr_Type_id i); 
+
 bool can_build_improvement(const struct city *pcity, Impr_Type_id id);
-bool can_eventually_build_improvement(const struct city *pcity, Impr_Type_id 
id);
+bool can_build_improvement_eventually(const struct city *pcity,
+                                     Impr_Type_id id);
+bool can_build_improvement_direct(const struct city *pcity, Impr_Type_id id);
+
 bool can_build_unit(const struct city *pcity, Unit_Type_id id);
 bool can_build_unit_direct(const struct city *pcity, Unit_Type_id id);
 bool can_eventually_build_unit(const struct city *pcity, Unit_Type_id id);
Index: common/improvement.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.c,v
retrieving revision 1.44
diff -u -r1.44 improvement.c
--- common/improvement.c        1 Sep 2004 03:16:47 -0000       1.44
+++ common/improvement.c        1 Sep 2004 20:11:39 -0000
@@ -379,8 +379,8 @@
   Whether player could build this improvement, assuming they had
   the tech req, and assuming a city with the right pre-reqs etc.
 **************************************************************************/
-bool could_player_eventually_build_improvement(struct player *p,
-                                             Impr_Type_id id)
+bool can_player_build_improvement_eventually(const struct player *p,
+                                            Impr_Type_id id)
 {
   struct impr_type *impr;
 
@@ -423,30 +423,28 @@
 }
 
 /**************************************************************************
-...
+  Can a player build this improvement somewhere?  Ignores the fact that 
+  player may not have a city with appropriate prereqs.
 **************************************************************************/
-static bool could_player_build_improvement(struct player *p, Impr_Type_id id)
+bool can_player_build_improvement(const struct player *p, Impr_Type_id id)
 {
-  if (!could_player_eventually_build_improvement(p, id))
+  if (!can_player_build_improvement_eventually(p, id)) {
     return FALSE;
-
-  /* Make sure we have the tech /now/.*/
-  if (get_invention(p, improvement_types[id].tech_req) == TECH_KNOWN)
-    return TRUE;
-  return FALSE;
+  }
+  if (get_invention(p, improvement_types[id].tech_req) != TECH_KNOWN) {
+    return FALSE;
+  }
+  return TRUE;
 }
-  
+
 /**************************************************************************
-  Can a player build this improvement somewhere?  Ignores the fact that 
-  player may not have a city with appropriate prereqs.
+  Can the player build the improvement?  This ignores whether it is
+  redundant or obsolete.
 **************************************************************************/
-bool can_player_build_improvement(struct player *p, Impr_Type_id id)
+bool can_player_build_improvement_direct(const struct player *p,
+                                        Impr_Type_id id)
 {
-  if (!improvement_exists(id))
-    return FALSE;
-  if (!player_knows_improvement_tech(p,id))
-    return FALSE;
-  return(could_player_build_improvement(p, id));
+  return can_player_build_improvement(p, id);
 }
 
 /**************************************************************************
Index: common/improvement.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.h,v
retrieving revision 1.31
diff -u -r1.31 improvement.h
--- common/improvement.h        1 Sep 2004 03:16:47 -0000       1.31
+++ common/improvement.h        1 Sep 2004 20:11:39 -0000
@@ -130,9 +130,11 @@
 void improvement_status_init(Impr_Status * improvements, size_t elements);
 
 /* player related improvement and unit functions */
-bool could_player_eventually_build_improvement(struct player *p, 
-                                               Impr_Type_id id);
-bool can_player_build_improvement(struct player *p, Impr_Type_id id);
+bool can_player_build_improvement(const struct player *p, Impr_Type_id id);
+bool can_player_build_improvement_eventually(const struct player *p, 
+                                            Impr_Type_id id);
+bool can_player_build_improvement_direct(const struct player *p,
+                                        Impr_Type_id id);
 
 /* city related improvement functions */
 void mark_improvement(struct city *pcity,Impr_Type_id id,Impr_Status status);
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.258
diff -u -r1.258 cityturn.c
--- server/cityturn.c   28 Aug 2004 19:15:39 -0000      1.258
+++ server/cityturn.c   1 Sep 2004 20:11:39 -0000
@@ -678,7 +678,7 @@
       Impr_Type_id new_target = improvement_upgrades_to(pcity, target);
 
       /* If the city can never build this improvement, drop it. */
-      if (!can_eventually_build_improvement(pcity, new_target)) {
+      if (!can_build_improvement_eventually(pcity, new_target)) {
        /* Nope, never in a million years. */
        notify_player_ex(pplayer, pcity->x, pcity->y, E_CITY_CANTBUILD,
                         _("Game: %s can't build %s from the worklist.  "