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

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

Everything is fine with the patch but I think you should do the renaming
suggested by you in email to Mike.  Don't leave until tomorrow... (what you
can do day after tomorrow).

Possibly you should consider changing belligerence to attack_rating (in
analogy to defence_rating used in combat.c).  "Belligerence" isn't one of the
commonest words...

Two comments below.


  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
[...]
> @@ -571,10 +566,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) *
> -             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);

why don't you feed 
(m == LAND_MOVING || player_knows_improvement_tech(pplayer, B_PORT)
as a parameter to base_unit_belligerence_primitive??
make it a new variable, say, bool will_be_veteran.
(BTW, here it is assumed that AI will build barracks (port) before building
anything military)

> +      if (m == LAND_MOVING || player_knows_improvement_tech(pplayer,
> B_PORT)) {
> +     a = (a * 3) / 2;
> +      }
>        if (acity) a += acity->ai.a;
>        a *= a;
>        /* b is unchanged */

[...]

> Index: ai/aiunit.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
> retrieving revision 1.180
> diff -u -r1.180 aiunit.c
> --- ai/aiunit.c       2002/02/25 19:05:13     1.180
> +++ ai/aiunit.c       2002/02/26 09:25:27

[..]

> @@ -1057,9 +1060,9 @@
>      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;
> -      d_val += j;
> +      d_val += base_get_attack_power(d_type,
> +                                  do_make_unit_veteran(dcity, d_type),
> +                                  SINGLE_MOVE) * unit_types[d_type].hp;

Here (as everywhere else) there should be belligerence instead of just
attack*hp (compare to stack_attack_value above).  I suspect it's a historical
thing:  firepower wasn't in Civ I.  Firepower is completely missing in
gotohand.c, for example.

>      }
>      d_val /= (unit_type(punit)->move_rate / SINGLE_MOVE);
>      if (unit_flag(punit, F_IGTER)) d_val /= 1.5;


The rest is fine.

Best,
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