[Freeciv-Dev] Re: AI cleanup V2 (PR#1171)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Sat, Dec 29, 2001 at 05:09:13PM CET, I got a letter,
where Raahul Kumar <raahul_da_man@xxxxxxxxx> told me, that...
> This AI cleanup touches mostly advmilitary.c in the AI directory. Makes
> basic changes getting rid of
> 
> one letter variables like c with more descriptive names
too little of that :(

> finishing the change made earlier by getting rid of hardcoded nos like
> 6:12 with 2 * SINGLE_MOVE:4 * SINGLE_MOVE
Well, simple grep (ok, a little manually filtered ;) gives me:

./advmilitary.c:  if (dist * dist < m * 3) { v *= m; v /= 3; } /* knights can't 
attack more than twice */
./advmilitary.c:            if (dist <= m * 3) urgency++;
./advmilitary.c:          if (dist <= m * 3) urgency++;
./advmilitary.c:  if (def) cur *= 3;
./advmilitary.c:  if (unit_type_flag(i, F_IGTER) && !def) cur *= 3;
./advmilitary.c:      q = (acity ? 1 : unit_types[n].move_rate * 
(unit_type_flag(n, F_IGTER) ? 3 : 1));
./advmilitary.c:      if (unit_type_flag(v, F_IGTER)) m *= 3; /* not quite 
right */
./advmilitary.c:        if (unit_flag(pdef, F_IGTER)) dist *= 3;
./advmilitary.c:      if (unit_type_flag(v, F_IGTER)) m *= 3; /* not quite 
right */
./aiunit.c:       (real_map_distance(punit->x, punit->y, dest_x, dest_y) < 3 &&
./aiunit.c:          else n = real_map_distance(punit->x, punit->y, aunit->x, 
aunit->y) * 3;
./aiunit.c:            if (unit_flag(aunit, F_IGTER)) n *= 3;
./aiunit.c:    if (city_got_building(pcity, B_PORT)) cur /= 3;
./aiunit.c:  square_iterate(punit->x, punit->y, 3, x, y) {
./advmilitary.c:    if (y >= 6 * THRESHOLD)
./advmilitary.c:              warmap.cost[x][y] <= (MIN(6, m) * THRESHOLD));
./aiunit.c:    if (warmap.seacost[x1][y1] <= 6 * THRESHOLD && t != T_OCEAN) {
./aiunit.c:        ok += (6 * THRESHOLD - warmap.seacost[x1][y1]);
./aiunit.c:  maxd = MIN(6, m) * THRESHOLD + 1;
./aiunit.c:                      warmap.seacost[acity->x][acity->y] <= 6 * 
THRESHOLD))) ||
./aiunit.c:  int best = 6 * THRESHOLD + 1, cur;
./aiunit.c:  if (best > 6 * THRESHOLD) return 0;
./advmilitary.c:/* if dist = 9, a chariot is 1.5 turns away.  NOT 2 turns away. 
*/
./advmilitary.c:  else pcity->ai.wallvalue = (danger * 9 - badmojo * 8) * 10 / 
(danger);
./advmilitary.c:          !unit_type_flag(i, F_IGWALL) && 
!city_got_citywalls(acity)) d *= 9;
./advmilitary.c:          !city_got_citywalls(acity)) d *= 9;
./aiunit.c:         unknown += 9 * (threshold - warmap.seacost[x1][y1]);
./aiunit.c:         unknown += 9 * (threshold - warmap.cost[x1][y1]);
./aiunit.c:         d *= 9;

You already fixed all these? If yes, good work! :)

> There are also a lot more comments added, and the addition of Raimar's
> unit_move_turns, inspired by me, coded by Raimar, and changes suggested
> by Gregory Berkolaiko.
Aha, so why do you change it in aicleanupv3? ;) I'm maybe just a bit
confused, sorry :).

> It's a nice clean addition against the Dec 25 CVS, and compiles with no 
> problems.
Well, basically same objections as to aicleanupv3 applies here.

