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

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 04:06 PM 02/01/04 +0000, Gregory Berkolaiko wrote:
>This is a mass reply to your comments.
>First of all, thanks for your time!
[...]
>To Petr: On formatting: when I view my file everything looks perfectly
>formatted.  Can it be that diff chews the tabs up somehow?
>Sorry Raahul, I think the crazy formatting in your diff was also because
>of this -- I didn't know that :(
[...]
>A slightly improved patch is attached.  All seemingly unformated lines
>are due to tabulation thing -- do I have to hunt them out?

diff -b will ignore whitespace differences and can produce very odd
patches. But otherwise, it should work fine. What platform/version
are you using.

Note, because diff adds 1or2 characters to the start of the line, tabs 
will still indent to where they normally do, but all-spaces lines will
be off by 2. This is just reading the diff, not what it does though.

If you do "set ts=8 ht=8 sw=8" all or singly in vi it will setup the
appropriate conditions for tab viewing. If someone used the wrong number
of spaces somewhere it will showup. Switch to a tabs 4 flavour and all
the space-only lines will be horribly indented. Use something like
expand/unexpand to redo the tabs (but caution, it is really only leading
tab/space indentation you want to touch and expand will do everything).
Finally run everything through indent into a temp file. Then diff the
tempfile against the original to see where the problems occur.

Some editors have a really hard time with Unix spaces and tabs formatting
and introduce all kinds of spurious spaces. Space only is the common
denominator, but is horribly inefficient, the spark of many flame-wars,
and not the mandated K&R style.

Cheers,
RossW
=====

BTW: ternary operators are valid "C" syntax not proscribed by any policy
     or program reformatting. There should not be an issue with such usage.
     There may be an issue with poorly coded or abuse of ternaries, though.
     If they get too complex or are not visually indented correctly, this
     can be a valid point of concern. For example either of the second two
     flavours are typically easier to recognize. The first is a -bbo, the
     second a better grouping to not split the sub-element constructs.

         needferry = ((!sanity && !boatid && is_ground_unit(punit)) ? 
                       40 : 0);

         needferry = ((!sanity && !boatid && is_ground_unit(punit)) 
                      ? 40 : 0);
         needferry = 
           ((!sanity && !boatid && is_ground_unit(punit)) ? 40 : 0);

