refactor(client): decouple dialogue_box from GameState and AudioManager (#558)
Replace 3 direct GameState.dialogue_active mutations and all AudioManager.apply_dip/clear_dip calls with signals: dialogue_state_changed, audio_dip_requested, audio_dip_cleared. dialogue_box.gd now has zero references to GameState or AudioManager. main.gd wires coordinator handlers in _ready() (D-020). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.6
@@ -596,3 +596,77 @@ func test_is_dialogue_active_true_after_show_dialogue() -> void:
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
assert_bool(box.is_dialogue_active()).is_true()
|
||||
box.queue_free()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# D-020 (#558): Signal decoupling — dialogue_box emits signals instead of
|
||||
# directly mutating GameState or calling AudioManager.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
func test_dialogue_state_changed_emits_true_on_show() -> void:
|
||||
## D-020: show_dialogue() must emit dialogue_state_changed(true).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
var received: Array = []
|
||||
box.dialogue_state_changed.connect(func(active): received.append(active))
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
assert_bool(received.has(true)).override_failure_message(
|
||||
"dialogue_state_changed(true) must be emitted on show_dialogue"
|
||||
).is_true()
|
||||
box.queue_free()
|
||||
|
||||
|
||||
func test_dialogue_state_changed_emits_false_on_hide() -> void:
|
||||
## D-020: hide_dialogue() must emit dialogue_state_changed(false).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
var received: Array = []
|
||||
box.dialogue_state_changed.connect(func(active): received.append(active))
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
box.hide_dialogue()
|
||||
assert_bool(received.has(false)).override_failure_message(
|
||||
"dialogue_state_changed(false) must be emitted on hide_dialogue"
|
||||
).is_true()
|
||||
box.queue_free()
|
||||
|
||||
|
||||
func test_audio_dip_requested_emits_dialogue_on_show() -> void:
|
||||
## D-020: show_dialogue() must emit audio_dip_requested("dialogue").
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
var received: Array = []
|
||||
box.audio_dip_requested.connect(func(profile): received.append(profile))
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
assert_bool(received.has("dialogue")).override_failure_message(
|
||||
"audio_dip_requested('dialogue') must be emitted on show_dialogue"
|
||||
).is_true()
|
||||
box.queue_free()
|
||||
|
||||
|
||||
func test_audio_dip_cleared_emits_on_hide() -> void:
|
||||
## D-020: hide_dialogue() must emit audio_dip_cleared.
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
var cleared := [false]
|
||||
box.audio_dip_cleared.connect(func(): cleared[0] = true)
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
box.hide_dialogue()
|
||||
assert_bool(cleared[0]).override_failure_message(
|
||||
"audio_dip_cleared must be emitted on hide_dialogue"
|
||||
).is_true()
|
||||
box.queue_free()
|
||||
|
||||
|
||||
func test_no_direct_game_state_mutation() -> void:
|
||||
## D-020: dialogue_box must not directly mutate GameState.dialogue_active.
|
||||
## After show_dialogue, GameState.dialogue_active should remain unchanged
|
||||
## (only the coordinator updates it via signal handler).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
GameState.dialogue_active = false
|
||||
box.show_dialogue("NPC", "Speech.", _make_options(["Reply"]))
|
||||
assert_bool(GameState.dialogue_active).override_failure_message(
|
||||
"GameState.dialogue_active must NOT be mutated directly by dialogue_box"
|
||||
).is_false()
|
||||
box.queue_free()
|
||||
GameState.dialogue_active = false
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
## Sprint 20 #558: dialogue_box.gd decoupling tests.
|
||||
##
|
||||
## Verifies D-020 compliance: dialogue_box.gd emits signals instead of mutating
|
||||
## GameState or calling AudioManager directly. main.gd wires the signal handlers.
|
||||
##
|
||||
## D-030: fixture-based, server-free, no subprocess required.
|
||||
class_name TestDialogueSprint20
|
||||
extends GdUnitTestSuite
|
||||
|
||||
|
||||
func _make_dialogue_box() -> Control:
|
||||
if not ResourceLoader.exists("res://ui/dialogue_box.tscn"):
|
||||
push_warning("TestDialogueSprint20: dialogue_box.tscn not found — scene tests skipped")
|
||||
return null
|
||||
var node: Control = load("res://ui/dialogue_box.tscn").instantiate()
|
||||
add_child(node)
|
||||
return node
|
||||
|
||||
|
||||
func before_test() -> void:
|
||||
GameState.dialogue_active = false
|
||||
|
||||
|
||||
func after_test() -> void:
|
||||
GameState.dialogue_active = false
|
||||
|
||||
|
||||
# -- dialogue_state_changed signal -------------------------------------------
|
||||
|
||||
func test_show_dialogue_emits_dialogue_state_changed_true() -> void:
|
||||
## show_dialogue() must emit dialogue_state_changed(true) not mutate GameState directly.
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
var received: Variant = null
|
||||
box.dialogue_state_changed.connect(func(active: bool): received = active)
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
assert_that(received).override_failure_message(
|
||||
"show_dialogue() must emit dialogue_state_changed(true) (#558)"
|
||||
).is_equal(true)
|
||||
|
||||
|
||||
func test_show_dialogue_does_not_mutate_game_state_directly() -> void:
|
||||
## Without a connected handler, GameState.dialogue_active must stay false.
|
||||
## Proves dialogue_box.gd has zero direct GameState mutation (D-020 #558).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
GameState.dialogue_active = false
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
assert_bool(GameState.dialogue_active).override_failure_message(
|
||||
"dialogue_box must not mutate GameState.dialogue_active directly (D-020 #558)"
|
||||
).is_false()
|
||||
|
||||
|
||||
func test_hide_dialogue_emits_dialogue_state_changed_false() -> void:
|
||||
## hide_dialogue() must emit dialogue_state_changed(false).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
var received: Variant = null
|
||||
box.dialogue_state_changed.connect(func(active: bool): received = active)
|
||||
box.hide_dialogue()
|
||||
assert_that(received).override_failure_message(
|
||||
"hide_dialogue() must emit dialogue_state_changed(false) (#558)"
|
||||
).is_equal(false)
|
||||
|
||||
|
||||
# -- audio_dip_requested / audio_dip_cleared signals -------------------------
|
||||
|
||||
func test_show_dialogue_emits_audio_dip_requested_dialogue() -> void:
|
||||
## show_dialogue() must emit audio_dip_requested("dialogue") not call AudioManager.
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
var received_profile: Variant = null
|
||||
box.audio_dip_requested.connect(func(profile: String): received_profile = profile)
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
assert_that(received_profile).override_failure_message(
|
||||
"show_dialogue() must emit audio_dip_requested('dialogue') (#558)"
|
||||
).is_equal("dialogue")
|
||||
|
||||
|
||||
func test_show_dialogue_does_not_call_audio_manager_directly() -> void:
|
||||
## Without a connected handler, AudioManager state must be unchanged by show_dialogue().
|
||||
## Verifies no direct AudioManager call in dialogue_box.gd (D-020 #558).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
var dip_before := AudioManager.get_active_dip()
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
var dip_after := AudioManager.get_active_dip()
|
||||
assert_str(dip_after).override_failure_message(
|
||||
"dialogue_box must not call AudioManager.apply_dip() directly (D-020 #558)"
|
||||
).is_equal(dip_before)
|
||||
|
||||
|
||||
func test_hide_dialogue_emits_audio_dip_cleared() -> void:
|
||||
## Ending a conversation must emit audio_dip_cleared not call AudioManager directly.
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
var cleared := false
|
||||
box.audio_dip_cleared.connect(func(): cleared = true)
|
||||
box.hide_dialogue()
|
||||
assert_bool(cleared).override_failure_message(
|
||||
"hide_dialogue() must emit audio_dip_cleared (#558)"
|
||||
).is_true()
|
||||
|
||||
|
||||
func test_audio_dip_cleared_count_on_conversation_end() -> void:
|
||||
## Verify audio_dip_cleared fires when conversation ends.
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
var cleared_count := 0
|
||||
box.audio_dip_cleared.connect(func(): cleared_count += 1)
|
||||
box.show_dialogue("NPC", "Speak.", [])
|
||||
box.hide_dialogue()
|
||||
assert_int(cleared_count).override_failure_message(
|
||||
"audio_dip_cleared must fire at least once when conversation ends (#558)"
|
||||
).is_greater_equal(1)
|
||||
|
||||
|
||||
# -- coordinator wiring verification -----------------------------------------
|
||||
|
||||
func test_signal_handler_wires_game_state() -> void:
|
||||
## Simulate main.gd: connect dialogue_state_changed to update GameState.dialogue_active.
|
||||
## Verifies the coordinator pattern works end-to-end (D-020 #558).
|
||||
var box := _make_dialogue_box()
|
||||
if box == null: return
|
||||
auto_free(box)
|
||||
box.dialogue_state_changed.connect(func(active: bool): GameState.dialogue_active = active)
|
||||
box.show_dialogue("NPC", "Hello.", [])
|
||||
assert_bool(GameState.dialogue_active).override_failure_message(
|
||||
"With handler wired, GameState.dialogue_active must be true after show_dialogue (#558)"
|
||||
).is_true()
|
||||
box.hide_dialogue()
|
||||
assert_bool(GameState.dialogue_active).override_failure_message(
|
||||
"With handler wired, GameState.dialogue_active must be false after hide_dialogue (#558)"
|
||||
).is_false()
|
||||
+17
-12
@@ -15,6 +15,11 @@ signal dialogue_dismissed # Walk-away or conversation end
|
||||
signal confrontation_monologue(text: String, duration: float) # D-063: beat monologue
|
||||
signal pause_requested # D-061: auto-pause — main.gd routes through input recording (#507)
|
||||
signal unpause_requested # D-061: auto-unpause
|
||||
# D-020 (#558): Decoupled signals — dialogue_box emits, main.gd (coordinator) handles.
|
||||
# Replaces direct GameState.dialogue_active mutation and AudioManager calls.
|
||||
signal dialogue_state_changed(active: bool)
|
||||
signal audio_dip_requested(profile: String)
|
||||
signal audio_dip_cleared
|
||||
|
||||
@onready var panel: PanelContainer = $PanelContainer
|
||||
@onready var dialogue_log: RichTextLabel = $PanelContainer/MarginContainer/VBoxContainer/DialogueLog
|
||||
@@ -286,10 +291,10 @@ func show_dialogue(npc_name: String, speech: String, options: Array = []) -> voi
|
||||
# Show panel
|
||||
_ensure_visible()
|
||||
mouse_filter = Control.MOUSE_FILTER_STOP
|
||||
GameState.dialogue_active = true # D-064: block movement while in conversation
|
||||
dialogue_state_changed.emit(true) # D-064: coordinator blocks movement
|
||||
|
||||
# D-069: Dialogue dip
|
||||
AudioManager.apply_dip("dialogue")
|
||||
# D-069: Dialogue dip — coordinator routes to AudioManager
|
||||
audio_dip_requested.emit("dialogue")
|
||||
|
||||
# D-061: auto-pause — signal to main.gd for input recording (#507)
|
||||
pause_requested.emit()
|
||||
@@ -311,14 +316,14 @@ func _end_player_conversation() -> void:
|
||||
entry.timestamp_msec = now
|
||||
_log_dirty = true
|
||||
|
||||
# D-069: Clear dialogue/confrontation dip
|
||||
AudioManager.clear_dip()
|
||||
# D-069: Clear dialogue/confrontation dip — coordinator routes to AudioManager
|
||||
audio_dip_cleared.emit()
|
||||
|
||||
# D-061: unpause — signal to main.gd for input recording (#507)
|
||||
unpause_requested.emit()
|
||||
|
||||
# D-064: unblock movement immediately — log entries stay visible but don't block input.
|
||||
GameState.dialogue_active = false
|
||||
# D-064: unblock movement immediately — coordinator handles GameState update.
|
||||
dialogue_state_changed.emit(false)
|
||||
|
||||
# If no entries remain, hide the panel with fade.
|
||||
if _log_entries.is_empty():
|
||||
@@ -331,11 +336,11 @@ func hide_dialogue() -> void:
|
||||
_end_player_conversation()
|
||||
return # _end_player_conversation may call hide_dialogue if log is empty
|
||||
|
||||
GameState.dialogue_active = false
|
||||
dialogue_state_changed.emit(false) # D-020: coordinator handles GameState update
|
||||
|
||||
|
||||
func is_dialogue_active() -> bool:
|
||||
return _in_player_conversation or GameState.dialogue_active
|
||||
return _in_player_conversation
|
||||
|
||||
|
||||
func has_active_entries() -> bool:
|
||||
@@ -428,12 +433,12 @@ func _start_confrontation_beat(response_id: String, text: String) -> void:
|
||||
_active_tween.tween_property(panel, "modulate:a", CONFRONTATION_DIM_ALPHA, 0.2)
|
||||
|
||||
confrontation_monologue.emit(UIStrings.get_text(CONFRONTATION_MONOLOGUE_KEY), CONFRONTATION_BEAT_DURATION)
|
||||
AudioManager.apply_dip("confrontation")
|
||||
audio_dip_requested.emit("confrontation") # D-069: coordinator routes to AudioManager
|
||||
|
||||
_beat_tween = create_tween()
|
||||
_beat_tween.tween_interval(CONFRONTATION_BEAT_DURATION)
|
||||
_beat_tween.tween_callback(func():
|
||||
AudioManager.clear_dip()
|
||||
audio_dip_cleared.emit() # D-069: coordinator routes to AudioManager
|
||||
option_selected.emit(response_id, text)
|
||||
_end_player_conversation()
|
||||
)
|
||||
@@ -443,7 +448,7 @@ func _cancel_beat() -> void:
|
||||
if _beat_tween and _beat_tween.is_valid():
|
||||
_beat_tween.kill()
|
||||
_beat_tween = null
|
||||
AudioManager.clear_dip()
|
||||
audio_dip_cleared.emit() # D-069: coordinator routes to AudioManager
|
||||
|
||||
|
||||
# -- Log rendering --
|
||||
|
||||
Reference in New Issue
Block a user