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

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

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?

> +static char *get_citizen_name(enum citizen_type citizen)

Add a const. Add a note that these shouldn't be i18n.

>    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 * width, 0, TRUE);
>    }

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Transported to a surreal landscape, a young girl kills the first woman
  she meets and then teams up with three complete strangers to kill again."
    -- TV listing for the Wizard of Oz in the Marin Independent Journal