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

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Petr Baudis <pasky@xxxxxxxxxxx> wrote: > 
 
<snip> 
> Updated patch attached. Compiles ;-).
> 
> I'm now not sure if ai_manage_explore() wouldn't be even more appropriate.
> What
> you guys think?
>

I think I liked GB's warmap patch better. But it looks like that will never see
the light of day. Your patch is certainly am improvement on the existing state
of the AI.


> .
> Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/
> > Index: ai/aiunit.c
> ===================================================================
> RCS file: /home/cvs/aiciv/freeciv-a2/ai/aiunit.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 aiunit.c
> --- ai/aiunit.c       19 Dec 2001 20:43:22 -0000      1.1.1.1
> +++ ai/aiunit.c       7 Jan 2002 21:50:43 -0000
> @@ -177,191 +177,313 @@
>  }
>   
>  /**************************************************************************
> -Explores unknown territory, finds huts.
> +Handle eXplore mode of a unit - explores unknown territory, finds huts.
>  Returns whether there is any more territory to be explored.
>  **************************************************************************/
> -int ai_manage_explorer(struct unit *punit)
> +int ai_manage_exploring(struct unit *punit)
>  {
> +  /* The unit's owner */
>    struct player *pplayer = unit_owner(punit);
> -  int x, y; /* is the position of the unit; updated inside the function */
> -  int con; /* continent the unit is on */
> -  struct city *pcity;
> -  int id = punit->id; /* we can now die because easy AI may accidently
> -                      stumble on huts it fuzzily ignored */
> -  int best_x = -1, best_y = -1;
> +  /* The position of the unit; updated inside the function */
> +  int x, y;
> +  /* Continent the unit is on */
> +  int continent;
> +  /* Unit's speed */
>    int move_rate = unit_move_rate(punit);
> +  /* Range of unit's vision */
>    int range;
>  
> +  /* Get the range */
> +
>    if (unit_profits_of_watchtower(punit)
>        && map_get_tile(punit->x, punit->y)->special & S_FORTRESS)
> -    range =get_watchtower_vision(punit);
> +    range = get_watchtower_vision(punit);
>    else
>      range = unit_type(punit)->vision_range;
>

GB got rid of the units profitting from the wachtower vision. He said that
apparently it made no difference, since this was the starting destination,
and our end point was elsewhere. I am sure he can explain it better.

  
> +  /* Idle unit */
> +
>    if (punit->activity != ACTIVITY_IDLE)
>      handle_unit_activity_request(punit, ACTIVITY_IDLE);
>  
> -  x = punit->x; y = punit->y;
> -  if (is_ground_unit(punit)) con = map_get_continent(x, y);
> -  else con = 0; /* Thanks, Tony */
> +  /* Localize the unit */
> +
> +  x = punit->x;
> +  y = punit->y;
> +  
> +  if (is_ground_unit(punit)) {
> +    continent = map_get_continent(x, y);
> +  } else {
> +    continent = 0;
> +  }
>  
>    /* CPU-expensive but worth it -- Syela */
>    generate_warmap(map_get_city(x, y), punit);
>  
> -  /* BEGIN PART ONE: Look for huts.  Non-Barbarian Ground units ONLY. */
> +  /*
> +   * PART 1: Look for huts
> +   * Non-Barbarian Ground units ONLY.
> +   */
> +  

I prefer your way. It looks like time for a jehad on old style comments.

>    if (!is_barbarian(pplayer)
> -      && is_ground_unit(punit)) { /* boats don't hunt huts */
> +      && is_ground_unit(punit)) {
> +    /* Maximal acceptable move cost to the target */
>      int maxcost = pplayer->ai.control ? 2 * THRESHOLD : 3;

You may want to add more stuff to README.AI on the wonders of threshold.

> +    /* Move cost to the best target (=> lower is better) */
>      int bestcost = maxcost * SINGLE_MOVE + 1;

You did not explain why we add 1. It's so that we can find other paths
that have the same movecost as the minimum cost one(due to djikstras).
If you do this elsewhere, forget it.

> +    /* Desired destination */
> +    int best_x = -1, best_y = -1;
>  
> -    /* Iterating outward so that with two tiles with the same movecost
> -       the nearest is used */
> +    /* We're iterating outward so that with two tiles with the same movecost
> +     * the nearest is used. */
>      iterate_outward(x, y, maxcost, x1, y1) {
>        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)) {
> -     best_x = x1;
> -     best_y = y1;
> -     bestcost = warmap.cost[best_x][best_y];
> +          && warmap.cost[x1][y1] < bestcost
> +          && (!ai_handicap(pplayer, H_HUTS) || map_get_known(x1, y1,
> pplayer))

A comment here saying which handicaps are affected by which ai difficulty
levels
may be handy. 

> +          && tile_is_accessible(punit, x1, y1)
> +          && ai_fuzzy(pplayer, 1)) {
> +        best_x = x1;
> +        best_y = y1;
> +        bestcost = warmap.cost[best_x][best_y];
>        }
>      } iterate_outward_end;
> +    
>      if (bestcost <= maxcost * SINGLE_MOVE) {
> +      /* We can die because easy AI may accidently stumble on huts it
> fuzzily
> +       * ignored - unit_id is used for the check of this */
> +      int unit_id = punit->id;
> +
> +      /* Go there! */
>        punit->goto_dest_x = best_x;
>        punit->goto_dest_y = best_y;
>        set_unit_activity(punit, ACTIVITY_GOTO);
>        do_unit_goto(punit, GOTO_MOVE_ANY, 0);
> -      if (!player_find_unit_by_id(pplayer, id))
> -     return 0; /* died */
> +      
> +      if (!player_find_unit_by_id(pplayer, unit_id)) {
> +        /* We're dead. */
> +        return 0;
> +      }
>  
>        if (punit->moves_left) {
> -     if (punit->x == best_x && punit->y == best_y) {
> -       return ai_manage_explorer(punit);
> -     } else {
> -       /* Something went wrong; fall through. This should almost never 
> happen.
> */
> -       if (punit->x != x || punit->y != y)
> -         generate_warmap(map_get_city(punit->x, punit->y), punit);
> -       x = punit->x; y = punit->y;
> -       /* Fallthough to next fase */
> -     }
> +        /* We can still move on... */
> +
> +        if (punit->x == best_x && punit->y == best_y) {
> +          /* ...and got into desired place. */
> +          return ai_manage_exploring(punit);
> +  
> +        } else {
> +          /* Something went wrong. This should almost never happen. */
> +          if (punit->x != x || punit->y != y)
> +            generate_warmap(map_get_city(punit->x, punit->y), punit);
> +          
> +          x = punit->x;
> +          y = punit->y;
> +          /* Fallthrough to next part. */
> +        }
> +
>        } else {
> -     return 1;
> +        return 1;
>        }
>      }
>    }
>  
> -  /* BEGIN PART TWO: Move into unexplored territory */
> -  /* move the unit as long as moving will unveil unknown territory */
> +  /* 
> +   * PART 2: Move into unexplored territory
> +   * Move the unit as long as moving will unveil unknown territory
> +   */
> +  
>    while (punit->moves_left) {
> +    /* Best (highest) number of unknown tiles adjectent (in vision range) */
>      int most_unknown = 0;
> -    int unknown;
> +    /* Desired destination */
> +    int best_x = -1, best_y = -1;
>  
> -    /* evaluate all adjacent tiles */
> +    /* Evaluate all adjacent tiles. */
> +    

Someone posted an algorithm to the mailing list, showing that moving in a zig
zag pattern was the best way to explore mostly uncovered territory.

>      square_iterate(x, y, 1, x1, y1) {
> -      unknown = 0;
> +      /* Number of unknown tiles in vision range around this tile */
> +      int unknown = 0;
> +      
>        square_iterate(x1, y1, range, x2, y2) {
> -     if (!map_get_known(x2, y2, pplayer))
> -       unknown++;
> +        if (!map_get_known(x2, y2, pplayer))
> +          unknown++;
>        } square_iterate_end;
>  
> -      if (unknown > most_unknown && (!unit_flag(punit, F_TRIREME)
> -                                  || trireme_loss_pct(pplayer, x1,
> -                                                      y1) == 0)
> -       && map_get_continent(x1, y1) == con
> -       && can_unit_move_to_tile_with_notify(punit, x1, y1, 0)
> -       && !((pcity = map_get_city(x1,y1))
> -            && (unit_flag(punit, F_DIPLOMAT)
> -                || unit_flag(punit, F_CARAVAN)))
> -       && !(is_barbarian(pplayer) && map_get_special(x1, y1) & S_HUT)) {
> -     most_unknown = unknown;
> -     best_x = x1;
> -     best_y = y1;
> +      if (unknown > most_unknown) {
> 

The patch looks ok, I have not tested it yet to see if it compiles.

__________________________________________________
Do You Yahoo!?
Send FREE video emails in Yahoo! Mail!
http://promo.yahoo.com/videomail/