Code changes addressing PR #135 review (Tyre + Hoshe): - **T3 (blocking):** test_merge_path_flows_sprint37.gd `_load_main_menu` and `_load_char_create` now assert the scene loaded instead of silently returning. Missing .tscn → red test, not falsely green. - **T1:** sim_bridge.gd signal `handshake_complete(protocol_version: int)` was D-192 residue with no listeners. Drop the int parameter entirely and the literal-0 emit. - **H4:** test_new_game_catalog_snapshot_resolves_loading_state now asserts SimBridge.state == CONNECTED terminus, not just the loading flag — guarantees full flow completion, not merely flag-clear. - **H5:** test_protocol_bridge.gd file-level comment refreshed; drops reference to removed protocol-version check tests. - **H6:** test_p0_regressions.gd `_make_snapshot_bytes` comment refreshed and version field removed from fixture dict (D-192: not required). - **H7:** test_merge_path_flows_sprint37.gd `_make_catalog_snapshot` drops version field from fixture dict (D-192). Follow-up tickets filed for reviewer suggestions: - **T2:** #889 — revive EntityRenderer sprite constants coverage (D-044 ENTITY_WIDTH/HEIGHT, asserted by deleted test_sprite_integration). - **T4:** #890 — UI timeout fallback for bookmark catalog wait in main_menu (systemic 'catalog never arrives' class beyond #872's TCP-batch race). - **T5/T6:** #891 — scene-flow test tier docs + test-only reset helpers (SimBridge.reset_for_test, MetaStack.reset_for_test) + minimal public API on scenes so UI refactors don't break all four flow tests simultaneously. Verification: - `make lint-client` — no script errors - `gdlint client/scripts/ client/ui/` — no problems - `make test-client` — 2428/2488 passing. 60 remaining failures are pre-existing, unrelated to sprint 37 (test_dialogue_sprint20 #558 signals, test_input_roundtrip integration-sans-server, etc.). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
331 lines
13 KiB
GDScript
331 lines
13 KiB
GDScript
## Sprint 37 — Scene-level merge-path UI flow tests (#873)
|
|
##
|
|
## Four flows that cover the critical paths through the pre-game UI.
|
|
## These tests are the merge-gate mechanism added in the Sprint 36 retro:
|
|
## regressions like #872 (New Game hang) must be caught here, not in post-merge
|
|
## smoke tests.
|
|
##
|
|
## Pattern: load scene → simulate input via button.pressed.emit() or direct
|
|
## handler call → assert terminal state. No pixel diffing, no xdotool.
|
|
##
|
|
## NOTE: Tests that end in a scene transition (change_scene_to_file) assert state
|
|
## synchronously before the deferred transition fires. The test scene is
|
|
## queue_freed in after_test() regardless.
|
|
##
|
|
## Reference: test_character_creation_sprint28.gd
|
|
## Ticket: #873 | motivating regression: #872
|
|
class_name TestMergePathFlowsSprint37
|
|
extends GdUnitTestSuite
|
|
|
|
const MAIN_MENU_SCENE_PATH := "res://scenes/main_menu.tscn"
|
|
const CHAR_CREATE_SCENE_PATH := "res://scenes/character_creation.tscn"
|
|
|
|
var _scene = null # MainMenu or CharacterCreation — untyped, varies per test
|
|
|
|
|
|
func before_test() -> void:
|
|
# Reset all shared state that these flows touch
|
|
SimBridge.disconnect_from_sim()
|
|
SimBridge._last_snapshot = null
|
|
SimBridge._outbound_buffer.clear()
|
|
GameState.bookmark_catalog = []
|
|
GameState.pending_load_path = ""
|
|
# Clear MetaStack from any leftover overlays to prevent push/pop ordering issues
|
|
MetaStack._stack.clear()
|
|
|
|
|
|
func after_test() -> void:
|
|
if is_instance_valid(_scene):
|
|
_scene.queue_free()
|
|
_scene = null
|
|
SimBridge.disconnect_from_sim()
|
|
SimBridge._outbound_buffer.clear()
|
|
MetaStack._stack.clear()
|
|
|
|
|
|
# -----------------------------------------------------------------------------
|
|
# Helpers
|
|
# -----------------------------------------------------------------------------
|
|
|
|
func _load_main_menu() -> void:
|
|
var packed := load(MAIN_MENU_SCENE_PATH) as PackedScene
|
|
# Hard-fail on missing scene (PR #135 review T3): silent skip turns a broken
|
|
# merge-path test into a uselessly green one.
|
|
assert_that(packed).override_failure_message(
|
|
"main_menu.tscn missing — merge-path coverage is broken, not skipped"
|
|
).is_not_null()
|
|
_scene = packed.instantiate()
|
|
add_child(_scene)
|
|
|
|
|
|
func _load_char_create() -> void:
|
|
var packed := load(CHAR_CREATE_SCENE_PATH) as PackedScene
|
|
# Hard-fail on missing scene (PR #135 review T3): silent skip turns a broken
|
|
# merge-path test into a uselessly green one.
|
|
assert_that(packed).override_failure_message(
|
|
"character_creation.tscn missing — merge-path coverage is broken, not skipped"
|
|
).is_not_null()
|
|
_scene = packed.instantiate()
|
|
# Seed required state so Start is not disabled (guard added in PR #134 / R2-Hoshe-1).
|
|
# Individual tests override these as needed. add_child() must run first so
|
|
# _ready() populates @onready vars (_footer_start, etc.) that
|
|
# _update_start_btn_state() dereferences.
|
|
add_child(_scene)
|
|
_scene._selected_bookmark_id = "test-bookmark"
|
|
_scene._selected_location_id = "test-location"
|
|
if _scene.has_method("_update_start_btn_state"):
|
|
_scene._update_start_btn_state()
|
|
|
|
|
|
func _make_catalog_snapshot() -> Dictionary:
|
|
## Minimal valid snapshot with a bookmark_catalog for flow-1 testing.
|
|
## D-192: no version field required; decode accepts snapshots with or without.
|
|
return {
|
|
"tick": 0,
|
|
"entities": [],
|
|
"game_time": {"day": 0, "time_of_day": 0, "day_phase": "Morning", "tick_rate": "Full"},
|
|
"player_facing": "North",
|
|
"player_stance": "Walk",
|
|
"player_inventory": [],
|
|
"visible_tiles": [],
|
|
"nearby_interactions": [],
|
|
"pending_recognitions": [],
|
|
"bookmark_catalog": {
|
|
"bookmarks": [
|
|
{
|
|
"id": "bm_tycoon_arion",
|
|
"title": "Arion Freight Broker",
|
|
"subtitle": "Start at Arion orbital",
|
|
"flavor": "Commodities and logistics.",
|
|
"default_location": "arion",
|
|
"allowed_locations": ["arion", "arion_low"],
|
|
"allowed_locations_cultures": ["arion"],
|
|
"career": "tycoon",
|
|
"starting_capital_tractus": 50000,
|
|
}
|
|
]
|
|
},
|
|
}
|
|
|
|
|
|
# =============================================================================
|
|
# Flow 1: main menu → new game → loading state → catalog received → resolved
|
|
# =============================================================================
|
|
# Regression guard for #872: New Game used to hang on "Connecting to simulation..."
|
|
# because bookmark_catalog was overwritten in receive_bytes before poll_snapshot consumed it.
|
|
# This test catches that regression by verifying the full state machine:
|
|
# pressed → loading visible → catalog signal → loading dismissed.
|
|
|
|
func test_new_game_shows_loading_screen() -> void:
|
|
## Pressing New Game must show the loading screen and set _waiting_for_catalog.
|
|
_load_main_menu()
|
|
if _scene == null:
|
|
return
|
|
|
|
assert_bool(_scene._waiting_for_catalog).override_failure_message(
|
|
"_waiting_for_catalog must be false before New Game is pressed"
|
|
).is_false()
|
|
|
|
# Press New Game via the button signal (same as real player input)
|
|
_scene._new_game_btn.pressed.emit()
|
|
|
|
assert_bool(_scene._waiting_for_catalog).override_failure_message(
|
|
"_waiting_for_catalog must be true after New Game pressed"
|
|
).is_true()
|
|
assert_that(_scene._loading_screen).override_failure_message(
|
|
"Loading screen instance must exist after New Game pressed"
|
|
).is_not_null()
|
|
assert_bool(_scene._loading_screen.visible).override_failure_message(
|
|
"Loading screen must be visible after New Game pressed"
|
|
).is_true()
|
|
|
|
|
|
func test_new_game_catalog_snapshot_resolves_loading_state() -> void:
|
|
## When snapshot_received fires with a bookmark_catalog, the loading state must clear.
|
|
## This is the exact regression introduced in #872 — if the catalog is never delivered,
|
|
## _waiting_for_catalog stays true and the screen hangs forever.
|
|
_load_main_menu()
|
|
if _scene == null:
|
|
return
|
|
|
|
# Simulate the New Game press to set up the signal subscription and loading state.
|
|
# In test mode, connect_to_sim() immediately fires CONNECTED, which triggers
|
|
# _on_sim_state_changed_for_new_game and connects snapshot_received.
|
|
_scene._new_game_btn.pressed.emit()
|
|
|
|
assert_bool(_scene._waiting_for_catalog).override_failure_message(
|
|
"Precondition: _waiting_for_catalog must be true before catalog arrives"
|
|
).is_true()
|
|
|
|
# Deliver the catalog via the signal path (same path the server uses in live mode).
|
|
# snapshot_received is emitted here directly because in test mode poll_snapshot()
|
|
# uses the harness snapshot (no catalog). The carry-forward fix (#872) ensures
|
|
# this signal path also works correctly in live mode when ticks batch.
|
|
var catalog_snapshot := _make_catalog_snapshot()
|
|
SimBridge.snapshot_received.emit(catalog_snapshot)
|
|
|
|
# Terminal state: loading resolved
|
|
assert_bool(_scene._waiting_for_catalog).override_failure_message(
|
|
"_waiting_for_catalog must be false after catalog snapshot delivered — #872 regression"
|
|
).is_false()
|
|
# PR #135 review H4: assert the SimBridge terminus, not just the loading flag.
|
|
# In test mode connect_to_sim() jumps state to CONNECTED synchronously; this
|
|
# guarantees the flow reached its terminal state, not merely that the catalog
|
|
# flag cleared.
|
|
assert_int(SimBridge.state).override_failure_message(
|
|
"SimBridge must be in CONNECTED terminus after catalog resolves — flow completion guard"
|
|
).is_equal(SimBridge.ConnectionState.CONNECTED)
|
|
assert_bool(GameState.bookmark_catalog.size() > 0).override_failure_message(
|
|
"GameState.bookmark_catalog must be populated after catalog snapshot applied"
|
|
).is_true()
|
|
assert_str(GameState.bookmark_catalog[0].get("id", "")).override_failure_message(
|
|
"Catalog entry must have the expected bookmark id"
|
|
).is_equal("bm_tycoon_arion")
|
|
|
|
|
|
# =============================================================================
|
|
# Flow 2: main menu → load game → save picker → save selected
|
|
# =============================================================================
|
|
|
|
func test_load_game_save_picker_shows_on_browse() -> void:
|
|
## Calling _on_load_game_browse() must show the save picker panel.
|
|
_load_main_menu()
|
|
if _scene == null:
|
|
return
|
|
|
|
assert_bool(_scene._load_panel.visible).override_failure_message(
|
|
"Load panel must be hidden before Load Game is pressed"
|
|
).is_false()
|
|
|
|
_scene._on_load_game_browse()
|
|
|
|
assert_bool(_scene._load_panel.visible).override_failure_message(
|
|
"Load panel must be visible after _on_load_game_browse()"
|
|
).is_true()
|
|
|
|
|
|
func test_load_game_save_selection_sets_pending_load_path() -> void:
|
|
## Selecting a save entry must set GameState.pending_load_path for main.gd to consume.
|
|
_load_main_menu()
|
|
if _scene == null:
|
|
return
|
|
|
|
var mock_save := {
|
|
"game_id": "20260421-120000-abc123",
|
|
"newest_save": "quicksave.sav",
|
|
}
|
|
|
|
# Call _on_save_selected directly — mirrors what the generated save-list button does.
|
|
_scene._on_save_selected(mock_save)
|
|
|
|
assert_str(GameState.pending_load_path).override_failure_message(
|
|
"pending_load_path must be set to the selected save's full path"
|
|
).is_equal("user://saves/20260421-120000-abc123/quicksave.sav")
|
|
|
|
|
|
# =============================================================================
|
|
# Flow 3: character creation → submit → sim_bridge receives correct payload
|
|
# =============================================================================
|
|
|
|
func test_character_creation_submit_sends_confirm_bookmark_action() -> void:
|
|
## _on_start() must queue a ConfirmBookmark action with the selected bookmark
|
|
## and location IDs. This is the payload the server uses to initialize the run.
|
|
_load_char_create()
|
|
if _scene == null:
|
|
return
|
|
|
|
# Ensure SimBridge is connected so send_named_action doesn't silently drop the action
|
|
SimBridge.connect_to_sim() # test mode: immediately CONNECTED
|
|
|
|
_scene._selected_bookmark_id = "bm_tycoon_arion"
|
|
_scene._selected_location_id = "arion"
|
|
if _scene.has_method("_update_start_btn_state"):
|
|
_scene._update_start_btn_state()
|
|
|
|
SimBridge._outbound_buffer.clear()
|
|
_scene._on_start()
|
|
|
|
var found := false
|
|
for entry in SimBridge._outbound_buffer:
|
|
if entry.get("action_name") == "ConfirmBookmark":
|
|
var data: Variant = entry.get("action_data")
|
|
if (
|
|
data is Dictionary
|
|
and data.get("bookmark_id") == "bm_tycoon_arion"
|
|
and data.get("starting_location_id") == "arion"
|
|
):
|
|
found = true
|
|
break
|
|
assert_bool(found).override_failure_message(
|
|
"_outbound_buffer must contain ConfirmBookmark{bookmark_id='bm_tycoon_arion', starting_location_id='arion'}"
|
|
).is_true()
|
|
|
|
|
|
# =============================================================================
|
|
# Flow 4: bookmark tab → select location → confirm → server gets bookmark action
|
|
# =============================================================================
|
|
|
|
func test_bookmark_tab_select_location_then_confirm_queues_action() -> void:
|
|
## Exercises the full selection path: picking a bookmark, picking a location, then
|
|
## confirming. Verifies the correct bookmark_id + starting_location_id reach the server.
|
|
## This is the flow the player actually takes — selection handlers must propagate
|
|
## to the outbound buffer correctly.
|
|
|
|
# Populate catalog before scene instantiation so _build_bookmark_cards() sees it
|
|
GameState.bookmark_catalog = [
|
|
{
|
|
"id": "bm_tycoon_arion",
|
|
"title": "Arion Freight Broker",
|
|
"subtitle": "Start at Arion orbital",
|
|
"flavor": "Commodities and logistics.",
|
|
"default_location": "arion",
|
|
"allowed_locations": ["arion", "arion_low"],
|
|
"allowed_locations_cultures": ["arion"],
|
|
"career": "tycoon",
|
|
"starting_capital_tractus": 50000,
|
|
}
|
|
]
|
|
|
|
_load_char_create()
|
|
if _scene == null:
|
|
return
|
|
|
|
SimBridge.connect_to_sim() # test mode: immediately CONNECTED
|
|
SimBridge._outbound_buffer.clear()
|
|
|
|
# Simulate the player selecting the bookmark card
|
|
var bm: Dictionary = GameState.bookmark_catalog[0]
|
|
_scene._on_bookmark_selected(bm)
|
|
|
|
assert_str(_scene._selected_bookmark_id).override_failure_message(
|
|
"_on_bookmark_selected must update _selected_bookmark_id"
|
|
).is_equal("bm_tycoon_arion")
|
|
assert_str(_scene._selected_location_id).override_failure_message(
|
|
"_on_bookmark_selected must populate _selected_location_id from default_location"
|
|
).is_not_empty()
|
|
|
|
# Simulate the player picking a specific allowed location
|
|
_scene._on_location_selected("arion_low")
|
|
|
|
assert_str(_scene._selected_location_id).override_failure_message(
|
|
"_on_location_selected must update _selected_location_id"
|
|
).is_equal("arion_low")
|
|
|
|
# Confirm — sends the action to the server
|
|
_scene._on_start()
|
|
|
|
var found := false
|
|
for entry in SimBridge._outbound_buffer:
|
|
if entry.get("action_name") == "ConfirmBookmark":
|
|
var data: Variant = entry.get("action_data")
|
|
if (
|
|
data is Dictionary
|
|
and data.get("bookmark_id") == "bm_tycoon_arion"
|
|
and data.get("starting_location_id") == "arion_low"
|
|
):
|
|
found = true
|
|
break
|
|
assert_bool(found).override_failure_message(
|
|
"_outbound_buffer must contain ConfirmBookmark{bookmark_id='bm_tycoon_arion', starting_location_id='arion_low'}"
|
|
).is_true()
|