[Freeciv-Dev] Re: [Patch] Cleanup of units and unittypes (PR#950)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

It looks good as far as it goes.

But there are a number of places where unit_types[] are not replaced.

Grepping for virtualunit and bodyguard will hit some. But it would 
probably be best to check all struct unit declarations.

Some just use ids from different structs than a unit, and some are
plain ids. Are these to be all generalized as well? 

While I like the unit_type() C++ flavour, I don't think C allows the
overloading flexibility that would really allow one to carry this out.

I think we should see how this is to be done for each case, before 
taking the first step. That way one won't have to backtrack, or get 
stopped halfway and leave things in a worse mess than currently.

And if there are other cases, then unit_type(struct unit *) should 
probably not be the form used for this case.

And if not done fully, just doing it for punit may tend to obscure the 
use of unit_types[] in various cases. Just converting all references to 
get_unit_type(int id) might be more general. Or one could just convert 
all to unit_type[id] which is both standard and compact.

Also, if this is a function. there are an awful lot of places where
unit_type[] is used, and putting a function call in place of an 
array lookup should be profiled carefully before it is committed to
CVS.

In short, this looks like it needs a bit more explanation, and maybe
thought.

BTW: what is the real value of getting rid of Unit_Type_ids that seems
     to be driving this?

Cheers,
RossW
=====

At 04:59 PM 01/09/09 +0200, Raimar Falke wrote:
>The attached patch introduces a new method: "struct unit_type
>*unit_type(struct unit *punit)". The patch replaces
>"get_unit_type(punit->type)->" and "unit_types[punit->type]." with
>"unit_type(punit)->".
>
>This patch is the first one is a series of patches to bring
>Unit_Type_id the same fate as "int unit_id": remove it and replace all
>it with "struct unit_type *" in every function argument.
>
>       Raimar
>
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
>  Windows: From the people who brought you edlin...
>
>Attachment Converted: "c:\program files\eudora\attach\unit_type1.diff.gz"
>