On Tue, Jun 20, 2006 at 04:52:24PM -0500, williamb at its.caltech.edu wrote: > Tue Jun 20 16:48:48 CDT 2006 williamb at its.caltech.edu > * Adding packet priority to sndSSelf This patch name is incorrect. It should be "show observers the geno" or some other user-visible description. See the STYLE file. If you have any doubt as to what goes here, imagine you are describing it to non-coding Netrek clue in a message to the ALL board in the game. > * genspkt.c: Certain sndSSelf packets get marked as critical, > as is done with SndSelf, so that information like player > whydead is sent as high priority. Fixes problem where players > would not get proper whydead on game end, most noticeable for > observers. This change log entry did not note the changed function in full. See http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html for standards. We need to be able to find the change later using the change log *or* whatever repository we have at the time. It also doesn't show any results of research or testing ... like is this change tested, did you verify that the packet order was wrong, how was it wrong, did your change fix it, how did it fix it ... > hunk ./Vanilla/ntserv/genspkt.c 1041 > + if (commMode == COMM_UDP) { > + if ( ntohl(youp->flags) != pl->p_flags || > + youp->armies != pl->p_armies || > + youp->swar != pl->p_swar) { > + youp->type=SP_S_YOU | 0x40; /* mark as semi-critical */ > + } > + if ( youp->whydead != pl->p_whydead || > + youp->whodead != pl->p_whodead || > + youp->pnum != pl->p_no) { > + youp->type=SP_S_YOU | 0x80; /* mark as critical */ > + } > + } This segment of code looks similar to the code in sndSelf, but it has differences as well. 1. if the code is meant to do the same thing, add a new static function, such as sp_s_you_criticality() or something, and call that function from both places that do this same thing ... this is called factoring, 2. if the code is not meant to do the same thing, then make it clearer why there are differences. It might also be time to make 0x40 and 0x80 more formally declared. Interesting that it causes server packets to be restricted to 0-63 type codes, wasting bits in the type char. Perhaps some day we should change sendClientPacket to remove this prioritisation overload. -- James Cameron mailto:quozl at us.netrek.org http://quozl.netrek.org/