[Freeciv-Dev] Re: At long last: the Worklist patch

[Jeff Mallatt <jjm@xxxxxxxxxxxx>]

At 1999/12/22 22:04 , Corin Anderson wrote:
>  I've been hinting that it's been coming for some time.  Well, it's
>finally here:  my Worklist patch.  This patch adds the city build queues
>to both the Xaw and Gtk clients.

Wonderful!  This has been badly needed.

>I've added "+worklist" to the capability string because
>these changes are pretty widespread.  (If there's call for it, I could
>change this to "worklist" -- without the + -- to allow non-worklist
>clients to play with a worklist server or vice versa.  It won't be fun,
>but if need be, I'll do it.)

My opinion is that this is a big enough change to warrant a mandatory
capability.  So, there's no need to do this.

>Worklists are persistent across game saves/loads -- the worklists are
>stored in the saved game file.  As such, old saved games are -not-
>compatible with the worklist-enabled server.  It would be easy enough to
>write a tool that converted .sav files to include worklist info, if
>there's interest.

I believe we must keep the latest servers compatible with old save files --
and not by requiring a conversion program.  It should be easy to use one of
the secfile_lookup_*_default() functions to determine if your new
information is present, and if not, then build defaults.

>I've included in my diff a patch for all the files I've modified that are
>also in CVS.  In particular, my diff includes patches for auto-generated
>code (Makefile.am's, Freeciv.h).  I've included these patches because
>they're in CVS, but if I shouldn't have, please let me know.  (Why are the
>auto-gen'd files in CVS, anyway?  Shouldn't they be built only as part of
>the config or make process, and not actually in the repo?)  Of course,
>I've made the appropriate changes to the "source" files for these
>auto-created files, too.

Actually, the "Makefile.am"s are not auto-generated -- they are where you
specify your list of source files, etc.  Most "Makefile.in"s are generated
from "Makefile.am"s by automake (not the one in ./po -- it's built at
./configure time), and are included in the CVS.

As for why these files are in the CVS, I don't know the reasoning of
whoever made the decision, but a reason I can think of is that we want to
make it easy for people to do development.  Providing consistent generated
files means that a developer doesn't need the "special" tools required to
build some of these generated files, just to fix a simple bug (this applies
especially to the automake/autoconf tools).

>There are a few known "issues"
>that I'll look at when I get a chance, but they're not serious.

If you don't mind mentioning them, what issues?

------

>On Thu, 23 Dec 1999, Paul Zastoupil wrote:
>
>> Minor note, when I look at the global worklist it shows "Barbarian
>> Leader" as one of my build choices... If I try to build it, the server
>> gets:
>> "1: received bad string in packet (type 19, len 80) from localhost"
>
>Gotcha.  I'll fix this.  Are there other build targets, besides the
>Barbarian Leader, that a player will never be able to build?

This probably happens because of the way in which Barbarian Leaders are
currently kept from being built by players -- the "obsolete_by" is set to
Settlers (which are always available -- see comment in
data/default/units.ruleset at unit_barbarian_leader).  It would be better
to add a new unit flag, called "NonPlayer", which would identify unit types
which could not be built by players.

------

I have a few comments based on my first, quick perusal:

- The Kingdom menu Worklists item shortcut ('L') is missing in gtk client.

- Move the Worklist button in both clients' city dialogs to below the
improvements list and to the right of the Sell button.  I think this would
make the layout look much nicer, and take up lots less space.

- In a city's worklist, you are only allowed to insert things you can build
right now (except when you copy in a global worklist).  I'd like a
togleable option where I could see everything in the list to the right,
even stuff I couldn't build yet.  (Though, it'd be nice if it wouldn't show
Wonders that have already been built, and maybe even not show obsolete
units and improvements.)

And, a couple of code style things:

- Put wldlg.[hc] & worklist.[hc] in alphabetical order in Makefile.am files.

- Put worklist.h in proper place in include lists in server/cityhand.c,
client/packhand.c (in both cases, it belongs in the group with the rest of
the include files from the ./common directory).  Also, reorganize the
include lists in the new files to group them by directory -- see the
freeciv_hackers_guide.txt, Mini Style Guide section, for how to group
include files.

jjm