Complete.Org: Mailing Lists: Archives: freeciv-dev: November 2001:
[Freeciv-Dev] Re: unit flags/capabilities
Home

[Freeciv-Dev] Re: unit flags/capabilities

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: unit flags/capabilities
From: Reinier Post <rp@xxxxxxxxxx>
Date: Thu, 29 Nov 2001 17:25:13 +0100

On Thu, Nov 29, 2001 at 08:51:25AM -0500, Andrew Sutton wrote:

> > 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.

Which doesn't belong in 'unit', for it is unknown at the client side.

> > 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.

(This would really make it more extensivble.)

> 1. it's a clumsy implementation.
> 2. it isn't very extensible

I don't think your idea makes it any more extensible in practice.

> 3. it groups widely different types of data together into one bitfield.
> 4. it separates out data that should be there.
> 5. adding a new implementation won't change the old code or the rulesets

All nice if you want it in the first place ...

> 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.

Yes, definitely, but it's a consequence of C's typing system.

> 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.

This doesn't happen, instead it's encoded in the unit *type* structure
using special flags.  Ideally, all unit behaviour should depend on flags,
ot the unit type, but flags are a later introduction and the conversion
is not complete.

> 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;
> };

So now, at every place in the code where units are used, we need to expect
a whole set of possible unit subtypes and test for the type with a case
construct?  You can write wrappers to take care of this, but it's going
to make the code much slower.  And I can't see much practical benefit:
adding a new property and associated behaviour will be *more* work than
it was before.

Are you sure this is what you intend to do?

> voila! graceful extension of the unit type (or something similar). this is, 
> by the way, the RIGHT way to code that.

Well, the code to handle the additional properties still isn't subtype
specific - the proper way would also add methods.  I.e. for every subtype
struct you'd have a separate .h/.c pair with the subtype specific code.
And of course there must be generic code to deal with the inheritance
and polymorphism.

What you're asking for is C++.  Freeciv is C code.  The X code
(see the Xaw client) implements a mechanism for inheritance, so you may
consider to use it here.  But doing your own mechanism seems to be the
wrong approach.

The problem you address is that some unit properties are only meaningful
to certain categories of units - but how often does this really happen?

> andy

-- 
Reinier


[Prev in Thread] Current Thread [Next in Thread]