godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Implement SceneTreeAudioPlayer #4569

Open SilencedPerson opened 2 years ago

SilencedPerson commented 2 years ago

Describe the project you are working on

Teaching teenagers how to use Godot. Current projects are JRPGs.

Describe the problem or limitation you are having in your project

It's very inconvenient having to store the Audio inside objects if they exist in a scene for time shorter than the length of the Audio. Leads to ugly cut-offs and having to write code to work around it.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Recently backported Tweens spoiled me, the SceneTreeAudioPlayer would work like the tweens. Alternatively, AudioStreamPlayer could spawn helper node in the root upon being played that would delete itself after finishing.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

var audio = create_audio_player("res://path to audio") Plays automatically upon creation and deletes after finishing.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It can be done but it is lot of lines for something so simple.

1) stored in var 2) has to be removed as a child 3) signal has to be connected 4) back into the SceneTree 5) played 6) cleaned up

Is there a reason why this should be core and not an add-on in the asset library?

Streamlining working with audio.

Calinou commented 2 years ago

How is positional audio taken into account with this approach? Sound can be positional in either 2D or 3D space.

SilencedPerson commented 2 years ago

It wasn't when I made the proposal, I forgot that was a thing. It was mostly for UI and transitions between scenes, effects that don't rally benefit from being positional. But similarly to the SceneTreeTween where you can bind the node to be deleted with specified node with bind_node(), 2D and 3D variants would just mimic global position of specified node until it ceased existing.

Mickeon commented 2 years ago

If this enhancement will not be used often, can it be worked around with a few lines of script?

https://gitlab.com/Xecestel/sound-manager kinda allows this. But frankly, I would love if part of its functionality were to be integrated within the Engine, but I am very aware that Godot currently doesn't have much in the Audio department.

It's a bit hard to explain exactly why it would be useful, but oftentimes, sounds really are just as throwaway as Timers and Tweens.

KoBeWi commented 2 years ago

Well, I use something like this all the time. I only had write my own method that takes a sample (and even position), so playing a sound anywhere is a one-liner. What I mean, this can easily be an addon.

h0lley commented 2 years ago

playing sound effects really seems like something of the same nature as timers and tweens, so I think this makes sense.

having to keep a game entity alive just because it is still running a sound effect always felt kind of awkward to me.

and while it is true that this can be solved with relatively few lines of code, why should everybody have do implement their little sound manager when it is something virtually every project would benefit from?

I would suggest that this would work similar to the Timer node and SceneTreeTimer, in that both options are available: the AudioStreamPlayer node and the SceneTreeAudioPlayer.

as a Node, its use-case would be mainly looping audio; BGM, ambient, positional audio, effects you expect to play very frequently or you want to adjust properties of in the inspector. on the SceneTree, its use-case would be for one-shot throw-away like sound effects.

SilencedPerson commented 2 years ago

Well, I use something like this all the time. I only had write my own method that takes a sample (and even position), so playing a sound anywhere is a one-liner. What I mean, this can easily be an addon.

I agree but, it can easily be done with vanilla GDScript but it is annoying paper-cut and I guarantee you will start to notice it in every project now that it was brought to your attention.

Mickeon commented 2 years ago

The current AudioStreamPlayers, as their names imply, focus more on the release of the sound, rather than the sound itself. If AudioStreamPlayer2D changes position, the perceived sound changes position. If any AudioStreamPlayer is freed, its sound stops existing. If AudioStreamPlayer's play() is constantly called, its sound will stop and restart accordingly, without stacking.

This isn't a bad thing at all, but there are indeed cases in which an AudioStream... just has to fire and be forgotten about.

Things like, general UI and quick prototyping. These don't particularly care about setting up how the sound is emitted, they just care about playing the sound, likely for decorative purposes.

One example right on top of my head is a spaceship playing a firing sound as it keeps shooting. Perhaps, it is desirable the sound not to cut off before firing another shot. Currently, something like this would need to be done


var shoot_scene := preload(PATH)
var shoot_sfx := shoot_scene.instance()
add_child(shoot_sfx)
shoot_sfx.play()
shoot_sfx.connect("finished", shoot_sfx, "queue_free")

It can be argued that this "workaround" can give much finer control than the proposal (e.g. limiting the sounds being played at once), but any amount of hypothetical SceneTreeAudioPlayer References could also stored during their creation, in a way that may be less bloated than the AudioStreamPlayer Nodes, whose additional properties visible in the Scene (volume, autoplay, position, pitch_scale...) albeit very useful, may not need to be tweaked on the Tree editing itself.

Unfortunately, the implementation of such a system really is up to debate. I'm personally concerned on its ease of access to the average user, but I do feel like would put it more in line with other Engines (currently thinking about Game Maker's audio_play_sound() for one)

SilencedPerson commented 2 years ago

Alternatively you could do this. Still lot more complicated than it needs to be.

