[CF-Devel] 1.1.0 client patches by Edgar

[Mark Wedel]

Edgar Toernig wrote:
>
     
     
     >
     
      Mark Wedel wrote:
     
     >
     
      >
     
     >
     
      > 1) His fix/check for libz is put in the configure file, and not in the
     
     >
     
      > configure.in where it should be.  As it is now, the next time configure is
     
     >
     
      > rebuilt via autoconf, that change will be lost.
     
     >
     
     
     >
     
      I'm aware of that.  But as I said, I'm not used to autoconf and someone
     
     >
     
      has to figure out how to modify configure.in.
     
     
 I've applied the patch to a copy I have.  I'll take care of the configure.in
setup - it is actually quite easy if you have the tools installed - you modify
configure.in, run 'autoconf', and it generates a new configure file for you.

>
     
     
     >
     
      > 2) The check for TCP_NODELAY should be possible in configure.
     
     >
     
     
     >
     
      Hmm... And then?  Changing the #ifdef TCP_NODELAY to #ifdef HAVE_TCP_NODELAY?
     
     
 Yeah, I actually don't see anything in configure, so keeping the TCP_NODELAY as
it is makes sense.


>
     
      Wrapping the NODELAY into an if(getenv) is mainly done for debugging.  It's
     
     >
     
      much easier to "CF_NAGLE=1 cfclient" then to change a define and recompile.
     
     
 Right, but even better is to be able to do something like gcfclient
-fastransmit or the like.  If something is meant for the end user to use, it
should be accessible in some easier manner.


>
     
      What I did in the patch is to turn on TCP_NODELAY and at least make sure that
     
     >
     
      a single crossfire command is send with one write.  That results in one net-
     
     >
     
      work packet per command.  A future optimization would be to collect more
     
     >
     
      commands and send them together.  I'll add that later.  (In fact, the reason
     
     >
     
      I did not do it right away was that crossfire uses very little bandwidth
     
     >
     
      so there was no immediate urge to add it.)
     
     
 The patch you did makes sense - the amount of data the client sends to the
server is relatively small, and you were able to form complete packets pretty
easily.  I don't have a problem with it - my only point was really that it
should be less cryptic for users than having to set an undocumented env
variable.  In all practicality, if available, it should always be used.

 I'd probably say don't bother with trying further consolidation for the
client->server commands - my initial thought is that it would get much more
complicated - the client would now need to have its own write buffer, and when
it then knows that nothing can get generated for a while (eg, about to sleep),
it sends out the buffer.  I mainly say this because the number of times the
client is likely to generate more than 1 command before such a sleep is probably
quite rare, and certainly infrequent enough that the extra tcp header probably
won't be that significant.

 Of course, if there were functions in the client which were going to send
multiple pieces and know it, then that function should optimize itself to only
do one write - I can't think of any such case right now, but an example I could
see would be if the client was smart enough to know about what the rings the
player had on (eg, had the paper doll type thing), if the player dragged a new
ring to a particular hand, it should unapply the old ring and apply the new one,
which amounts to two commands to the server, which should be in one packet.