Complete.Org: Mailing Lists: Archives: freeciv-dev: November 2001:
[Freeciv-Dev] Re: PATCH: AI cleanup Version 2
Home

[Freeciv-Dev] Re: PATCH: AI cleanup Version 2

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Petr Baudis <pasky@xxxxxxxxxxx>
Cc: Raahul Kumar <raahul_da_man@xxxxxxxxx>, Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>, freeciv development list <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: PATCH: AI cleanup Version 2
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 23 Nov 2001 21:54:27 +0100
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Fri, Nov 23, 2001 at 08:42:06PM +0100, Petr Baudis wrote:
> > Added the patch that Raimar sent. I'm almost ready to finish the changes to
> > the ai. Version 3 will wrap up all changes. This patch applies cleanly
> > against Nov 20 CVS, the autogames are not changed.
> Made some cleanups, another indentation changes, some variable naming fixes,
> and that findvictim cleanup. Applies on the top of your patch. Just cosmetical
> changes, no real code changed afaik.

> -  if (is_tiles_adjacent(x0, y0, myunit->x, myunit->y)
> -      && !is_non_allied_unit_tile(map_get_tile(x0, y0),
> -                               unit_owner(myunit)))
> +  
> +  if (is_tiles_adjacent(x0, y0, myunit->x, myunit->y) &&
> +      !is_non_allied_unit_tile(map_get_tile(x0, y0), unit_owner(myunit)))

This is a formatting change. See past threads about such changes.

> -  if (result == MR_ZOC) {
> +  if (result == MR_ZOC)
>      if (could_be_my_zoc(punit, src_x, src_y))
>        return -1;
> -  }
> +  

I will definitely not accept changes which remove {}. 

> +  
>    if (pcity->ai.building_want[B_SAM] > 0 && danger4) {
>      i = assess_defense_igwall(pcity);
> +    
>      if (!i) pcity->ai.building_want[B_SAM] = 100 + urgency;
>      else if (danger4 > i * 2) pcity->ai.building_want[B_SAM] = 200 + urgency;
>      else pcity->ai.building_want[B_SAM] = danger4 * 100 / i;
>    }
> +  
>    if (pcity->ai.building_want[B_SDI] > 0 && danger5) {
>      i = assess_defense_igwall(pcity);
> +    
>      if (!i) pcity->ai.building_want[B_SDI] = 100 + urgency;
>      else if (danger5 > i * 2) pcity->ai.building_want[B_SDI] = 200 + urgency;
>      else pcity->ai.building_want[B_SDI] = danger5 * 100 / i;
>    }
> +  
>    pcity->ai.urgency = urgency; /* need to cache this for bodyguards now -- 
> Syela */
>    return(urgency);
>  }

Such changes are only noise. Also note that i can be declared local
(which I prefer).

Overall I have to look at the other changes in detail. It may also be
nice if you add some docu to the variables. Like in

/* 
 * Desire to get the unit denotes by "punit". Value range is [0,100)
 * where 0 means no interest. 
 */
int udesire;

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I do feel kind of sorry for Microsoft. Their attornies and marketing
  force must have tons of ulcers trying to figure out how to beat (not
  just co-exist with) a product that has no clearly defined (read
  suable) human owner, and that changes on an hourly basis like the
  sea changes the layout of the sand on a beach. Severely tough to
  fight something like that."
    -- David D.W. Downey at linux-kernel


[Prev in Thread] Current Thread [Next in Thread]