[Freeciv-Dev] Re: [RFC] Move cost map interface

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Apr 25, 2002 at 06:10:51PM +0100, Gregory Berkolaiko wrote:
> I have no major complaints.  However I expect big arguments when it comes 
> to implementation.  Also, I don't think I can spend much time on it until 
> late May.
> 
> On Wed, 24 Apr 2002, Raimar Falke wrote:
> 
> > On Tue, Apr 16, 2002 at 09:57:39AM +0200, Raimar Falke wrote:
> > > Attached a version which have the requested changes. You would want to
> > > wrap an int in the void *.
> > 
> > And now version 8. Changes:
> >  - added move_backward flag
> 
> right.
> I still think that it's better to have one map_type enum instead of three
> different flags, but it's not important.
> 
> >  - added move_expended array
> 
> The information given by 
>       moves_expended, initial_moves
> and by 
>       turn, move_left
> is equivalent.  So I think it is excessive to have both of them.
> If I were you, I would convert moves_left to an array though (however it 
> only makes sense for MINIMAL and MAXIMAL modes).
> Again, this is not an area of my immediate interest.
> 
> I also have many problems reading this bit of code:
> =============================================================
> /*
>  * Construct the whole path to the given position and write it into
>  * the given path. The position has to be obtained from
>  * pf_get_next_position.
>  */
> void pf_construct_path(pf_map_t map, const struct pf_position *position,
>                        const pf_location_t location);
> 
> /*
>  * The last position of the next best path is written in the given
>  * struct pf_position. Only the fields int x, y and
>  * remaining_move_points are set. Returns FALSE if no more positions
>  * are available in this map. Behavior is undefined after pf_get_path
>  * was called.
>  */
> bool pf_get_next_position(pf_map_t map, pf_location_t *location);
> ===================================================================
> 
> 1. In construct_path the second argument is probably struct pf_path *, not 
> position.

Yes.

> 2. The comment of get_next_position is wrong too.  It's obsolete.

Yes "Only ...set" should be removed.

> 3. The name of get_next_position is obsolete too.  Should be 
> get_next_location IMO.

Yes.

> 4. If you do 3, change the comment to construct_path.

Ok.

> 5. I can foresee a lot of confusion with position / location names.  Thus 
> I reccommend to change pf_position to pf_node (as in node of the route).

We can define the terms position and location at the top. Problem
solved IMHO.

> 6. I hope that you do not consider this interface as wrought in stone.

I have no problem changing the interface in small areas.

> Circumastances might change, new tasks might demand new capabilities like 
> user-suppliable BMC functons.

In such case I would prefer adding some parameters but this whole
thing depends on the situation.

I'm currently adapting the goto agent code to use the new
interface. This should give us a playground for more experimentations.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Using only the operating-system that came with your computer is just
  like only playing the demo-disc that came with your CD-player."