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
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

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/