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

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

I have made another updated governments ruleset patch (based 
on previous patches by Sune Kirkeby <sune@xxxxxxxxxxxxx> and 
Rizos Sakellariou <rizos@xxxxxxxxxxx>, and my previous update).

The patch is at:
  ftp://ftp.freeciv.org/pub/freeciv/incoming/governments_rs_d2.diff.gz
(against current CVS "now").

This is a more substantial update, and I think this is now
mostly correct at least for the default government ruleset.

Specifically, a goal of this patch is to be _exactly_ backward
compatible with the existing code, to allow precise regression
testing.  That is, you should be able to run two games with and 
without this patch, with identical user input (and same random 
seeds, or from a savegame with randoms initialized), and the 
results, in terms of savefiles etc, should be identical (except 
an extra governments="default" line in the savefile).

I have found such regression testing to be _very_ useful at 
finding nasty subtle bugs, and suggest its use whenever possible
when making structural changes.  (Although it can be a bit tedious
tracking down unimportant changes, and sometimes finding bugs in 
the existing version which were fixed by the changes...) 

To allow such regression testing, this patch does _not_ include
some of the new code to dynamically evaluate governments etc. 
Instead for now it just uses the old code, ignoring that the 
enum values may not correspond to the ruleset governments.
(The new code is included, but ifdef-ed out, and it needs updating
due to some changes such as discussed below.)

While updating the patch I came across a few problems related to
the added generality of the new government rulesets and unit upkeeps, 
where situations arise which do not exist in any of the normal Civ 
games and it is unclear how best to handle some situations.  

For example, under Republic you may have one aggressive/field unit
before incuring unhappiness for further such units.  A generalization
is to allow some number N such units.  But at the same time different
unit types may now incur different levels of unhappines, so it 
matters _which_ N units are counted as free of unhappiness costs 
-- So unless care it taken, the calculation order matters :-(
To avoid such problems I've instead taken the generalization to 
be that the first N unhappiness _points_ are free, not the first 
N units, which also seems probably more reasonable anyway.

Other changes since previous such patches include:

- Instead of Democracy adding 1 unhappy for unhappy-causing units,
I made this a factor, with Demoacracy=2, Republic=1 and others=0,
thus also avoiding requiring a "special case" for no unhappiness.

- Added separate production bonus and penalty parameters for
celebrating cities, to cover case that non-Republic/Democ count
as "higher" governments for celebrating cities.  (I thought this
was a bit more flexible than explicitly pointing to the effect
of another government.)

- Added martial_law_per to handle improved martial law under
Communism; altered meaning of empire_size_mod (was empire_size_factor);
added garphics offset parameter.

- Fixed various bugs and backward-compatability/regression-testing
issues.

There are still quite a few things on the TODO list in governments.c:

 * o Update government.ruleset to be untabulated (like the new
 *   units.ruleset).
 * o Update and turn on evaluation code currently disabled for
 *   regression testing purposes.
 * o Undo some other regression-testing constraints, and check
 *   exact government rules vs Civ1,Civ2.
 * o The clients display wrong upkeep icons in the city display,
 *   not sure what to do here.  
 * o When change government, should send more updated info to client
 *   (in particular unit upkeep icons are strange).
 * o Rizos changes to have AI understand unit unhappiness better.
 * o Implement actual cost effect for unit gold upkeep.
 * o ai_manage_cities() in aicity.c assigns tech_want to A_CODE,
 *   A_REPUBLIC and A_MONARCHY presumably for government purposes...
 * o Move some functions from player.c to government.c.
 * o Update help system: meaning of Nonmil and FieldUnit unit flags;
 *   dynamic help on governments etc.
 * o Test the new government evaluation code (AI).
 
However if there are no significant problems I plan to check
this patch in and then work from there.

Regards,
-- David