> diff -ruN -X /tmp/freeciv/diff_ignore freeciv/ai/advmilitary.c 
> freeciv-cvs/ai/advmilitary.c
> --- freeciv/ai/advmilitary.c  Sun Sep 16 01:53:45 2001
> +++ freeciv-cvs/ai/advmilitary.c      Fri Nov 23 16:42:54 2001
> @@ -264,7 +264,7 @@
>      if (i != pcity->owner) {
>        aplayer = &game.players[i];
>        boatspeed = (get_invention(aplayer, game.rtech.nav)
> -                == TECH_KNOWN ? 12 : 6);
> +                == TECH_KNOWN ? 4*SINGLE_MOVE : 2*SINGLE_MOVE);
please make spaces around operators ;-)
>        boatid = find_boat(aplayer, &x, &y, 0);
>        if (boatid) boatdist = warmap.seacost[x][y];
>        else boatdist = -1;
> @@ -471,7 +471,7 @@
>    Unit_Type_id i;
>    Unit_Type_id bestid = 0; /* ??? Zero is legal value! (Settlers by default) 
> */
>    Tech_Type_id tech_req;
> -  int j, k, l, m, n;
> +  int j, k, l, move_type, n;
what about other variables?
>    int best= 0;
>    int walls = city_got_citywalls(pcity);
>    int desire[U_LAST]; /* what you get is what you seek */
> @@ -481,24 +481,24 @@
>    memset(desire, 0, sizeof(desire));
>    for (i = 0; i < game.num_unit_types; i++) {
>      if (!is_ai_simple_military(i)) continue;
> -    m = unit_types[i].move_type;
> -    if ((m == LAND_MOVING || m == SEA_MOVING)) {
> +    move_type = unit_types[i].move_type;
> +    if ((move_type == LAND_MOVING || move_type == SEA_MOVING)) {
>        k = pplayer->ai.tech_turns[unit_types[i].tech_requirement];
>        j = unit_desirability(i, 1);
>        if (!isdef && unit_type_flag(i, F_FIELDUNIT)) j = 0;
>        j /= 15; /* good enough, no rounding errors */
>        j *= j;
>        if (can_build_unit(pcity, i)) {
> -        if (walls && m == LAND_MOVING) { j *= pcity->ai.wallvalue; j /= 10; }
> +        if (walls && move_type == LAND_MOVING) { j *= pcity->ai.wallvalue; j 
> /= 10; }
please put statement to separate line
>          if ((j > best || (j == best && get_unit_type(i)->build_cost <=
>                                 get_unit_type(bestid)->build_cost)) &&
>              unit_types[i].build_cost <= pcity->shield_stock + 40) {
>            best = j;
>            bestid = i;
>          }
> -      } else if (k > 0 && (shore || m == LAND_MOVING) &&
> +      } else if (k > 0 && (shore || move_type == LAND_MOVING) &&
>                  unit_types[i].tech_requirement != A_LAST) {
> -        if (m == LAND_MOVING) { j *= pcity->ai.wallvalue; j /= 10; }
> +        if (move_type == LAND_MOVING) { j *= pcity->ai.wallvalue; j /= 10; }
ditto
>          l = k * (k + pplayer->research.researchpoints) * game.researchcost /
>           (game.year > 0 ? 2 : 4); /* cost (shield equiv) of gaining these 
> techs */
>          l /= city_list_size(&pplayer->cities);

> diff -ruN -X /tmp/freeciv/diff_ignore freeciv/ai/aiunit.c 
> freeciv-cvs/ai/aiunit.c
> --- freeciv/ai/aiunit.c       Mon Nov  5 02:00:24 2001
> +++ freeciv-cvs/ai/aiunit.c   Fri Nov 23 16:17:48 2001
> @@ -132,30 +132,56 @@
>    return 0;
>  }
>  
> -/********************************************************************** 
> -  ...
> -***********************************************************************/
> -static int unit_move_turns(struct unit *punit, int x, int y)
> -{
> -  int m, d;
> -  m = unit_type(punit)->move_rate;
> -  if (unit_flag(punit, F_IGTER)) m *= SINGLE_MOVE;
> -  if(is_sailing_unit(punit)) {
> -    struct player *pplayer = unit_owner(punit);
> -    if (player_owns_active_wonder(pplayer, B_LIGHTHOUSE)) 
> -      m += SINGLE_MOVE;
> -    if (player_owns_active_wonder(pplayer, B_MAGELLAN))
> -      m += (improvement_variant(B_MAGELLAN)==1) ? SINGLE_MOVE : 2 * 
> SINGLE_MOVE;
> -    m += player_knows_techs_with_flag(pplayer,TF_BOAT_FAST) * SINGLE_MOVE;
> +/**********************************************************************
> + Precondition: A warmap must already be generated for the punit
What about moving this after the basic description?
> + Returns the minimal amount of turns required to reach the given
> + destination position. The actual turn at which the unit will
> + reach the given point depends on the movement points it has remaining.
  +
> + For example: Unit has a move rate of 3, path cost is 5, unit has 2   
> + move points left
  +
> + path1 costs: first tile = 3, second tile = 2
> +
  -
> + turn 0: points=2, unit has to wait
> + turn 1: points=3, unit can move, points=0, has to wait
> + turn 2: points=3, unit can move, points=1
> +
> + path2 costs: first tile=2, second tile=3
> +  
  -
> + turn 0: points=2, unit can move, points=0, has to wait
> + turn 1: points=3, unit can move, points=0
> +
> + In spite of the path costs being the same, these two units will arrive
> + at different turns. This function also does not take into account ZOC.
  +
> + Note: even if a unit has only fractional move points left, there is 
> + still a possibility it could cross the tile
> +  ***********************************************************************/
> +  
> +  static int unit_move_turns(struct unit *punit, int x, int y)
please no no no! :) i cried for fixing of the indendation ages ago :).
> +  {
> +   int result, move_rate = unit_type(punit)->move_rate;
> +
> +   switch (unit_type(punit)->move_type) {
> +   case LAND_MOVING:
please indent cases appropriatelly ;p
> +     if (unit_flag(punit, F_IGTER)) {
> +       move_rate *= SINGLE_MOVE;
> +     }
> +     result = warmap.cost[x][y] / move_rate;
> +     break;
> +
> +   case SEA_MOVING:
> +     if (player_owns_active_wonder(unit_owner(punit), B_LIGHTHOUSE)) {
> +       move_rate += SINGLE_MOVE;
> +     }
> +     if (player_owns_active_wonder(unit_owner(punit), B_MAGELLAN)) {
> +       move_rate += (improvement_variant(B_MAGELLAN) == 1) ?
> +         SINGLE_MOVE : 2 * SINGLE_MOVE;
> +     }
> +     if (player_knows_techs_with_flag(unit_owner(punit), TF_BOAT_FAST)) {
> +       move_rate += SINGLE_MOVE;
> +     }
> +     result = warmap.seacost[x][y] / move_rate;
> +     break;
> +
> +   case HELI_MOVING:
> +   case AIR_MOVING:
> +     result = real_map_distance(punit->x, punit->y, x,y) * SINGLE_MOVE / 
> move_rate;
> +     break;
> +
> +   default:
> +     assert(0);
> +     exit(1);
> +     result = -1;
> +   }
> +   return result;
>    }
>  
> -  if (unit_type(punit)->move_type == LAND_MOVING)
> -    d = warmap.cost[x][y] / m;
> -  else if (unit_type(punit)->move_type == SEA_MOVING)
> -    d = warmap.seacost[x][y] / m;
> -  else d = real_map_distance(punit->x, punit->y, x, y) * SINGLE_MOVE / m;
> -  return(d);
> -}
> +  
> +
> +  
>  
>  /**************************************************************************
>    is there any hope of reaching this tile without violating ZOC? 
> @@ -1244,8 +1270,8 @@
>    if (ferryboat) really_generate_warmap(map_get_city(ferryboat->x, 
> ferryboat->y),
>                         ferryboat, SEA_MOVING);
>  
> -  if (ferryboat) boatspeed = (unit_flag(ferryboat, F_TRIREME) ? 6 : 12);
> -  else boatspeed = (get_invention(pplayer, game.rtech.nav) != TECH_KNOWN ? 6 
> : 12);
> +  if (ferryboat) boatspeed = (unit_flag(ferryboat, F_TRIREME) ? 
> 2*SINGLE_MOVE : 4*SINGLE_MOVE);
> +  else boatspeed = (get_invention(pplayer, game.rtech.nav) != TECH_KNOWN ?  
> 2*SINGLE_MOVE : 4*SINGLE_MOVE);
spaces...
>  
>    if (is_ground_unit(punit) && !punit->id &&
>        is_terrain_near_tile(punit->x, punit->y, T_OCEAN)) harborcity++;
> @@ -1271,7 +1297,7 @@
>            if (is_ground_unit(punit)) {
>              if (!sanity) {
>                c = (warmap.seacost[acity->x][acity->y]) / boatspeed; /* kluge 
> */
> -              if (boatspeed < 9 && c > 2) c = 999; /* tired of Kaput! -- 
> Syela */
> +              if (boatspeed < 3 * SINGLE_MOVE && c > 2) c = 999; /* tired of 
> Kaput! -- Syela */
...like here you do! ;)
>                if (ferryboat) c += (warmap.cost[bx][by] + m - 1) / m + 1;
>                else c += 1;
>                if (unit_flag(punit, F_MARINES)) c -= 1;
> 

-- 

                                Petr "Pasky" Baudis

UN*X programmer, UN*X administrator, hobbies = IPv6, IRC, FreeCiv, (e)links
.
Firewall in a way is like the doorkeeper of a local pub. If you don't have your
I.D. on you, or if for some reason you do not qualify to enter, the doorkeeper
will not permit you to enter. In some extreme cases these doorkeepers will not
let you out, or at least give you a hard time before they finally let you out.
.
Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/