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

["Ben Webb" <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jun 09, 2003 at 03:26:07PM -0700, Jason Short wrote:
> --On Monday, June 09, 2003 08:07:23 -0700 Ben Webb 
> <ben@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > I've just updated the FreecivAC patch to add borders to Freeciv, at
> > http://freecivac.sf.net/ (newborders-20030609.patch).
> 
> I've only looked at it briefly, and tested it out with gui-gtk...
> 
> It looks very good.

Well, that's because I've incorporated all the style fixes and map
drawing stuff that you put into the last version of the patch; it seems
only natural that you'd like your own code. ;)

> 1.  The fog-of-war "bug" is still there.

Yes. I haven't tackled that in this update - it's largely to add XAW
support and get it working again with current CVS.

> For instance if you explore the whole island, then found a new city you 
> won't "know" (even though it is obvious) what land you actually control. 
> The borders will end up being drawn right on the edge of the fog.  And if 
> you explore the fog from an alternate direction you'll see some odd-looking 
> behavior (see the attached borders-bug-1.png).

Yes, I've seen this too.

> Solving this is not trivial; the problem is similar to the one with 
> continent numbering but has more effect on gameplay.  The current method is 
> clearly flawed IMO.  But an omniscient POV is not so good either.

AFAIK, Alpha Centauri does this. This does mean that you can guess at
enemy city locations rather accurately (if the region is explored). The
original borders patch used only client-side information to determine
the borders, which avoided this problem, but it did mean that different
players thought the borders were in different places, which was a little
confusing (and which was shot down by the then Freeciv maintainers, IIRC).
I think I do mention this in the README.

My proposed solution to the problem: the server determines the
ownership of each tile, and sends it to clients, even if the tile is
fogged, but only in that case for your own tiles or those directly adjacent
to them (consider this as receiving formal notice from another civ that
they're annexed some of your territory). Borders are not drawn between
fogged and non-fogged tiles (i.e. a fogged tile for which ownership
information is not known counts as "indeterminate" rather than "no owner").

> 2.  When loading a savegame, an assertion is encountered.  This seems to be 
> reproducable - if not I have a savegame.  I haven't looked at it closely.

I think this was introduced by your changes to the patch, actually - but
correct me if I'm wrong. ;)

> #4  0x8064347 in update_city_tile_status_map (pcity=0x84bd008, map_x=1,
>     map_y=0) at citytools.c:1931

In the last version of the patch I released, this part of the code did:
+      city_map_iterate(x, y) {
+        update_city_tile_status(pcity, x, y);
+      } city_map_iterate_end;
+    } cities_iterate_end;

You changed this to:
+      city_map_iterate(x, y) {
+        update_city_tile_status_map(pcity, x, y);
+      } city_map_iterate_end;

If I'm not mistaken, you're passing city_map coordinates to a function
that expects map coordinates... or am I getting confused?

> 3.  The code to draw borders in iso view is gui-specific (it is in 
> gui-xxx/mapview.c), but it doesn't need to be.

True. In fact, the gui-gtk and gui-gtk-2.0 functions are identical.

> IMO the tile_draw_borders_iso() function should go in mapview_common
> and should call a GUI function (draw_line or draw_border) just to draw
> the line itself.  This is similar to what the non-iso code does.

Indeed. I was tricked by the map grid code being GUI-specific, and just
followed that example. Shouldn't that be moved to mapview_common as
well?

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"Verbosity leads to unclear, inarticulate things."
        - Vice President Dan Quayle, 11/30/88