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

["Vasco Alexandre Da Silva Costa via RT" <rt@xxxxxxxxxxxxxx>]

On Sun, 8 Dec 2002, Mike Kaufman via RT wrote:

> Here is the first draft of the first step toward general-improvements.
> This is the initial framework.

And it is great stuff indeed. I like it a lot! ;)

> void effect_iterator_init(struct effect_iter *iter, unsigned int ranges,
>                           struct player *pplayer, struct city *pcity,
>                           int id, enum effect_type type);
>
> which describes the kind of search to do and:
>
> struct effect *effect_iterator_get_next_active(struct effect_iter *iter);
>
> which returns an (active) matching effect.
>
>   struct effect_iter iter;
>   struct effect *eff;
>   int failure_chance = 25;
>
>   effect_iterator_init(&iter, EFR_LAST, pplayer, pcity,
>                        walls_impr_id, EFT_SPY_RESISTANT);
>   while( (eff = effect_iterator_get_next_active(&iter)) ) {
>     failure_chance += eff->defn->amount;
>   }
>
> my, wasn't that nice? A word on the init function the range EFR_LAST is a
> synonym for iterate through all applicable ranges (as defined by pplayer,

I think it would be nicer if you added an EFR_ALL #define which just ORed
all the flags or something. It would be a more intuitive name since that
is a bitvector argument.

Another personal nit is I think function arguments 2-5 look odd in that
function. I suggest you make it thus:

range.type   = EFR_ALL;
range.player = pplayer;
range.city   = pcity;
range.impr   = walls_impr_id;
effect_iterator_init(&iter, &range, EFT_SPY_RESISTANT);

However MicroSoftie that may seem :)

Another idea is that you could add a macro wrapper for cases like the one
above to ease coding. Thus:

range.type   = EFR_ALL;
range.player = pplayer;
range.city   = pcity;
range.impr   = walls_impr_id;

effect_iterate(iter, &range, EFT_SPY_RESISTANT) {
  failure_chance += iter->defn->ammount;
} effect_iterate_end;

> pcity, and walls_impr_id). If you needed just the World and Island ranges
> for some reason, you would do:
>
>   effect_iterator_init(&iter, EFR_WORLD|EFR_ISLAND, pplayer, pcity,
>                        walls_impr_id, EFT_SPY_RESISTANT);
>
> so ranges can be ORed together. If you needed to look for all active
> effects, use EFT_LAST instead of EFT_SPY_RESISTANT.

Keep on with the good stuff :-)

---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa