[FIXED] Selection sound played on target death

[zoot]

This is a bug that I have experienced since Alpha 8, so I am surprised it has survived: when an attacking unit is selected and its target dies, its selection sound is played. I am not sure if it applies to all units in all cases, but I often hear a subtle "Ti esti?" every time something dies and its pretty annoying

[historic_bruno]

Create a Trac ticket and assign it to stwf.

[zoot]

Create a Trac ticket and assign it to stwf.

I can't seem to login to Trac. It isn't accepting my password even if I reset it.

[zoot]

I've attached a save game for testing this issue. In the save game, a group of your units will be chasing a single enemy cavalry unit in the lower part of the map. To reproduce the bug, select the units chasing the cavalry unit and follow them until they kill it. When the cavalry unit dies, you will hear a "Ti esti?". If the units are not selected, the "Ti esti?" sound will not be played.

selectionsoundondeath.0adsave.zip

[historic_bruno]

Confirmed the bug.

I will let Philip know about your Trac issue, is your username there also "zoot"?

[zoot]

I will let Philip know about your Trac issue, is your username there also "zoot"?

Yup.

[historic_bruno]

From IRC:

<Philip`> historicbruno: They're still in the .htdigest file, with the same password as they had on Oct 22

One thing to keep in mind is that the password reset doesn't actually work, despite it sending emails to the contrary.

[zoot]

Thanks, I did get in with my old password now. Maybe just a temporary outage.

[historic_bruno]

For some reason, I don't get this bug anymore. Can you confirm, zoot?

[zoot]

I can still reproduce it on most maps (oddly, not on the combat demo map). It only happens when I manually target an enemy unit - not when the UnitAI did it automatically.

[zoot]

Okay, I've done a bit of sleuthing on this. I take the above back, the bug also applies to UnitAI kills. The offending call seems to be on selection.js:318. Apparently, what happens is that when a unit in the selection is promoted (as a consequence of killing its target), the selection gets rebuilt and the side-effect of this is that the selection sound is played.

The easy fix is to add a flag to the rebuildSelection() function to indicate whether the sound should be suppressed: https://github.com/zootzoot/0ad/compare/1713

[historic_bruno]

Nice job tracking that down. Fixed in r12793

[Lion.Kanzen]

Okay, I've done a bit of sleuthing on this. I take the above back, the bug also applies to UnitAI kills. The offending call seems to be on selection.js:318. Apparently, what happens is that when a unit in the selection is promoted (as a consequence of killing its target), the selection gets rebuilt and the side-effect of this is that the selection sound is played.

The easy fix is to add a flag to the rebuildSelection() function to indicate whether the sound should be suppressed: https://github.com/z...ad/compare/1713

that is i like from this guy, go ahead Zoot.

[luziferius]

a minor note from my side: don’t use double negations in boolean expressions when you can avoid them.

instead of

 if (playsound !== false && owner == playerID || owner == 0 || g_DevSettings.controlAll)
_playSound(added[0]);

use

 if (playsound == true && owner == playerID || owner == 0 || g_DevSettings.controlAll)
_playSound(added[0]);

or even

 if (playsound && owner == playerID || owner == 0 || g_DevSettings.controlAll)
_playSound(added[0]);

that saves you 2 byte of disk space ( ) and is easier to understand

consider this example pseudo-c snippet(ask yourself which 'if' implements the desired behaviour; play the sound when sound is enabled)

if(disable_sound != false)
  playsound(whatever);
if(enable_sound == true)
  playsound(whatever);

in theory you could remove every occurance of "!== false" in the codebase…

EDIT: i just looked into the linked changeset, everything is ok, never said anything

Edited December 11, 2012 by luziferius

[leper]

in theory you could remove every occurance of "!== false" in the codebase…

You know that the variable could be undefined or null right? (Though your argument is valid)

[zoot]

a minor note from my side: don’t use double negations in boolean expressions when you can avoid them.

As leper implies, I was altering the prototype of an existing function. I couldn't have used "playsound == true" because existing code might be calling the function with no playsound argument, while still expecting the sound to be played. historic_bruno's ultimate formulation is less confusing, though.

[luziferius]

since that check isn’t copied 1:1 into the codebase i see my complain as [iNVALID], so only one last thing from my side

ok, I don’t know javascript. If there’s a semantic difference between var !== false and var == true for a boolean variable, your point is valid. isn’t there a warning when calling a function with too few parameters?

so there’s a difference between these 3 pseudo-js-functions?

function f1(bool_var)
{
if(bool_var !== false)
  do_smth();
}
function f2(bool_var)
{
if(bool_var == true)
  do_smth();
}
function f3(bool_var)
{
if(bool_var)
  do_smth();
}

when calling those like this?

f1();
f2();
f3();

You know that the variable could be undefined or null right?

since it’s boolean algebra, you can’t have a semantic 'undefined' or 'null'; undefined is a random true or false and null normally is false and everything else is true

Edited December 13, 2012 by luziferius

[leper]

Ah I misread your earlier post slightly.

Since JavaScript is weakly typed you can have bool_var === undefined (though the check for !== false, works just as expected).

No, there isn't a warning issued as that can be defined/expected behaviour. Indeed === is just for type and value comparision, so it doesn't make a difference for booleans.

f1 does something, f2 and f3 don't. ('function f1' would make that example even correct js)