Add button hover sound effect - #2885
Conversation
There was a problem hiding this comment.
I've already removed that line of code
There was a problem hiding this comment.
There are two (different) Hover.wav files, which one is the correct oen?
There was a problem hiding this comment.
The correct file is sounds/ui/Hover.wav. The duplicate in scenes/ has been removed.
| func _connect_once(sig: Signal, callable: Callable) -> void: | ||
| if not sig.is_connected(callable): | ||
| sig.connect(callable) |
There was a problem hiding this comment.
I don't think you meant to delete the _connect_once function!
There was a problem hiding this comment.
When I tried to submit the pull request, I got an error before it was sent. The error appeared starting at the line “<<<<HEAD,” so I tried moving the code around until the error disappeared. However, it has already been reverted.
There was a problem hiding this comment.
I think this duplicate scene is unnecessary?
There was a problem hiding this comment.
Yes! The .wav file that plays is the one in the iu/Hover.wav folder. The duplicate has already been removed.
| if node is BaseButton: | ||
| _connect_once(node.mouse_entered, _on_button_hovered) |
There was a problem hiding this comment.
Have you tried navigating with the keyboard or a gamepad? I wonder if we should play the same sound for the Control.focus_entered signal.
There was a problem hiding this comment.
Yes, the sound is the same whether you click the button with the mouse or navigate the menu with the keyboard. I don't know about gamepads; I don't have one to test it with.
I've been testing it, and it does recognize the keyboard!
There was a problem hiding this comment.
Why only BaseButton? There are other Control subclasses that can be focused/hovered, e.g. Slider. I agree that we don't want to bind to Control.mouse_entered on every control though.
There was a problem hiding this comment.
Ahhh, I hadn't realized that. I updated the hover function to check whether the node is a BaseButton or a Slider: that way, the sliders now also play the hover sound when you hover over them.
Actually, now that I’ve done the latest test, I think it would be good if the music slider—instead of the click sound—used the random sound feature to play the melody from “The Musical Rocks.”
… focus_entered and clean up files :wq :wq
|
I've already updated the pull request. |
|
Test build no longer available. |
|
@FinalRed please address the comments made by Will. Thanks! |
|
All of Will's comments have been addressed! The accidental files were removed, project.godot was reverted, _connect_once was restored. |
| [editor] | ||
|
|
||
| movie_writer/movie_file="user://recording.ogv" | ||
| movie_writer/movie_file="user://recording.mp4" |
There was a problem hiding this comment.
This should be reverted! To verify, come to the Files Changed tab of this pull request and check that there project.godot is not in the list of changed files.
There was a problem hiding this comment.
project.godot is gone
manuq
left a comment
manuq
left a comment
There was a problem hiding this comment.
Excellent! All works as expected. I agree that applying this just to BaseButton and Slider seems limiting, but I couldn't find an existing control that doesn't play the sound when hovered. We can enlarge the list of affected Control nodes in the future.
3 participants
Fixes #1027
This time, the sound was created acoustically by moving the teeth of a hair comb.
Added hover sound effect to UI buttons:
mouse_enteredsignal connection forBaseButtonnodes inmenu_ui_player.gd.%HoverSFXPlayernode tomenu_ui_player.tscnassigned to theSFXbus.max_polyphonyto prevent audio dropouts during fast cursor movement across menu items.