[Freeciv-Dev] Re: [IAMBACK] [PATCH] [1.3] cleanup of proccess_*_want() (PR#1295)

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx> wrote:

 
> The below comment is nonsense.  The clause does not apply to battleships.
> 
> +  /* I was getting four-figure desire for battleships otherwise. -- Syela */
> +  if (!walls && unit_types[best_unit_type].move_type == LAND_MOVING) {
>      best *= pcity->ai.wallvalue;
> +    best /= POWER_FACTOR;
> +  }
> 

What does the comment apply to then?
 
>  /************************************************************************** 
> +This function decides, what unit would be best for erasing enemy. It is
> called,
> +when we just want to kill something, we've found it but we don't have the
> unit
> +for killing that built yet - here we'll choose the type of that unit.
> 
> This function does something really strange, not quite what you say.
> But I guess your explanation is as good as any.

What does it do? Petr comment is a reasonable explanation. Don't be so vague.
How do you want it changed?



> Remove "not quite right" below.

Agree fully.
 
> +      if (unit_type_flag(unit_type, F_IGTER)) {
> +        /* Not quite right... */
> +        /* TODO: Use something like IGTER_MOVE_COST. -- Raahul */
> +        move_rate *= SINGLE_MOVE;
> +      }
> 
> +      if (unit_types[unit_type].move_type == LAND_MOVING) {
> +        if (boatspeed > 0) {
> +          /* It's either city or too far away, so don't bother with
> +           * victim_move_rate. */
> +          move_time = (warmap.cost[boatx][boaty] + move_rate - 1) /
> move_rate
> +                      + 1 + warmap.seacost[x][y] / boatspeed; /* kludge */
> +          if (unit_type_flag(unit_type, F_MARINES)) move_time -= 1;
> +          
> +        } else if (warmap.cost[x][y] <= move_rate) {
> +          /* It's adjacent. */
> +          move_time = 1;
> +
> +        } else {
> 


> +        /* Citywalls give a defensive bonus of 300%. So for units that lack
> the
> +         * ability to ignore city walls, the lack of a city wall makes them
> 3
> +         * times as dangerous. Yet in this check we multiply by 9.
> WHY????????
> +         * (Pulls out hair and screams). -- Raahul */
> +        /* FIXME: Use acity->ai.wallvalue? --pasky */
> +        vuln *= 9;
> +      }
> 
> Raahul, please stop ruining your hairstyle!
> City walls give bonus of 200% (that's multiplying by 3)
> vulnerability is quadratic, so we perform mental calculation
> 3*3 = 9 and fill in the result
 
Back the truck up there G. I'm not quite sure I follow. Yes, vulnerability is
quadratic. From there to multiplying city wall value by 3 is beyond me. Explain
it to me without the missing steps.
 
> All of my complaints are about your comments which is non-essential.
> I haven't checked line-by-line correspondence but if you run a few 
> autogames, I think the patch can be safely committed and human resources 
> directed to pastures new (like the dreaded and bugged f_s_t_k).

Not the f_s_t_k. Anything but that. What have I done to deserve this?

__________________________________________________
Do You Yahoo!?
Yahoo! Tax Center - online filing with TurboTax
http://taxes.yahoo.com/