[Freeciv-Dev] Re: Layer view patch

[Thue <thue@xxxxxxx>]

On Thursday 21 December 2000 18:34, Daniel M Church wrote:
> On Thu, 21 Dec 2000, Thue wrote:
> > On Friday 10 November 2000 07:24, Daniel M Church wrote:
> > > Updated.  It now has both Xaw and GTK; it also has both trident and
> > > engels tilesets.  I added two options to the View menu; one to control
> > > whether or not the focus unit is displayed even while others are not,
> > > and one to control Fog of War.  I also added a background tile for use
> > > when viewing with terrain off. Since the patches to the xpm files were
> > > getting a little big, I've stored the patch at
> > > ftp://ftp.freeciv.org/freeciv/incoming/layerview.diff.gz .  Once again,
> > > I built this off the October 31 CVS image.
> >
> > The patch seems to have been deleted from the ftp site; could you make it
> > available again?
>
> Done.  If it doesn't patch nicely (stuff has been known to change
> dramatically before), let me know and I'll freshen the patch to the latest
> CVS version.

Some comments:
-IMHO you should automatically turn off city names and city production when 
not displaying cities.
-You seem to have allocated extra colors in the tilesets, even though colors 
existed which were pretty close to the ones you used.
-You spelled "bacground" in the tilespecs :)
-You didn't indent the menu items in menu_items[] consistently with the rest 
of the code in gui-gtk/menu.c
-Some people want shortcut letters to all items in the menus, so you might as 
well put them in right away :) (you will notice that all other menu items 
already have them.)

Otherwise it looks fine, and I will commit it if nobody have further comments.

(yes, I am too lazy to make these changes myself when the author offered to 
freshen the patch himself:))
-Thue