[Freeciv-Dev] Re: somebody fix struct *player! (was: FoW remove player bug)

[Thue Janus Kristensen <thue@xxxxxxx>]

On Wed, 17 May 2000, Reinier Post wrote:
> On Tue, May 16, 2000 at 10:00:39PM +0200, Thue Janus Kristensen wrote:
> > On Tue, 16 May 2000, Reinier Post wrote:
> 
> [...]
>  
> > > Somebody (not me, I'm afraid) could do us all a great favour by rewriting
> > > the code such that struct *player pointers are permanent, immutable
> > > player identifiers.  [...]
> 
> > But then it would be a pain to map bitvectors to players.
> 
> No, why?  game.players[] is still available.
>  
> > Actually, for
> > this reason in most of the code what you need is the player id, not the
> > player pointer.
> 
> There would be no need for a player id anymore.

As an ID I mean it's position in the player array, which is actually the
same... And to avoid running through all the players in the array to count
it you would store the number in the player struct; voila - player ID!

> > You also need to compare players, but as long as either
> > the pointer or the id is used consistently it works.
> 
> The problem is, everything in the code that needs to store information
> about players must use player attributes.  An example is the player's
> command access level.  I wanted to store this in a separate data structure,
> because it isn't communicated to the client and  hence is out of place in
> struct player (which is in common/), but I found that this would require
> the addition of a
> 
>   update_my_data_structure_after_deletion_of_player(int plid)
> 
> call that would need to be invoked from anywhere in the code,
> whenever a player is deleted.  You apparently ran into the same problem.
> 
> This is broken datastructure design.  It may only be a detail
> but we'll keep running into it.

Yes, some parts of the code it is smartest to use the player pointer. So
use that there. It is just not smart when manipulating units, maps and
bitvectors.

> > Currently it is a little mixed, but I would prefer to use the player id
> > because we need that anyway for the bitvectors, and because it is already
> > used in the unit and city struct. (And I have actively begun converting
> > to that in my patches already :P)
> 
> Well, practically all the code seems to use struct *player pplayer to
> pass in arguments.  I don't believe there are player ids in the code that
> do not change over time.  It's OK to use a player id that is the index of
> the player in an array, but the index must not change in the course of
> a game if it is to be used to store information about players.
> 
> > Yes, the current implementation gets a bit ugly when renumbering, but it
> > is not that bad. No more than 50 lines of code.
> 
> The problem is not the amount of code, but the fact that eveyone who needs
> to store information about players has to deal with this problem; it's
> too easy to forget or to do it wrong.
> 
> > If you really wanted to
> > get rid of the renumbering a much easier solution would be to disable the
> > player removing ability once the game has started.
> 
> No, all you need to do is to keep the references around by which players
> are identified.  Removal will be an operation on game.players[] as it is
> now, or maybe a boolean .exists can be added to the player struct
> for convenience.

As far as I can see that was excactly what I suggested :)

> -- 
> Reinier

-Thue