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

Voice: safe guard against invalid DB volume values #1335

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EntranceJew
Copy link
Contributor

maybe fixes #1334 if the database stuff doesn't

@ZenBre4ker
Copy link
Contributor

ZenBre4ker commented Jan 29, 2024

This shouldnt be necessary. As far as I can see, when the sql.query returns nil or anything else, its always converted to a number in VOICE.GetPreferredPlayerVoiceVolume. Therefore I believe, that the scalingfunction could be the problem. Maybe just add an
out_vol = func( vol ) or vol or 1

@EntranceJew EntranceJew mentioned this pull request Jan 30, 2024
@saibotk
Copy link
Member

saibotk commented Jan 30, 2024

We should fix the root cause of this instead of a bandaid patch.
Maybe check the scaling function result?

But if this already affects users im happy to merge to mitigate weird db values from impacting users

@saibotk saibotk changed the title make volume do something if someone's voicevolume is fucked up Voice: safe guard against invalid DB volume values Jan 30, 2024
@ZenBre4ker
Copy link
Contributor

I tested all three scaling functions with weird values and it seemed fine, linear is also default, which also just puts out_vol = vol, therefore the error is rather in an overwrite of a function.The sql table getter outputs well defined numbers in any case.

Im not fan of this fix, as it might not neutralize the problem itself. In case the db is fine, we have a database write everytime someone starts talking.

@EntranceJew
Copy link
Contributor Author

The code that I made is pretty airtight, and all the checks have been working flawlessly for users on a server with an existing database and for users client-sided in a new database that were specifically set up to join and play.

I can't find a reason why this would happen. All the checks that are in place are designed to minimize the amount of values flowing into C functions and to the database, but still have the potential to update dynamically to prevent a player from being dramatically louder or quieter between circumstances.

The only probable causes in my mind are:

  • numeric comparison precision accuracy problems?
  • database type coercion?
  • a client or server has a read-only database and thus has the schema but not the new records

@ZenBre4ker
Copy link
Contributor

ZenBre4ker commented Feb 27, 2024

In case this should be worked on again. IMO we should just implement a default value with or in case of nil. I'm against setting the value, because it could be something else is broken and this code just wirtes to the db everytime nil comes up. It's not safe to say, that by setting the preferred value, that the actual error will be fixed in the next call.

@EntranceJew
Copy link
Contributor Author

This shouldnt be necessary. As far as I can see, when the sql.query returns nil or anything else, its always converted to a number in VOICE.GetPreferredPlayerVoiceVolume. Therefore I believe, that the scalingfunction could be the problem. Maybe just add an out_vol = func( vol ) or vol or 1

I don't think so. Similar to the shop issues we discussed, the only scaling function that could have this flaw is linear, but that is because the data is arriving to it as a string and not a number. The existing conditions assume nil or a falsey value which is not the expected number. Why would something recorded as a numeric type be permitted to come back as a string?

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

Successfully merging this pull request may close these issues.

Voice Chat Bug
3 participants