Results of a small investigation into the plugins.

[rawnar]

I have had a look in the source code of the Positional Audio plug-ins to check the calculations. While I was going through them I also found some other thinks that looked off. I will list them here in three sections. Errors these are mistakes that can have effect on the usage of the plug-ins. Improvements things that can be changed to optimize the plug-ins. Questions some part of the code I did not understand. Each item will begin with a list of plug-ins that are related to the comment.

ERRORS:

• cod4, cod5, codmw2, etqw, wolfet Fetch() is used to check the function of the plug-in before sending true back in trylock(). Only the fetch function has a width range of values for the state check that will cause the function to return true. So different versions of the executable will return true for trylock(). This can result in a strange behaviour of the plug-in.
  
• bf2142, bfbc2, bfheroes The avatar vectors are filled before making a clean exit when the state vectors says your are not in a map. The avatar vector will still be send via the server to the other users in the channel. So if someone else in the channel is in the map he will hear you positionally while you are not in the map.
  
• l4d, l4d2 Sting assignments within a for loop. The context and identity string are assigned three times with the same value.
  
• arma2 Checking a variable before you have checked if the variable was read correctly by peekProc().
  
• bfbc2 Call to generic_unlock() in the fetch function. This seems incorrect, when compared to other plug-ins.
  
• etqw According to the comments the first and last entry of the front vector are incorrectly calculated. They need to be avatar_front[0] = cos(vievHor) * cos(viewVer); and avatar_front[2] = sin(viewHor) * cos(viewVer);
  
• lotro According to the comments the axis system is already left-handed, but it is made right-handed by switching to axis. The axis needs to by cycled. To get the y-axis pointing upward use, x->z, y->x, z->y. For the heading I can not give advice as I can not find comments how the angle is defined. (Talked to the coder and the comments in the source were wrong, so this remark can scratched)

 

IMPROVEMENTS:

• aoc, bf2, cod4, cs, css, dods, dys, etqw, gmod, hl2dm, insurgency, tf2, ut2004, ut3 The namespace std is used in the code. I did not put this in the error section, but some coders probably think it would be.
  
• aoc, css, dods, dys, gmod, hl2dm, insurgency, tf2 In fetch() move the string handling after possible quick escapes. The strings are filled while in a later part of the function there is a check that can exit the function with false.
  
• arma2, lotro, wow In these plugins there are several variables, functions or included headers that are not used. Cleaning up the code will make them better readable.
  
• cod2, cod4, cod5, codmw2, codmw2so, etqw, wolfet When angle are used to calculated the front vector also calculated the top vector and not provide a top vector which is almost never orthogonal to the front vector. The positional audio will work, but mumble has to recreate the top vector be finding the azimuth and inclination angle.
  
• arma2, bf1942, ut2004 There is not check if one is in a map or not. This can result in strange behaviour of the plugin. I have not put it in the error section, as I know how hard it can be to find a good state value.;-)
  

 

QUESTIONS:

• aoc, cs, css, dods, dys, gmod, hl2dm, insurgency, l4d, l4d2,tf2 The angles to calculate the front and top vector are checked, but why is that specific range picked? The range for the angles are -360
• bf2, bf2142, bfheroes Why return the fetch function with false when the user is not logged in? Are the pointers not working or is there no option to play a multiplayer game when you are not loggedin?
  

 

There you have it, another walk of text by the one and only forum spammer Mr. Rawnar. :D

Edited November 25, 2010 by rawnar

[rawnar]

Probably the first error, which is about the call to fetch within the trylock function, his not so prudent. Maybe we only need to be sure that the executable we are reading data from is the version we expect it to be. What I mean is do a simple peekProc on a string, so it can be checked against something expected. This way you know that you are connected to the executable you have written the plugin for. The call for fetch is only an additional test.

Edited October 21, 2010 by rawnar

[rawnar]

Going through the plug-ins again showed me that only cod4, cod5, codmw2, etqw and wolfet need an addition check in the trylock before returning true. In the fetch() call the state value is check to be unequal to a single number before returning true. Which means a large range of numbers will return true, so there is no good test on the executable version.

