Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2001:
[Freeciv-Dev] Re: worklists (PR#722)

[Freeciv-Dev] Re: worklists (PR#722)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: worklists (PR#722)
From: Kero van Gelder <kero@xxxxxxxxxxxxxxxxxxxxx>
Date: Sun, 11 Mar 2001 12:25:56 +0100
Reply-to: kero@xxxxxx

> Full_Name: Heikki Kniivilä
> Version: 
> Distribution: Mandrake binary

Reproducable from current cvs.

> Client: Both (or N/A)
> OS: Mandrake 7.2
> Submission from: (NULL) (
> If you open menu: Kingdom... Worklists...
> And make 2 lists (Different names)
> Open one by pressing "edit" list. Add something, and save.
> Then open both.
> And save both.
> TADAA!!!
> The names of both lists are same!

Worse, their content are the same!
Try playing with three lists. It is a mess...

Upon that, closing main worklist-window, then closing the
worklist-editor gives a gtk-compaint (but at least the client keeps

--- solution ---

OK, so the callback depends on an index in the parent dialog.
Bad habit!

a) it changes when you open the second list,
b) the index is used for all other references as well (like renaming),
   NB: it is really bad habit to use one var for >1 purpose.

Even when the indexed is passed to the child dialog:

c) the index can change when a list before the indexed one is removed.
d) the same list can be edited twice.
   NB: this is really bad, too.

Furthermore, the actual change seems to be applied via the
network. Why? What has the network got to do with something that is
(or could/should be) strictly client-side? [uh-oh, I found out, see below]

Possible solutions:
1) disable the parent dialog while a child dialog is active; this is
   done in many places in freeciv gtk, but the disadvantage is the
   user loses flexibility,
2) reference to the list in a unique way,
3) edit the list ITSELF, instead of via reference. It is passed to the
   function anyway!

NB: you need a way to lock the list, so it can't be edited twice at
the same time; I do not know the multi-threading behaviour of gtk,
but I know freeciv has no mechanisms implemented by itself. And no, a
simple boolean as field in the worklist will not work, of

--- end solution ---

So, at first I thought this would be simple to fix. It isn't.
For one, I do not understand the network-part [I do now...]
For two, there is no simple, clean, complete solution.
For three, I am afraid this behaviour is in many more places in the
code. Patching one place solves nothing.
For four, the solution must be independent of gtk, since there are
many more guis around for freeciv (which is a Good thing).

Hey, Kingdom -> Worlists for the second time does not open a second
window for worklists! Nice! Will it be strictly once, even if I press
Shift-L twice _very fast_? Short look at the code: apart from some
gtk-threading (of which I know /nothing/), no, it will not be strictly
once. There is nothing atomic, like a check-and-set function.

The network-stuff: when I start a new game, the worklists are not
remembered, though they were when I reconnected to a running server;
you are not maintaining work-lists in the server, are you? yes, you
are... So now I understand the network part. That leaves only solution
2) as workable option. Have fun implementing (the server is a single
thread, so that simplifies the editing). Wish: remember those
worklists in the client, too. Upload them to the server when I
connect. Well, at least worklists are not visible to other users.

Sidenote: wldlg.c is a somewhat meaningless name, unless you deduct
dlg is dialog and wl is worklist (well...). Given the rest of the
filenames, worklist.c would be just fine (stripping dlg from all of
them does not throw away any info).


+--- Kero --------------------------------- kero@xxxxxx ---+
|  Don't split your mentality without thinking twice       |
|                          Proud like a God -- Guano Apes  |
+--- M38c ------------------ ---+

[Prev in Thread] Current Thread [Next in Thread]