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

[Thue Janus Kristensen <thue@xxxxxxx>]

On Tue, 16 May 2000, Reinier Post wrote:
> On Tue, May 16, 2000 at 08:39:51PM +0200, Thue Janus Kristensen wrote:
> > I kinda forgot taking server_remove_player(), and the player renumbering
> > that takes place, into account when I ade the FoW patch. This should make
> > it work again.
> > (Actually I had no idea it existed...)
> 
> Why oh why are players identified by their position in game.players[]
> when this position can change during the game?  This is horrible,
> as shown by the awkward copying that goes on in your patch.
> 
> 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.  All this would take is
> 
>  - a big array or linked list to hold all players that ever come
>    into existence in the course of a game
>  - changing game.players[] to be an array of struct player *pointers*
>  - removing player.plrid and changing all references to it
>  - changing all code that tries to identify players by their attributes
> 
> A bonus: another unique player identifier will be its index in the big
> array/list; this can be saved into savegames to allow players to be
> identified across different savegames of the same game.  This feature
> will allow the implementation of the /load foo.sav and /rename Foo
> commands.  It will also simplify the implementation of reporting
> tools thAT operate on savefiles.
> 
> -- 
> Reinier Post

But then it would be a pain to map bitvectors to players. Actually, for
this reason in most of the code what you need is the player id, not the
player pointer. You also need to compare players, but as long as either
the pointer or the id is used consistently it works.
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)

Yes, the current implementation gets a bit ugly when renumbering, but it
is not that bad. No more than 50 lines of code. 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.

-Thue