Repository navigation
Player: Add sound effect for throwing the grapple string - #2902
Conversation
|
Test build no longer available. |
manuq
left a comment
There was a problem hiding this comment.
I'm not an expert but I think this sounds very well! Check my request on unique names. Also your pull request description seems outdated:
Adds a sound effect that plays when the player throws the grappling hook string, using the "string_thrown" signal already emitted in "_new_hook_string()". The sound "Whip woosh" by ErikCruzDev (CC0) was used, licensed via a ".license" file following the project's REUSE convention. This addresses part of #1918 — specifically the "throwing thread" sound.
This PR is playing the sound directly in _new_hook_string(). This means that it will also play when making contact with the first hookable item (pin, needle, or any other). I don't think it's bad, I just mention it because reading the ticket #1918 that case could have a differrent sound effect.
Probably you intended to use the "string_thrown" signal and add the sound to the player scene. Another option would be to add it to the AnimationPlayer throw_string animation. I think what you have is good for now, just update the description to match the implementation.
Also please add the following as the last line in the description:
Helps https://github.com/endlessm/threadbare/issues/1918
We use "Helps" when the PR doesn't fully adress the issue. Thanks!
| @onready var phantom_camera_2d: PhantomCamera2D = %PhantomCamera2D | ||
|
|
||
| ## Plays a sound effect when the string is thrown. | ||
| @onready var throw_audio_player: AudioStreamPlayer2D = $ThrowAudioPlayer |
There was a problem hiding this comment.
Please give a unique name to the new node, and reference it here with %. See the other @onready variables in this same file. This is documented in https://github.com/endlessm/threadbare/wiki/Contributing#coding-style
There was a problem hiding this comment.
Done, thanks for the pointer, updated all 5 audio player nodes to use unique names
Also change when to play them in the script, and always play the throw effect combined with the others. Also rename the nodes to match the code.
|
@Codex-1010 I did a number of fixes myself because we would like to add this into the next release. Please continue iterating on the sound effects if you want, in a separate pull request. I found that the "fail" sound wasn't transmiting failure, and the opposite was happening with the "hooked" sound, it was a bit too sad. So I played with the volume and pitch. All of the sound effects were playing at the default 0 db recorded sound. Also:
|
|
@Codex-1010 these two license files are empty:
Sorry but I will have to leave this out of the next release if you don't provide them. Don't worry though, there will be another release soon! |
|
@manuq Sorry about that! Just fixed both license files grapple_hit.wav.license, grapple_collect.wav.license, they now have the proper CC0 attribution. Thanks a lot for all the fixes and for taking the time to polish this for the release, really appreciate the feedback on the sound |
|
@Codex-1010 thank you! |
Adds sound effects to PlayerHook:
Resolves #1918