[Freeciv-Dev] Re: [PATCH] get rid of floating point calculations at advmilitary.c (PR#1320)

[Petrus Viljoen <viljoenp@xxxxxxxxxxx>]

Petr Baudis wrote:

> Dear diary, on Sun, Mar 10, 2002 at 05:58:30PM CET, I got a letter,
> where Markus Linnala <maage@xxxxxxxxx> told me, that...
> >
> > Get rid of floating point calculations at advmilitary.c.
> >
> > diff -ur -X freeciv/diff_ignore freeciv/ai/advmilitary.c 
> > freeciv-rid-fp-2/ai/advmilitary.c
> > --- freeciv/ai/advmilitary.c  Wed Mar  6 12:05:09 2002
> > +++ freeciv-rid-fp-2/ai/advmilitary.c Sun Mar 10 18:56:40 2002
> > @@ -449,8 +449,8 @@
> >    else cur *= a; /* wanted to rank Legion > Catapult > Archer */
> >  /* which we will do by munging f in the attacker want equations */
> >    if (unit_type_flag(i, F_IGTER) && !def) cur *= 3;
> > -  if (unit_type_flag(i, F_PIKEMEN) && def) cur *= 1.5;
> > -  if (unit_types[i].move_type == LAND_MOVING && def) cur *= 1.5;
> > +  if (unit_type_flag(i, F_PIKEMEN) && def) cur += cur / 2;
> > +  if (unit_types[i].move_type == LAND_MOVING && def) cur += cur / 2;
> >    return(cur);
> >  }
> >
> > @@ -595,7 +595,7 @@
> >
> >        m = get_virtual_defense_power(i, n, x, y);
> >        m *= unit_types[n].hp * unit_types[n].firepower;
> > -      if (vet) m *= 1.5;
> > +      if (vet) m += m / 2;
> >        m /= 30;

More optimized >>
-      if (vet) m *= 1.5;
-      m /=30;
+      m /= (vet ? 20 : 30) ;

or  to be more descriptive of what's happening ..
+      m /= (vet ? 30*2/3 : 30) ;

the Compiler should do the constant folding here..

>
> >        m *= m;
> >        d = m;
> > @@ -735,7 +735,7 @@
> >        n = ai_choose_defender_versus(acity, v);
> >        m = get_virtual_defense_power(v, n, x, y);
> >        m *= unit_types[n].hp * unit_types[n].firepower;
> > -      if (do_make_unit_veteran(acity, n)) m *= 1.5;
> > +      if (do_make_unit_veteran(acity, n)) m += m / 2;
> >        m /= 30;

Same As Above :
-      if (do_make_unit_veteran(acity, n)) m *= 1.5;
-      m /= 30;
+      m /= (do_make_unit_veteran(acity, n) ? 30*2/3 : 30) ;



>
> >        if (c > 1) {
> >          d = m * m;
> > @@ -752,7 +752,7 @@
> >  tech progression beyond all description.  Only when adding the override 
> > code
> >  did I realize the magnitude of my transgression.  How despicable. -- Syela 
> > */
> >          m = get_virtual_defense_power(v, pdef->type, x, y);
> > -        if (pdef->veteran) m *= 1.5; /* with real defenders, this must be 
> > before * hp -- Syela */
> > +        if (pdef->veteran) m += m / 2; /* with real defenders, this must 
> > be before * hp -- Syela */
> >       m *= (myunit->id != 0 ? pdef->hp : unit_type(pdef)->hp) *
> >           unit_type(pdef)->firepower;
> >  /*        m /= (pdef->veteran ? 20 : 30);  -- led to rounding errors.  
> > Duh! -- Syela */

/*        m /= (pdef->veteran ? 20 : 30);  -- led to rounding errors. 
Duh! -- Syela */
Can someone enlighten me in regards to this comment ......

If I have any number foo     then  ((foo*3)/2)/30   = foo/20
No Rounding errors can happen by reducing  ((foo*3)/2)/30 to foo/20

Best to remove  confusing comments !!

If  the 30 used in this code is an arbitrary number  and not a constant 
it is a different case though,
But then it is better to give it some symbolic name.
to make it clear that it can be an arbitrary number .


like
#define     POWER_DIVIDER 30
and user POWER_DIVIDER in the code not 30.

In that case it is better to do the following.

    if (vet) {
        m = (3*m) / (2*POWER_DIVIDER);
    }
    else {
        m /= POWER_DIVIDER;
    }

The Compiler will do the constant folding for (2*POWER_DIVIDER)