Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2002:
[Freeciv-Dev] Re: remove nearest_real_pos uses (PR#1211)
Home

[Freeciv-Dev] Re: remove nearest_real_pos uses (PR#1211)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: jdorje@xxxxxxxxxxxxxxxxxxxxx
Cc: freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: remove nearest_real_pos uses (PR#1211)
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Mon, 7 Jan 2002 13:01:36 +0100
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Mon, Jan 07, 2002 at 02:39:16AM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> The attached patch removes two unnecessary (incorrect AFAICT) uses of 
> nearest_real_pos.
> 
> 1.  In map_get_player_tile(), I've replaced the nearest_real_pos call 
> with a CHECK_MAP_POS check (included in map_inx).  I haven't verified 
> that all calls to the code are safe, but (1) the error message that is 
> currently displayed for non-real tiles is never shown and (2) 
> extensively running autogames has yet to turn up a failed assert.
> 
> Note that Ross's corecleanups return NULL in the case of a non-real map 
> position, but this isn't desired IMO and isn't feasible at this time 
> (since it would require converting all uses to check the return value, 
> which they currently do not do).
> 
> 2.  In auto_settler_do_goto(), it has been simply replaced by 
> CHECK_MAP_POS.  Here I've scanned all the calls to the function, and it 
> looks like they should all pass Good coordinates.  Again there have been 
> no failed assertions in my autogames.
> 
> Note that Ross's corecleanups handle this incorrectly, as they call 
> normalize_map_pos on the coordinates and ignore the return value 
> (verboten!  Ross, you should know better!).  This will result in 
> unpredictible behavior if non-real coordinates are ever passed in; most 
> likely (and hopefully) a failed CHECK_MAP_POS check later on when the 
> coordinates are actually used.
> 

> If this patch is applied, the only remaining nearest_real_pos use will 
> be in map_pos_to_canvas_pos in mapview_common.  If one of these places 
> does need to use nearest_real_pos (which seems inconceivable), a comment 
> should be added explaining why.

Can't nearest_real_pos then be moved into mapview_common? So nobody
can use this "dangerous tool". However this would spread topology
knowledge (which is also bad). What about just ignoring clicks which
are outside the map?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Living on earth may be expensive, but it includes an annual free trip
  around the sun.


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