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

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Thu, Feb 21, 2002 at 06:07:19AM CET, I got a letter,
where Mike Kaufman <kaufman@xxxxxxxxxxxxxxxxxxxxxx> told me, that...
> /*************************************************************************
> This function looks at tiles directly neighbouring the unit in order to find
> something to kill or explore.  It prefers tiles in the following order:
> 
> 1. Undefended cities
> 2. Huts
> 3. Enemy units weaker than the unit
> 4. Land barbarians also like unfrastructure tiles (for later pillage)
> 
> If none of the following is there, nothing is chosen and dest_x, dest_y is set
> to actual punit's position.
> 
> Returns value of the tile which has been chosen (MAY BE NEGATIVE!).
> **************************************************************************/
> static int ai_military_findvictim(struct player *pplayer, struct unit *punit,
>                                   int *dest_x, int *dest_y)
> {
> >>  /* Sets the target tile as the best (with value new_best). */
>   /* Set the current tile as the best (with value new_best). */

Ok.

> #define SET_TARGET(new_best) \
>   do { best = (new_best); *dest_x = x1; *dest_y = y1; } while (0)
> >> I think I do like SET_BEST better (as long as there's a comment as above)

Fine :).

>   int bellig = unit_belligerence_primitive(punit);
>   int x = punit->x, y = punit->y;
>   int best = 0;
>   
>   *dest_y = y;
>   *dest_x = x;
>   
> >> add agreed upon comment here

Done.

>   if (punit->unhappiness > 0) {
>     best = 0 - 2 * MORT * TRADE_WEIGHTING; /* desperation */
>   }
> 
>   /* Ferryboats with passengers do not attack. -- Syela */
>   if (punit->ai.passenger > 0) {
>     return 0;
>   }
> 
>   adjc_iterate(x, y, x1, y1) {
>     struct unit *pdef = get_defender(punit, x1, y1);
>     
>     if (pdef) {
>       struct unit *patt = get_attacker(punit, x1, y1);
> 
>       if (!can_unit_attack_tile(punit, x1, y1))
>         continue;
> 
>       /* If we are in the city, let's deeply consider defending it - however,
>        * horsemen in city refused to attack phalanx just outside that was
>        * bodyguarding catapult; thus, we get the best attacker on the tile as
>        * well, for the case when there are multiple different units on one
>        * title. Thus we force punit to attack a stack of units if they are
>        * endangering punit seriously, even if they aren't that weak. */
> 
> >> /* FIXME: there ought to be a better metric for determining this */

Ok.

>       /* FIXME: The get_total_defense_power(pdef, punit) should probably use
>        * patt rather than pdef. */
>       if (map_get_city(x, y)
>           && get_total_defense_power(pdef, punit) *
> >> this comment wrap here. fix it.

Ok.

>              get_total_defense_power(punit, pdef) >= /* didn't like > -- 
> Syela */
>              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);
> >> I thought we discussed this... I think you're going to need to go with: 
> 
>   int attack;  /* The total possible attack power we can throw on the victim. 
>                 * that we will even square this. */
>   int benefit; /* Something like 'attractiveness' of the victim, how nice it
>                 * would be to destroy it. Larger value, worse loss for enemy. 
> */
>   ...
> 
>   attack = reinforcements_value(punit, pdef->x, pdef->y) + bellig;
>   attack *= attack;
>   benefit = unit_type(pdef)->build_cost;
> 
>   ...
> 
> >> to satisfy me. (see how much nicer that looks).

I like mine more :). Still I do. And I see others are reusing my style of
comments as well. So let's stop it now or keep it for some long time. I'll ask
people publicly what they think and if they'll agree with you, I'll do that so,
otherwise I'll scream and resist .. ;-)

> 
>         /* The total possible attack power we can throw on the victim. Note
>          * that we will even square this. */
> >> why squaring? Isn't this value totally dependent on the stats of the 
> >> attacking unit?

It is. It contradicts? The squaring is there probably because .. well, Syela
used it in such cases ;). It emphasizes the effect sufficiently :).

>         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;
>         /* We're only dealing with adjacent victims here. */
>         /* TODO: Remove this when moving the desire equation to separate
>          * function. */
>         int move_cost = 0;
>         /* The possible loss when we would loose this unit. */
> >> rant: what is it with the word "loose?" yes, it is _possible_ to use this 
> >> word here as in "we shall loose our weapons upon the enemy", but, I'm not 
> >> sure you meant this here. I think you want 
> >> "'production' loss if our unit dies in attack"
> >> PS: out of curiosity: when did "loose" become synonymous with "lose"
> >> is this a 'net thing? is it a l33t hacker thing? or is it just Europeans 
> >> using english as a second language? 

Ok.

It is just Europeans using english as a second language :). I myself use
sometimes l33t hacker things in ironical context, but never otherwise.

>         /* TODO: Possibly remove this when moving the desire equation to
>          * separate function? */
>         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)) {
> >> perhaps a mention that the 40 here is Syela "arbitrariness"

Ok.

Improved patch attached. Hopefully we're getting near.

-- 

                                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