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

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Fri, Feb 22, 2002 at 05:41:38PM CET, I got a letter,
where Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> told me, that...
> On Fri, Feb 22, 2002 at 03:34:10PM +0100, Petr Baudis wrote:
> >  1. Undefended cities
> >  2. Huts
> >  3. Enemy units weaker than the unit
> >  4. Land barbarians also like unfrastructure tiles (for later pillage)
> 
> > +99999   means empty city
> > +99998   means hut
> > +sane number means ordinary value of the victim
> > +1       means barbarians wanting to pillage
> > +0       means nothing found or error
> > +-2*MORT*TRADE_WEIGHTING means nothing found and punit causing unhappiness
> 
> Can't these two blocks be merged?

Done.

> > +      struct unit *patt = get_attacker(punit, x1, y1);
> > +
> 
> > +      if (!can_unit_attack_tile(punit, x1, y1))
> > +        continue;
> 
> {}

Yes sir.

> > +      /* 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
> 
> s/on the tile/on the tile (x1, y1)/
> 
> > +       * 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
> 
> s/on one title/on the tile (x1, y1)/

Done.

> > +       * endangering punit seriously, even if they aren't that weak. */
> > +      /* FIXME: The get_total_defense_power(pdef, punit) should probably 
> > use
> > +       * patt rather than pdef. There also ought to be a better metric for
> > +       * determining this. */
> > +      if (map_get_city(x, y)
> > +          && get_total_defense_power(pdef, punit) *
> 
> > +             get_total_defense_power(punit, pdef) >= /* didn't like > 
> > --Syela */
> 
> Who doesn't like it? I would say: remove it.

That's speech of a man! :)

> > +             get_total_attack_power(patt, punit) *
> > +             get_total_attack_power(punit, pdef)
> 
> Can't this test be replaced with win_chance?

That's something that can be hidden under 'better metric'. Certainly outside
scope of this patch, I think.

> > +          && unit_list_size(&(map_get_tile(punit->x, punit->y)->units)) < 2
> 
> Constant. Should be moved outside the loop.

Done. Also reformated the condition, I wonder how much are you going to like it
now ;).

> > +        if (map_get_city(x1, y1)) {
> > +          /* 40 is something like value of a city, as used in many other 
> > parts
> > +           * of code (?). --pasky */
> > +          benefit = (benefit + 40) * punit->hp / unit_type(punit)->hp;
> 
> s/40/unit_value(get_role_unit(F_CITIES, 0))/

*THAT* is it!

> > +          && could_unit_move_to_tile(punit, punit->x, punit->y, x1, y1) != > > 0
> 
> Are you sure that you don't mean can_unit_move_to_tile here?

Why I should? It looks to me that only difference is that
could_unit_move_to_tile takes ZOC into account - and as we are going to move
there now, it seems as a good thing (tm) to me.

> > +          && !is_barbarian(unit_owner(punit))
> > +          && punit->ai.ai_role != AIUNIT_ESCORT
> 
> > +          && punit->ai.charge == 0 /* Above line doesn't seem to work. :( 
> > */
> 
> Please explain.

That was Syela! :) Sorry, but I fear I can't explain it well yet. It's possible
that we're not consistent in keeping those two values in sync; not really an
idea.

> > +          && get_tile_infrastructure_set(map_get_tile(x1, y1)) != 0
> 
> Although the return type of get_tile_infrastructure_set is an int it
> really returns enum tile_special_type. So please use S_NO_SPECIAL.

Yes sir.

> >  /*************************************************************************
> > Index: aiunit.h
> > ===================================================================
> > RCS file: /home/cvs/aiciv/freeciv-a2/ai/aiunit.h,v
> > retrieving revision 1.1.1.4
> > retrieving revision 1.6
> > diff -u -r1.1.1.4 -r1.6
> 
> Something missing?

CVS is buggy. This is some arcane residual thing after explorer cleanup, no
idea how to get rid of it, lazy to manually clean it everytime and it's not
that much data :).

> Side note: it would be nice if you put a serial number on the patch
> file/subject.

Good idea. I'll put revision number of aiunit.c there.

Improved patch attached. Also is_non_allied_city_tile went there.

-- 

                                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