[Freeciv-Dev] Re: Military amortize (PR#1196)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Thu, Jan 03, 2002 at 07:20:35PM CET, I got a letter,
where Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx> told me, that...
> Hi,
> 
> Throughout military calculations in the AI some crazy amortize scheme is
> used repeatedly.  I do not claim I understand it, but I thought it's
> worthwhile separating it into a separate procedure, military_amortize.
> 
> It makes the code more concise and removes one imaginatively named
> variable (a0) from 3 functions!
> 
> >From todays CVS,
> Compiles smooth.
> Savegames identical.
I like it. Only minor objections...

> -      if (b0 > 0) {
> -        a0 = amortize(b0, MAX(1, c));
> -        e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / ((fprime + needferry) * 
> MORT);
> -      } else e = 0;  
> +      e = military_amortize(b0, MAX(1, c), fprime + needferry);
I just feel soo better :).. The code has new fresh breath ;)) Just can't we do
MAX(1, c) in the function itself?

> +  simply_amortized = amortize(value, delay);
> +  fully_amortized = (value * simply_amortized 
> +                  / (MAX(1, value - simply_amortized)) 
> +                  * 100 / build_cost / MORT);
Nothing terribly exciting about the operators in new line, but can you please
at least indent them into the parenthesis?

And I would like the original...

  fully_amortized = ((value * simply_amortized)
                     / (MAX(1, value - simply_amortized) * 100
                     / (build_cost * MORT));

... much more. It gives you IMHO much better idea how the formula was actually
constructed.

> @@ -1314,35 +1334,31 @@
>                        g * SHIELD_WEIGHTING / (acity->ai.a * acity->ai.a + g 
> * d);
>            }
>            b0 -= c * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING : 
> SHIELD_WEIGHTING);
> -          if (b0 > 0) {
> -            a0 = amortize(b0, MAX(1, c));
> -            if (!sanity && !boatid && is_ground_unit(punit))
> -              e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / ((fprime + 40) * 
> MORT);
> -            else e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / (fprime * MORT);
> - /* BEGIN STEAM-ENGINES-ARE-OUR-FRIENDS KLUGE */
> -          } else if (!punit->id && !best) {
> -            b0 = b * SHIELD_WEIGHTING;
> -            a0 = amortize(b0, MAX(1, c));
> -            if (!sanity && !boatid && is_ground_unit(punit))
> -              e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / ((fprime + 40) * 
> MORT);
> -            else e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / (fprime * MORT);
> -            if (e > bk) {
> +       /* FIXME: sheild_cost of ferry */
shield_cost.. and can you please rather use spaces?
> +          needferry = ((!sanity && !boatid && is_ground_unit(punit)) ? 
> +                    40 : 0);
this deserves an if
> +       e = military_amortize(b0, MAX(1, c), fprime + needferry);
> +       /* BEGIN STEAM-ENGINES-ARE-OUR-FRIENDS KLUGE */
> +          if (b0 <= 0 && !punit->id && !best) {
> +         int bk_e = military_amortize(b * SHIELD_WEIGHTING, 
> +                                      MAX(1, c), fprime + needferry);
> +            if (bk_e > bk) {
>                if (punit->id && is_ground_unit(punit) &&
> -                       !unit_flag(punit, F_MARINES) &&
> -                       map_get_continent(acity->x, acity->y) != con) {
> +               !unit_flag(punit, F_MARINES) &&
> +               map_get_continent(acity->x, acity->y) != con) {
uhm, yes, you just exchange tabs and spaces here (no idea why), but when you
already touch it, what about moving that boolean opeators? ;)

> -          if (b0 > 0) {
> -            a0 = amortize(b0, MAX(1, c));
> -            e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / (fprime * MORT);
> -          } else e = 0;
> +       e = military_amortize(b0, MAX(1, c), fprime);
this indendation looks odd - just tab-hallucination?



-- 

                                Petr "Pasky" Baudis

UN*X programmer, UN*X administrator, hobbies = IPv6, IRC, FreeCiv, (e)links
.
The advantage of GUI is that you can see everything you can change.
The disadvantage of GUI is that you can change only what you can see.
.
Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/