Bug 3787 - better handling of voip target bitfields
Status: RESOLVED FIXED
Alias: None
Product: ioquake3
Classification: Unclassified
Component: Misc
Version: GIT MASTER
Hardware: PC Linux
: P3 enhancement
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2008-09-22 12:07 EDT by Ben Millwood
Modified: 2011-07-27 11:48:46 EDT
1 user (show)

See Also:


Attachments
the described patch (12.67 KB, patch)
2008-09-22 12:08 EDT, Ben Millwood
fix buffer underflow (12.88 KB, patch)
2008-09-22 12:21 EDT, Ben Millwood
only parse voip targets on a new generation (4.88 KB, patch)
2010-10-19 09:02 EDT, Ben Millwood

Description Ben Millwood 2008-09-22 12:07:56 EDT
This patch has the following effects:

1. The voip code is made independent of MAX_CLIENTS by substituting clc.voipTarget[1-3] with clc.voipTargets, an array of uint8_t which is always just big enough. This makes a client and server with different values of MAX_CLIENTS incompatible but I assume that's more or less the case anyway.
For efficiency I could have used the value of sv_maxclients for how much I want to send, but I decided against this in favour of simplicity.
This unfortunately changes the voip protocol version, which brings me to:

2. Some code in aid of specifying one or more protocol versions has been added, although my attempts to maintain backwards compatibility ultimately proved futile and I've since been advised they were unnecessary. The macro VOIP_PROTOCOL_OK() is now used on the protocol version numbers instead of merely storing whether or not they are supported as booleans.
For what it's worth: supporting more than one protocol version was easy to code for but very difficult to actually use, because you have to either keep sv_voip and cl_voip the same, thus refusing to accept the new style, or change them, losing backward compatibility anyway. What would be useful would be some kind of list of supported protocols, perhaps space- or comma-separated in the two variables instead of a single integer. When connecting, a client and server would agree to use the highest number that appeared in both lists. If this is deemed a good idea, I may well begin work on it, but at present it seems like it would complicate things needlessly.

3. Instead of being parsed each time it is changed, cl_voipSendTarget is parsed every time the voip key is pressed. This is necessary because even while the string value remains constant, the target it represents may change (e.g. with "crosshair" or "attacker").

Probably I should separate these features out and submit them separately, but they kind of depend upon each other, so I'm only going to put the work in if someone tells me that one or two of the features are not wanted (and the other[s] are).
Comment 1 Ben Millwood 2008-09-22 12:08:29 EDT
Created attachment 1875 [details]
the described patch
Comment 2 Ben Millwood 2008-09-22 12:21:31 EDT
Created attachment 1876 [details]
fix buffer underflow

The previous patch occasionally allowed negative numbers to be used as array indices. Fixed.
Comment 3 Zachary J. Slater 2010-10-19 01:51:08 EDT
Does this need to be updated to be applied or is it still good?
Comment 4 Ben Millwood 2010-10-19 09:02:28 EDT
Created attachment 2464 [details]
only parse voip targets on a new generation

It seems like the patch doesn't apply anymore. It also had at least one bug (using the wrong vmcall).

As for updating it, I think that point 2. requires discussion and point 1. can't go ahead without it, but here's point 3. all on its own and cleaned up for you.

It compiles, but I can't test it; I'm pretty sure it should work but it would be a good idea for someone who has the whole mic setup to give it a try. I think that before if you had the target being "crosshair" and you pointed at someone, started talking, and then pointed away, they would stop being able to hear you. With this patch applied as long as you were pointing at them when you started talking then the channel should remain open (but conversely, if you weren't, then moving your crosshair over them mid-conversation won't help). So that's what needs testing.
Comment 5 Thilo Schulz 2011-03-14 13:44:42 EDT
Hey Ben, as you may have noticed I'm right now weeding out bug-reports and fixim them. Yours seems to be valid and I'd like to have the improvement in ioquake3. any updates on this?
Comment 6 Ben Millwood 2011-03-15 07:58:29 EDT
I think this is still relevant but I think it could do with some testing because it's possible I've misunderstood how generations etc. work. I can't do any work on it myself until the weekend.
Comment 7 Thilo Schulz 2011-07-27 11:48:46 EDT
partly applied r2102