Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E2 - soundLevel() chops off sounds (especially on loaded servers) #3250

Open
Noxyro opened this issue Jan 24, 2025 · 8 comments
Open

E2 - soundLevel() chops off sounds (especially on loaded servers) #3250

Noxyro opened this issue Jan 24, 2025 · 8 comments

Comments

@Noxyro
Copy link

Noxyro commented Jan 24, 2025

Explanation of issue:
While working with long-range and "global" sounds by using soundLevel(), I have noticed an issue with sounds getting chopped off / stopping immediately after starting to play as soon as the new sound level has been applied, but it's only really reproducible on (loaded) servers.

I have tried multiple combinations of using different other sound functions around and with soundLevel(), different attempts of creating multiple sounds on different entities, different indexes and different parameters for setting the sound duration, to see if the issue is persistent to only appear when using soundLevel() specifically.
So far I can confirm that there is no issues whatsoever when changing any other "normal" property of the sound like pitch or volume and this issue is contained to only appear as soon as soundLevel() is used.


To Reproduce:
Steps to reproduce the behavior:

  1. Create a sound via soundPlay() first (or else soundLevel() can not be applied obviously)
  2. Change the sound level via soundLevel() to anything else
  3. Optional: Do this with a repeating timer multiple times to highlight the issue

Code example:

if (first()) {
    soundPlay(1, 2, "ambient/explosions/exp3.wav")
    soundLevel(1, 110)
    
    timer(2.5, 0, function() {
        soundPlay(1, 2, "ambient/explosions/exp3.wav")
        soundLevel(1, 110)
    })
}

Expected behavior:
The sound should just play / be restarted with the new sound level applied, without being chopped off / stopped.


Possible origin:
After already having looked at the implementation of the soundLevel() function, I suspect the culprit to be the stopping and starting of the sound in the same tick. That this can lead to unreliable and unexpected behavior has also been mentioned in another currently open issue:

Unfortunately, playing E2 sounds on the same id/entity within a few ticks of the previous one ending is still unreliable (in my experience), so if you need some seamless looping sound you might need to pingpong it between two different sound IDs and entities (I like to use holograms)
Originally posted by @Fasteroid in #3091

Sadly the workaround mentioned can not be used in this case, as the user has no granual control over the stopping and starting of the sound when using the soundLevel() function.


PS: The same chopping / stopping behaviour can also be noticed when trying to soundPlay() the same sound again too quickly after the first, without the previous sound having ended or being properly stopped with some delay before playing the new sound again.

PPS: The same issue probably also applies to soundDSP() as it uses the same stopping and starting logic within its function.


Code reference:

e2function void soundLevel( index, level )
local sound = getSound( self, index )
if not sound then return end
-- We need to set the level while the sound is stopped
sound:Stop()
sound:SetSoundLevel( math.Clamp( level, 0, wire_expression2_sound_level_max:GetInt() ) )
sound:Play()
end

@thegrb93
Copy link
Contributor

Might be worth checking if the Stop+Play can be removed. If not, then a garrysmod-issue should be created.

@Noxyro
Copy link
Author

Noxyro commented Jan 26, 2025

Might be worth checking if the Stop+Play can be removed. If not, then a garrysmod-issue should be created.

I was thinking that instead of stopping and starting the pre-existing sound, it could be fixed by just simply re-creating the sound.
The sound has to be stopped / "restarted" in any case, so from a practical view it does not make a big difference.
Also I don't think it would lead to any unforeseen behaviours or overhead when "spamming" this, as users are already free to stop and start as many sounds as they want per tick by using E2s soundStop() and soundStart(), which would be no different as eventually re-creating the sound in soundLevel().

On top of that, by looking at the code carefully again, I've noticed that the way soundLevel() is implemented currently, it will create an inaccuracy in the timer that is supposed to stop the sound after its defined duration, as it will not be restarted together with setting the new sound level and therefore (if not used in the exact same tick) will make the sound stop earlier than it should have been stopped.
This would also be fixed by properly stopping the sound via the soundStop() function and creating it again via the soundCreate() function, stopping the old and creating a new timer with it.

