[Freeciv-Dev] Re: bugs and problems (PR#1040)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Oct 29, 2001 at 09:50:03PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> While playing around with topologies where normal!=regular, I've found
> some weird behavior.  Some of it is currently correct (but doesn't work
> with different topologies), while some seems to be a part of subtle and
> little-occuring bugs.
> 
> 
> Here's the biggest thing: when a unit is created, its goto_dest_x and
> goto_dest_y are initialized to 0.  Later, these values are used when
> they really shouldn't be.  Two examples of this are in
> ai_manage_ferryboat() [1] and another in is_already_assigned [2] (this
> latter one has a comment from Syela "I'm still not sure this is exactly
> right").  First of all, this will not work with topologies where
> regular!=normal (the check_map_pos change does its job well in catching
> the bug).  Secondly, it could lead to weird bugs in cases where the
> check for (0,0) actually matches (for instance, the unit may think it is
> ferrying to whatever city happens to lie there - I'm not exactly sure). 
> It's easy to fix the problem by doing a check for is_normal_map_pos
> before using the coordinates, but fixing the bug is slightly trickier. 
> My best idea is to make goto_dest a pointer to a struct map_position,
> that is initialized as NULL and later set up properly (and maybe
> reverted to NULL later).  Another idea would be to have a separate flag
> indicating whether the goto coordinates had any validity.  Using (-1,
> -1) as a target to indicate invalid coordinates is not a good solution
> (see below).
> 
> There is a somewhat similar problem in some of the server's plrhand.c
> code.  Here it's not a bug, but will become a problem once topologies
> may span all coordinates.  The function vnotify_conn_ex [3] takes a
> coordinate pair (x, y).  Several pieces of code pass in (-1, -1) to
> indicate no coordinates.  On a torus map, these coordinates are real and
> so the check within the function behaves wrongly.  This is easy enough
> to correct by checking for normal coordinates instead of real ones, but
> I do not think this is the correct fix.  A better solution that I've
> used is passing in a pointer to a struct map_position.  NULL is passed
> in to indicate no coordinates.  I haven't looked at this all that
> deeply, though - the coordinates are translated and passed on, and I'm
> not sure what will happen to them then.  It's also likely that more
> places like this exist in the code.

Using NULL may be possible but you have to think about the
representation in packets. So I think (-1,-1) is ok for both problems
since all now all map position don't have to be real but normal.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  One nuclear bomb can ruin your whole day.