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

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

I've decided to respond to your comments.

> > one letter variables like c with more descriptive names
> too little of that :(

Don't worry, far more of that is coming soon. It's a little bit difficult
following the logic where the same variable name is used to mean different
things. I've had a little bit of trouble getting my patches to work properly,
so when I've tracked down the problem you'll see most of the one letter
variables
disappear.
 
> > 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 */

The above is correct, it cannot be replaced with single_move, because it
has nothing to do with the pre-existing move costs.

> ./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));

i'm not going to touch the igter stuff, because GB is handling that. Also,
it is correct to multiply igter stuff by 3, because the warmap does not
return 1/3 movement cost for igter units.

> ./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;

Those look like they should be fixed. I'll get on it. Actually, I thought I
had fixed it, probably in aicleanupv4 or 5.

> ./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;

I thought you were handling the threshold stuff. If you'd like me to send in
patches about threshold I will.


> ./advmilitary.c:  else pcity->ai.wallvalue = (danger * 9 - badmojo * 8) * 10
> / (danger);

No idea what wallvalue, 9, 8 and 10 mean, so nothing for the moment.

> ./advmilitary.c:          !unit_type_flag(i, F_IGWALL) &&
> !city_got_citywalls(acity)) d *= 9;

I think I know what that is about. The defensive bonus of a city wall is
3, but since the defensive power stuff divides by 15, it is probably necessary
to use 9 here.


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

I've touched only a very few of those. For reasons I mentioned above, just
because the number is 3 and relates to movement cost does not mean you can
substitute SINGLE_MOVE.
 
> Aha, so why do you change it in aicleanupv3? ;) I'm maybe just a bit
> confused, sorry :).
>

Because I am a moron and sent the old version. Don't worry, the newer version
will be out soon.
 


__________________________________________________
Do You Yahoo!?
Send your FREE holiday greetings online!
http://greetings.yahoo.com