Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2003:
[Freeciv-Dev] Re: (PR#2715) introducing tech_type_iterate
Home

[Freeciv-Dev] Re: (PR#2715) introducing tech_type_iterate

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: jdorje@xxxxxxxxxxxxxxxxxxxxx
Cc: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#2715) introducing tech_type_iterate
From: "Raimar Falke via RT" <rt@xxxxxxxxxxxxxx>
Date: Wed, 8 Jan 2003 13:04:11 -0800
Reply-to: rt@xxxxxxxxxxxxxx

On Fri, Jan 03, 2003 at 10:25:45PM -0800, Jason Short via RT wrote:
> 
> After the patch to add tech graphics to the client, I noticed (again)
> that several desirable iterators are missing.  One of them is
> tech_type_iterate, which is provided by the attached patch.
> 
> The concept is muddied by the existence of A_NONE and also by
> "non-existent" techs.
> 
> A_NONE==0 is hard-coded as a placeholder, empty tech.  tech_type_iterate
> should skip it and start with A_FIRST.  But many loops don't do this;
> they just start with 0.  I suspect most of these loops simply don't
> realize that A_NONE is irrelevant, but I haven't changed any of them yet
> (another patch can go over them in detail).  The one exception is the
> loop Rafal and I just added in tilespec.c.
> 
> tech_exists can be called to tell if a tech exists.  This isn't a very
> helpful explanation, and in fact it's not a very helpful function.  It
> will check for out-of-range techs (<0 or >= game.num_techs).  But it
> will return TRUE for A_NONE and FALSE for "non-existent" techs.  An
> example of a non-existent tech is environmentalism - it exists in the
> default ruleset but is impossible to research.  Per speculated that
> these techs were kept around from when the techs were hard-coded in the
> program, and said Raimar wanted to get rid of them.  But for now I just
> ignore the possibility when iterating, so tech_exists is NOT checked. 
> It could probably just as easily be done the other way (but none of the
> callers except the tilespec one bother with it, so I didn't either).

There seems to be some confusion what A_NONE is. It is the root
tech. It should be named A_ROOT. You _always_ know this tech. This
tech has itself as requirement. Thats it. Nothing more special. Given
these it is clear that code like:

    return (req == A_NONE
            || (get_invention(pplayer, req) == TECH_KNOWN)
            || player_owns_active_govchange_wonder(pplayer));

can and _should_ be written as

    return (get_invention(pplayer, req) == TECH_KNOWN
            || player_owns_active_govchange_wonder(pplayer));

This sin is repeated throughout the code. But this doesn't make A_NONE
more special. There are however some misuses: like setting researching
to A_NONE for denoting the researching of a future tech. A_LAST should
be used here. Or a new A_FUTURE.

So please change the code for tech_type_iterate to start with A_NONE
(or 0) and also convert the remaining cases
  grep -Irn for.*game.num_tech_types .

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Microsoft does have a year 2000 problem. I'm part of it. I'm running Linux.




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