[2.13] Sound parameter 0 overrides

[stgatilov]

Yet another breaking change, I'm afraid: 6346

Sounds have a bunch of parameters:

• minDistance
• maxDistance
• volume
• shakes
• soundClass

The base value for each parameter is set in sound shader.
However, it can be overridden with a different value in spawnargs (e.g. "s_volume" "-10") or in C++ engine code with SetSoundVolume (used extensively for footsteps).

Unfortunately, Doom 3 engine has a special case: setting some parameter to zero means it will not override the base value.
So there is no way to override sound volume with 0, because setting zero would mean "use value from sound shader", while setting 0.1 or -0.1 would mean "use volume = 0.1 or -0.1".

This behavior causes confusion.
It is especially bad when volume is set programmatically, because e.g. volume of player footsteps is computed as a sum of many modifiers (run, crouch, creep, in water, etc.) and it is hard to be sure you don't get zero sum in the end.

The idea is to fix this mess and add a "don't override" special value in the system.
Speaking of spawnargs, it would work like this:

• "s_volume" "13.4" = override with value 13.4
• "s_volume" "0" or "s_volume" "0.0" = override with zero
• "s_volume" "" (empty string) = don't override

Right now there are tons of zero values set in these spawnargs. It is not clear where the author intended to override with zero, and where he wanted to drop inherited override and use base value.

I guess for compatibility reasons I'll have to replace spawnargs "s_volume" "0" with "s_volume" "" in all missions.

[stgatilov]

I implemented a tool to replace spawnarg automatically.

Here is the list of missions using the paradigm of settings zero sound spawnargs:

Spoiler