func play_sound():
    var sound = $AudioStreamPlayer.duplicate()
    get_tree().get_root().add_child(sound)
    sound.connect("finished",sound,"queue_free")
    sound.play()
me2beats commented 2 years ago

Sometimes I use Audio singleton. This is a scene with AudioStreamPlayer nodes, also I have Timers and Tweens there if needed. And I can create methods in this Audio singleton like play_win(), play_fail() etc. So I can play a sound from any node using Audio.play_something()

dalexeev commented 2 years ago

My SFX singleton (4.0):

extends Node

const MAX_POLYPHONY_MAP = {
    # ...
}

var _players := {}

func play(path: String) -> void:
    var player := _players.get(path) as AudioStreamPlayer
    if player:
        player.play()
        return

    player = AudioStreamPlayer.new()
    player.bus = &'SFX'
    player.stream = load('res://sfx/' + path)
    player.max_polyphony = MAX_POLYPHONY_MAP.get(path, 1)
    add_child(player)
    _players[path] = player
    player.play()
amarraff commented 9 months ago

I made a similar post about this, #8903, because I'm experiencing a similar issue. I want to play multiple sounds in rapid succession from the same AudioStreamPlayer (for example, machine gun bullets) and have each individual audio stream play to completion without getting cut-off by a new one. The code workaround to achieve this commonly desired behavior feels hacky, and it would be great for the engine to support it out of the box like Unity.

There is a type of AudioStream called "AudioStreamPolyphonic" that appears to achieve this functionality from its description in documentation, but I do not understand how to implement it. Edit -> After some experimenting thanks to code guidance from @AThousandShips, I understand how to make it work.

You must set the stream of the AudioStreamPlayer to "PolyPhonic" and then run "play()" on the AudioStreamPlayer before ever attempting to play sounds through it. This opens the player's stream playback object so that we can access it.

Then, play your desired streams through the AudioStreamPlayer's opened up stream playback object.

The code in my project looks like this:

@export var player:AudioStreamPlayer
@export var my_stream:AudioStream

func _ready():
    player.play()

func _play_my_sound(): # Called from elsewhere to play the sound as needed.
    player.get_stream_playback().play_stream(my_stream)
AThousandShips commented 9 months ago

It works fine to play the same stream multiple times, assuming that stream is polyphonic, I've made it work fine, but assumes the underlying stream is polyphonic, depends on the stream, but most of the raw file based streams should be, aka. WAV, OGG, etc.

amarraff commented 9 months ago

It works fine to play the same stream multiple times, assuming that stream is polyphonic

@AThousandShips I finally understand how to make this work, thank you for all of your help with sending me projects/examples and having patience as I learned. It wasn't working for me because I wasn't putting player.play() in my _ready() function. So, Godot was telling me that it was null, and I didn't understand why. I now understand that the AudioStreamPlayer's stream playback has to be initialized before you can send streams through its AudioStreamPolyphonic.

Thanks again!

AThousandShips commented 9 months ago

That would have worked if you followed my instructions 🙂 but I'm happy it works now!

I sent you an actual project doing exactly that which you tested? 🙃

amarraff commented 9 months ago

That would have worked if you followed my instructions 🙂 but I'm happy it works now!

I did honestly try, I just didn't understand how to do it. Sorry. Thank you again.

amarraff commented 9 months ago

I sent you an actual project doing exactly that which you tested? 🙃

Yeah, I did test both projects you sent me with my own sounds. I did not understand the concept of the audio stream playback object (I still don't fully, but I'm getting it) and the importance of having to run .play() on the AudioStreamPlayer first, outside wherever you try to play the sound. I was trying to run it like this in my project:

func _play_my_sound(): 
    player.play()
    player.get_stream_playback().play_stream(my_stream)

which seems to have been resetting/emptying the stream playback object each play, instead of like this:

func _ready():
    player.play()

func _play_my_sound(): # Called from elsewhere to play the sound as needed.
    player.get_stream_playback().play_stream(my_stream)

which now works exactly as intended and is how you had it set up in the example project.

AThousandShips commented 9 months ago

Well good that that's worked out, but a suggestion for the future is that if someone provides you with an example, and you say it doesn't work, it'll save everyone a lot of time if you don't say you tried it when you didn't actually try their code, if someone send you code just try that code instead of modifying it

I'll mark this as off topic as it's very much so, but I'll leave your example above as it's the correct code

amarraff commented 9 months ago

Well good that that's worked out, but a suggestion for the future is that if someone provides you with an example, and you say it doesn't work, it'll save everyone a lot of time if you don't say you tried it when you didn't actually try their code

I'll mark this as off topic as it's very much so, but I'll leave your example above as it's the correct code

I understand. I genuinely thought I was trying your code, sorry. I didn't understand how to use it. I was going back and forth between your project and mine trying to figure out what I was doing wrong.