[Freeciv-Dev] Re: (PR#1870) borders patch feedback

["Ben Webb" <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Jul 02, 2003 at 02:51:41PM -0700, ChrisK@xxxxxxxx wrote:
> I've played around with CVS 01 JULY + newborders-20030630.patch

Thanks! Feedback is good.

> 1. Coastal ocean tiles should be included in the territory IMHO.

The trouble is this would be inconsistent with the way SMAC does things;
the borders patch was written originally to replicate SMAC borders.

>    All city tiles (to be worked on) should also be included.
>    Attached PNG shows how ugly it is now.

How about we just don't draw borders along coasts? SMAC doesn't either.
Whether to count ocean squares as national or international waters is
kind of dependent on what you think the distance scale to be. Claiming
tiles in the city radius would only help in the particular case you
show; it wouldn't help for other inland lakes/oceans.

> 2. The radius calculation is not exact, which results in ugly "circles".
>    Should be calculated from tile center to tile center. Guess there is a
>    int() instead of a round() somewhere.

We use the standard map_distance_vector function to calculate the
distance - what's wrong with it? The borders won't be circles, because
units can move diagonally at the same rate as horizontally and
vertically, even though the distance is greater.

> 3. The "claimed by" tile info is somewhat superfluous for city tiles.

True; I'll remove that.

> 4. Suggestion (maybe for later): draw borders so that the *side* can be
>    seen. Fx a semicircle instead of dashed line, with the arc to the outer
>    world.

I'm not quite sure what you mean here.

> 5. In the beginning when land is being discovered, there is a lot of
>    flickery due to redraw of city names/description.

Yes, that's mentioned in the README. I think this is more a problem of
map_info packet spam than borders stuff. Ideally all of these redraws
would be frozen until the whole group of map_info packets have been
received. The borders code shouldn't result in extra map_info packets
being sent here; if it does, it's a bug.

> 6. I've got a server crash when switching of fogofwar, and, later switch on
>    again, on turn done:
> 
> #0  0x400b1861 in kill () from /lib/libc.so.6
> #1  0x400b1665 in raise () from /lib/libc.so.6
> #2  0x400b2c81 in abort () from /lib/libc.so.6
> #3  0x400aba52 in Letext () from /lib/libc.so.6
> #4  0x08075f1f in decrement_pending_seen (pplayer=0x81660ec, x=0, y=0) at
> maphand.c:446

Hmm. None of the functions mentioned in the backtrace are modified by
the borders code, and pending_seen certainly isn't fiddled with. Borders
could be the culprit if it's overwriting random memory, I suppose. Are
you using the semifog patch as well? The standard borders patch doesn't
alter the player_tile struct.

>    Not reproducible.

Odd. Can you get a similar crash without the borders patch?

> 7. "Color" column in player report better named "Border"

Ah, but the function we use is "player_color". It's not only used for
borders.

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx         http://bellatrix.pcl.ox.ac.uk/~ben/
"The future will be better tomorrow."
        - Vice President Dan Quayle