[Freeciv-Dev] Re: Question about attributes

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Jan 10, 2002 at 09:14:38AM +0200, Teemu Kurppa wrote:
> 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.

You see the attributes aren't really used. I agree that a seperate
attribute_length is the way to go. Please send a patch.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "It is not yet possible to change operating system by writing
  to /proc/sys/kernel/ostype."              sysctl(2) man page