[Freeciv-Dev] Re: (PR#2650) Overflow in military_amortize

["Gregory Berkolaiko via RT" <rt@xxxxxxxxxxxxxx>]

Quoting "Per I. Mathisen via RT" <rt@xxxxxxxxxxxxxx>:

> 
> On Thu, 26 Dec 2002, Gregory Berkolaiko via RT wrote:
> > The expression in military_amortize() in aiunit.c
> > fully_amortized = ((value * simply_amortized) * 100
> >                    / (MAX(1, value - simply_amortized))
> >                    / (build_cost * MORT));
> > overflows on a regular basis.
> >
> > This is rather damaging as can trun attractive objective / build into an
> > unattractive (negative) one.
> 
> I suppose military_amortize() is meant to balance the use of build cost in
> the kill_desire() function, which gives us want.
> 
> But if we really want to do this, we should calculate (in aidata.c) the
> average city production each turn, divide build cost on that and add the
> result to the delay parameter to normal amortize instead. This way we
> really know what impact the loss of build_cost shields.

I was thinking more in the direction of fixing the overflow.  Well,thanks for
turning me around.  What you say makes perfect sense.  We wait for comments for
some time and then I commit, ok?

Two remarks: average production (over the whole nation) canbe very different
from what we have in this given city.  But I guess we shouldn't bother about it,
it's all relative anyway.

There is a danger that changing amortize in such a brutal way will unsynch the
weight in other areas (like tech want).  But I wouldn't bother about it either,
we won't learn until we break it.

> This makes sense for what-do-we-want-to-build uses of this function, while
> for find-target uses, the only way it makes a difference is when comparing
> a we-need-ferry mission to a mission where we don't (since we add the
> build cost of our ferry to the mess).
> 
> So military_amortize would be like this:
> 
> /**************************************************************************
>   Amortize a want modified by the shields (build_cost) we risk losing.
> **************************************************************************/
> int military_amortize(struct player *pplayer, int value, int delay, int
>                       build_cost)
> {
>   struct ai_data *ai = ai_data_get(pplayer);
>   int build_time = ai->stats.average_production / build_cost;
> 
>   if (value <= 0) {
>     return 0;
>   }
> 
>   return amortize(value, delay + build_time);
> }
> 
> Patch attached. Only autogame tested.
> 
> The big question is if we need this at all. Simpe amortize might just get
> the job done, too.

With what delay?

G.