[Freeciv-Dev] Re: [RFC] square_dxy_iterate

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Sat, Feb 23, 2002 at 05:29:47PM CET, I got a letter,
where Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> told me, that...
> Index: ai/aiunit.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
> retrieving revision 1.176
> diff -u -r1.176 aiunit.c
> --- ai/aiunit.c       2002/02/22 13:14:38     1.176
> +++ ai/aiunit.c       2002/02/23 16:28:32
> @@ -601,17 +601,37 @@
>    return(val);
>  }
>  
> -static void invasion_funct(struct unit *punit, bool dest, int n, int which)
> -{ 
> +/*************************************************************************
> +  Mark an invasion threat of punit in the surrounding cities. The
> +  given radius limites the area which is searched for cities. The
> +  center of the area is either the unit itself (dest == FALSE) or the
> +  destiniation of the current goto (dest == TRUE). The invasion threat
> +  is marked in pcity->ai.invasion via ORing the which argument (to
> +  tell attack from sea apart from ground unit attacks). Note that
> +  which should only have one bit set.
> +**************************************************************************/
> +static void invasion_funct(struct unit *punit, bool dest, int radius,
> +                        int which)
> +{
>    int x, y;
> -  if (dest) { x = punit->goto_dest_x; y = punit->goto_dest_y; }
> -  else { x = punit->x; y = punit->y; }
> +  bool flag = (dest || punit->activity != ACTIVITY_GOTO);

I would rename it too. What about just want_attack?

> + 
> +  if (dest) {
> +    x = punit->goto_dest_x;
> +    y = punit->goto_dest_y;
> +  } else {
> +    x = punit->x;
> +    y = punit->y;
> +  }
>  
> -  square_iterate(x, y, n, i, j) {
> +  square_iterate(x, y, radius, i, j) {

x1, y1 would be better.

>      struct city *pcity = map_get_city(i, j);
> -    if (pcity && pcity->owner != punit->owner)
> -      if (dest || punit->activity != ACTIVITY_GOTO || !has_defense(pcity))
> -     pcity->ai.invasion |= which;
> +
> +    if (pcity && pcity->owner != punit->owner

I agree with you that there's no urgent need to check for allies here now.
Maybe leaving a little TODO around would help in the future, though.

> +     && (pcity->ai.invasion & which) != which &&

Move the last && to next line.

> +     (flag || !has_defense(pcity))) {
> +      pcity->ai.invasion |= which;
> +    }
>    } square_iterate_end;
>  }

It's polluted by tabs ;) thus making the patch hard to read etc.

It's certainly an improvement (and the behaviour shouldn't be changed, except
that it will be faster ;). I've no other problems with the patch.

-- 

                                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/