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

[Reinier Post <rp@xxxxxxxxxx>]

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.

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

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

-- 
Reinier