ahouseoflockedsecrets\ahouseoflockedsecrets.pk4_dir\maps\manor.map: replaced s_volume 14 times
ahouseoflockedsecrets\ahouseoflockedsecrets.pk4_dir\maps\monastery.map: replaced s_shakes 1 times
alberic3\alberic3.pk4_dir\maps\alberic.map: replaced s_mindistance 8 times
alberic3\alberic3.pk4_dir\maps\alberic.map: replaced s_volume 3 times
alchemist\alchemist.pk4_dir\maps\alchemist.map: replaced s_volume 5 times
altham\altham.pk4_dir\maps\altham.map: replaced s_mindistance 106 times
altham\altham.pk4_dir\maps\altham.map: replaced s_maxdistance 6 times
antr\antr.pk4_dir\maps\antr.map: replaced s_mindistance 3 times
at1_lucy\at1_lucy.pk4_dir\maps\lucy.map: replaced s_volume 21 times
away0\away0.pk4_dir\maps\away0.map: replaced s_volume 1 times
bcd\bcd.pk4_dir\maps\bcd.map: replaced s_mindistance 62 times
bcd\bcd.pk4_dir\maps\bcd.map: replaced s_volume 46 times
black_mage\black_mage.pk4_dir\maps\tbm.map: replaced s_volume 1 times
briarwood\briarwood.pk4_dir\maps\briarwoodcathedral.map: replaced s_volume 2 times
briarwood_manor\briarwood_manor.pk4_dir\maps\briarwood.map: replaced s_volume 16 times
briarwood_manor\briarwood_manor.pk4_dir\maps\briarwood_autosave.map: replaced s_volume 16 times
bridge2far\bridge2far.pk4_dir\maps\bridge.map: replaced s_volume 3 times
bridge2far\bridge2far.pk4_dir\maps\bridge_autosave.map: replaced s_volume 3 times
broads\broads.pk4_dir\maps\builderroads.map: replaced s_mindistance 1 times
builders_influence\builders_influence.pk4_dir\maps\builders_influence.map: replaced s_volume 1 times
business3\business3.pk4_dir\maps\business.map: replaced s_volume 3 times
caduceus\caduceus.pk4_dir\maps\caduceus.map: replaced s_volume 13 times
cathedral\cathedral.pk4_dir\maps\cathedral.map: replaced s_volume 7 times
cauldron_v2_2\cauldron_v2_2.pk4_dir\maps\cauldron_v2.map: replaced s_volume 3 times
chalice1_1\chalice1_1.pk4_dir\maps\chalice.map: replaced s_volume 1 times
cleanneighbourhood\cleanneighbourhood.pk4_dir\maps\CleaningUp.map: replaced s_mindistance 3 times
cleanneighbourhood\cleanneighbourhood.pk4_dir\maps\CleaningUp.map: replaced s_volume 2 times
closemouthed_shadows\closemouthed_shadows.pk4_dir\maps\closemouthed_shadows.map: replaced s_volume 1 times
cos2_precpos\cos2_precpos.pk4_dir\maps\precpos2.map: replaced s_volume 2 times
cos3_sacricide\cos3_sacricide.pk4_dir\maps\cos3.map: replaced s_volume 2 times
cos3_sacricide\cos3_sacricide.pk4_dir\maps\cos3_autosave.map: replaced s_volume 2 times
crystalgrave2_1\crystalgrave2_1.pk4_dir\maps\krysztalowygrob1.map: replaced s_volume 69 times
delisle\delisle.pk4_dir\maps\delisle.map: replaced s_mindistance 8 times
delisle\delisle.pk4_dir\maps\delisle.map: replaced s_maxdistance 1 times
delisle\delisle.pk4_dir\maps\delisle.map: replaced s_volume 1 times
delivery\delivery.pk4_dir\maps\delivery.map: replaced s_volume 1 times
delivery\delivery_l10n.pk4_dir\maps\delivery.map: replaced s_volume 1 times
elixir\elixir.pk4_dir\maps\elixir.map: replaced s_mindistance 7 times
elixir\elixir.pk4_dir\maps\elixir.map: replaced s_volume 1 times
good\good.pk4_dir\maps\good.map: replaced s_mindistance 1 times
good\good.pk4_dir\maps\good.map: replaced s_volume 1 times
hazard\hazard.pk4_dir\maps\hazard_night.map: replaced s_volume 13 times
heartstmattis\heartstmattis.pk4_dir\maps\heartstmattis.map: replaced s_volume 2 times
heartv2\heartv2.pk4_dir\maps\heart.map: replaced s_volume 1 times
hhta\hhta.pk4_dir\maps\hhta.map: replaced s_mindistance 1 times
hhta\hhta.pk4_dir\maps\hhta.map: replaced s_volume 7 times
hhtlc\hhtlc.pk4_dir\maps\hhtlc.map: replaced s_volume 2 times
highex\highex.pk4_dir\maps\fsx.map: replaced s_mindistance 7 times
highex\highex.pk4_dir\maps\fsx.map: replaced s_maxdistance 3 times
inplainsight\inplainsight.pk4_dir\maps\dmx.map: replaced s_mindistance 6 times
inplainsight\inplainsight.pk4_dir\maps\dmx.map: replaced s_maxdistance 2 times
iris\iris.pk4_dir\maps\iris.map: replaced s_mindistance 95 times
iris\iris.pk4_dir\maps\iris.map: replaced s_maxdistance 5 times
kingofdiamonds\kingofdiamonds.pk4_dir\maps\kod.map: replaced s_mindistance 1 times
kingofdiamonds\kingofdiamonds.pk4_dir\maps\kod.map: replaced s_volume 1 times
knighton_manor\knighton_manor.pk4_dir\maps\knightons_manor.map: replaced s_mindistance 1 times
knighton_manor\knighton_manor.pk4_dir\maps\knightons_manor.map: replaced s_volume 3 times
living_expenses\living_expenses.pk4_dir\maps\living_expenses.map: replaced s_volume 2 times
lockdown1_2_1\lockdown1_2_1.pk4_dir\maps\lockdown.map: replaced s_volume 38 times
lordsnlegacy\lordsnlegacy.pk4_dir\maps\lordsnlegacy.map: replaced s_volume 1 times
matterofhours\matterofhours.pk4_dir\maps\speedbuild7.map: replaced s_mindistance 4 times
nhat3\nhat3.pk4_dir\maps\anoott.map: replaced s_mindistance 39 times
nhat3\nhat3.pk4_dir\maps\anoott.map: replaced s_maxdistance 11 times
nhat3\nhat3.pk4_dir\maps\anoott.map: replaced s_volume 15 times
nhat3\nhat3.pk4_dir\maps\forest.map: replaced s_mindistance 12 times
nhat3\nhat3.pk4_dir\maps\forest.map: replaced s_maxdistance 7 times
nhat3\nhat3.pk4_dir\maps\forest.map: replaced s_volume 3 times
nhat3\nhat3.pk4_dir\maps\politics.map: replaced s_mindistance 11 times
nhat3\nhat3.pk4_dir\maps\politics.map: replaced s_maxdistance 2 times
nhat3\nhat3.pk4_dir\maps\politics.map: replaced s_volume 7 times
northdale1\northdale1.pk4_dir\maps\m1.map: replaced s_mindistance 3 times
northdale1\northdale1.pk4_dir\maps\m1.map: replaced s_volume 1 times
northdale2\northdale2.pk4_dir\maps\m2.map: replaced s_mindistance 1 times
nowandthen\nowandthen.pk4_dir\maps\nowandthen.map: replaced s_volume 1 times
oldhabits2\oldhabits2.pk4_dir\maps\oldhabits.map: replaced s_volume 5 times
oldman\oldman.pk4_dir\maps\hpl31.map: replaced s_mindistance 4 times
oldman\oldman.pk4_dir\maps\hpl31.map: replaced s_maxdistance 1 times
onesteptoofar\onesteptoofar.pk4_dir\maps\onesteptoofar.map: replaced s_mindistance 6 times
onesteptoofar\onesteptoofar.pk4_dir\maps\onesteptoofar.map: replaced s_maxdistance 4 times
onesteptoofar\onesteptoofar.pk4_dir\maps\onesteptoofar.map: replaced s_volume 10 times
painterswife\painterswife.pk4_dir\maps\city.map: replaced s_mindistance 76 times
painterswife\painterswife.pk4_dir\maps\city.map: replaced s_volume 15 times
pandoras_box\pandoras_box.pk4_dir\maps\airship.map: replaced s_volume 2 times
pearlsnswine\pearlsnswine.pk4_dir\maps\pearls8.map: replaced s_mindistance 3 times
pearlsnswine\pearlsnswine.pk4_dir\maps\pearls8.map: replaced s_volume 1 times
penny2_1\penny2_1.pk4_dir\maps\allthewayup.map: replaced s_mindistance 3 times
penny2_1\penny2_1.pk4_dir\maps\allthewayup.map: replaced s_volume 3 times
penny3\penny3.pk4_dir\maps\erasing.map: replaced s_mindistance 4 times
penny3\penny3.pk4_dir\maps\erasing.map: replaced s_volume 1 times
peril\peril.pk4_dir\maps\peril.map: replaced s_volume 11 times
poets\poets.pk4_dir\maps\poets.map: replaced s_volume 1 times
ptb0\ptb0.pk4_dir\maps\movingday187.map: replaced s_volume 3 times
ptb0\ptb0.pk4_dir\maps\movingday187_autosave.map: replaced s_volume 3 times
quinn\quinn.pk4_dir\maps\chase.map: replaced s_volume 3 times
requiem\requiem.pk4_dir\maps\requiem.map: replaced s_shakes 1 times
returntothecity\returntothecity.pk4_dir\maps\rttc.map: replaced s_mindistance 1 times
returntothecity\returntothecity.pk4_dir\maps\rttc.map: replaced s_volume 23 times
rift\rift.pk4_dir\maps\rift.map: replaced s_volume 3 times
rightful\rightful.pk4_dir\maps\rightful.map: replaced s_volume 1 times
river\river.pk4_dir\maps\invernesse.map: replaced s_mindistance 1 times
river\river.pk4_dir\maps\invernesse.map: replaced s_volume 5 times
river\river.pk4_dir\maps\river.map: replaced s_volume 23 times
samhain\samhain.pk4_dir\maps\samhain.map: replaced s_volume 1 times
score_to_settle\score_to_settle.pk4_dir\maps\spring13.map: replaced s_mindistance 1 times
score_to_settle\score_to_settle.pk4_dir\maps\spring13.map: replaced s_volume 2 times
scroll\scroll.pk4_dir\maps\scroll.map: replaced s_mindistance 16 times
scroll\scroll.pk4_dir\maps\scroll.map: replaced s_volume 1 times
seeking\seeking.pk4_dir\maps\seeking.map: replaced s_mindistance 179 times
seeking\seeking.pk4_dir\maps\seeking.map: replaced s_maxdistance 5 times
seeking\seeking.pk4_dir\maps\seeking.map: replaced s_volume 13 times
sk_cooks\sk_cooks.pk4_dir\maps\cookbook.map: replaced s_mindistance 2 times
sk_cooks\sk_cooks.pk4_dir\maps\cookbook.map: replaced s_volume 2 times
sneak_destroy\sneak_destroy.pk4_dir\maps\sneak.map: replaced s_mindistance 2 times
sneak_destroy\sneak_destroy.pk4_dir\maps\sneak.map: replaced s_maxdistance 2 times
snowed_inn\snowed_inn.pk4_dir\maps\snowed_inn.map: replaced s_mindistance 2 times
solarescape1\solarescape1.pk4_dir\maps\solarescape1.map: replaced s_volume 2 times
sons_of_baltona_1\sons_of_baltona_1.pk4_dir\maps\sons_of_baltona_1_beta.map: replaced s_volume 2 times
spiderfinch\spiderfinch.pk4_dir\maps\g1.map: replaced s_volume 1 times
springcleaning\springcleaning.pk4_dir\maps\sc.map: replaced s_volume 1 times
storm\storm.pk4_dir\maps\storm.map: replaced s_volume 1 times
sw1_ebound\sw1_ebound.pk4_dir\maps\sw1_ebound.map: replaced s_shakes 25 times
talbot\talbot.pk4_dir\maps\kiss.map: replaced s_mindistance 24 times
talbot\talbot.pk4_dir\maps\kiss.map: replaced s_maxdistance 1 times
talbot\talbot.pk4_dir\maps\kiss.map: replaced s_volume 2 times
thecreeps\thecreeps.pk4_dir\maps\003.map: replaced s_volume 12 times
thecreeps\thecreeps.pk4_dir\maps\003.map: replaced s_shakes 3 times
thiefsden\thiefsden.pk4_dir\maps\thiefs_den.map: replaced s_volume 1 times
threepenny\threepenny.pk4_dir\maps\threepenny.map: replaced s_mindistance 1 times
transaction\transaction.pk4_dir\maps\transaction.map: replaced s_volume 1 times
ulysses_genesis\ulysses_genesis.pk4_dir\maps\ulysses_genesis.map: replaced s_mindistance 6 times
volta1_3\volta1_3.pk4_dir\maps\volta_v1.map: replaced s_mindistance 3 times
volta1_3\volta1_3.pk4_dir\maps\volta_v1.map: replaced s_volume 4 times
volta1_3\volta1_3.pk4_dir\maps\snapshots\volta_v1.map.0.map: replaced s_mindistance 3 times
volta1_3\volta1_3.pk4_dir\maps\snapshots\volta_v1.map.0.map: replaced s_volume 4 times
written\written.pk4_dir\maps\written.map: replaced s_mindistance 144 times
written\written.pk4_dir\maps\written.map: replaced s_maxdistance 19 times
written\written.pk4_dir\maps\written.map: replaced s_volume 3 times
written\written.pk4_dir\maps\written.map: replaced s_shakes 1 times
ws1_north\ws1_north.pk4_dir\maps\ws1_north.map: replaced s_volume 1 times
ws2_homeagain\ws2_homeagain.pk4_dir\maps\ws2_homeagain.map: replaced s_volume 1 times
ws4_warrens\ws4_warrens.pk4_dir\maps\ws4_warrens.map: replaced s_volume 1 times
ws5_commerce\ws5_commerce.pk4_dir\maps\ws5_commerce.map: replaced s_volume 3 times
altham\altham.pk4_dir\def\custom.def: replaced s_volume 1 times
iris\iris.pk4_dir\def\welli_mementos.def: replaced s_volume 2 times

