[Freeciv-Dev] Re: [PATCH] city_map_size fix and idea

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Raimar Falke wrote:
> 
> On Wed, Aug 22, 2001 at 09:54:05AM -0400, Ross W. Wetmore wrote:
> > The movement system does not use Pythagorean distances, and the "size"
> > of a city radius probably needs to be
> 
> > more closely associated with the number of moves rather than an
> > absolute distance.
> 
> Sounds very complicated.

Associating it with a number of moves will just make for an orthogonal
(diagonal) square city map.  Using a near-circular city map would be
much more appropriate.

> > While generalizing the city size to one which has an arbitrary radius
> > seems nice, I suspect that the myriad places where you need to do a fast
> > check to determine which of the [CITY_MAP_SIZE][CITY_MAP_SIZE] locations
> > is valid might become tricky.
> 
> Currently I don't see a problem here.

Fitting it into the [CITY_MAP_SIZE][CITY_MAP_SIZE] array isn't too
difficult since the same type of coordinate system will be kept.

What is difficult is fitting in the "specialty" iterators like
city_map_iterate_outwards (or whatever it's called).

> > Also, I doubt whether city sizes will ever be more than 3 and even this
> > would be a difficult sell because of the incompatibility with stock CIV
> > rules. Note increasing the upper bound on a city's size/productivity has
> > impact on a lot of game balance aspects like the value of a marketplace.

Yes, this is a good point but not really a reason not to do it IMO.  A
completely different ruleset could certainly use different sized cities.

> For me the primary goal isn't to enable to code to has a changeable
> CITY_SIZE but currently just code cleanup. "pcity->x + x +
> CITY_MAP_SIZE/2" is just dead ugly. A transformation method would be
> nice something with more a abstract numbering even better.

I agree wholeheartedly on this one.  I just believe that once these
things are cleaned up it will take just a few lines of code to make the
changes I point out.

As for a transformation macro, it's very similar to the MAP_STEP macro. 
You're basically talking about diffs like

- map_x = pcity->x + city_x - CITY_MAP_SIZE/2;
- map_y = pcity->y + city_y - CITY_MAP_SIZE/2;
- if (normalize_map_pos(&map_x, &map_y)) {
+ if (CITY_TO_MAP(pcity, city_x, city_y, map_x, map_y)) {

all over the code.  I also looked at using the city map iterators in
more places, but most of the places that don't use them currently do so
for good reasons.

Speaking of MAP_STEP, this macro turns the following code

- x1 = x + DIR_DX[dir];
- y1 = y + DIR_DY[dir];
- if (normalize_map_pos(&x1, &y1)) {
+ if (MAP_STEP(x, y, x1, y1, dir)) {

in a number of places.  About 20 of these places should be replaced with
adjc_dir_iterate instead, though, which should happen first IMO.

Neither of these make the code particular more legible IMO but they do
make it cleaner and more maintainable.

jason