[Freeciv-Dev] (PR#10090) multiple applications of a single effect type

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10090 >

A single effect type may be applied multiple times.

For instance under the default ruleset in get_city_science_bonus(pcity) 
a bunch of different effects may be totaled: +50 from library, +50 from 
university, +50 from research lab, +50 from Copurnicus', +100 from Isaac 
Newton's. These are totaled additively.  (After this the bonus is halved 
for certain governments.)

However elsewhere there is a problem with additive summing.  In 
city_pollution() there is a check for the applicable effect amount from 
EFT_POLLU_PROD_PCT.  If this value exists then the pollution is divided 
by that ammount.  However in the previous rules these were combined 
multiplicatively, so the two effects of amounts "2" and "3" would 
combine to a 6x bonus.

A related problem is in base_get_trade_tile, where there are a whole 
bunch of effects to add to or multiply the tile's trade output by. 
These are applied in a hard-coded order that cannot be controlled by the 
ruleset.  Moreover there are some government, terrain, and special-based 
effects (bonuses and penalties which aren't implemented as effects yet, 
but should be) that must be applied in a particular order as well.  With 
the way the effects code is currently written there is no way for this 
ordering to come from the rulesets, so this government effect cannot be 
written as an effect.

My idea to solve this is to add a new parameter to each effect, called 
"which" or "order" or some better name that we come up with.  This will 
be an integer parameter that defaults to 0.  For each effect type you 
may have multiple "which" values for different effects.  When 
calculating an effect bonus, you can only calculate for one "which" 
value at a time.  Thus get_city_bonus(pcity, effect_type) becomes 
get_city_bonus(pcity, effect_type, which) and sums up all the bonus 
associated with "which".

"which" should be renumbered during ruleset loading so that it is uses 
non-negative integers.  The order is preserved but the values themselves 
are inconsequential.

A new function is added to return the number of "which" values for the 
effect.  It's called something like effect_order_count() (that's a bad 
name, but I don't have a better one just now).

Thus get_city_shield_bonus() is rewritten as

   int bonus = 100, i, count = effect_order_count(EFT_PROD_BONUS);

   for (i = 0; i < count; i++) {
     bonus += (bonus * get_city_bonus(EFT_PROD_BONUS, i)) / 100;
   }

   return bonus;

and there is no need for a government adjustment, since it can be done 
via an effect.

city_pollution is rewritten in a similar way so that the two 
pollution-affecting buildings can have a multiplicative effect.

base_get_trade_tile is a bit harder since there are multiple effects 
here.  These effects would need to be renumbered together (so you can 
alternate between one effect and another).  If that were done then all 
the bonuses in this function - even the celebration one - can be done by 
effects.  However eventually it should be possible to rewrite it to 
something very simple like:

   int output = 0, i, count = effect_order_count(EFT_TRADE_ADD_TILE);

   for (i = 0; i < count; i++) {
     output += get_city_tile_bonus(EFT_TRADE_ADD_TILE, i);
     if (output > 0) {
       output += get_city_tile_bonus(EFT_TRADE_INC_TILE, i);
     }
     output += output * get_city_tile_bonus(EFT_TRADE_PER_TILE, i) / 100;
   }

   return output;

And the same for all sorts of other functions.

The end result isn't too much more complicated than what we have now 
(the main complexity lies in generating the ruleset cache, I think). 
It's a LOT simpler than what the original gen-impr patch had, with 
separate effects for additively versus multiplicatively cumulative 
effects.  And it allows a lot of flexibility once government and other 
types of effects are added (which should probably be done first).

Thoughts?

jason