[Freeciv-Dev] Re: [Patch] [RFC] Path finding version 14

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Sat, Sep 14, 2002 at 02:51:22PM +0100, Gregory Berkolaiko wrote:
> On Thu, 12 Sep 2002, Raimar Falke wrote:
> 
> > On Thu, Aug 15, 2002 at 09:59:18AM +0200, Raimar Falke wrote:
> > > 
> > > Changes to the interface
> > >  - remove PF_IGNORE_COST
> > >  - add enum tile_behavior
> > >  - add get_TB
> > >  - add ignore_enemy and omniscience flags
> > > 
> > > The first three should be obvious. I will explain the last one latter.
> > > 
> > > Changes to the implementation:
> > >  - add bucket list heap (activate with USE_HEAP2)
> > >  - add some inlining (activate with USE_INLINE) This is a moderate
> > >  inline which brings the path finding in the same area as
> > >  really_generate_warmap. The 8 small helper function are all moved
> > >  into plain_get_next_position and so plain_get_next_position and
> > >  really_generate_warmap are now the big functions.
> > >  - support for the get_TB, ignore_enemy and omniscience
> > >  - added a missing heap_destroy call
> > 
> > And a new version. Changes:
> >  - remove the testing path finding user from gotohand.c
> >  - add some documentation and license headers
> > 
> > There were no comments on the last version. So I would think that this
> 
> Lots of comments now:
> 
> 1. In struct pf_position why would you need BMC_of_next_step and COP?

Both are information without a "hard" value. BMC_of_next_step could
for example display in the GUI if the user chooses a path. No idea
about COP.

> 2. What are ignore_enemy and omniscience and why these options
> cannot be implemented through the already existing callbacks?

I added them to be able to compare path finding to the current warmap
which avoid some checks. And no these flags can't implemented through
the existing callbacks.

> 3. Usual objections to get_COP.  I think the parameters to this useless 
> function should be (int TEC, int cost, void *), where cost is either BMC 
> or (turn + 1) * move_rate - moves_left.

I though about this and the term "(turn + 1) * move_rate - moves_left"
is bad. If you use EC in some way you have to scale the EC by
move_rate. You don't want this. You want the term "turn +
moves_left/move_rate" instead. But this is a float which we don't
want.

> 4. I thought pf_next_get_position should be non-destructive, just lifting 
> info.

I don't understand:

void pf_next_get_position(pf_map_t pf_map, struct pf_position *pos)
{
  assert(pf_map->last_pos_is_valid);
  memcpy(pos, &pf_map->last_pos, sizeof(*pos));
}

It is clear that we have to overwrite the pos from the caller.

> 5. Please use standard comments before each function.  Most
> functions are already well commented elsewhere, but the line of
> ****** makes the code so much easier to browse.

This is easy.

> 6. Which heap is better performing?

Try for yourself. See my original posting. The difference is very
minimal because the heap isn't the bottleneck.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 Windows: Where do you want to go today?
 Linux: Where do you want to go tomorrow?
 BSD: Are you guys coming or what?