[freeciv-ai] Re: (PR#3619) coordinate fix in find_city_to_diplomat

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Raimar Falke wrote:
> On Wed, Mar 05, 2003 at 12:09:06AM -0800, Jason Short wrote:
> 
>>In find_city_to_diplomat, there is an initializer:
>>
>>  int best_dist = MAX(map.xsize, map.ysize);
>>
>>this is no good under gen-topologies, because it is possible for a
>>distance to be larger than this (map.xsize and map.ysize are the
>>_native_ dimensions of the map, while the distance is in _map_ coordinates).
>>
>>It's easy to fix it to simply be a larger value.  But how large?  The
>>most obviously correct thing to me is just to use MAXINT.  But I'm not
>>sure if this is portable (Raimar?).  Aside from that, its correctness
>>should be obvious.

> It is easier and safer to set it to a value which is an invalid
> distance like -1 and treat this value as unset.

Err, right.  In fact that's particularly easy in this case since the 
"unset" comparison is already done (ctarget starts out NULL).

So I guess that means this code actually is safe - it's just misleading. 
  Patch attached.

jason

Index: ai/aidiplomat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidiplomat.c,v
retrieving revision 1.14
diff -u -r1.14 aidiplomat.c
--- ai/aidiplomat.c     2003/02/27 22:14:37     1.14
+++ ai/aidiplomat.c     2003/03/05 09:09:03
@@ -312,7 +312,7 @@
   bool has_embassy;
   int incite_cost = 0; /* incite cost */
   int move_cost = 0; /* move cost */
-  int best_dist = MAX(map.xsize, map.ysize);
+  int best_dist = -1;
   int continent = map_get_continent(x, y);
   bool dipldef; /* whether target is protected by diplomats */
   bool handicap = ai_handicap(pplayer, H_TARGETS);
@@ -355,12 +355,15 @@
               || (incite_cost < (pplayer->economic.gold - 
pplayer->ai.est_upkeep)
                   && can_incite && !dipldef))) {
         /* We have the closest enemy city so far on the same continent */
+       /* The first target found will initialize both ctarget (from NULL)
+        * and best_dist (from -1). */
         ctarget = acity;
         best_dist = move_cost;
       }
     } city_list_iterate_end;
   } players_iterate_end;
 
+  assert(ctarget == NULL || best_dist >= 0);
   return ctarget;
 }