I wonder: does it happen automatically in DarkRadiant somehow?
Or everyone explicitly specify zero?
And if you intentionally specify zero, what do you want to achieve this it?

[Frost_Salamander]

41 minutes ago, stgatilov said:

I wonder: does it happen automatically in DarkRadiant somehow?
Or everyone explicitly specify zero?
And if you intentionally specify zero, what do you want to achieve this it?

I just had a quick look at the usage in my missions:

• s_mindistance = 0 is set on some speakers used for weather sounds (e.g. wind), probably because it didn't make sense for that sound to fade.  But I think setting both s_mindistance and s_maxdistance to the same value and also setting s_omni to '1' would probably have been a better way to do it.
• The prefab furniture/tables/desk3.pfbx (and desk3_old) has both s_mindistance and s_maxdistance set to 0 by default. I guess it is meant to be tweaked by the mapper and I didn't notice it.
• the prefab mechanical/machines/printing_press.pfb has a speaker called 'speaker_machine_stop' and both s_mindistance and s_maxdistance are set to 0 by default.  Looks like I missed that one too.

Edited March 30 by Frost_Salamander

[stgatilov]

As far as I see in the code, minDistance/maxDistance specify the range in meters where volume falloff happens.

Let's introduce F(d) as piecewise linear function.
Then:

