From 5a9af4191efeb84d812ac1e6d9d3d95c8112737f Mon Sep 17 00:00:00 2001 From: autocommit Date: Thu, 30 Apr 2026 00:08:31 -0700 Subject: [PATCH] =?UTF-8?q?refactor(audio-manager):=20=E2=99=BB=EF=B8=8F?= =?UTF-8?q?=20Replace=20direct=20signal-to-SFX=20mapping=20with=20a=20tabl?= =?UTF-8?q?e-based=20system=20in=20AudioManager=20to=20reduce=20code=20dup?= =?UTF-8?q?lication?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Lilith Autocommit --- .../engine/src/autoloads/audio_manager.gd | 182 +++++++----------- .../engine/tests/unit/test_audio_manager.gd | 16 ++ 2 files changed, 84 insertions(+), 114 deletions(-) diff --git a/src/game/engine/src/autoloads/audio_manager.gd b/src/game/engine/src/autoloads/audio_manager.gd index ff0bd71d..c2345777 100644 --- a/src/game/engine/src/autoloads/audio_manager.gd +++ b/src/game/engine/src/autoloads/audio_manager.gd @@ -189,32 +189,51 @@ func stop_music() -> void: # --------------------------------------------------------------------------- +## Maps an EventBus signal name → SFX manifest key. Each entry here turns +## "fire SFX X when signal Y emits" into a single line of data instead of a +## bespoke handler. Signals that need branching (perspective stings, music +## crossfades, owner-aware variants, throttling) get explicit handlers +## below — this table is for the trivial 1:1 cases. +const SIMPLE_ROUTES: Dictionary = { + "turn_started": "turn_started", + "turn_ended": "turn_ended", + "city_founded": "city_founded", + "tech_researched": "tech_researched", + "tech_research_started": "research_start", + "culture_researched": "culture_researched", + "city_grew": "city_grew", + "city_starved": "city_starved", + "city_border_expanded": "border_expanded", + "unit_promoted": "unit_promoted", +} + + func _connect_event_bus() -> void: - EventBus.turn_started.connect(_on_turn_started) - EventBus.turn_ended.connect(_on_turn_ended) - EventBus.city_founded.connect(_on_city_founded) - EventBus.tech_researched.connect(_on_tech_researched) + for sig_name: String in SIMPLE_ROUTES.keys(): + var sfx_key: String = SIMPLE_ROUTES[sig_name] + EventBus.get(sig_name).connect(_make_simple_route_handler(sfx_key)) + # Branching handlers — each needs more than play_sfx(literal). EventBus.unit_destroyed.connect(_on_unit_destroyed) EventBus.wonder_built.connect(_on_wonder_built) EventBus.era_changed.connect(_on_era_changed) EventBus.combat_resolved.connect(_on_combat_resolved) + EventBus.combat_started.connect(_on_combat_started) EventBus.unit_moved.connect(_on_unit_moved) EventBus.victory_achieved.connect(_on_victory_achieved) - # p2-33 — categorical / additional wires. - EventBus.combat_started.connect(_on_combat_started) - EventBus.unit_promoted.connect(_on_unit_promoted) - EventBus.city_grew.connect(_on_city_grew) - EventBus.city_starved.connect(_on_city_starved) EventBus.golden_age_started.connect(_on_golden_age_started) EventBus.golden_age_ended.connect(_on_golden_age_ended) - EventBus.city_border_expanded.connect(_on_border_expanded) - EventBus.tech_research_started.connect(_on_tech_research_started) - EventBus.culture_researched.connect(_on_culture_researched) EventBus.wild_creature_spawned.connect(_on_wild_creature_spawned) EventBus.weather_event_applied.connect(_on_weather_event) EventBus.player_eliminated.connect(_on_player_eliminated) +## Returns a Callable that plays `sfx_key` and ignores all signal args. +## Closures over the key so SIMPLE_ROUTES drives connect. +func _make_simple_route_handler(sfx_key: String) -> Callable: + return func(_a: Variant = null, _b: Variant = null, _c: Variant = null) -> void: + play_sfx(sfx_key) + + func _build_music_players() -> void: _music_player_a = AudioStreamPlayer.new() _music_player_a.name = "MusicA" @@ -445,22 +464,6 @@ func _unit_id_of(unit: Variant) -> String: # --------------------------------------------------------------------------- -func _on_turn_started(_turn_number: int, _player_index: int) -> void: - play_sfx("turn_started") - - -func _on_turn_ended(_turn_number: int, _player_index: int) -> void: - play_sfx("turn_ended") - - -func _on_city_founded(_city: Variant, _player_index: int) -> void: - play_sfx("city_founded") - - -func _on_tech_researched(_tech_id: String, _player_index: int) -> void: - play_sfx("tech_researched") - - func _on_unit_destroyed(unit: Variant, killer: Variant) -> void: # Two layers, conceptually distinct: # 1. Species death sound (wolf yelp, dwarf grunt) — neutral, plays for @@ -473,8 +476,8 @@ func _on_unit_destroyed(unit: Variant, killer: Variant) -> void: var unit_id: String = _unit_id_of(unit) if not unit_id.is_empty(): play_for_entity(unit_id, "death") - var victim_human: bool = _is_human_owner(unit) - var killer_human: bool = _is_human_owner(killer) + var victim_human: bool = _is_human(unit) + var killer_human: bool = _is_human(killer) if victim_human: play_sfx("unit_defeated") elif killer_human: @@ -483,12 +486,19 @@ func _on_unit_destroyed(unit: Variant, killer: Variant) -> void: play_sfx("unit_killed") -func _is_human_owner(holder: Variant) -> bool: - if holder == null: - return false - if not ("owner" in holder): - return false - var idx: int = int(holder.get("owner")) +## Resolve a player_index from `who` (Variant), then check is_human. +## Accepts either an int (player_index) or any RefCounted that exposes +## an `owner: int` field — entities emitted by EventBus carry the latter, +## elimination/victory signals carry the former. Returns false on any +## resolution failure (out of range, missing field, non-RefCounted). +## Replaces the prior _is_human_player(int) and _is_human_owner(Variant) +## DRY duplicates. +func _is_human(who: Variant) -> bool: + var idx: int = -1 + if who is int: + idx = who as int + elif who != null and "owner" in who: + idx = int(who.get("owner")) if idx < 0 or idx >= GameState.players.size(): return false var player: RefCounted = GameState.players[idx] as RefCounted @@ -515,11 +525,7 @@ func _on_wonder_built(_wonder_id: String, player_index: int) -> void: ## fallback walk) — so a missing variant warns once instead of silently ## degrading to a different sound. func play_sfx_for_owner(key: String, player_index: int) -> void: - var suffix: String = "rival" - if player_index >= 0 and player_index < GameState.players.size(): - var player: RefCounted = GameState.players[player_index] as RefCounted - if player != null and "is_human" in player and bool(player.get("is_human")): - suffix = "own" + var suffix: String = "own" if _is_human(player_index) else "rival" var variant_key: String = "%s.%s" % [key, suffix] if _sfx_events.has(variant_key): play_sfx(variant_key) @@ -550,18 +556,6 @@ func _on_combat_started(attacker: Variant, _defender: Variant) -> void: play_sfx("combat_started") -func _on_unit_promoted(_unit: Variant, _promotion: String) -> void: - play_sfx("unit_promoted") - - -func _on_city_grew(_city: Variant, _new_pop: int) -> void: - play_sfx("city_grew") - - -func _on_city_starved(_city: Variant, _new_pop: int) -> void: - play_sfx("city_starved") - - func _on_golden_age_started(_player_index: int) -> void: play_sfx("golden_age_swell") play_music("golden_age") @@ -573,18 +567,6 @@ func _on_golden_age_ended(_player_index: int) -> void: stop_music() -func _on_border_expanded(_city: Variant, _tile: Vector2i) -> void: - play_sfx("border_expanded") - - -func _on_tech_research_started(_tech_id: String, _player_index: int) -> void: - play_sfx("research_start") - - -func _on_culture_researched(_tradition_id: String, _player_index: int) -> void: - play_sfx("culture_researched") - - func _on_wild_creature_spawned(unit: Variant, _lair_pos: Vector2i) -> void: var unit_id: String = _unit_id_of(unit) if not unit_id.is_empty(): @@ -612,64 +594,36 @@ func _on_victory_achieved(player_index: int, victory_type: String) -> void: # A win is the listener's win only if the winner is the local human. # Otherwise the human is being defeated by this winner's strategy and # we play the matching defeat-by- track. - if _is_human_player(player_index): + if _is_human(player_index): play_sfx("victory_fanfare") - play_music(_pick_victory_track(victory_type)) + play_music(_pick_from_pool(_victory_pool, victory_type, "victory")) else: play_sfx("defeat_stinger") - play_music(_pick_defeat_track(victory_type)) + play_music(_pick_from_pool(_defeat_pool, victory_type, "defeat")) -## Pick a music track id for the given victory type. Looks the type up in -## `_victory_pool`; if multiple track ids are listed, picks one at random -## so a player who triggers the same victory across multiple games hears -## variation. Falls back to the manifest's "victory" track id when the -## type is unmapped, then to default_track_id. -func _pick_victory_track(victory_type: String) -> String: - if _victory_pool.has(victory_type) and _victory_pool[victory_type] is Array: - var pool: Array = _victory_pool[victory_type] as Array - if pool.size() > 0: - return String(pool[_rng.randi_range(0, pool.size() - 1)]) - if _music_tracks.has("victory"): - return "victory" +## Pick a music track id from a per-victory-type pool. If `key` is mapped +## to a non-empty array of track ids, return a random one; otherwise fall +## back to `fallback_id` (a known generic track), and finally to +## `_music_default_id`. Replaces the prior _pick_victory_track and +## _pick_defeat_track which were structurally identical. +func _pick_from_pool(pool: Dictionary, key: String, fallback_id: String) -> String: + if pool.has(key) and pool[key] is Array: + var arr: Array = pool[key] as Array + if arr.size() > 0: + return String(arr[_rng.randi_range(0, arr.size() - 1)]) + if _music_tracks.has(fallback_id): + return fallback_id return _music_default_id -## Mirror of _pick_victory_track for `defeat_pool`. Returns a defeat track -## id keyed to *how* the human player was defeated. Falls back to the -## generic "defeat" track when the victory_type is unmapped. -func _pick_defeat_track(victory_type: String) -> String: - if _defeat_pool.has(victory_type) and _defeat_pool[victory_type] is Array: - var pool: Array = _defeat_pool[victory_type] as Array - if pool.size() > 0: - return String(pool[_rng.randi_range(0, pool.size() - 1)]) - if _music_tracks.has("defeat"): - return "defeat" - return _music_default_id - - -## Helper: is `player_index` the local human player? Returns false on -## out-of-range indices and on players that don't expose `is_human`. -func _is_human_player(player_index: int) -> bool: - if player_index < 0 or player_index >= GameState.players.size(): - return false - var player: RefCounted = GameState.players[player_index] as RefCounted - if player == null or not ("is_human" in player): - return false - return bool(player.get("is_human")) - - -## Defeat is the human-player counterpart of victory_achieved. The signal -## fires for any eliminated player; we only swap to defeat audio when the -## eliminated player is the local human, otherwise the listener gets -## defeat music for an AI's loss which is wrong. +## Defeat is the human-player counterpart of victory_achieved. Fires for +## last-unit-destroyed eliminations etc. (no victory_type carried). When +## the elimination is also a victory_achieved (an AI just won), that +## handler already swapped to the defeat-by-X track; re-asserting the +## generic "defeat" here is harmless (same Music bus, crossfade tweens). func _on_player_eliminated(player_index: int) -> void: - # This signal carries no victory_type — fires for last-unit-destroyed - # eliminations etc. When the elimination is also a victory_achieved - # (an AI just won), that handler already swapped to the defeat-by-X - # track via _pick_defeat_track; re-asserting the generic "defeat" - # here is harmless (same Music bus, crossfade tweens). - if not _is_human_player(player_index): + if not _is_human(player_index): return play_sfx("defeat_stinger") play_music("defeat") diff --git a/src/game/engine/tests/unit/test_audio_manager.gd b/src/game/engine/tests/unit/test_audio_manager.gd index ee23dcf4..f5b6c2ac 100644 --- a/src/game/engine/tests/unit/test_audio_manager.gd +++ b/src/game/engine/tests/unit/test_audio_manager.gd @@ -289,6 +289,22 @@ func test_every_weather_kind_has_manifest_entry() -> void: ) +func test_simple_routes_have_manifest_entries() -> void: + # Closure for the routes table: every signal-name → sfx-key mapping in + # AudioManager.SIMPLE_ROUTES must point at a real manifest entry. + # Catches accidental drift (e.g. renaming a manifest key without + # updating the route, or vice-versa). + var AudioManagerScript: GDScript = load("res://engine/src/autoloads/audio_manager.gd") + var routes: Dictionary = AudioManagerScript.SIMPLE_ROUTES + assert_gt(routes.size(), 0, "SIMPLE_ROUTES must have entries") + for sig_name: String in routes.keys(): + var sfx_key: String = routes[sig_name] + assert_true( + AudioManager._sfx_events.has(sfx_key), + "SIMPLE_ROUTES[%s] = %s — no manifest entry exists for that key" % [sig_name, sfx_key] + ) + + func test_unknown_entity_chain_does_not_resolve() -> void: # Mirror of the closure test: an unknown entity_id with no DataLoader # registration must NOT resolve to anything. The runtime then emits