[Freeciv-Dev] Re: unit flags/capabilities

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Nov 29, 2001 at 08:51:25AM -0500, Andrew Sutton wrote:
> On Thursday 29 November 2001 04:04 am, Raimar Falke wrote:
> > On Wed, Nov 28, 2001 at 03:59:47PM -0500, Andrew Sutton wrote:
>  > it'd take a considerable amount of patching to work the changes back to
> > > the server and client, but IMHO i think it's not a bad idea.
> > >
> > > thoughts?
> >
> > Why? The server core only uses enum unit_flag_id. And this is good
> > IMHO. The server AI needs extra infos about how to use certain unit
> > types: enum unit_role_id was added.
> >
> > So what are your problems with this? Running out of flags? We can fix
> > this in a transparent way (no changes of the format of the rulesets,
> > no changes of the code, just the network code, unit_flag and ruleset
> > reading). It may necessary to have other infos about the units (for
> > example if you program an AI). But I don't see that you are doing
> > this.
> 
> 1. it's a clumsy implementation.

From both points of view it is ok: the ruleset writer can do a nicely
  flags         = "IgTer", "IgZOC"
  roles         = "DefendGood", "Partisan", "BarbarianTech"
and the programmer can do nicely
  ./client/control.c:    if(unit_flag(punit_focus, F_CARAVAN))

So these are the end points (the interfaces). You may now say that the
implementation between these endpoints is bad. I don't think so.

> 2. it isn't very extensible

You first have to state in which direction and why you want to
extend. What is the goal? What is the interface you want to achive?

> 3. it groups widely different types of data together into one bitfield.

Data which are all flags (booleans) and may be used independently. You
can have a unit which have the F_SPY,F_MARINES,F_IGTIRED flags. A
bitfield is a quite good implementation.

> 4. it separates out data that should be there.

??

> 5. adding a new implementation won't change the old code or the rulesets
> 
> this is a more graceful way to introduce capabilities to units than to 
> continually define and insert enumerations and bitfields into a structure. 
> there's been alot of talk about coding style? that's bad.
> 
> the point is that all of that information describes some capability of the 
> unit AND follows the same base subscription pattern (can have multiple 
> properties). therefore, it deserves to be in one place - type definition.
> 
> if you wanted to gracefully add new informatics for a new type of unit, then 
> sticking that data into the unit structure is bad style too. now, you end up 
> with a single structure that contains all the information for all units and 
> its overkill. it's clumsy. it's harder to maintain. it's harder to extend.
> 
> if you really want to add a new unit that defines a new field, say fuel (for 
> aircraft), then the correct thing to do would be to "derive" a new structure 
> for it. you'd end up with this:
> 
> struct unit
> {
>       unit_type       type;
>       unit_flags      flags;
>       // other generic information
> };
> 
> struct aircraft
> {
>       struct unit     base;
>       unsigned        fuel;
> };
> 
> voila! graceful extension of the unit type (or something similar). this is, 
> by the way, the RIGHT way to code that.

We both know that such a way would require a major
rewrite. Fortunately freeciv doesn't have this many types and would is
doesn't require a big flexibility because the set of types is more or
less constant. I agree that from a programmers point of view it may be
nicer to have a finer grained type set like struct land_unit, struct
water_unit and struct air_unit.

> the whole point is that people have been kludging this game together for a 
> long time. i'm starting to go thru field by field and undo some of those 
> problems. i'm sure some parts of the game are actually gracefully and 
> artfully written, but not unit code.

Another way for example is to change:

  int fuel;
  enum unit_activity activity;
  int activity_count;
  int activity_target;
  int connecting;

into

  struct {
    int fuel;
  } air_unit;
  struct {
    enum unit_activity activity;
    int activity_count;
    int activity_target;
    int connecting;
  } settler;

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Make it idiot-proof and someone will make a better idiot."