>G.
>
>__________________________________________________
>Do You Yahoo!?
>Everything you'll ever need on one web page
>from News and Sport to Email and Music Charts
>http://uk.my.yahoo.com? rfile1
>? rfile2
>? saves
>? ai/aiunit.mod.c
>Index: ai/advmilitary.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
>retrieving revision 1.80
>diff -u -r1.80 advmilitary.c
>--- ai/advmilitary.c   2001/12/21 11:17:30     1.80
>+++ ai/advmilitary.c   2002/01/04 16:01:34
>@@ -543,7 +543,7 @@
> { 
>   Unit_Type_id i;
>   Tech_Type_id j;
>-  int a, c, d, e, a0, b0, f, g, fprime;
>+  int a, c, d, e, b0, f, g, fprime;
>   int k, l, m, q;
>   int shore = is_terrain_near_tile(pcity->x, pcity->y, T_OCEAN);
>   struct city *acity = map_get_city(x, y);
>@@ -624,10 +624,7 @@
>         b0 -= l * SHIELD_WEIGHTING;
>         b0 -= c * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING :
SHIELD_WEIGHTING);
>       }
>-      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);
>       if (e > 0) {
>         if (k) {
>           pplayer->ai.tech_want[j] += e;
>@@ -659,7 +656,7 @@
> {
>   int a, b, c, d, e, f, g; /* variables in the attacker-want equation */
>   Unit_Type_id v, n;
>-  int m, vet, dist, a0, b0, fprime;
>+  int m, vet, dist, b0, fprime;
>   int x, y, unhap = 0;
>   struct unit *pdef, *aunit, *ferryboat;
>   struct city *acity;
>@@ -822,10 +819,7 @@
>                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));
>-      e = ((a0 * b0) / (MAX(1, b0 - a0))) * 100 / ((fprime + needferry) *
MORT);
>-    } else e = 0;  
>+    e = military_amortize(b0, MAX(1, c), fprime + needferry);
> 
> #ifdef DEBUG
>     if (e != fstk && acity) {
>Index: ai/aiunit.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
>retrieving revision 1.158
>diff -u -r1.158 aiunit.c
>--- ai/aiunit.c        2001/11/05 01:59:59     1.158
>+++ ai/aiunit.c        2002/01/04 16:01:59
>@@ -490,6 +490,26 @@
> }
> 
> /**************************************************************************
>+Military "want" estimates are amortized in this complicated way.
>+COMMENTME: Why not use simple amortize? -- GB
>+**************************************************************************/
>+int military_amortize(int value, int delay, int build_cost)
>+{
>+  int simply_amortized, fully_amortized;
>+
>+  if (value <= 0) {
>+    return 0;
>+  }
>+
>+  simply_amortized = amortize(value, delay);
>+  fully_amortized = ((value * simply_amortized) 
>+                     / (MAX(1, value - simply_amortized)) * 100
>+                     / (build_cost * MORT));

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

This is perhaps slightly better done by moving the multipliers up so
that integer truncation effects are minimized when simply_amortized is
much less than value. The only issue with this is overflow, but I
don't think values are more than a short, and simply_amortized should
always be less than value.

>+
>+  return fully_amortized;
>+}
>+
>+/**************************************************************************
>  this is still pretty dopey but better than the original -- Syela
> **************************************************************************/
> static int reinforcements_value(struct unit *punit, int x, int y)
>@@ -1162,14 +1182,14 @@
> **************************************************************************/
> int find_something_to_kill(struct player *pplayer, struct unit *punit,
int *x, int *y)
> {
>-  int a=0, b, c, d, e, m, n, v, i, f, a0, b0, ab, g;
>+  int a=0, b, c, d, e, m, n, v, i, f, b0, ab, g;
> #ifdef DEBUG
>   int aa = 0, bb = 0, cc = 0, dd = 0, bestb0 = 0;
> #endif
>   int con = map_get_continent(punit->x, punit->y);
>   struct player *aplayer;
>   struct unit *pdef;
>-  int best = 0, maxd, boatid = 0;
>+  int best = 0, maxd, boatid = 0, needferry;
>   int harborcity = 0, bx = 0, by = 0;
>   int fprime, handicap;
>   struct unit *ferryboat = 0;
>@@ -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) {
>-              if (punit->id && is_ground_unit(punit) &&
>-                       !unit_flag(punit, F_MARINES) &&
>-                       map_get_continent(acity->x, acity->y) != con) {
>+        /* FIXME: build_cost of ferry */
>+          needferry = ((!sanity && !boatid && is_ground_unit(punit)) ? 

*** This is a 8 spaces == 1 tab problem. Alternatively the FIXME and
    2 lines at 1 down have tabs the surrounding code does not.

>+                     40 : 0);
>+        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) {
>                 if (find_beachhead(punit, acity->x, acity->y, &xx, &yy)) {
>                   *x = acity->x;
>                   *y = acity->y;
>-                  bk = e;
>+                  bk = bk_e;
>                 } /* else no beachhead */
>               } else {
>                 *x = acity->x;
>                 *y = acity->y;
>-                bk = e;
>+                bk = bk_e;
>               }
>             }
>-            e = 0; /* END STEAM-ENGINES KLUGE */
>-          } else e = 0;
>+        }
>+        /* END STEAM-ENGINES KLUGE */
> 
>         if (punit->id && ferryboat && is_ground_unit(punit)) {
>           freelog(LOG_DEBUG, "%s@(%d, %d) -> %s@(%d, %d) -> %s@(%d, %d)"
>@@ -1410,10 +1426,7 @@

>           else if (c > THRESHOLD) b0 = 0;
>           else b0 = ((b * a - f * d) * SHIELD_WEIGHTING / (a + d)) - 
>             c * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING :
SHIELD_WEIGHTING);

It would be nice to clean this up so the "b0 =" are aligned and not part
of run-on statements.

>-          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);
>           if (e > best && ai_fuzzy(pplayer,1)) {
> #ifdef DEBUG
>             aa = a; bb = b; cc = c; dd = d; bestb0 = b0;
>Index: ai/aiunit.h
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.h,v
>retrieving revision 1.24
>diff -u -r1.24 aiunit.h
>--- ai/aiunit.h        2001/01/10 18:50:45     1.24
>+++ ai/aiunit.h        2002/01/04 16:02:27
>@@ -34,6 +34,7 @@
> int unit_vulnerability_basic(struct unit *punit, struct unit *pdef);
> int unit_vulnerability_virtual(struct unit *punit);
> int unit_vulnerability(struct unit *punit, struct unit *pdef);
>+int military_amortize(int value, int delay, int build_cost);
> 
> int is_on_unit_upgrade_path(Unit_Type_id test, Unit_Type_id base);
> int is_ai_simple_military(Unit_Type_id type);
>