[Freeciv-Dev] Re: (PR#6585) Delta version 5

["Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>]

On Thu, Oct 30, 2003 at 12:54:58AM -0800, Per I. Mathisen wrote:
> > > > So a bitvector function doesn't follow the usual lowercase function
> > > > names. We need to convert floats to 4 byte integer values. And
> > > > worklists and raw memory have a generic function name but different
> > > > arguments.
> > >
> > > We should consider altering higher-level codea little to make lower-level
> > > code simpler.
> >
> > Can you be more specific?
> 
> Such as renaming the bitvector function to lowercase, for example. I am
> just offering a general idea here. It is you who know the python script. I
> am barely able to read it.

Such changes would make the patch even larger. And yes it would cause
more C and less python changes.

> > I think that the ability to turn fields off is not needed.
> 
> So what if we remove a field from a packet?

It stays. Delta will ensure that it doesn't use more than 1 bit of
extra bandwidth. Just zero it at the pre-send hook.

> > I think you misunderstood the issue with default values. I wrote in my
> > first posting that the generator will not support default values. The
> > generator however supports pre-send and post-receive hooks. Default
> > values can be implemented with these hooks.
> 
> Ok. I still think it is not necessary. We can implement different values
> in client/packhand.c and friends with capabilities, it does not need to be
> crammed into the delta code.

Where you put this special case code does indeed not matter. However
the pre-send and post-receive hooks have the nice property that
everything goes through them. In the other cases you have to change
every caller of the send* functions and/or the function(s) after the
packet is received. Look we also have currently such a special case:

void pre_send_packet_chat_msg(struct connection *pc,
                              enum packet_type packet_type,
                              struct packet_chat_msg *packet)
{
  if (packet->x == -1 && packet->y == -1) {
    /* since we can currently only send unsigned ints... */
    assert(MAP_MAX_WIDTH < 255 && MAP_MAX_HEIGHT < 255);
    packet->x = 255;
    packet->y = 255;
  }
}

void post_receive_packet_chat_msg(struct connection *pc,
                                  enum packet_type packet_type,
                                  struct packet_chat_msg *packet)
{
  if (packet->x == 255 && packet->y == 255) {
    /* unsigned encoding for no position */
    packet->x = -1;
    packet->y = -1;
  }
}

You can push this conversion into all the callers of this send method
(about 45 in CVS HEAD) but you see the difference. Another solution in
this case is to add a "bool have_position" or new packet
"packet_generic_message_nopos". But these are also not much better.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I heard if you play the NT-4.0-CD backwards, you get a satanic message."
 "That's nothing, if you play it forward, it installs NT-4.0"