[Freeciv-Dev] Re: [FreeCiv-Cvs] thue: Move the seen field from the common map to the...

[Thue Janus Kristensen <thue@xxxxxxx>]

On Sat, 20 May 2000, David Pfitzner wrote:
> > Many functions take
> > struct player *pplayer
> > as argument when what they need is really the player id (for lookup in a
> > bitvector). This patch corrects a few cases of that.
> 
> I'm a bit late on this, but will comment anyway...

It can still be reverted.

> This change seems to me a bit gratuitous: it results in widespread
> code changes, yet is of limited desirability IMO.
> 
> I don't think there is any real efficiency argument, since it is
> very simple to transform in either direction between pplayer and
> playerid.
> 
> And the programmer convenience issue is relatively small, since it 
> is trivial to do pplayer->player_no (hmm, that should be player_num 
> IMO), or get_player(playerid) (which could be macro if wanted).
> 
> So I think the "standard" should be to pick the one which is more
> "natural"/useful in most cases, and use that except where there is
> clear reason to prefer the other.  To me this means the standard
> should be pplayer.  Though I suspect Thue has been doing lots of 
> stuff outside the player struct recenty, so finds playerid more 
> natural...  But still I think most cases this can be hidden 
> inside the map etc functions rather than having patches with lots
> of changes where the caller has do the change, as:
> -  else if( map_get_known_and_seen(xu, yu, victim) )
> +  else if( map_get_known_and_seen(xu, yu, victim->player_no) )

Actually most of the time you don't need one fo the other, but need only
to compare two players, and either will do.
Since units and cities identify their owners with playerids, it
actually seemed to me to be easier to use playerids most of the time.
But we could make units and cities have player pointers in their
structs... (talk about big conversions...)
I also argued that the only need for the identification at the low level
was for indices in bitvectors, which actually gives a preference for
playerids

> One advantage of using pplayer more than playerid is that it 
> provides some "type safety", in that its harder to accidently
> use a unit_id or something, whereas playerid is just an int.

*Thue says something about wanting to use a strongly typed language
instead of using arguments ilke that.*
I agree with the type safety argument, and have indeed hunted down a
nasty random secfault in my own code resulting from it :). But in a mature
programming language that should not be an argument... Maybe we could
define a type playerid and turn on a compiler option to be strictly
warned of convertions.

> > Change the order of function argument from (player, x, y) to (x, y, player) 
> > in
> > some functions for consistency. More functions need to be changed.
> 
> (This change also seems a bit unnecessary to me, without a more
> clearly elaborated "consistency", but nevermind...)

Ok, half the functions take (arguments, player) , the other half take
(player, arguments). It became pretty irritating to me, so I figured I
would start transforming while I needed to change all the functions
manipulating the seen value anyway... This one has a clear benifit, at
least.

> Regards,
> -- David