A nice way around this issue would have been to leave the stopping and starting to the user altogether, with the hint of the sound needing to be stopped to apply the sound level ... but this doesn't work, because of the sound and its sound object being kicked out of existence if stopped and therefore not being able to apply sound level after stopping or even in a delayed context.

The only other workaround (and maybe even the cleanest one?), would be to add another parameter to the soundPlay() function, providing the sound level right when the sound is started, as right now you always have to use soundPlay() followed by soundLevel() if you want to change it, which internally starts, stops and starts the sound again.
By implementing it into the soundPlay() function, the sound level can be applied after the sound object is created and before it starts playing.
This does not ultimately fix the issue of soundLevel() stopping and starting the sound itself, but at least it would circumvent the issue with what I think is the widest use-case of soundLevel() anyways.

Correct me if I missed anything on that.

@thegrb93
Copy link
Contributor

I think the best solution is to allow changing those settings while the sound is playing, but gmod dev needs to do it

@Noxyro
Copy link
Author

Noxyro commented Jan 27, 2025

I think the best solution is to allow changing those settings while the sound is playing, but gmod dev needs to do it

I mean yes, not being able to adjust the sound level while the sound is already playing is a topic better addressed towards the GMod devs, I agree.

However, by playing around with some small test addon to try and verify / re-create the same behaviour, it seems to me that the specific issue of the sounds getting chopped off by stopping and starting them in rapid succession, seems to be only isolated to E2s implementation. No matter if I used server-side or client-side sounds, I was not able to in any way re-create the same chopping off I observed with the soundPlay() and soundLevel() functions from E2, by just simply creating a sound, playing it, stopping it, adjusting the sound level of it and starting it again in rapid (every tick) succession, even under server load.

Comparing the definitions of the soundCreate() function which E2s soundPlay() uses to how I did it, they seem pretty much identical to me, with the only difference of E2s implementation always using RecipientFilter(true), making it effectively always use the unreliable channel. So at this point I'm convinced the origin of my initially stated issue of the chopping off behaviour is exactly that - the unreliable channel.

Now, I do understand that net messages not being received, because of the unreliable channel being overloaded is not specifically an issue of E2 or Wiremod by that extend, but it begs the question why it is -forcing- all script-related sounds to be send through the unreliable channel without any option given and therefore making them always ... well ... unreliable?

The sound emitter code also uses the reliable channel and has an adjustable limit of how many can be created per player the same way as E2 has a limit of how many sounds can be created per player, so I don't see why the limitation to always using the unreliable channel needs to be hard-coded for E2?
Obviously to prevent flooding the reliable channel, but then again at what cost, if it makes the starting and stopping of E2 sounds completely unreliable under already slight to medium server load?

I have several ideas of how to work around this issue, which require more or less reworking some of the current sound library for E2, but the first most simple quick fix which doesn't break anything else coming to mind is obviously at least creating a convar for server owners to decide if they want a reliable or an unreliable sound channel for E2, instead of the hard-coded limitation.

Opinions?

@thegrb93
Copy link
Contributor

I believe the RecipientFilter was added to prevent PVS/range limiting and it was probably made unreliable so that it has less latency.

@thegrb93
Copy link
Contributor

thegrb93 commented Jan 28, 2025

It could probably be made reliable if we can prove that latency isn't increased and that e2 can't overload the reliable channel.

@Noxyro
Copy link
Author

Noxyro commented Feb 1, 2025

I believe the RecipientFilter was added to prevent PVS/range limiting and it was probably made unreliable so that it has less latency.

Yes, it allows the effective range to be removed, but for that it doesn't matter which channel the filter is set to.

It could probably be made reliable if we can prove that latency isn't increased and that e2 can't overload the reliable channel.

I have never seen an issue with other things or addons using the reliable channel for sounds, as I said, technically by spamming Wiremod sound emitters the same behaviour can already be achieved at this time.

In my opinion making it reliable and also adding a convar for server owners would be the best compromise, as it will then by default work fine for the majority and in the very rare case that it could potentially become a problem (maybe on really big servers?), the option is there to make it unreliable and avoid the overload.

@thegrb93
Copy link
Contributor

thegrb93 commented Feb 1, 2025

Sound emitter inputs can only take 8 triggers per frame which may not be enough to overflow the buffer. Will need to test with e2 with the filter set to reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants