[Freeciv-Dev] Re: [PATCH] [1.2] cleanup of proccess_*_want() (PR#1295)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Mon, Mar 04, 2002 at 04:48:54AM CET, I got a letter,
where Raahul Kumar <raahul_da_man@xxxxxxxxx> told me, that...
> 
> --- Petr Baudis <pasky@xxxxxxxxxxx> wrote:
> > Hello,
> <snip>
> 
> >   I think it's ready to actually go in, altough I'm open to objections and
> > ideas how to extend/correct it even more (I expect many hints for better and
> > more comments, and possibly also for better variable names - in fact, all
> > those
> > one-letter variables have usually same mean in the whole AI - but I didn't
> > i.e.
> > renamed f to 'loss', but to 'bcost' here, contrary to findvictim()).
> >
> 
> I'd like a little consistency.

So would I. But then I would rather change loss to bcost in findvictim() - I
really can't look into future..

> > +  bool defended = has_a_normal_defender(pcity);
> > +  /* Technologies we would like to have. */
> > +  int tech_desire[U_LAST];
> > +  /* Our favourite unit. */
> > +  int best = 0;
> > +  Unit_Type_id best_unit_type = 0; /* FIXME: 0 is legal value! */
> > +  /* Iterator. */
> > +  Unit_Type_id unit_type;
> > +
> > +  memset(tech_desire, 0, sizeof(tech_desire));
> > +  
> > +  for (unit_type = 0; unit_type < game.num_unit_types; unit_type++) {
> 
> Replace this with unit_type_iter.

Where it lives? Grep doesn't know and I saw only a proposal on ml.

> > +    if (!is_ai_simple_military(unit_type)) continue;
> > +    
> > +    if (move_type == LAND_MOVING || move_type == SEA_MOVING) {
> > +      /* How many technologies away it is? */
> > +      int tech_dist = num_unknown_techs_for_goal(pplayer,
> > +                        unit_types[unit_type].tech_requirement);
> > +      /* How much we want the unit? */
> > +      int desire = unit_desirability(unit_type, TRUE);
> > +
> 
> The range of unit desirability is ? Does negative numbers have any meaning?

Nope. AFAIK it's guaranteed to be -le 0.

> > +      /* We won't leave the castle empty when driving out to battlefield. 
> > */
> > +      if (!defended && unit_type_flag(unit_type, F_FIELDUNIT)) desire = 0;
> > +      
> > +      desire /= 15; /* Good enough, no rounding errors. */
> > +      desire *= desire;

This is good place for POWER_DIVIDER/2 ;).

> > +      
> > +      if (can_build_unit(pcity, unit_type)) {
> > +        /* We can build the unit now... */
> > +        
> > +        if (walls && move_type == LAND_MOVING) {
> > +          desire *= pcity->ai.wallvalue;
> > +          desire /= 10;
> > +        }
> 
> Huh??? Is this trying to model the fact that one unit defending behind city
> walls is better than 2 without?

Who knows, maybe...

> > +    if ((move_type == LAND_MOVING || (move_type == SEA_MOVING && shore))
> > +        && tech_req != A_LAST
> > +        && (tech_dist > 0 ||
> > +            !can_build_unit_direct(pcity,
> > unit_types[unit_type].obsoleted_by))
> > +        && unit_types[unit_type].attack_strength /* or we'll get SIGFPE */
> > +        && move_type == orig_move_type) {
> > +      bool will_be_veteran = (move_type == LAND_MOVING ||
> > +                              player_knows_improvement_tech(pplayer,
> > B_PORT));
> 
> If you're going to check that, why not check B_AIRPORT as well?

That's Greg's job :).

> > +      /* Cost (shield equivalent) of gaining these techs. */
> > +      /* FIXME? Katvrr advises that this should be weighted more heavily in
> > big
> > +       * danger. */
> > +      int tech_cost = total_bulbs_required_for_goal(pplayer,
> > +                        unit_types[unit_type].tech_requirement) / 4
> > +                      / city_list_size(&pplayer->cities);
> > +      int move_rate = unit_types[unit_type].move_rate;
> > +      int move_cost;
> > +      int bcost_balanced = build_cost_balanced(unit_type);
> > +      /* See description of kill_desire() about this variables. */
> > +      int bcost = unit_types[unit_type].build_cost;
> > +      int vuln;
> > +      int attack = base_unit_belligerence_primitive(unit_type,
> > will_be_veteran,
> > +                     SINGLE_MOVE, unit_types[unit_type].hp);
> > +      /* Computed values.. */
> > +      int desire, want;
> > +      
> > +      if (acity) attack += acity->ai.attack;
> > +      
> > +      attack *= attack;
> > +
> > +      if (unit_type_flag(unit_type, F_IGTER)) {
> > +        /* Not quite right... */
> > +        move_rate *= SINGLE_MOVE;
> 
> Bad Petr. Create your own define, IGTER_MOVE_COST.

Why? This would mean changing on much more places. And this stuff is IIRC your
or Greg's job, isn't it? ;))) (oh, I'm evil..)

> > +      /* Appreciate resistance of the enemy. */
> > +      /* It's like assess_defense_unit(), but get_virtual_defense_power()
> > +       * instead of get_defense_power(). */
> > +      
> > +      vuln = get_virtual_defense_power(unit_type, victim_unit_type, x, y);
> > +      vuln *= unit_types[victim_unit_type].hp *
> > +              unit_types[victim_unit_type].firepower;
> > +      if (veteran) vuln *= 1.5;
> > +      vuln /= 30;
> > +      vuln *= vuln;

Ok, here I should use POWER_

> > +      if (unit_types[unit_type].move_type == LAND_MOVING && acity
> > +          && move_cost > (player_knows_improvement_tech(city_owner(acity),
> > +                                                        B_CITY) ? 2 : 4)
> > +          && !unit_type_flag(unit_type, F_IGWALL)
> > +          && !city_got_citywalls(acity)) {
> > +        /* FIXME: Use acity->ai.wallvalue? --pasky */
> > +        vuln *= 9;
> 
> I wish I knew why used 9 instead of 3. I can only assume this is due to those
> goddamn scaling factors, and this probably explains why we divide results by
> 30.

Why should we use 3?

> > -  pcity->ai.a = val2;
> > -  pcity->ai.f = val3;
> > +  pcity->ai.attack = val2;
> > +  pcity->ai.bcost = val3;
> 
> Crap names.

Hm? Tell me better.

Improved patch attached (use of POWER_DIVIDER on two places, few comments
added).

-- 

                                Petr "Pasky" Baudis

* elinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
"If you have acquired knowledge, what do you lack?
    If you lack knowledge, what have you acquired?"
Lev. R. 1:6
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/

want.patch
Description: Text document