[Freeciv-Dev] Re: (PR#2370) Path finding

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Wed, Feb 19, 2003 at 04:40:35PM +0000, Per I. Mathisen wrote:
> On Wed, 19 Feb 2003, Raimar Falke wrote:
> > My ordered list of goal for development is
> > still: correctness, maintainability, speed. Warmap had great speed but
> > low maintainability. You change this with PF but you can't get both
> > easily.
> 
> Maybe the warmap code itself wasn't easily maintained, but code which
> _uses_ the warmap can be written in an easily maintained manner. Code
> which uses pf directly is going to be very ugly, and rather
> unmaintainable, simply because the pf API is so big.

> Correctness be damned, Raimar. 

> Maintainability is much more important in the long run.

Maintainability is important in the long run. No doubt about this. But
correctness is still more important.

> Compare:
> 
> warmap:
>   generate_warmap(map_get_city(x, y), punit);
>   WARMAP_COST(x, y);
> 
> pf:
>   pf_map_t UPI_map;                                         \
>   struct pf_parameter UPI_parameter;                        \
>   struct pf_path UPI_path;                                  \
>   pft_fill_default_parameter(&UPI_parameter);               \
>   pft_fill_unit_parameter(&UPI_parameter, punit);           \
>   pft_adjust_param_to_overlap(&UPI_parameter, punit);       \
>   UPI_map = pf_get_map(&UPI_parameter);                     \
>   while (pf_next(UPI_map)) {                                \
>     int x, y, movecost;                                     \
>     int px = punit->x;                                      \
>     int py = punit->y;                                      \
>     struct pf_position UPI_pos;                             \
>     pf_next_get_position(UPI_map, &UPI_pos);                \
>     movecost = UPI_pos.total_BMC;                           \
>     x = UPI_pos.x;                                          \
>     y = UPI_pos.y;                                          \
>     pf_next_get_path(UPI_map, &UPI_path);                   \
>     if (UPI_path.positions_used > 0) {                      \
>       px = UPI_path.positions[UPI_path.positions_used-1].x; \
>       py = UPI_path.positions[UPI_path.positions_used-1].y; \
>     }

This is comparing apples with peaches. Based on pf14.diff is more
like:

>   pf_map_t UPI_map;                                         \
>   struct pf_parameter UPI_parameter;                        \
>   struct pf_path UPI_path;                                  \
>   pft_fill_default_parameter(&UPI_parameter);               \
>   pft_fill_unit_parameter(&UPI_parameter, punit);           \
>   pft_adjust_param_to_overlap(&UPI_parameter, punit);       \
>   UPI_map = pf_get_map(&UPI_parameter);                     \
    pf_get_path(UPI_map, x, y, &UPI_path);
    if(UPI_path.is_valid) .... else ....
    return pf_last_position(&UPI_path)->total_BMC;

With the pf_get_position Gregory wanted you can remove two lines. The
init stuff can be folded nicely in a function like ai_get_pf_map. So
this will look like:

    pf_map_t UPI_map;
    pf_position pos;
    UPI_map = ai_get_pf_map(punit);
    if(pf_get_position(UPI_map, x, y, &pos))
        return pos.total_BMC;

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "- Amiga Y2K fixes (a bit late, wouldn't you say?)"
    -- Linus Torvalds about linux 2.4.0 at 4 Jan 2001