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

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Petr Baudis <pasky@xxxxxxxxxxx> wrote: 
[..]
> >
> >  a. determines unit's vision range at (x,y) -- if the player has no
> >  info on
> >  (x,y), assume there is no watchtower
> > 
> >  b. count the number of unknown location within the vision range. 
> >  This
> >  function would be used at least in 2 places, so it's worth it.
> And it's worth separate patch. I don't want to get my patch too big.
> Are you willing to make it?

I am willing but incapable right now.  So maybe some time...
I would prefer if you removed the watchtower code or at least added a
FIXME, because it is a bug.

> > 2. Comments like
> > > +  /* The unit's owner */
> > >    struct player *pplayer = unit_owner(punit);
> > are completely useles IMO, as the function name says the same thing.
> There were some notes about commenting etc ;p. I wanted to see how many
> people will catch this and throw it on me - you were the only, quite
> funny ;).

I don't think many people read your patch :(

> > 3. Splitting of long if (...) condition into many if() continue;
> > is nice.  There is another place that you can do the same.
> Which one?

I think I mant this one:
>        if (map_get_special(x1, y1) & S_HUT
> -       && warmap.cost[x1][y1] < bestcost
> -       && (!ai_handicap(pplayer, H_HUTS) || map_get_known(x1, y1,
pplayer))
> -       && tile_is_accessible(punit, x1, y1)
> -       && ai_fuzzy(pplayer, 1)) {

But it's all non-critical.

> Improved patch attached. I also added symbolic constant for the '9' in
> part 3 move cost applying to unknown. I would still like to get some
> more explanation of this, if anyone has an idea.

not likely.
the underlying idea is surely to say "if we go a tad further but can
uncover lot's of uncharted land, go for it".  But then 9 is too large a
value for this, I would have gone for 9/SINGLE_MOVE at most...

Again, I think that changing function name is noise.

If savegames are identical, the patch should be applied and man-resources
should be used on cleaning up something more "dirty" than expolring code.
:)

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