godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 83 forks source link

Add `get_current_beat()` function to AudioSteam #6166

Open IntangibleMatter opened 1 year ago

IntangibleMatter commented 1 year ago

Describe the project you are working on

A custom music engine

Describe the problem or limitation you are having in your project

It's rather complex to get the currently playing beat of an AudioStream. There's several lines of scripting that you have to do that can easily break, and don't necessarily deal well with edge cases.

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

We've got AudioStream.get_bpm(), so obviously we should have a method for getting the current beat! It would return either an integer (if it were quantized) or a float, which returns the current beat of the song, with a decimal that represents the percentage of the beat that has past. For example, 2 sixteenth notes after the 7th beat of a song would return something along the lines of 7.50.

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

Here's an example of a simple rhythm game manager script with this feature enabled

extends Node

@onready var music := $AudioStreamPlayer.stream

signal beat(beat)

func _process(delta) -> void:
    # normalize the current beat, then emit the beat signal if the beat approximately just changed.
    if is_equal_approx(music.get_beat() % 1, 0)
        emit_signal("beat", floor(music.get_beat()))

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

It kind of can, but considering how useful this would be to implement for anyone trying to sync things to music in their games, and to anyone who wants to measure the progress of their music in a way that makes sense for music, it makes sense to implement a method into the class to work well with AudioStream.get_bpm()

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

It's a modification to an in-engine class, and as such it would be difficult to implement as an addon.

lemilonkh commented 1 year ago

I would add that signals that are emitted each beat or bar would be great to have as well. It seems that this is the direction the engine is going with the BPM values in the audio import dialog.

If this is wanted in the engine I'd be happy to implement this feature :smile:

IntangibleMatter commented 1 year ago

I think that it's a natural fit! It would enable significantly easier syncing with music, which would mean that people who wanted to have stuff like the dancing koopas in New Super Mario bros (as an example) would have a significantly easier time doing so.

IntangibleMatter commented 1 year ago

So yes, I think this is wanted in the engine, it would be used by a lot of people.

lemilonkh commented 1 year ago

Glad to hear, my proposal for the implementation would be:

These would use the BPM setting found in the import settings of an audio file.

Questions:

IntangibleMatter commented 1 year ago

I think that your ideas for return types make sense.

I think that the methods and signals should be exposed through the AudioStreamPlayer and related items, as that's the easiest to code with in most cases. It also as you said would make it so you don't have to re-hook things up if the stream changes, which is likely to happen in games.

lemilonkh commented 1 year ago

Starting to implement this now, this is what I have settled on:

The reason these methods are moved to AudioStreamPlayback instead of AudioStream directly is that AudioStream doesn't hold any information about the current position in the stream.

The signals will be exposed on AudioStreamPlayer[2D/3D] where possible, but the methods will stay on AudioStreamPlayback and its subclasses (since for them it doesn't really matter if the stream changes).

Update: Since AudioStreamPlayback doesn't expose any public methods (other than virtual ones), the methods will be accessible from AudioStreamPlayer in GDScript.

lemilonkh commented 1 year ago

Update: I got it working :sparkles: Here's a recording of my example project (will be downloadable from the PR so you can try it out):

https://github.com/godotengine/godot-proposals/assets/93539/86534a86-5ec8-4bd1-9e3e-fff6203375d8

And the code for this:

extends Node

@onready var music: AudioStreamPlayer = $AudioStreamPlayer

@onready var beat_1_rect: ColorRect = %Beat1Rect
@onready var beat_2_rect: ColorRect = %Beat2Rect
@onready var beat_3_rect: ColorRect = %Beat3Rect
@onready var beat_4_rect: ColorRect = %Beat4Rect
@onready var beat_progress_bar: ProgressBar = %BeatProgressBar
@onready var bar_progress_bar: ProgressBar = %BarProgressBar
@onready var beat_label: Label = %BeatLabel
@onready var bar_label: Label = %BarLabel

var pause_position := 0.0

func _process(_delta: float) -> void:
    if !music.has_stream_playback():
        return

    beat_label.text = "Beat: " + str(music.get_current_beat())
    bar_label.text = "Bar: " + str(music.get_current_bar())

    beat_1_rect.color = Color.CRIMSON if music.get_current_beat() == 0 else Color.BLACK
    beat_2_rect.color = Color.CRIMSON if music.get_current_beat() == 1 else Color.BLACK
    beat_3_rect.color = Color.CRIMSON if music.get_current_beat() == 2 else Color.BLACK
    beat_4_rect.color = Color.CRIMSON if music.get_current_beat() == 3 else Color.BLACK

    beat_progress_bar.value = music.get_beat_progress() * 100
    bar_progress_bar.value = music.get_bar_progress() * 100

func _on_play_button_pressed() -> void:
    if !music.playing:
        music.play(pause_position)
    else:
        pause_position = music.get_playback_position()
        music.stop()
mchamby commented 4 weeks ago

I'd like to +1 this old suggestion. All of the needed beat/BPM info is there in the AudioStream. It's just missing a method in the AudioStream or a signal in the AudioStream player, as others have suggested. Either one would be great!