[Freeciv-Dev] Re: (PR#2516) introducing an enum citizen_type

["Raimar Falke via RT" <rt@xxxxxxxxxxxxxx>]

On Sun, Dec 08, 2002 at 01:23:31AM -0800, Jason Short via RT wrote:
> On Sun, 2002-12-08 at 04:10, Raimar Falke via RT wrote:
> > On Sat, Dec 07, 2002 at 08:43:46PM -0800, Jason Short via RT wrote:
> > > 
> > > The current client code tracks 11 different types of citizens: 2 each
> > > for happy, content, unhappy, angry citizens, plus elvises, scientists,
> > > and taxmen.  The order of these 11 is hard-coded in several places.  The
> > > sprites for all 11 are loaded manually, and the accesses to them are
> > > often also hard-coded.
> > > 
> > > This is inelegant and inextensible; see PR#2495.
> > > 
> > > The attached patch rewrites a lot of this code.  Now there are 7 types
> > > of citizens: happy, content, unhappy, angry, elvis, scientist, taxman. 
> > > Each type may have one or more sprites, as defined by the tileset. 
> > > Accessing the sprite is done through the get_citizen_sprite function. 
> > > This means a future change to the sprite system (such as PR#2495) can be
> > > done without any GUI changes.
> > > 
> > > The patch itself is pretty lengthy and touches every GUI.
> > > 
> > > A lot of the happiness code could be removed with some common
> > > functions.  This would be easier to do _after_ the enum citizen_type is
> > > put into place, but could be done before to make the patches slightly
> > > smaller.  I would prefer that this patch go in first.
> > 
> > Very nice.
> > 
> > First point: if we also allow different city styles for the citizen
> > sprites (as suggested by Rafal) shouldn't we also add a player
> > parameter now or will it be added later or do you plan to do this in
> > another way?
> 
> Good point.  Can we ever see enemy citizens?  If so, a player parameter
> is needed, which will be passed city_owner(pdialog->pcity) in most
> cases.

Yes. Diplomat investigating an enemy city.

> > >    for (n = 0; n < pcity->ppl_happy[4]; n++, i++) {
> > >      gtk_pixcomm_copyto(GTK_PIXCOMM(pdialog->citizen_pixmap),
> > > -                get_citizen_sprite(5 + i % 2), i * width, 0, TRUE);
> > 
> > > +                get_citizen_sprite(CITIZEN_HAPPY, i),
> > 
> > The i should be n IMHO. More predictable.
> 
> I disagree.  Using n, if one content citizen becomes happy all other
> content citizens will have their sprites toggled.  Using i all citizens
> have their graphics remain the same (unless they change status, e.g.,
> content->happy).
> 
> Note that MUI does use n here (I didn't bother to change this yet), and
> that behavior is intended to be unchanged by the patch (except for the
> MUI bug that's fixed).

Ok. I agree with you.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "Heuer's Law: Any feature is a bug unless it can be turned off."