• F(0) = F(minDistance) = 1
• F(maxDistance) = F(+infinity) = 0
• F(d) changes linearly from minDistance to maxDistance

The sound gain is multiplied by F(d)^2 (i.e. falloff is turned quadratic).

If we talk about real physics, the falloff from point sound should linear without any caps.
The gain should be multiplied by (1 / d).

However, real sound sources are never perfectly concentrated at a point.
If a sound source has size S, then the volume at distances up to O(S) will look like constant.
I suppose default minDistance is nonzero to replicate this effect.

The existence of maxDistance is harder to justify physically.
Perhaps it is here for optimization reasons (don't process sounds farther than this).

As for quadratic falloff instead of linear, I think the simply wanted the sound to fade away to zero smoothly.

[stgatilov]

So the sound has full volume on distances up to minDistance, and no volume at distances above maxDistance.
I have not found any special meaning for maxDistance = 0 in the code, so I assume such a sound will never be heard due to zero volume.

Also, there are some default values littered around the code.
Most importantly, sound shader gets minDistance = 1 and maxDistance = 10 by default.
Also I think I have seen min=0, max = 10 somewhere in the code.

 

12 hours ago, Frost_Salamander said:

s_mindistance = 0 is set on some speakers used for weather sounds (e.g. wind), probably because it didn't make sense for that sound to fade.  But I think setting both s_mindistance and s_maxdistance to the same value and also setting s_omni to '1' would probably have been a better way to do it.

There is little difference between minDistance = 1 and minDistance = 0.
Mostly the difference is that fading does not happen within 1 meter of the sound source.

Supposedly, setting minDistance = maxDistance = 0 will cause sound to be completely muted.

If you want a sound to always have full volume, I think you should set e.g. minDistance = maxDistance = 1000.
Of perhaps set "global" flag instead, which disables distance computation completely. Although this probably has different effect (like maybe global sound passes through walls too).

Or if you want it to be sharply disabled at 10 meters, then set minDistance = maxDistance = 10... but I see no point in sounds that can be disabled based on distance without any fade.

Finally, as I have described at the very beginning of the post, "minDistance" "0" as spawnarg has no effect on current TDM versions.

 

Quote

• The prefab furniture/tables/desk3.pfbx (and desk3_old) has both s_mindistance and s_maxdistance set to 0 by default. I guess it is meant to be tweaked by the mapper and I didn't notice it.
• the prefab mechanical/machines/printing_press.pfb has a speaker called 'speaker_machine_stop' and both s_mindistance and s_maxdistance are set to 0 by default.  Looks like I missed that one too.

I don't see mindistance/maxdistance settings in these two prefabs in SVN.
Maybe this were the issues that someone fixed recently.

Or... could it be that you confuse spawnargs with sound shader settings?
Actually, I don't see the distance settings on sound shaders either.

[Frost_Salamander]

30 minutes ago, stgatilov said:

I don't see mindistance/maxdistance settings in these two prefabs in SVN.
Maybe this were the issues that someone fixed recently.

Or... could it be that you confuse spawnargs with sound shader settings?
Actually, I don't see the distance settings on sound shaders either.

The settings are on the speaker entities contained in the prefabs.  Is that not what you meant?  If not, could you change your auto-replace tool you showed in this post to print the name of the entity where the replace is happening?

I forgot to say which speaker in the desk, but it's in the one for the clock ('ernst_clock_ticking2')

I mentioned the speaker for the printing press, it's called speaker_machine_stop (or speaker_machine_stop2)

Just open up DR, add those prefabs, click the speaker entities and look at the spawargs, they should be as I describe.

[stgatilov]

38 minutes ago, Frost_Salamander said:

I forgot to say which speaker in the desk, but it's in the one for the clock ('ernst_clock_ticking2')

I mentioned the speaker for the printing press, it's called speaker_machine_stop (or speaker_machine_stop2)

Just open up DR, add those prefabs, click the speaker entities and look at the spawargs, they should be as I describe.

I imported them into DR, still don't see any s_xxxDistance spawnargs there:

Spoiler

      

 

[Frost_Salamander]

Strange, they are definitely there for me.  What version of DR do you have?  I'm on 3.9.0 now but this was the same in 3.8.0...

[Frost_Salamander]

and it's not defined in the prefabs.  I'm not sure how those spawnargs are getting there.  It's not from inherited properties either.

// entity 6
{
"classname" "speaker"
"name" "speaker_machine_stop2"
"origin" "-17.25 22.75 66.25"
"s_shader" "machine_noise08_loop"
"s_waitfortrigger" "1"
}

<entity number="17">
  <primitives/>
  <keyValues>
    <keyValue key="classname" value="speaker"/>
    <keyValue key="name" value="ernst_clock_ticking2"/>
    <keyValue key="origin" value="36.5 -41 76"/>
    <keyValue key="s_looping" value="1"/>
    <keyValue key="s_shader" value="clock_tick01_loop"/>
    <keyValue key="s_volume" value="-5"/>
  </keyValues>

 

Edited March 31 by Frost_Salamander

[stgatilov]

I have DR 3.0.0.
(I rarely update it since I rarely use it)

[Frost_Salamander]

36 minutes ago, stgatilov said:

I have DR 3.0.0.
(I rarely update it since I rarely use it)

I installed DR 3.0.0 (portable version) and I see what you see now, so it's something to do with DR:

The change seems to have been introduced in 3.1.0 which I also tried:

There were a bunch of sound shader-related fixes done in the 3.1.0 release: https://bugs.thedarkmod.com/changelog_page.php?version_id=101

perhaps this one? https://github.com/codereader/DarkRadiant/commit/541f2638c810588ada12e9a28360f16df6143d45#diff-104c4215e1bcd3ef19a7c943fec728649b1b0ba3bccf30600084b28dc1a8e67d

@greebo

[stgatilov]

Yes, this behavior of DR would become a big problem after this change.
Because all sound sources will be efficiently muted...

Meanwhile, I think most of the maps were created before this behavior.
So other opinions on setting zero sound spawnargs are welcome.

[stgatilov]

The new behavior is available in the latest dev17026-10712.
If you set cvar s_overrideParmsMode to 1, then you get the new behavior.
The old behavior is under value 0, which is default yet.

Also there is "debug mode" if you set value to 2.
In this case the new behavior is used, but console warnings are posted when a sound being started shows difference in behavior (i.e. the engine computes both behaviors and complains if they are different).
Right now you'll see regular warnings about various AI sounds: they have wrong effective volume in TDM 2.12 and before due to this issue.

As for modifying the missions, I think the main blocker is the new DarkRadiant behavior.
@greebo @OrbWeaver, could you please comment why DR automatically sets s_minDistance and s_maxDistance spawnargs since recently?

[OrbWeaver]

The commit which introduced unconditional writing of the s_mindistance and s_maxdistance spawnargs was this one:

https://github.com/codereader/DarkRadiant/commit/541f2638c810588ada12e9a28360f16df6143d45

and it appears it was intended to fix this bug:

https://bugs.thedarkmod.com/view.php?id=6062

The current logic is to set the spawnargs to the same values as in the sound shader, if a shader is set:

// Write the s_mindistance/s_maxdistance keyvalues if we have a valid shader
if (!_spawnArgs.getKeyValue(KEY_S_SHADER).empty())
{
	// Note: Write the spawnargs in meters
	_spawnArgs.setKeyValue(KEY_S_MAXDISTANCE, string::to_string(_radii.getMax(true)));
	_spawnArgs.setKeyValue(KEY_S_MINDISTANCE, string::to_string(_radii.getMin(true)));
}

This happens in the freezeTransform method which is called after performing some manipulation of the speaker entity such as moving or resizing it. In this case _radii is the object which contains the modified speaker radii, so this code is persisting the modified radii into the relevant spawnargs. This seems to be working correctly when I manipulate a speaker with a valid sound shader.

The only way I can get 0 is by creating a speaker with a sound shader like blackjack_swing which does not have radii defined. In this case the speaker has a default minimum radius of 0.0 and a default maximum radius of 10.0. We could avoid setting a radius at all, but then the speaker just appears as an entity box rather than a sphere/circle, which I assume is the original reason for setting a default value.

Right now I have no idea what code path would lead to having both a minimum and a maximum of 0.0. I think we'd need more detailed reproduction steps.

This is the current logic for setting the spawnargs on speaker construction (rather than manipulation, which is the previous code):

// Initialise the speaker with suitable distance values
auto radii = soundShader->getRadii();
entity.setKeyValue("s_mindistance", string::to_string(radii.getMin(true)));
entity.setKeyValue("s_maxdistance", radii.getMax(true) > 0 ? string::to_string(radii.getMax(true)) : "10");

So there is a specific check that s_maxdistance is greater than 0 before setting it as a spawnarg. Code similar to this has existed for many years, as far as I can see, and I have to go as far back as 2009 to find something different (originally all speakers just had hardcoded 16/32 radii to make them visible).

[stgatilov]

But sound spawnargs are merely overrides.
You should not set them at all if you don't intend to override the values set in sound shader.

Also, how does moving anything affect sound distances?
Whenever you move sound, you should only change its position/orientation and nothing else, shouldn't you?

[stgatilov]

On 4/15/2024 at 8:22 PM, OrbWeaver said:

The only way I can get 0 is by creating a speaker with a sound shader like blackjack_swing which does not have radii defined. In this case the speaker has a default minimum radius of 0.0 and a default maximum radius of 10.0. We could avoid setting a radius at all, but then the speaker just appears as an entity box rather than a sphere/circle, which I assume is the original reason for setting a default value.

I believe the engine sets mindistance = 1 and maxdistance = 10 if they are not set explicitly in the sound shader.
At least that's what I see here: https://github.com/stgatilov/darkmod_src/blob/trunk/sound/snd_shader.cpp#L154

Anyway, if DR sets spawnargs to the same values as in sound shader, that's not a problem for the suggested change in the meaning of zero value. But setting maxdistance = 0 will be
 

[OrbWeaver]

10 hours ago, stgatilov said:

But sound spawnargs are merely overrides.
You should not set them at all if you don't intend to override the values set in sound shader.

That's what we originally did, but mappers (or at least one mapper) complained in bug 6062 about the spawnargs disappearing if they were the same as the sound shader. I suppose we could have considered that "not a bug" and kept the original behaviour, but perhaps there are situations where the old behaviour was problematic — there would be no way to distinguish between "this speaker has default radii from the shader" and "this speaker has fixed radii which happen to be the same as the shader but must not change, even if the shader is edited". I don't know how common such a situation is in practice.

10 hours ago, stgatilov said:

Also, how does moving anything affect sound distances?
Whenever you move sound, you should only change its position/orientation and nothing else, shouldn't you?

Correct, but the freezeTransform method is called after the end of any transformation, and does not distinguish between what type of transformation was previously happening. I imagine resizing the speaker to the same size as the shader would also have triggered bug 6062, but speakers are resized less often than they are moved and hitting an exact size with the mouse would be rare, so the issue was only noticed when moving speakers.

10 hours ago, stgatilov said:

Anyway, if DR sets spawnargs to the same values as in sound shader, that's not a problem for the suggested change in the meaning of zero value. But setting maxdistance = 0 will be
 

That's what I'm confused about. I have yet to see any situation in which DR will set a max distance of 0 on a sound shader, other than by explicitly editing the spawnarg to have a "0" value.

[stgatilov]

8 hours ago, OrbWeaver said:

Correct, but the freezeTransform method is called after the end of any transformation, and does not distinguish between what type of transformation was previously happening. I imagine resizing the speaker to the same size as the shader would also have triggered bug 6062, but speakers are resized less often than they are moved and hitting an exact size with the mouse would be rare, so the issue was only noticed when moving speakers.

You can check if transform has no scaling (3x3 submatrix is orthogonal up to epsilon) and leave sound spawnargs "as is" in this case. I think that would make more sense than setting them on every movement.

[OrbWeaver]

As of 09e5ec1cae16b8350097fc97839de64cf96c4e88 I have changed the logic to write spawnargs if EITHER the speaker radii are different from the shader default, OR if there were min/max spawnargs to begin with. Spawnargs will no longer be created (or deleted) as a result of a speaker move, but will appear and change if the speaker is resized, which I assume is closer to the expected behaviour.