Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2001:
[Freeciv-Dev] Re: the directional system
Home

[Freeciv-Dev] Re: the directional system

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: jdorje@xxxxxxxxxxxxxxxxxxxxx
Cc: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: the directional system
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 7 Sep 2001 14:06:14 +0200
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Fri, Sep 07, 2001 at 06:59:52AM -0400, Jason Dorje Short wrote:
> First, there's an experimental patch for testing attached.  This patch
> changes the DIR_D[XY] arrays to use the DIR_D[XY]2 directional system. 
> It replaces a last few magic numbers that I've found.  Most importantly,
> everything seems to work cleanly (although I have not done real tests
> like comparisons of profiles).
> 
> Given this, I have to ask you Ross: what was the problem you were having
> with changing the DIR_D[XY] arrays?  Can you still reproduce it?
> 
> 
> I'd like to bring up the proposed plans for changing the directional
> system again:
> 
> - Ross's plan is to switch over slowly.  All of the core server and
> common code will be changed to use the DIR_DX2 arrays directly.  The GUI
> code (that he had had problems with before) will continue to use the
> DIR_DX arrays until that problem is solved.
> 
> - My plan is to switch over all at once, but not until everything is
> ready.  First off drop the DIR_DX2 arrays and just use the DIR_DX ones
> for now.  Then find and replace all remaining magic numbers/code so that
> the directional system is all encapsulated within map.h.  Finally make
> the switchover all at once.
> 
> 
> Ross, I did find one extra place where "magic" code is used: get_drawn()
> in client/goto.c.  This was easily spotted because the assertions of
> July 28th that were added by Thue (before that I'm not sure if there
> would have been a failed assertion check).  Could this account for the
> problems you were having before?
> 
> 
> To move forward on making this changeover, one of the plans needs to be
> chosen.  The next step for each of them would be quite different, so
> until a decision is made no step can be taken.
> 
> Ross: if there is no structural problem with the GUI code preventing the
> use of the new system (as it appears to me there is not), would you
> still be opposed to my system?  If you can still reproduce the problem
> you were having before, I'd like to take a look at it.

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?

$ grep -Ir DIR_DX .|grep -v DIR_DX2|grep -v CAR_DIR_DX|cut -d":" -f1|uniq -c
      2 ./ChangeLog
      1 ./client/goto.c
      6 ./client/gui-gtk/mapview.c
      2 ./client/gui-mui/graphics.c
      4 ./client/gui-mui/mapclass.c
      6 ./client/gui-win32/mapview.c
      3 ./client/gui-xaw/mapview.c
      6 ./common/map.c
      5 ./common/map.h
     14 ./server/gotohand.c

I assume common/map.[ch] is ok. client/gui-gtk/mapview.c's references
to DIR_DX are only about drawing the goto line. I assume all the other
mapview.c are similar. From looking at server/gotohand.c it looks like
it will also can cope with a different direction
ordering. server/gotohand.c may also make use of adjc_iterate.

I'm suprised at how few instances of DIR_DX are left.

> diff -u -r1.49 tilespec.c
> --- client/tilespec.c 2001/08/24 08:22:01     1.49
> +++ client/tilespec.c 2001/09/07 10:42:25
> @@ -1097,8 +1097,8 @@
>    }
>  
>    for (dir=0; dir<8; dir++) {
> -    int x1 = x + DIR_DX2[dir];
> -    int y1 = y + DIR_DY2[dir];
> +    int x1 = x + DIR_DX[dir];
> +    int y1 = y + DIR_DY[dir];
>      if (normalize_map_pos(&x1, &y1)) {
>        ttype_near[dir] = map_get_terrain(x1, y1);
>        tspecial_near[dir] = map_get_special(x1, y1);

We should replace the constants at 

  ttype_north      = ttype_near[0];
...
  tspecial_north_west = tspecial_near[7];

with names.

The other changes look ok for me (I'm not very deep into this). 

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I do feel kind of sorry for Microsoft. Their attornies and marketing
  force must have tons of ulcers trying to figure out how to beat (not
  just co-exist with) a product that has no clearly defined (read
  suable) human owner, and that changes on an hourly basis like the
  sea changes the layout of the sand on a beach. Severely tough to
  fight something like that."
    -- David D.W. Downey at linux-kernel


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