[Freeciv-Dev] Re: Profiling Civserver again

[Trent Piepho <xyzzy@xxxxxxxxxxxxx>]

On Mon, 30 Jul 2001, Jason Dorje Short wrote:
> "Ross W. Wetmore" wrote:
> > 
> > Attached is a quick hack of my accumulated macro patches into map.c, map.h
> > for yesterday's CVS.
> 
> I'm not sure how to fit these together, but I have made changes to
> current CVS that do the following:

I didn't sturdy Ross's system extensively, but isn't it an extension of the
system I outlined?  He added the neighbor cells within a radius of 2 by
assigning them directions after 7.  It seems like you can use the same code,
except now you can iteratore over a radius of 2 by using directions 0 to 24,
or just the outer ring of tiles by using directions 8 to 24.

I suppose it matters if his enumeration has the nice properties the DX2 one
has.  One property is that you can do the cardinal diections by incrementing
the direction by two each time.  That also means that you can test if a
direction is cardinal by just looking to see if it is odd/even.  Another
property is that it is easy to calculate the reverse of a direction,
(dir+4)&7.

I suppose that since there are only 24 (or 25, does the center tile get
included somewhere?) directions, is_cardinal and reverse can just be easily
done with lookup tables.


> - Wrote a few simple macros, functions, and variables. 
> is_normal_map_pos(x, y) returns true iff (x, y) is in its normal
> (canonical) form.  get_dir(x, y) returns the direction from the (x,y)
> pair.  reverse(dir) returns the reverse of the direction [1].  DIR_NAMES
> holds the names ("N", "NW", etc.) of each direction.  is_cardinal(dir)
> behaves as you would expect.

I can see the name reverse() clashing with something.  Maybe dir_reverse() or
dir_rev()?

Also, what about creating an enum type for directions?  That way you don't
have code with "magic numbers" like I see now in the client.

enum dir_type { NORTH = 0, NORTHEAST = 1, etc. }

Would putting DIR_NAMES inside a function, const char *dir_name(enum dir_type
dir), help with i18n?  

> [1]  The biggest trouble I've had was figuring out that 128>>dir was
> really supposed to be 1<<reverse(dir).  That should have been commented!

When I first saw 128>>dir in the code, it took me a few seconds to figure out
what was going on.  Cleverness like that should get commented, so people can
appreciate it.  Too bad C doesn't have a way to access a machine bit rotate
instruction, because 16 rotated left by dir is the same thing as
1<<((dir+4)&7) in one operation instead of three.

> There are three advantages here: the adjc_dir_iterate macro is simpler
> (and harder to mess up) than manual iteration.  The new directional
> system is _much_ easier to work with.  Finally, enhancing freeciv to
> allow different map wrapping systems will be slightly easier.

It's faster too, since the adjc_* macros are more efficient than the manual
code that is in use now.

> I haven't quite finished upgrading all references to DIR_DX/DIR_DY, but
> when I do I'll send the patch along.  Right now both server and (GTK)
> client compile and run just fine (on an AI simulation).  Obviously, it
> should not be applied before 1.12.0.

There are a few, aren't there?  I saw a few case statements in the client that
used direction numbers, but I guess you caught those if the client works.