[Freeciv-Dev] Re: Question about attributes

[Teemu Kurppa <tkurppa@xxxxxxxxxx>]

On Wed, 9 Jan 2002, Raimar Falke wrote:

> On Tue, Jan 08, 2002 at 07:26:05PM +0200, Teemu Kurppa wrote:
> > On Tue, 8 Jan 2002, Raimar Falke wrote:
> > 
> > > On Tue, Jan 08, 2002 at 02:51:30PM +0200, Teemu Kurppa wrote:
> > > > This makes coding a little bit cumbersome compared to solution, in
> > > > which persistent data is restored to client side data structures,
> > > > when client reconnects.
> > > 
> > > Can you save the client side data structures directly in the
> > > attributes?
> > 
> > With a current data structure, list, no. And I think that a list is a
> > natural choice for markers. I'd like to keep them in list instead of
> > an array.
> 
> You do something like this:
> 
> +struct tile_marker* find_marker_at(int x, int y)
> +{
>    int i, size, markers;
>    struct tile_marker *p;
> 
>    size=attr_player_get(ATTR_TILE_MARKERS, game.player_idx, 0, NULL);

I see you what you meant now. However, this doesn't work. I think we
should add something like attribute_length and attribute_player_length. It
would emphasize the correct usage of attributes. For example I didn't
notice that I can query a length of an attribute beforehand. Obviously,
the fact that at the moment I can't, may have something to do with it ;). 

Why doesn't your proposal work for a current code, see :
 
int attribute_get(int key, int id, int x, int y, int max_data_length,
                  void *data)
{

  struct attr_key pkey;
  void *pvalue;
  int length;

  freelog(ATTRIBUTE_LOG_LEVEL, "attribute_get(key=%d, id=%d, x=%d, y=%d, "
          "max_data_length=%d, data=%p)", key, id, x, y, max_data_length,
          data);

  assert(attribute_hash);

  pkey.key = key;
  pkey.id = id;
  pkey.x = x;
  pkey.y = y;

  pvalue = hash_lookup_data(attribute_hash, &key);

  if (pvalue == NULL) {
    freelog(ATTRIBUTE_LOG_LEVEL, "  not found");
    return 0;
  }

  length = ((int *) pvalue)[0];
  assert(max_data_length >= length);     <-- doesn't allow query 
  memcpy(data, (char *)pvalue + sizeof(int), length); <- also memcpy to NULL

  freelog(ATTRIBUTE_LOG_LEVEL, "  found length=%d", length);
  return length;
}
 
I can introduce necessary changes in a separate patch, if you let me :) As
I wrote, I favor separate functions for length queries. It makes the
purpose clearer.

>    markers=size/sizeof(struct tile_marker);
>    p=malloc(size);
>    attr_player_get(ATTR_TILE_MARKERS, game.player_idx, size, p);
> 
>    for(i=0;i<markers;i++)
>    {
>      if(p[i]->x == x && p[i]->y == y) {
>        return marker_clone(p[i]);         // needed because p gets freed
>      }
>    }
>    free(p);
> }
> 
> and
> +void add_marker(struct tile_marker* pnew)
> {
>    int i, old_size, new_size, new_marker;
>    struct tile_marker *p;
> 
>    old_size=attr_player_get(ATTR_TILE_MARKERS, game.player_idx, 0, NULL);
>    new_size=old_size+sizeof(struct tile_marker);
>    new_marker=old_size/sizeof(struct tile_marker);
>    p=malloc(new_size);
>    attr_player_get(ATTR_TILE_MARKERS, game.player_idx, old_size, p);
>    p[new_marker]->x=pnew->x;
>    p[new_marker]->y=pnew->y;
>    p[new_marker]->num=pnew->num;
>    attr_player_get(ATTR_TILE_MARKERS, game.player_idx, new_size, p);
> }

> > 
> > Yeah I know, and attached "patch" will flush attributes, when player
> > disconnects ;)  
> 
> Applied.
> 

I hesitated with that patch. It's not enough. If a player just 
disconnects, but doesn't quit, attributes are still not flushed. Necessary
addition is a one-liner, but I'll send it with other changes to attribute.c. 
  
Teemu Kurppa