[Freeciv-Dev] Re: (PR#2521) general effects framework

["Mike Kaufman via RT" <rt@xxxxxxxxxxxxxx>]

On Tue, Jan 14, 2003 at 03:19:42AM -0800, Ben Webb via RT wrote:
> On Mon, Jan 13, 2003 at 03:07:13PM -0800, Mike Kaufman via RT wrote:
> > On Mon, Jan 13, 2003 at 03:46:59AM -0800, Ben Webb via RT wrote:
> > > I'm not talking about nuts and bolts; I'm talking about your interface
> > > being consistent. What's to stop a user feeding in a NULL city pointer
> > > and then saying he wants City-range effects? You need to add error
> > > checking to the function to enforce this. impr-gen just assumes that if
> > > you put in a NULL city pointer you don't want City-range effects.
> > with a city of NULL, then we'll iterate over all cities at City-range.
> 
> That seems rather counter-intuitive. Surely we have city_iterate already
> for iterating over all cities? What if you don't want to iterate over
> all cities? (As I've already said, there are a few places where you want
> to consider only Player- and World- range effects.)

then you put a particular city pointer in the init function...I don't see
how this is counter-intuitive: if you don't want a particular city, you
don't assign one.

> City if they're in a city). I was just curious at how your iterator
> function compared with that in impr-gen for unit effects.

we'll have to add a struct unit *punit into the iterator I imagine, but
then it'll be just like cities.

> bool pending:1, which seems eminently sensible, of course. (Although the
> C standard does seem to suggest that a bitfield can only be of type int,
> signed int, or unsigned int. I'm not sure if typedefs which just happen
> to resolve to ints are acceptable.)

yeah, GCC seems to think unsigned char is ok, but we'll stick to the C
standard I guess.

> Er, it's not just a single effect; you'd need to add additional effects
> for Science_Bonus, Science_Pct, Tax_Bonus, Tax_Pct, Unit_Defend,
> Unit_Attack, Unit_Attack_Firepower, Unit_Defend_Firepower, Unit_Recover,
> Unit_Vet_Combat, Upgrade_One_Step, Upgrade_One_Leap, Upgrade_All_Step
> and Upgrade_All_Leap, in the current code, and more if you add more unit
> effect types. For example, in the default ruleset, the Unit_Recover
> effect of United Nations works on units in the field, while the various
> Unit_Defend effects work only on units in cities.

After thinking about this, you may be right. Great Wall's UnitDefend effect
I think convinces me.

> You are in a government which confers a 20% science bonus (e.g.
> Knowledge). You have ten cities, each producing 1 science. This gives a
> total science output of 10, plus 20% bonus, i.e. 12. A 20% bonus acting
> on each city individually would give a per-city science output of 1.2,
> which would be rounded down to 1, i.e. it would have no effect.

I see: a product of integer arithmetic. Yes this could be a problem, but I
suggest that these should be different effects.

> That might work, although you still have to look after destroyed buildings
> that no longer have an owner - i.e. they belong to dead players.

dead players are saved in savegames.

-mike