[Freeciv-Dev] Re: [Patch] Cleanup of attack power calculations

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Feb 25, 2002 at 05:27:18PM -0800, Raahul Kumar wrote:
> 
> --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > To understand combat.c better I have cleaned up the calculation of the
> > attack power. Best part: you can now tell the differnce between
> > unit_belligerence_basic and unit_belligerence_primitive.
> > 
> 
> It's a good patch. Thumbs up. Minor nitpicks follow.
> 
> >     Raimar
> > 
> > -- 
> >  email: rf13@xxxxxxxxxxxxxxxxx
> >  "Life is too short for reboots."
> > > Index: ai/advmilitary.c
> > ===================================================================
> > RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
> > retrieving revision 1.90
> > diff -u -r1.90 advmilitary.c
> > --- ai/advmilitary.c        2002/02/21 09:44:50     1.90
> > +++ ai/advmilitary.c        2002/02/25 17:10:28
> > @@ -571,10 +569,11 @@
> >        l /= city_list_size(&pplayer->cities);
> >  /* Katvrr advises that with danger high, l should be weighted more heavily
> > */
> >  
> > -      a = unit_types[i].attack_strength *  ((m == LAND_MOVING ||
> > -          player_knows_improvement_tech(pplayer, B_PORT)) ? 15 : 10) *
> 
> Notice that here the POWER_FACTOR is 15 instead of 10 if the player knows
> how to build a port. That is not the case in your replacement below. I'm not
> quite clear on why the power factor is necessary. Put in comments 
> explaining the power factor.

See MARK.

> > -             unit_types[i].firepower * unit_types[i].hp;
> > -      a /= 30; /* scaling factor to prevent integer overflows */
> > +      a = base_unit_belligerence_primitive(i, FALSE, SINGLE_MOVE,
> > +                                      unit_types[i].hp);

MARK:
> > +      if (m == LAND_MOVING || player_knows_improvement_tech(pplayer,
> > B_PORT)) {
> > +   a *= 1.5;
> > +      }

> >        if (acity) a += acity->ai.a;
> >        a *= a;
> >        /* b is unchanged */
> > @@ -703,9 +702,7 @@
> >      v = myunit->type;
> >      vet = myunit->veteran;
> 
> V is fairly bad. How about my_unit_type.

It could be resolved to an even simpler version.

> You can remove this now or rename base_unit_belligerence. I find two 
> functions,
> one which does the actual work and a wrapper which calls it annoying. It means
> I have to read two functions instead of one to work out what is really
> happening.

I leave it to you (the AI cleanup people) to choose the interface. It
is also possible to merge them and add a flag "bool
consider_moves_left".

> >  int unit_belligerence(struct unit *punit)
> > @@ -1040,8 +1043,8 @@
> >      d_val = stack_attack_value(dest_x, dest_y) * 30;
> >      if ((dcity = map_get_city(dest_x, dest_y))) {
> >        d_type = ai_choose_defender_versus(dcity, punit->type);
> > -      j = unit_types[d_type].hp * (do_make_unit_veteran(dcity, d_type) ? 15
> > : 10) *
> > -          unit_types[d_type].attack_strength;
> > +      j = base_get_attack_power(d_type, do_make_unit_veteran(dcity, 
> > d_type),
> > +                           SINGLE_MOVE) * unit_types[d_type].hp;
> 
> A better name than j would be appreciated.

j has been removed entirely.

> >  int get_attack_power(struct unit *punit)
> >  {
> > +  return base_get_attack_power(punit->type, punit->veteran,
> > +                          punit->moves_left);
> > +}
> > +
> 
> You can get rid of this as well.

There are three users left.

./common/combat.c:300:  int attackpower = get_attack_power(attacker);
./server/gotohand.c:1109:           attack_of_enemy = get_attack_power(aunit);
./server/unittools.c:151:    return (get_attack_power(punit)>0);

> > +  power = get_unit_type(type)->attack_strength * POWER_FACTOR;
> > +  if (veteran) {
> > +    power = (power * 3) / 2;
> > +  }
> 
> power = power * 1.5; /* 50% bonus to attack and defence gained by veteran     
>  
>                          units*/

1.5 involves the FPU. Comment added.

> > +  if (!unit_type_flag(type, F_IGTIRED) && moves_left < SINGLE_MOVE) {
> > +    power = (power * moves_left) / SINGLE_MOVE;
> 
> Finally. I always found it annoying that the attack power calcs were wrong
> if the unit had fractional moves left.

??

> These lines below could be replaced with the equivalent of
> 
> int get_defence_power(Unit_Type_id type, bool veteran, int moves_left)

Defence comes in the next patch.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "> WHY?! Isn't it better to put $(shell cat cscope.files) on the list of
  I only have a yellow belt in makefile kungfu.  These fancy gnu make things
  are relatively new to some of us..."
    -- Mark Frazer to Vassilii Khachaturov in linux-kernel

attack_power2.diff
Description: Text document