The initial post has been updated.

[hacst]

Thanks for reviewing the plugins. With the corrections everything seems to be valid to me. I guess all that's left now is fixing the code :lol:

ERRORS

As point one in the errors category requires you to have those games (installed) it is the most tricky one. I guess Snares has most of them and maybe we can get Timer to do Cod 5. I could install wolfet and cod4 if no one else steps up.

The rest, at least if the comments in the lotro plugin are correct, are valid catches and should be easy fixes. We just have decide on who does what.

IMPROVEMENTS

1. Doesn't strike me as very problematic but since its a simple string replacement thing can be easily fixed

2. Since the plugin unlinks I'm not really sure whether the string updates do still matter. But for the sake of good style the assigments should be moved, you are right.

3. Ok

4. When I wrote those (cod2/4/wolfet were my first contributions to mumble iirc :roll: ) I left out the top vectors since Mumble got them right on its own. I guess they could be added though I'm not quite sure what this would gain. No implicit behaviour?

Thanks again for taking time to review those.

ps: I was just writing to you about that when you left in IRC and yesterday I couldn't answer you because you left before I was back at my computer ;-) You should think about setting up a bouncer or something like that. Helps IRC communications greatly.

[rawnar]

IMPROVEMENTS

4. If you look at the code in mumble you will see that if the top vector is not perpendicular to the front vector it will recalculate the top vector. Recreating the top vector involves using the front vector together with acos() and atan2() to get the angles, then rotate the colatitude(called inclination in mumble) by 90 degrees and call the top vector with cos() and sin(). So the improvement lies in the fact that the calls to acos() and atan2() will not be needed. It is not much of an improvement as the two function calls will probably only take 100 clock cycles, but adding the top to the plugin is easy when you have the angles. ;)

The games I own are bf2, etqw, cs, css, tf2, ut3. And as far as I know wolfet, dys and insurgency are free.

[kissaki]

dys and insurgency are free.

Free mods, you’ll need a bought source engine game. But dd0t has one as well. :)

I got a lot of those games as well, so feel free to ask if you need help with one of them.

No CoD5 though.

[rawnar]

Additional note on improvement 4. You can also calculate the top vector without the use of acos and atan2. There is only one call to sqrt needed. If the values of the front vector are (x_f, y_f, z_f) then the top vector values are,

x_t = -x_f * y_f / sqrt(x_f * x_f + z_f * z_f) = -x_f * y_f / y_t
y_t =  sqrt(x_f * x_f + z_f * z_f) = sqrt(1 - y_f * y_f)
z_t = -z_f * y_f / sqrt(x_f * x_f + z_f * z_f) = -z_f * y_f / y_t

Also the right vector can be calculated this way,

x_r = z_f / sqrt(x_f * x_f + z_f * z_f) = z_f / y_t
y_r =  0
z_r = -x_f / sqrt(x_f * x_f + z_f * z_f) = -x_f / y_t

The zero value of y_r is because this vector will always be in the xy-plane. These formulas only hold when the front vector is a unit vector, which means x_f * x_f + y_f * y_f + z_f * z_f = 1. This method will show instability when the front vector point upwards as y_t will be close to zero. But this also holds for the atan2 method as atan2 is undefined when the first and second argument are zero.

An additional note on the implementation of the positional audio. The values of the positional vector are used to check if positional audio is performed. In my opinion using the front vector for that is a better choice. There are two reason. The first one is, the position vector can contain all zero's while you are in a map, but the front vector always non-zero in length. The change seems small, but if one uses the manual plug-in and forgets to set his position other then the default it will not work. This can cause confusion with the user. The second one is, if the front vector is zero but not the position vector the front and top vector are recreated in a fixed direction. This will mean that a when you are in-game and the person seems to be at your right. Turning your head to the right, the audio will not shift to the front but stay at the right. In other word the positional audio is useless.

P.S. Using the front vector for the check, can help plug-ins that use the avatars_pos vector to check if they are on a map or not.

And the wall keeps on growing. :D