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

Move Loop from an import option to a property in the AudioStreamPlayer nodes #3120

Closed
MichaelShaulskiy opened this issue Aug 11, 2021 · 10 comments
Labels
breaks compat Proposal will inevitably break compatibility topic:audio topic:import
Milestone

Comments

@MichaelShaulskiy
Copy link

Describe the project you are working on

Applies to anything

Describe the problem or limitation you are having in your project

While participating in a game jam, I had voice lines in ogg format which I wanted to use in game.

After putting them in an AudioStreamPlayer/2D node and setting looping to off in the AudioStreamPlayer, my voicelines kept looping.

I didn't know why, even the finished() signal from the AudioStreamPlayer was never emitted.
Perplexed, I googled my problem and found a fix for my Problem: by default, looping is turned on in the import tab of my ogg audio asset.

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

It doesn't make sense to have a looping option in the import tab of all things. There already is a looping option in the AudioStreamPlayer Node, which will have no effect, if looping is turned on in the import options.

Looping isn't an inherent property of audio files. They should be just that: audio files, containing sound data.

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

When a User imports an audio file, there will be either no option called looping or the looping option will still be present, but not enabled by default as it is now.

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

No because the AudioStreamPlayer looping option has no effect on audio assets imported with looping turned on in the import settings

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

It is a source of confusion and not logical, looping should be an option in the AudioStreamPlayer only

@Calinou Calinou added this to the 4.0 milestone Aug 11, 2021
@Calinou Calinou changed the title Looping shouldn't be an option in the import settings tab for audio assets Move Loop from an import option to a property in the AudioStreamPlayer nodes Aug 11, 2021
@golddotasksquestions
Copy link

golddotasksquestions commented Aug 12, 2021

It maybe would make more sense to have ogg looping disabled by default, but not all games have voice overs (given how Godot is used predominantly by hobbyists still, it's fair to assume most games don't have voice overs).

I would therefore vote against removing it completely from the import settings.

For games that don't have voice overs, having an import option enable the oggs loop property by default makes a lot of sense as ogg is meant to be used for game music, not sound effects. Having to remind yourself to check "enable loop" every time you load music is at least as irritating and annoying as your situation, only it will effect much more people. (see the whole "enable raycast" controversy).

For voice acting ogg is of course fine. These sound sniplets are usually substantially longer than your average sound effect and a tiny delay is also not completely immersion breaking.

For any short repeated sound effects however like impact sounds, swooshes, shooting sounds, footsteps, menu and ui sounds and the like, use wav. Especially those that are important gameplay indicators in fast paced games since decoding the ogg can take more time and processing power.

@dalexeev
Copy link
Member

Resources should manage data, not behavior. Therefore, this is a very good proposal.

Moreover, we have already introduced such a change:

@me2beats
Copy link

Loop should be turned off imo.
turn it off

Now I have to disable this extremely often, this is annoing.
I keep Loop on for background music etc but the number of such files is much less.
I believe loop should be off for all audio formats.

@YuriSizov
Copy link
Contributor

This may already be addressed by godotengine/godot#62896.

@mieldepoche
Copy link

I have WAV files that have loop regions.
Without a per-AudioStreamPlayer loop mode, it is impossible (without an ugly workaround) to play both the "short" sample and the looped one as it will always loop no matter what.

My current workaround is:

var asp := $AudioStreamPlayer
asp.play()
get_tree().create_timer( asp.stream.get_length() ).timeout.connect( asp.stop )

This may already be addressed by godotengine/godot#62896.

This does not seem to adress the issue I'm having, which would be solved by this proposal, so I'd say no.

@Landeplage
Copy link

I've worked with audio in a Godot project lately, and I agree that having a checkbox for looping in the import stage is confusing, and a bit irritating. Additionally, the AudioStream resource itself has settings like LoopMode and loop start/end. This means looping is configured in two places.

To me, it makes more sense to move loop settings to the AudioStreamPlayer node, because it is linked more to the behaviour of the audio, as opposed to content. This is also how Unity handles it.

One advantage of keeping this setting at the import stage would be to be able to check the filename of the audio, and look for "loop". If it has "loop" in the name, we more than likely want to loop the sound. The importer could automatically check the "Loop" box for us. This would be great as an optional project setting, or even a setting right in the import options.

@Mickeon
Copy link

Mickeon commented Aug 21, 2022

I am working on keeping both the Import setting and allowing AudioStreamPlayers to override the playback looping behaviour. So far so good.

In my personal projects this feature would be beneficial, as it does make sense to me for Godot to keep the original audio's looping settings in mind (otherwise sounds that have been set up to loop with loop_begin and loop_end may not behave consistently), while still allowing it to be overridden by whatever Node is playing the source.

@fguillen
Copy link

fguillen commented Jun 1, 2023

Any advance on this?

@Calinou
Copy link
Member

Calinou commented Jun 2, 2023

Any advance on this?

See godotengine/godot#65797 (comment). Please don't cross-post, as it makes the discussion harder to follow 🙂

@akien-mga
Copy link
Member

The need for this was replaced by godotengine/godot#86473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:audio topic:import
Projects
Status: Implemented
Development

Successfully merging a pull request may close this issue.