[quake3-bugzilla] [Bug 4962] Protocol version 69 + legacy support

[bugzilla-daemon at icculus.org]

https://bugzilla.icculus.org/show_bug.cgi?id=4962

--- Comment #13 from Tim Angus <tim at ngus.net> 2011-06-27 07:01:17 EDT ---
OK, I've had a pretty good look at it now. I have a few points and questions:

* There are changes in cvar.c and Com_sprintf that look unrelated to this
patch; if that's the case they should probably just be committed anyway,
separately.

* On the same theme, the fix for the q3noclient issue should probably be in a
subsequent patch, to avoid clouding what is changing here.

* There is a line of commented code sending a disconnect command using
NET_OutOfBandPrint. If it's not necessary it should be removed completely. If
it needs to remain commented for some reason, this reason should be explained
in comments.

* The cvar names protocol and protocolver are pretty horrible and misleading. I
see why you've named them like this, but I think a better solution would be a
well placed hack to translate requests for the value of "protocol" coming from
legacy qvms to "oldprotocol".

* Actually, "oldprotocol" would be better as "legacyprotocol". This goes for
all the preprocessor tokens etc. too.

* Now a slightly bigger suggestion. If we're going to have all this upheaval
then we might as well do it right. To that end, I suggest we ditch identifying
protocol by a simple number and switch to using strings. There are probably
half a dozen things out there using protocol numbers in the range ~69-75, none
of which are remotely compatible. If we use strings instead we avoid all this
mess and also make it more difficult to (attempt) to connect to something where
it has no business working. If strings are a problem (for whatever reason) we
could alternatively use FOURCC codes like video codecs. This is assuming we
have 32bits to play with.

-- 
Configure bugmail: https://bugzilla.icculus.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.