Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2003:
[Freeciv-Dev] Re: (PR#2370) Path finding
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#2370) Path finding
From: "Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>
Date: Sat, 8 Mar 2003 17:05:47 -0800
Reply-to: rt@xxxxxxxxxxxxxx

Quoting Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>:

> On Thu, Feb 27, 2003 at 01:51:34PM -0800, Gregory Berkolaiko wrote:
> 
> > + *      Ou est la toilette la plus proche?
> 
> I can guess what this means ;) But it would be better to formulate
> this in English.

done

> 
> > + * B) the caller doesn't know the coordinates of the goal yet (but knows
> 
> 
> s/coordinates/map position/ also used on other places.

done

> 
> > +  enum direction8 dir_to_next_pos; /* Unsed only in struct_path */
> 
> It should be set to an invalid value otherwise.

done 

> 
> > +/* Iterates the map until it reaches (x, y).  Then fills the info
> > + * about it into pos.  Returns FALSE if position is unreachable.
> > + * Contents of pos in this case is not defined. */
> > +bool pf_get_position(struct pf_map *pf_map, int x, int y,
> > +                struct pf_position *pos);
> 
> We should write that it is a shortcut for 
> 
>   if(!pf_get_path(...)) return FALSE;
>   *pos = pf_last_position(path);

done, in a way.


> 
> > +/* Return the full info on the position reached in the last call to 
> > + * pf_next. */
> > +void pf_next_get_position(const struct pf_map *pf_map,
> > +                     struct pf_position *pos);
> 
> Same.

DRY :)

> 
> It looks like you forgot a pf_destroy_path function. I know why I
> prefer static allocation when it is possible.

done

> 
> INITIAL_PATH_LENGTH is ugly. We should do a loop first to count and
> then allocate.

Ughh...
done.
I do a loop to count in main path-finding.
In path-finding with danger, it is much much easier to count steps in a separate
 field, so I did that.  We pay with speed, but it's not going to be used much,
hopefully.

I hope it can now be committed.  Per, if you commit it, can you change the names
of the iterators to "iterate" like in "simple_unit_path_iterate"?

Thanks,
G.


Attachment: pf28.diff.gz
Description: pf28.diff.gz


[Prev in Thread] Current Thread [Next in Thread]