Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2001:
[Freeciv-Dev] Re: [Patch] Cleanup of units and unittypes (PR#951)
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [Patch] Cleanup of units and unittypes (PR#951)
From: rf13@xxxxxxxxxxxxxxxxxxxxxx
Date: Mon, 10 Sep 2001 03:01:03 -0700 (PDT)

On Mon, Sep 10, 2001 at 02:33:06AM -0400, Ross W. Wetmore wrote:
> 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.

Ahh virtualunit used "." instead of "->".

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

No. There will however be a city_unit_type() for these cases:
$ grep -Ir currently .|grep unit_type|wc -l
     38

> 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.

This is correct. But unit_type is just short for unit_unit_type.

> 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.

It can't get wrost than the currect situation ;)

> 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.

I see no problem making this method a macro (with the same name) if it
turns out as a problem.

> 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?

1) Allmost all methods of common/unittype should accept "struct
unit_type *".
2) Finding and replacing common code constructs is a good thing IMHO.
3) the "type" field in "struct unit *" may change to "struct unit_type
*" in the long run.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I heard if you play the NT-4.0-CD backwards, you get a satanic message."
 "That's nothing, if you play it forward, it installs NT-4.0"



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