[Freeciv-Dev] Re: the directional system

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

"Ross W. Wetmore" wrote:
> 
> At 06:59 AM 01/09/07 -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?
> 
> The client dumps core when you try to do goto lines. There were also
> some problems with rivers not joining and other spritely things. The
> server core is clean, i.e. autogames run fine. But if you start doing
> manual things with the GUI interface, or look closely at what is being
> drawn there are problems. Note, some of the sprite issues may be fixed
> if the non-isometric changes in tilespec.c that Raimar didn't like go
> in, i.e. all the ttype_north changes. But this isn't all the problems.

I see none of these problems.  I've tested things out briefly with
trident, engels, and hires tilesets with no problems.  I'm currently
playing a full game under engels tileset.

> While your plan is to wait until all the GUI issues are resolved and
> fixed. I don't think the two issues need to be linked. The GUI did its
> own thing before and can continue to do it after.

While it's certainly possible to unlink the two, doing so introduces
even more confusion IMO.  Certainly it forces you to make changes that
you're just going to change back once the GUI code is brought up to
speed.

> The linkage is what is new, and I think it doubles the size and probably
> squares the risk of problems and extra quibble chances.

The linkage is there now; everything except for one function is linked
to the DIR_DX arrays.  It's removing the linkage that takes work. 
Although you've already done this work, I think it's more complex than
it needs to be and that will be make it hard to get a maintainer to
sponsor the change.

> This clearly leaves only DIR_DX2 in the core - it wouldn't compile
> otherwise. And thus is a good way to check that you did everything right
> rather than just claiming to not have seen any residual problems.

No, you still have to claim that you haven't seen any residual problems
unless you look over each piece of code to make sure the change is
safe.  Changing the names to be DIR_DX2 may be convenient for this
switchover, but I don't see any reason why that change should go into
CVS.

> You can't do anything until you resolve all potential GUI issues including
> the places where it loads the graphics from datafiles into DIR_DX ordered
> memory, and then builds all the composite tile elements based on directional
> matches.

If such code exists, then it will indeed have to be corrected before we
can move on.  Fixing any "magic" code should be easy as soon as we find
it.

> First, change the order to start at the origin which is the NW corner.

This is just a change to the directional system itself.  My goal is to
make it so that all code outside of map.h is independent of the
directional system.  (But yes, I agree this system may be slightly
better.)

> It doesn't look like you fixed dir_ok in server/gotohand.c. The best way
> to do this is to compute straightest_direction, then reverse it and see
> if you are in the range -1/0/1.

No, I haven't made any changes to dir_ok.

> But the current straightest direction
> is slightly buggy in that it always moves "diagonally" first, then runs
> straight down the cartesian coordinate, rather than "straightest" which
> would have it move in the "largest" direction first.

Yes, I have noticed this for some time.  One of these days I'll submit a
patch to fix it.

> Until I can do a diff, I can't be sure if you have caught all the rest
> of the fixes in the previous submissions. There are sufficient omissions
> here to make me think this would be really worthwhile before getting into
> serious testing.

That would be much appreciated.

> But if you haven't they will come up as "rediscovered" bugs under testing,
> so at this point in the development it won't matter that much. One really
> needs to bang on things a bit though to one's raise confidence levels.

Yep.

> In the end, though this needs to be done 100%. You can't really afford to
> miss something, or have things dropped out for stupid reasons, or get them
> fouled up by amateur massaging along the way when it comes to final release.

This isn't quite true.  Trying to be 100% isn't worth it.  But 99% or
99.9% or 99.99999% may be reasonable.

> That's one of the reasons why I don't favour the kitchen sink approach.

I'm not advocating the kitchen sink approach.  I agree with you that a
full review will be needed before a switchover occurs.  Fortunately,
you've already reviewed the core code so all that remains (I think) is
the GUI code.

jason