[Freeciv-Dev] Re: [Patch] Cleanup of tile_move_cost_ai

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> I thought about using the move_cost field of struct tile for the goto
> agent. However I had hard time to understand tile_move_cost_ai and the
> (to me mysterious) -3. The attached patch should allow others not the
> fall into this.
> 

The -3 move cost does seem to be mysterious. As far as I know, that number
means nothing in particular, i.e -3 is never added or subtracted to in any way,
could as easy be -5 or any other negative number. If that number does mean
something,
could you please add that in the comments?

> It doesn't change any behavior. Well at least I hope so ;)
> 
>       Raimar
> 
> -- 
>  email: rf13@xxxxxxxxxxxxxxxxx
>  "That's fundamental game play!  My main enemy is *ALWAYS* fighting 
>   a 4-front war.  I make sure of it!"
>     -- Tony Stuckey, freeciv-dev
> > Index: client/goto.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
> retrieving revision 1.26
> diff -u -r1.26 goto.c
> --- client/goto.c     2001/09/23 16:09:34     1.26
> +++ client/goto.c     2001/10/07 11:48:26
> @@ -390,7 +390,7 @@
>                  || is_non_allied_city_tile(pdesttile, unit_owner(punit))) {
>         add_to_queue = 0;
>         move_cost = SINGLE_MOVE;
> -     } else if (psrctile->move_cost[dir] != -3) {/*is -3 if sea units can 
> move
> between*/
> +     } else if (psrctile->move_cost[dir] != MOVE_COST_FOR_VALID_SEA_STEP) {
>         continue;
>       } else if (unit_flag(punit, F_TRIREME) 
>                  && trireme_loss_pct(unit_owner(punit), x1, y1) > 0) {
> Index: common/map.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
> retrieving revision 1.93
> diff -u -r1.93 map.c
> --- common/map.c      2001/10/01 11:08:13     1.93
> +++ common/map.c      2001/10/07 11:48:28
> @@ -999,27 +999,46 @@
>  }
>  
>  /***************************************************************
> -  Similar to tile_move_cost_ptrs, but for pre-calculating
> -  tile->move_cost[] values for use by ai (?)
> -  If either terrain is T_UNKNOWN, then return 'maxcost'.
> -  If both are ocean, or one is ocean and other city,
> -  return -3, else if either is ocean return maxcost.
> -  Otherwise, return normal cost (unit-independent).
> +  tile_move_cost_ai is used to fill the move_cost array of struct
> +  tile. The cached values of this array are used in server/gotohand.c
> +  and client/goto.c. tile_move_cost_ai returns the move cost as
> +  calculated by tile_move_cost_ptrs (with no unit pointer to get
> +  unit-independent results) EXCEPT if either of the source or
> +  destination tile is an ocean tile. Then the result of the method
> +  shows if a ship can take the step from the source position to the
> +  destination position (return value is MOVE_COST_FOR_VALID_SEA_STEP)
> +  or not (return value is maxcost).
> +
> +  A ship can take the step if:
> +    - both tiles are ocean or
> +    - one of the tiles is ocean and the other is a city or is unknown
>  ***************************************************************/
>  static int tile_move_cost_ai(struct tile *tile0, struct tile *tile1,
>                            int x, int y, int x1, int y1, int maxcost)
>  {
>    assert(is_real_tile(x, y));
> -  assert(!is_server || (tile0->terrain != T_UNKNOWN && tile1->terrain !=
> T_UNKNOWN));
> +  assert(!is_server
> +      || (tile0->terrain != T_UNKNOWN && tile1->terrain != T_UNKNOWN));
> +
> +  if (tile0->terrain == T_OCEAN && tile1->terrain == T_OCEAN) {
> +    return MOVE_COST_FOR_VALID_SEA_STEP;
> +  }
> +
> +  if (tile0->terrain == T_OCEAN
> +      && (tile1->city || tile1->terrain == T_UNKNOWN)) {
> +    return MOVE_COST_FOR_VALID_SEA_STEP;
> +  }
> +
> +  if (tile1->terrain == T_OCEAN
> +      && (tile0->city || tile0->terrain == T_UNKNOWN)) {
> +    return MOVE_COST_FOR_VALID_SEA_STEP;
> +  }
>  
> -  if (tile0->terrain == T_OCEAN) {
> -    return (tile1->city || tile1->terrain == T_OCEAN
> -         || tile1->terrain == T_UNKNOWN) ? -3 : maxcost;
> -  } else if (tile1->terrain == T_OCEAN) {
> -    return (tile0->city || tile0->terrain == T_UNKNOWN) ? -3 : maxcost;
> -  } else {
> -    return tile_move_cost_ptrs(NULL, tile0, tile1, x, y, x1, y1);
> +  if (tile0->terrain == T_OCEAN || tile1->terrain == T_OCEAN) {
> +    return maxcost;
>    }
> +
> +  return tile_move_cost_ptrs(NULL, tile0, tile1, x, y, x1, y1);
>  }
>  
>  /***************************************************************
> Index: common/map.h
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
> retrieving revision 1.94
> diff -u -r1.94 map.h
> --- common/map.h      2001/09/27 22:49:53     1.94
> +++ common/map.h      2001/10/07 11:48:28
> @@ -20,7 +20,8 @@
>  
>  struct Sprite;                       /* opaque; client-gui specific */
>  
> -#define NUM_DIRECTION_NSEW 16
> +#define NUM_DIRECTION_NSEW           16
> +#define MOVE_COST_FOR_VALID_SEA_STEP (-3)
>  
>  struct map_position {
>    int x,y;
> Index: server/gotohand.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
> retrieving revision 1.121
> diff -u -r1.121 gotohand.c
> --- server/gotohand.c 2001/10/06 12:01:29     1.121
> +++ server/gotohand.c 2001/10/07 11:48:30
> @@ -342,11 +342,13 @@
>          move_cost = SINGLE_MOVE;
>          move_cost += warmap.seacost[x][y];
>          if (warmap.seacost[x1][y1] > move_cost && move_cost < maxcost) {
> -       /* by adding the move_cost to the warmap regardless if we can move
> between
> -          we allow for shore bombardment/transport to inland positions/etc. 
> */
> +       /* by adding the move_cost to the warmap regardless if we
> +          can move between we allow for shore bombardment/transport
> +          to inland positions/etc. */
>            warmap.seacost[x1][y1] = move_cost;
> -          if (ptile->move_cost[dir] == -3) /* -3 means ships can move
> between */
> +       if (ptile->move_cost[dir] == MOVE_COST_FOR_VALID_SEA_STEP) {
>           add_to_mapqueue(move_cost, x1, y1);
> +       }
>       }
>       break;
>        default:
> @@ -702,9 +704,11 @@
>       if (warmap.seacost[x1][y1] <= warmap.seacost[x][y])
>         continue; /* No need for all the calculations */
>  
> -     if (psrctile->move_cost[dir] != -3 /* is -3 if sea units can move 
> between
> */
> -         && (dest_x != x1 || dest_y != y1)) /* allow ships to target a shore 
> */
> +     /* allow ships to target a shore */
> +     if (psrctile->move_cost[dir] != MOVE_COST_FOR_VALID_SEA_STEP
> +         && (dest_x != x1 || dest_y != y1)) {
>         continue;
> +     }
>       else if (unit_flag(punit, F_TRIREME)
>                && trireme_loss_pct(unit_owner(punit), x1, y1) > 0) {


>         move_cost = 2*SINGLE_MOVE+1;

Isn't this line done in GB's warmap_func2.diff? He does it in the warmap
instead of this function, seems to be a duplication of code. 


__________________________________________________
Do You Yahoo!?
NEW from Yahoo! GeoCities - quick and easy web site hosting, just $8.95/month.
http://geocities.yahoo.com/ps/info1