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

yaml AudioParams are often ignored #34318

Open
5 tasks
ElectroJr opened this issue Jan 9, 2025 · 0 comments
Open
5 tasks

yaml AudioParams are often ignored #34318

ElectroJr opened this issue Jan 9, 2025 · 0 comments
Labels
DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. P3: Standard Priority: Default priority for repository items. S: Help Wanted Status: Requesting additional help for this to be completed. T: Cleanup Type: Code clean-up, without being a full refactor or feature

Comments

@ElectroJr
Copy link
Member

ElectroJr commented Jan 9, 2025

In many cases, systems that play sound ignore the audio options associated with a SoundSpecifier. I.e., they don't use the SoundSpecifier.Params field. Instead they often just bulldoze it with the default options. This should be fixed, so that sound options can be consistently specified in yaml.

E.g., stuff like

Audio.PlayPvs(cartridge.EjectSound, entity, AudioParams.Default.WithVariation(SharedContentAudioSystem.DefaultVariation).WithVolume(-1f));

needs to be replaced with something like

var @params = cartridge.EjectSound?.Params ?? AudioParams.Default;
@params = @params.AddVolume(-1f).WithVariation(SharedContentAudioSystem.DefaultVariation);
Audio.PlayPvs(cartridge.EjectSound, entity, @params);

Here are some the common mistakes/issues that need to be fixed:

  • Most uses of AudioParams.WithVolume() should probably be replaced with AudioParams.AddVolume()
  • AudioHelpers.WithVariation() needs to be removed.
    • There is now a AudioParams.Variation field that should be set. Though again, in many cases that should be set in yaml not in C#, as that just bulldozes the yaml value.
    • Maybe there should be a AddVariation() method, akin to AddVolume()?
  • Many sounds just don't use the sound specifier's options.
    • These are pretty easy to find by just looking for where AudioParams.Default is used.
  • Some systems convert sound specifiers into strings before using PlaySound.
    • Most of the time this is probably unnecessary, and they probably also forget to check the specifier's audio parameters.
    • These can often be found by looking at uses of AudioSystem.GetSound()
  • Sometimes audio parameters are specified inconsistently.
    • These are likely holdovers from before SoundSpecifier.Params was added and need to be updated.
    • E.g., the PlaySound construction action has its own AudioParams instead of just using the SoundSpecifier's.
@ElectroJr ElectroJr added S: Help Wanted Status: Requesting additional help for this to be completed. P3: Standard Priority: Default priority for repository items. T: Cleanup Type: Code clean-up, without being a full refactor or feature DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. labels Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. P3: Standard Priority: Default priority for repository items. S: Help Wanted Status: Requesting additional help for this to be completed. T: Cleanup Type: Code clean-up, without being a full refactor or feature
Projects
None yet
Development

No branches or pull requests

1 participant