-
Notifications
You must be signed in to change notification settings - Fork 258
Add AudioManager with click sounds #1670
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
base: main
Are you sure you want to change the base?
Conversation
added AudioManager global added click sounds to audio assets added script and scene to manager audio added various global click sounds to menu options (default, back, toggle)
|
Play this branch at https://play.threadbare.game/branches/kphero/add-clicks. (This launches the game from the start, not directly at the change(s) in this pull request.) |
manuq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
For audio we have 2 buses for this project (defined in default_bus_layout.tres):
I think this PR is adding the sounds to the master bus. Have you considered using the SFX bus instead? Basically, do we want these menu clicks to be muted when the sounds are muted in the game settings? I'm not sure about this, maybe is worth checking what other games use to do.
I am not so sure about adding a global / autoload for this. What do you think? So far we haven't needed a global for the SFXs, although there is one for the music.
| @@ -0,0 +1,20 @@ | |||
| # SPDX-FileCopyrightText: The Threadbare Authors | |||
| # SPDX-License-Identifier: MPL-2.0 | |||
| @tool | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this really need to be a @tool ?
| # SPDX-FileCopyrightText: The Threadbare Authors | ||
| # SPDX-License-Identifier: MPL-2.0 | ||
| @tool | ||
| extends Node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should give it a class_name to this node and add it to the menu scenes, rather than adding yet another autoload to the game. What do you think? I don't see this being used anywhere else than inside scenes/menus/. If so then I would propose menu_audio.gd or similar for the file (and MenuAudio or similar for the class name) instead of audio_manager.gd, which seems more global.
I think we should do one of these two things:
|
wjt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following from what Manuel has said: the approach implemented here adds a new autoload but also requires writing code for each button.
If we are prepared to write code for every button in the game then we don't need an autoload. And if we're going to ask the developer to do something for every button in the game, IMO it could be a node that we make a child of each button that plays a sound; or a script that we can attach directly to each control.
A middle ground would be to add a ControlSoundEffects node that we place into every scene that has controls that should make noise. We could either connect every button in the scene's pressed signal to a method on this node; or have it walk the scene on ready to find controls.
But a more comprehensive option would be to use an autoload that automatically connects to every control that is ever used in the game. Something like this:
func _ready() -> void:
get_tree().node_added.connect(_on_node_added)
func _on_node_added(node: Node) -> void:
if node is CheckButton:
node.toggled.connect(_on_check_button_toggled)
elif node is Button:
node.pressed.connect(_on_button_pressed)
elif node is Slider:
node.value_changed.connect(_on_slider_value_changed)
func _on_check_button_toggled(_toggled_on: bool) -> void:
ui_click_sound.play()
# ... etc ...Then every control in the game will make the desired noises - it's impossible to forget to add the relevant code.
Description:
This PR introduces a global
AudioManagerto handle audio assets consistently across the project. It also adds click sound effects from #1028.Changes
clickassets from Create a UI sound effect (click/select) #1028.AudioManageras a global utility.This PR is starting to implement #1030, with only functionality in the main menu currently. But should be easy to implement the click sounds wherever needed.