Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2001:
[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxxxxxxx
Cc: Gaute B Strokkenes <gs234@xxxxxxxxx>, jdorje@xxxxxxxxxxxxxxxxxxxxx, freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming
From: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>
Date: Sat, 01 Sep 2001 14:53:01 -0400

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.

>>  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.

Cheers,
RossW




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