[Freeciv-Dev] Re: RFC: Fixing movement code

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Nov 21, 2001 at 01:35:15AM -0800, Raahul Kumar wrote:
> This patch is tidying up the AI for my further patches. The aim is to allow
> the replacement of 3 = Single move cost with 30. This will allow finer grained
> movement costs such as 1/6 for an igter unit upgrade, as well as maglev costs.
> 
> List of proposed changes
> 
> If there are any other places in the AI code where there are changes to be 
> made
> let me know. 
> 
> 
>  if (ferryboat) boatspeed = (unit_flag(ferryboat, F_TRIREME) ? 6 : 12);
>   else boatspeed = (get_invention(pplayer, game.rtech.nav) != TECH_KNOWN ? 6 :
> 12);
> 
> Not very portable at all. What is supposed to be the move rate for triremes?

I don't know the answer but I want to point out

./ai/advmilitary.c:676

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

> warmap.seacost[acity->x][acity->y] <= 6 * THRESHOLD)))
> 
> I have to get rid of all these 6 * threshold nos. I could always
> just multiply threshold by 10.

Please make it a symbol like in:

/* docu */
#define SOME_MOVE_THRESHOLD     6

Also the original THRESHOLD in server/gotohand.h has no docu.

Also this
 ./server/gotohand.c:275:  int maxcost = THRESHOLD * 6 + 2; /* should be big 
enough without being TOO big */
looks like rather like some arbitrary value.

> if (boatspeed < 9 && c > 2)
> Have to multiply that by single move cost. What does c do in this function?

I have no clue. This function is 290 lines long. These are 250 too
much IMHO.

> total_distance in ai_tools.c
> 
> total_distance = 36 This must be 12 squares. Therefore my patch will have
> 360 as total distance.
> 
> 
> 
> File aiunit.c
> 
> 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;
>   }
> 
>   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);
> 
> m = max_move_rate
> d = min_turns_to_dest

I would use for a variable like d the name "result". But this is a
personal thing. You could write the "min turns to dest" in the header
of the function as comment.

I want to point out that "is_sailing_unit(punit)" ==
"unit_type(punit)->move_type == SEA_MOVING". This means that the
if(is_sailing_unit(punit)) part can be moved into the "if
(unit_type(punit)->move_type == SEA_MOVING)" test.

> ai_military_find_victim
> 
> Any ideas for something better than a,b,c,d,e,f
> 
> f should be unit_build_cost
> 
> what the hell is b? 
>           b = unit_type(pdef)->build_cost;
> b = (b + 40) * punit->hp / unit_type(punit)->hp;
> 
> find_something_to_kill is also pretty bad
> 
> m = max_move_rate
> 

No idea.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 Make a software that is foolproof, and only fools will want to use it.