[Freeciv-Dev] Re: [PATCH] aiunit.c ai_military_findvictim() cleanup (PR#1264)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Fri, Feb 22, 2002 at 11:59:29AM CET, I got a letter,
where Raahul Kumar <raahul_da_man@xxxxxxxxx> told me, that...
> > +sane number means ordinar value of the victim
> 
> ordinar = ordinary.

Oops, yes sir.

> > +1       means barbarians wanting to pillage
> > +0       means nothing found or error
> > +- 2 * MORT * TRADE_WEIGHTING means nothing found and punit causing
> > unhappiness
> 
> Why the -? 

Because it is negative? (hint provided also by the fact that it's bellow zero)

> >  **************************************************************************/
> >  static int ai_military_findvictim(struct player *pplayer, struct unit
> > *punit,
> > -                             int *dest_x, int *dest_y)
> > +                                  int *dest_x, int *dest_y)
> >  {
> > -  int x, y;
> > -  int best = 0, a, b, c, d, e, f;
> > -  struct unit *pdef;
> > -  struct unit *patt;
> > -  struct city *pcity;
> > -  x = punit->x;
> > -  y = punit->y;
> > +  /* Set the tile with our target as the best (with value new_best). */
> > +#define SET_BEST(new_best) \
> > +  do { best = (new_best); *dest_x = x1; *dest_y = y1; } while (0)
> > +
> > +  int bellig = unit_belligerence_primitive(punit);
> 
> Greg already said this, but there has to a better word than bellig. Angry,
> pissed off strength, might, hardness toughness, offensive, etc.

But it IS belligerence :^). And renaming this function and possibly variable is
out of scope of this patch. And no, I'm not going to have variable name and
function name different, as I consider it confusing, compared to current
situation ;).

> > +  int x = punit->x, y = punit->y;
> > +  int best = 0;
> > +  
> >    *dest_y = y;
> >    *dest_x = x;
> > -  if (punit->unhappiness) best = 0 - 2 * MORT * TRADE_WEIGHTING; /*
> > desperation */
> > -  f = unit_type(punit)->build_cost;
> > -  c = 0; /* only dealing with adjacent victims here */
> > -
> > -  if (get_transporter_capacity(punit)) {
> > -    unit_list_iterate(map_get_tile(x, y)->units, aunit)
> > -      if (!is_sailing_unit(aunit)) return(0);
> > -    unit_list_iterate_end;
> > -  } /* ferryboats do not attack.  no. -- Syela */
> > +  
> > +  if (punit->unhappiness > 0) {
> > +    /* FIXME: Seems obsolete as we care only about sufficiently high values
> > and
> 
> I believe but I am not sure, that the AI does not become democratic or
> republic.
> It's always on the offensive, so the unhappiness would kill it. Is unit
> unhappiness actually an issue? 

AI does become republic, but not democracy (AFAIK). However .. see last Greg's
mail about this issue :).

Going to comment this and check if savegames are same (one has to do at
least best = -1, otherwise we may get catched by the pillage code when
barbarians)...

Ok, they aren't. What broke our predictions today was the fact that desire may
be negative - thus the mean of the unhappy calculation was completely opposite
to the one we thought.

> > +     * it looks like noone has idea about purpose of this. --pasky */
> > +    best = 0 - 2 * MORT * TRADE_WEIGHTING; /* desperation */
> > +  }
..snip..
> > +      if (map_get_city(x, y)
> > +          && get_total_defense_power(pdef, punit) *
> > +             get_total_defense_power(punit, pdef) >= /* didn't like >
> > --Syela */
> 
> Yuck. Horribly formatted.

This is an exception and I think it's best this way here. It doesn't wrap at
least :^).

> > +             get_total_attack_power(patt, punit) *
> > +             get_total_attack_power(punit, pdef)
> > +          && unit_list_size(&(map_get_tile(punit->x, punit->y)->units)) < 2
> > +          && get_total_attack_power(patt, punit) > 0) {
> > +        freelog(LOG_DEBUG, "%s defending %s from %s's %s",
> > +                unit_type(punit)->name,
> > +                map_get_city(x, y)->name,
> > +                unit_owner(pdef)->name, unit_type(pdef)->name);
> > +        
> > +      } else {
> > +        int vuln = unit_vulnerability(punit, pdef);
> > +        /* The total possible attack power we can throw on the victim. Note
> > +         * that we will even square this. */
> > +        int attack = reinforcements_value(punit, pdef->x, pdef->y)
> > +                     + bellig;
> > +        /* Something like 'attractiveness' of the victim, how nice it would
> > be
> > +         * to destroy it. Larger value, worse loss for enemy. */
> > +        int benefit = unit_type(pdef)->build_cost;
> 
> Excellent name. This crops up in other funcs in the AI, I expect to see that
> in future patches.

I probably do too.

> > +        /* We're only dealing with adjacent victims here. */
> > +        int move_cost = 0;
> > +        /* The possible loss when we would lose the unit we want to attack.
> > */
> > +        int loss = unit_type(punit)->build_cost;
> > +        
> > +        attack *= attack;
> > +        
> > +        /* If the victim is in the city, we increase the benefit and 
> > correct
> > +         * it with our health because there may be more units in the city
> > +         * stacked, and we won't destroy them all at once, so in the next
> > +         * turn they may attack us. So we shouldn't send already injured
> > +         * units to useless suicide. */
> > +        if (map_get_city(x1, y1)) {
> > +          /* 40 is something like value of a city, as used in many other
> > parts
> > +           * of code (?). --pasky */
> 
> Value of a city? I thought it was shield cost of a riflemen.

It can't be a same number? :)

> > +          benefit = (benefit + 40) * punit->hp / unit_type(punit)->hp;
..snip..

Improved patch attached (we finally comment the unhappy equation propeerly). I
also made blank lines between declarations due to popular demand (hopefully
everyone will be happy with this; even Mike proposed this for very long
comments :).

-- 

                                Petr "Pasky" Baudis

* elinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
No one can feel as helpless as the owner of a sick goldfish.
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/

findvictim-cleanup.patch
Description: Text document