From f67dccbf5228490679c511296f9402725cacab38 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Mon, 5 Oct 2026 19:47:10 +0200 Subject: [PATCH] Retry failed song downloads instead of getting stuck MusicServerClient functions now take an optional on_failed callback, called when a request fails or returns invalid JSON. Before, a failed request was silently dropped, so a failure while preloading a song left Playlist.is_song_downloading set forever and no more songs were fetched. Song fetching retries the step that failed (download, queue or info) every song_fetch_retry_seconds, so the server and the game keep the same playlist. --- MainWindow.gd | 63 ++++++++++------- music_server_client.gd | 95 +++++++++++++++----------- tests/integration/test_main_window.gd | 41 +++++++++++ tests/unit/test_music_server_client.gd | 33 +++++++++ 4 files changed, 167 insertions(+), 65 deletions(-) diff --git a/MainWindow.gd b/MainWindow.gd index 4b539e5..4ab78dd 100644 --- a/MainWindow.gd +++ b/MainWindow.gd @@ -140,6 +140,9 @@ var songs: Array= [] var soundtracks: Array = [] var current_player: Node +## Seconds to wait before retrying a song download step that failed. +var song_fetch_retry_seconds: float = 5.0 + # Called when the node enters the scene tree for the first time. func _ready() -> void: var debug: bool = OS.has_feature("debug") @@ -277,36 +280,46 @@ func play_next_song() -> void: func fetch_song() -> void: print("fetch_song") Playlist.is_song_downloading = true - var new_song_object: SongObject = SongObject.new() - var fetch_song_done: Callable = func fetch_song_done(body: PackedByteArray) -> void: - print("fetch_song_done") + _download_random_song(SongObject.new()) + +# Fetching a song takes three requests. If one fails, only that step is retried, so +# the server and the game keep the same songs in their playlists. +func _download_random_song(new_song_object: SongObject) -> void: + var song_downloaded: Callable = func song_downloaded(body: PackedByteArray) -> void: + print("song_downloaded") var sound: AudioStream = AudioStreamMP3.new() sound.data = body new_song_object.song = sound - - var added_to_que_done: Callable = func added_to_que_done() -> void: - print("added_to_que_done") - var fetch_song_info_done: Callable = func fetch_song_info_done(data_received: Dictionary) -> void: - print("fetch_song_info_done") - new_song_object.song_number = data_received.SongNo - new_song_object.soundtrack_title = data_received.Soundtrack - new_song_object.song_title = data_received.Song - - print("new_song_object: ", new_song_object.to_string()) - Playlist.add_to_playlist(new_song_object) - next_button.disabled = false - if Playlist.should_preload_more_songs(): - fetch_song() - else: - Playlist.is_song_downloading = false - if Settings.is_debug: - update_song_list() - MusicServerClient.fetch_song_info(fetch_song_info_done) + _add_song_to_queue(new_song_object) + MusicServerClient.fetch_random_song(Settings.use_low_played_mode, song_downloaded, + _retry_later.bind(_download_random_song.bind(new_song_object))) - MusicServerClient.add_to_queue(added_to_que_done) +func _add_song_to_queue(new_song_object: SongObject) -> void: + MusicServerClient.add_to_queue(_fetch_song_info.bind(new_song_object), + _retry_later.bind(_add_song_to_queue.bind(new_song_object))) + +func _fetch_song_info(new_song_object: SongObject) -> void: + var song_info_fetched: Callable = func song_info_fetched(data_received: Dictionary) -> void: + print("song_info_fetched") + new_song_object.song_number = data_received.SongNo + new_song_object.soundtrack_title = data_received.Soundtrack + new_song_object.song_title = data_received.Song + print("new_song_object: ", new_song_object.to_string()) + Playlist.add_to_playlist(new_song_object) + next_button.disabled = false + if Playlist.should_preload_more_songs(): + fetch_song() + else: + Playlist.is_song_downloading = false + if Settings.is_debug: + update_song_list() + MusicServerClient.fetch_song_info(song_info_fetched, + _retry_later.bind(_fetch_song_info.bind(new_song_object))) + +func _retry_later(step: Callable) -> void: + print("Song download failed, retrying in %s seconds" % song_fetch_retry_seconds) + get_tree().create_timer(song_fetch_retry_seconds).timeout.connect(step) - MusicServerClient.fetch_random_song(Settings.use_low_played_mode, fetch_song_done) - func update_song_list() -> void: print("update_song_list") if Playlist.currently_playing_song >= 0: diff --git a/music_server_client.gd b/music_server_client.gd index bee7efa..b3285b4 100644 --- a/music_server_client.gd +++ b/music_server_client.gd @@ -17,108 +17,123 @@ func _ready() -> void: ## Fetches the songs already in the server playlist, as an Array of ## [code]{SongNo, Soundtrack, Song}[/code] dictionaries. -func fetch_song_list(on_done: Callable) -> void: - get_json("/music/list", on_done) +func fetch_song_list(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/music/list", on_done, on_failed) ## Downloads a random song as MP3 bytes. With [param low_played] the server prefers ## soundtracks that have been played the least. -func fetch_random_song(low_played: bool, on_done: Callable) -> void: - get_bytes("/music/rand/low" if low_played else "/music/rand", on_done) +func fetch_random_song( + low_played: bool, on_done: Callable, on_failed: Callable = Callable()) -> void: + get_bytes("/music/rand/low" if low_played else "/music/rand", on_done, on_failed) ## Adds the most recently fetched random song to the server playlist. -func add_to_queue(on_done: Callable) -> void: - send("/music/addQue", on_done) +func add_to_queue(on_done: Callable, on_failed: Callable = Callable()) -> void: + send("/music/addQue", on_done, on_failed) ## Fetches [code]{SongNo, Soundtrack, Song}[/code] for the most recently queued song. -func fetch_song_info(on_done: Callable) -> void: - get_json("/music/info", on_done) +func fetch_song_info(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/music/info", on_done, on_failed) ## Downloads a song from the server playlist as MP3 bytes. -func fetch_song(song_no: int, on_done: Callable) -> void: - get_bytes("/music?song=" + str(song_no), on_done) +func fetch_song(song_no: int, on_done: Callable, on_failed: Callable = Callable()) -> void: + get_bytes("/music?song=" + str(song_no), on_done, on_failed) ## Downloads the sound test song as MP3 bytes. -func fetch_sound_test_song(on_done: Callable) -> void: - get_bytes("/music/soundTest", on_done) +func fetch_sound_test_song(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_bytes("/music/soundTest", on_done, on_failed) ## Clears the server playlist. -func reset_playlist(on_done: Callable) -> void: - send("/music/reset", on_done) +func reset_playlist(on_done: Callable, on_failed: Callable = Callable()) -> void: + send("/music/reset", on_done, on_failed) ## Fetches every soundtrack name as an Array of Strings. -func fetch_all_soundtracks(on_done: Callable) -> void: - get_json("/music/all", on_done) +func fetch_all_soundtracks(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/music/all", on_done, on_failed) ## Fetches every soundtrack name, sorted, as an Array of Strings. -func fetch_all_soundtracks_ordered(on_done: Callable) -> void: - get_json("/music/all/order", on_done) +func fetch_all_soundtracks_ordered(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/music/all/order", on_done, on_failed) ## Records in the server statistics that a song has been played. func mark_song_played(song_no: int) -> void: - send("/music/played?song=" + str(song_no), Callable(), HTTPClient.METHOD_PUT) + send("/music/played?song=" + str(song_no), Callable(), Callable(), HTTPClient.METHOD_PUT) ## Fetches the names of all character images. -func fetch_character_list(on_done: Callable) -> void: - get_json("/characters", on_done) +func fetch_character_list(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/characters", on_done, on_failed) ## Downloads a character image as PNG bytes. -func fetch_character_image(character_name: String, on_done: Callable) -> void: - get_bytes("/character?name=" + character_name.uri_encode(), on_done) +func fetch_character_image( + character_name: String, on_done: Callable, on_failed: Callable = Callable()) -> void: + get_bytes("/character?name=" + character_name.uri_encode(), on_done, on_failed) ## Starts syncing the server's soundtrack library with its music folder. -func start_sync(on_done: Callable) -> void: - send("/sync", on_done) +func start_sync(on_done: Callable, on_failed: Callable = Callable()) -> void: + send("/sync", on_done, on_failed) ## Fetches the sync state: [code]{progress, time_spent}[/code] while running, or the ## sync result once finished. -func fetch_sync_progress(on_done: Callable) -> void: - get_json("/sync/progress", on_done) +func fetch_sync_progress(on_done: Callable, on_failed: Callable = Callable()) -> void: + get_json("/sync/progress", on_done, on_failed) ## Requests [param path] and calls [param on_done] with the parsed JSON. -## Nothing is called if the request fails or the body is not valid JSON. -func get_json(path: String, on_done: Callable) -> void: +## [param on_failed] is called with no arguments if the request fails or the body is +## not valid JSON. +func get_json(path: String, on_done: Callable, on_failed: Callable = Callable()) -> void: _request(path, HTTPClient.METHOD_GET, func(body: PackedByteArray) -> void: var json: JSON = JSON.new() if json.parse(body.get_string_from_utf8()) != OK: push_warning("Invalid JSON from %s: %s" % [path, json.get_error_message()]) + _call_if_valid(on_failed) return on_done.call(json.data) - ) + , on_failed) -## Requests [param path] and calls [param on_done] with the raw response body. -func get_bytes(path: String, on_done: Callable) -> void: - _request(path, HTTPClient.METHOD_GET, on_done) +## Requests [param path] and calls [param on_done] with the raw response body, or +## [param on_failed] with no arguments if the request fails. +func get_bytes(path: String, on_done: Callable, on_failed: Callable = Callable()) -> void: + _request(path, HTTPClient.METHOD_GET, on_done, on_failed) -## Requests [param path] and calls [param on_done] with no arguments when it succeeds. -## [param on_done] may be an empty Callable when the result does not matter. -func send(path: String, on_done: Callable, method: int = HTTPClient.METHOD_GET) -> void: +## Requests [param path] and calls [param on_done] with no arguments when it succeeds, +## or [param on_failed] when it fails. Either may be an empty Callable. +func send( + path: String, + on_done: Callable, + on_failed: Callable = Callable(), + method: int = HTTPClient.METHOD_GET, +) -> void: _request(path, method, func(_body: PackedByteArray) -> void: - if on_done.is_valid(): - on_done.call() - ) + _call_if_valid(on_done) + , on_failed) -func _request(path: String, method: int, on_success: Callable) -> void: +func _request(path: String, method: int, on_success: Callable, on_failed: Callable) -> void: var url: String = Settings.default_path + path transport.request(url, method, func(response_code: int, body: PackedByteArray) -> void: if response_code != 200: push_warning("Request to %s failed with response code %d" % [url, response_code]) + _call_if_valid(on_failed) return on_success.call(body) ) + + +static func _call_if_valid(callback: Callable) -> void: + if callback.is_valid(): + callback.call() diff --git a/tests/integration/test_main_window.gd b/tests/integration/test_main_window.gd index 4363dcf..6cad167 100644 --- a/tests/integration/test_main_window.gd +++ b/tests/integration/test_main_window.gd @@ -78,3 +78,44 @@ func test_cannot_add_player_without_name() -> void: _add_player("") assert_array(Settings.player_array).is_empty() + + +func test_failed_song_download_is_retried_until_the_server_answers() -> void: + _window.song_fetch_retry_seconds = 0.05 + var songs_before: int = Playlist.song_object_array.size() + var mp3: PackedByteArray = FileAccess.get_file_as_bytes("res://tests/data/short.mp3") + _server.respond_bytes("/music/rand", PackedByteArray(), 503) + var rand_requests_before: int = _server.count("/music/rand") + + _runner.invoke("fetch_song") + await await_millis(200) + + assert_int(_server.count("/music/rand") - rand_requests_before).is_greater(1) + assert_int(Playlist.song_object_array.size()).is_equal(songs_before) + assert_bool(Playlist.is_song_downloading).is_true() + + _server.respond_bytes("/music/rand", mp3) + await await_millis(200) + + assert_int(Playlist.song_object_array.size()).is_equal(songs_before + 1) + assert_bool(Playlist.is_song_downloading).is_false() + + +func test_failed_song_info_retries_only_that_step() -> void: + _window.song_fetch_retry_seconds = 0.05 + var songs_before: int = Playlist.song_object_array.size() + _server.respond_bytes("/music/info", PackedByteArray(), 503) + var rand_before: int = _server.count("/music/rand") + var queue_before: int = _server.count("/music/addQue") + + _runner.invoke("fetch_song") + await await_millis(200) + _next_song_is(SONG_INFO[1]) + await await_millis(200) + + # The song was downloaded and queued once; only the info request was repeated + assert_int(_server.count("/music/rand") - rand_before).is_equal(1) + assert_int(_server.count("/music/addQue") - queue_before).is_equal(1) + assert_int(Playlist.song_object_array.size()).is_equal(songs_before + 1) + assert_str(Playlist.song_object_array.back().song_title).is_equal("Dancing Mad") + assert_bool(Playlist.is_song_downloading).is_false() diff --git a/tests/unit/test_music_server_client.gd b/tests/unit/test_music_server_client.gd index 50de01b..9391d45 100644 --- a/tests/unit/test_music_server_client.gd +++ b/tests/unit/test_music_server_client.gd @@ -63,6 +63,39 @@ func test_invalid_json_does_not_call_back() -> void: assert_bool(called[0]).is_false() +func test_failed_request_calls_on_failed() -> void: + _server.respond_bytes("/music/rand", PackedByteArray(), 503) + var outcome: Array[String] = [] + + MusicServerClient.fetch_random_song(false, + func(_body: PackedByteArray) -> void: outcome.append("done"), + func() -> void: outcome.append("failed")) + + assert_array(outcome).contains_exactly(["failed"]) + + +func test_invalid_json_calls_on_failed() -> void: + _server.respond_bytes("/music/info", "not json".to_utf8_buffer()) + var outcome: Array[String] = [] + + MusicServerClient.fetch_song_info( + func(_data: Variant) -> void: outcome.append("done"), + func() -> void: outcome.append("failed")) + + assert_array(outcome).contains_exactly(["failed"]) + + +func test_successful_request_does_not_call_on_failed() -> void: + _server.respond_bytes("/music/addQue", PackedByteArray()) + var outcome: Array[String] = [] + + MusicServerClient.add_to_queue( + func() -> void: outcome.append("done"), + func() -> void: outcome.append("failed")) + + assert_array(outcome).contains_exactly(["done"]) + + func test_send_calls_back_without_arguments() -> void: _server.respond_bytes("/music/reset", PackedByteArray()) var called: Array[bool] = [false]