[Freeciv-Dev] patch: variable number of unit types (PR#108)

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

This patch allows specifying a variable number of unit types in the 
units ruleset file.  (Just include more or fewer unit_* sections.)
There are a number of issues with this though, discussed below.
(This patch has been somewhat delayed due to having to write
down this outline of these issues ;-)

This patch essentially removes the unit_type_id enum
(U_SETTLERS to U_LAST) in unit.h.  I think this is a good 
thing because due to rulesets these enum values should not 
be used, and indeed are _not_ used in the current code except 
for U_LAST.

(One could still consider them useful for debugging, since for
the "official" rulesets they _do_ apply, and this is not likely 
to change (see below).  But I think I prefer to get rid of them
nevertheless...)

However, although the enum _values_ other than U_LAST are not 
used in the code, the enum type itself _is_ used in quite a 
few places (variables, function prototypes etc).  So its not
trivial to remove it.  In this patch (for simplicity, as a first
step), the enum type is left in, but with all the values removed,
as just:  (see below for MAX_NUM_ITEMS)

enum unit_type_id {
  U_FIRST=0,
  U_LAST=MAX_NUM_ITEMS
  /*
     U_LAST is a value which is guaranteed to be larger than all
     actual unit_type values.  It is used as a flag value; it can
     also be used for fixed allocations to ensure able to hold
     full number of unit types.
  */
};

I think in practice this should be fine, though it could critisized
that we are using an enum type where most of the values used (small
integers) do not appear in the enumeration!  Probably what should
ultimately happen is that all instances of the enum type be replaced
by a new typedef (probably typedefed to unsigned char).


The second issue is related to MAX_NUM_ITEMS and U_LAST, as appear 
above.  In many places in the current code, U_LAST is used as a flag 
value, and as a value for fixed-size allocations to hold all the unit 
types.  In principle these cases could all be made dynamic, but its 
much simpler/quicker to just keep U_LAST in these roles, with a larger 
value which is the new _maximum_ number of unit types.  Thats what 
I've done here, with MAX_NUM_ITEMS that new maximum, so named because
I expect to use this also for the maximum number of advances, and maybe
other 'items'.

Then a question arises: what should be the values of MAX_NUM_ITEMS?

Against having it too large, is that it should be less that 256,
so unit_type values will fit in an unsigned 8-bit int, for packet 
purposes.  (Also if use unsigned char for typedef as above).
Also, since its used for fixed-size allocations, it should not
be too much larger than likely useful values.

Against having it too small is to not unnecessarily limit alternative 
rulesets/modpacks.  For reference, currently (or in Civ2) there are 
51 unit types, 68 buildings, and 89 advances (including A_NONE?)

In this patch I've chosen MAX_NUM_ITEMS = 250, to fit in 255 
with allowance for maybe a few extra special flag values 
U_LAST+1,+2 etc.  Though maybe 200 would be well sufficient...


With this patch it might be tempting to remove all the "unused"
(tech_req = "Never") units in the civ1 ruleset file, but this 
turns out not to be such a good idea, because it would break 
old savegames (since unit type ids in savegame would suddenly 
mean different unit types).  And its not really any harm to 
leave them in.


It would make sense to do a similar things with the number
of advances in techs.ruleset; a complication is that some
tech enum values _are_ still used in the code.  The governments
patch will remove some, and others (eg, Bridge Building, 
Construction for fortresses) should be converted to entries
in ruleset files (probably simply as extra entries in
techs.ruleset [a_special] section). 

Currently it _doesn't_ really make sense to do the same for
buildings (wonders/improvements), since currently the enum
values _are_ crucial, and used throughout the code.  It is 
intended to improve this situation, but this is a big job.

Governments, when ruleset patch is included, will already
have variable numbers allowed from the start.  I'm not sure
about the terrain ruleset in this respect...

Regards,
-- David

num_units1.diff.gz
Description: GNU Zip compressed data