[Freeciv-Dev] patch: updated governments ruleset (PR#105)

[dwp@xxxxxxxxxxxxxx]

I have made a updated governments ruleset patch; it is an 
updated version of freeciv-upkeep+government.diff, which was 
by Sune Kirkeby <sune@xxxxxxxxxxxxx>.  Also I think it
includes code to evaluate governments based on patch by
Rizos Sakellariou <rizos@xxxxxxxxxxx>.

The updated patch is at:
  ftp://ftp.freeciv.org/pub/freeciv/incoming/governments_rs_d1.diff.gz

This is a fairly minimal update:
- updated to latest CVS; this patch also assumes 
  ftp://ftp.freeciv.org/pub/freeciv/incoming/newform_remaining_rs.diff.gz
  which is not yet in CVS but probably will be soon;
- generalized Xaw govt selection dialog;
- fixed bug in receive_packet_ruleset_government() that
  packet->rapture_size was not read (leading to off-by-one-field 
  errors.  Oops :-)  This fixed the "extra char in Xaw govt names"
  problem (I guess Gtk refused to print weird char values?)
- added uk_* fields in civ2/units.ruleset

There are still some TODO and WISHLIST items in government.c,
especially changing the ruleset format to new non-tabular style.
(Though I would prefer _not_ to attempt the WISHLIST items
until after basic patch is in cvs.)


Other things I've noticed but not fixed:

-- There should be a server option to specify to governments ruleset!
And save tag in savegame etc.

-- Should also have a civ2/governments.ruleset, I guess pretty much
the same as current default, but for future expansion.

-- Should not have to put double quotes on numbers in ruleset
file for column of mixed numbers and flags: instead add new 
registry function to check type.  (Discussed previously.)

-- I would prefer flag strings etc in ruleset file not to be 
shouting (not all caps).

-- If there are more than the default number of governments, 
will have problems with government icon in client.  (After
Democracy will get global warming icons...  Even for same or
fewer, may get inappropriate icons.  Not sure of best solution
here; full-blown graphics spec files would maybe help...  
At minimum: government ruleset should specify some sort of
graphics offset.  We could also pre-allocate some extra spaces
in small.xpm for future use or for modpack use.

-- In lots of places have lines like:

+  struct government *g = &governments[ get_government(pcity->owner) ];
+  struct government *g = &governments[ pplayer->government ];

How about convenience functions like:
   struct government *get_government(int gov);
   struct government *get_gov_pplayer(struct *pplayer);
   struct government *get_gov_iplayer(struct *pplayer);
(These could then also do range checking.)

-- In ai/advdomestic.c:

   if (could_build_improvement(pcity, B_COURTHOUSE)) {
-    values[B_COURTHOUSE] = (pcity->corruption * t)/2;
-    if (gov == G_DEMOCRACY) values[B_COLOSSEUM] += building_value(1, pcity, val
);
+    /* Could someone elaborate one what COLOSSEUM does here? -- SKi */

This old code looks like a bug/typo to me: I think that should
be values[B_COURTHOUSE] all the way through.
"Under a Democracy, a Courthouse makes 1 unhappy citizen content."

-- Also in ai/advdomestic.c:

-  if (est_food > (get_government(pcity->owner) >= G_REPUBLIC ? 2 : 1)) {
+  if (est_food > (1 /*settler.food_cost*/ + g->extra_food_cost)) {

I think instead of "1 /*settler.food_cost*/" here, could use
something like: 
  get_unit_type(best_role_unit(pcity, F_SETTLERS))->food_cost

-- I don't think Suffrage/Police and Suffrage variant are
done properly with this patch.  (Plus note that current code 
which caches (pre-calculates) police/suffrage values is there 
for a reason: calling city_got_effect(pcity, B_POLICE) is not 
so cheap.)

-- In common/government.h:

Government flags such as NO_UPKEEP etc should have a
distinctive prefix (namespace), eg GF_NO_UPKEEP.
(Maybe also use an enum for the flags, with values 1,2,3,4,5...
and then use functions to do the bit tests; eg like unit flags.)

For magic constants, what about -1?  (Assuming signed values; 
careful in packets etc).   Or 127 or 255 as natural 8-bit values.

-- Comment:  I notice this patch makes Settlers and Engineers cost 
shield upkeep as well as food upkeep.  This matches Civ1 and Civ2, 
so I guess is reasonable, but we should remember to note this 
change in NEWS for the next release.

-- Loading existing savegames doesn't seem to work right.  
(One should correctly/best-effort load old savegames if at 
all possible!)


Regards,
-- David