[Freeciv-Dev] Re: the directional system

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 07, 2001 at 08:50:04AM -0400, Jason Dorje Short wrote:
> Raimar Falke wrote:
> > 
> 
> > DIR_DX2 is only used in
> > client/tilespec.c:fill_tile_sprite_array_iso. Correct? Looking at it,
> > it looks save for me to change the underlying direction
> > system. Somebody has said that the graphics also depend on the
> > direction system. It looks to me that the graphics only depend on
> > INDEX_NSEW. Correct?
> 
> This particular file does; however this file uses the *new* directional
> system whereas Ross says the graphics depend on the *old* system
> (although I don't see this).
> 
> I think I may not have been clear in my previous letter: I'm not really
> looking for a deep analysis of the patch itself.  My main questions are
> these: does FreeCiv run correctly under this patch, and are there any
> other places where "magic" code or constants are used?
> 
> Also, yes I know the magic numbers in tilespec need to be removed.  I
> intend to submit a real patch that removes the DIR_D[XY]2 arrays as well
> as fixing the locations that still have magic code (there are three of
> them that I know of).  But, this patch will only be worthwhile if we
> convert to the new directional system as I've suggested; if we do it as
> Ross has suggested then I'm only doing work that he'll have to undo
> later.
> 
> One other clarification: I've removed the DIR_D[XY]2 arrays, but I've
> changed the DIR_D[XY] arrays to use that directional system so really
> it's everything else that has changed.  Trying to find magic code by
> searching for DIR_D[XY] usages is pretty futile; the use of DIR_D[XY]
> itself is quite safe but the problem is when the code tries to
> manipulate the directions directly.  This often occurs several levels
> down (for instance in straightest_direction() and get_char() which don't
> use the arrays at all).  So, it will be difficult to tell by inspecting
> the code that all "magic" has been removed.
> 
> The question remains: does FreeCiv work correctly under this patch?

Ross may have an funded answer for this. I can only provide a possible
test: replace DIR_X access with a function, replace the numbers in
enum direction8 and in the "for (dir_itr = 0; dir_itr < 8; dir_itr++)"
with a new set of values. For example "42-49". See what breaks. It is
more likely to notice any coded assumption with such a test case than
with your current patch.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "That's fundamental game play!  My main enemy is *ALWAYS* fighting 
  a 4-front war.  I make sure of it!"
    -- Tony Stuckey, freeciv-dev