[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Sep 01, 2001 at 02:53:01PM -0400, Ross W. Wetmore wrote:
> At 10:26 AM 01/09/01 +0200, Raimar Falke wrote:
> >On Sat, Sep 01, 2001 at 05:41:06AM +0200, Gaute B Strokkenes wrote:
> [...]
> >> I've also taken out the access changes to the map_[gs]et_* functions;
> >> while I think that they are desirable in general I do not think the
> >> codebase in general is quite ready for them yet.
> 
> At some point we should take the plunge. When things stabilize, I'll run
> autogame testing on a CVS snapshot to see whether it is time.
> 
> >> +    /* FIXME: The above used to be "break; out of adjc_iterate",
> >> +    but this is incorrect since the current adjc_iterate() is
> >> +    implemented as two nested loops.  */
> >
> >You fixed it. So the comment is useless.
> 
> At some point the *_iterate code should all be converted to single loops,
> and hopefully the same loop code :-). But until then, goto hacks will be 
> necessary, and one shouldn't make any assunptions about the logic used
> in *_iterates. The comment at least remminds one to clean this up when
> the time comes.

Put it on my todo list.

> >>  static void find_city_beach( struct city *pc, struct unit *punit, int
> *x, int *y)
> >> -  int search_dist = real_map_distance( pc->x, pc->y, punit->x, punit->y );
> >> +  int search_dist = real_map_distance(pc->x, pc->y, punit->x, punit->y)
> - 1;
> >> +  
> >> -  square_iterate(punit->x, punit->y, search_dist - 1, xx, yy) {
> >> +  square_iterate(punit->x, punit->y, search_dist, xx, yy) {
> >
> >Well this is equivalent but why did you changed it?
> 
> Actually I like Gaute's change. It at least makes it clearer what the bug
> in this code is. Note this was discussed, and the bug is that you cannot 
> find_city_beach if you are adjacent to the city with this routine. 
> 
> Actually, I take this back. If you are in a city or apply the rivers
> patch, the square_iterate can actually succeed. But cities aren't allowed
> to be adjacent with most rulesets, and finding a convenient adjacent
> river won't be that easy in most cases.

Please look: if you the expression right to "int search_dist =" into
the square_iterate call you get in both cases
"real_map_distance(...)-1". So what is your point. Any Gautes?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "> WHY?! Isn't it better to put $(shell cat cscope.files) on the list of
  I only have a yellow belt in makefile kungfu.  These fancy gnu make things
  are relatively new to some of us..."
    -- Mark Frazer to Vassilii Khachaturov in linux-kernel