diff --git a/client/scripts/prototype_economy_hud_section.gd b/client/scripts/prototype_economy_hud_section.gd index c0e6541..368a341 100644 --- a/client/scripts/prototype_economy_hud_section.gd +++ b/client/scripts/prototype_economy_hud_section.gd @@ -85,7 +85,7 @@ func _apply_hud_root_scroll_bounds() -> void: return var available_height: float = position.y - HUD_TOP_MARGIN - HUD_ECONOMY_GAP hud_scroll.position = Vector2(SIDE_MARGIN, HUD_TOP_MARGIN) - hud_scroll.size = Vector2(PANEL_WIDTH, max(80.0, available_height)) + hud_scroll.size = Vector2(PANEL_WIDTH, maxf(0.0, available_height)) func _measure_header_height() -> float: diff --git a/client/scripts/quest_hud_controller.gd b/client/scripts/quest_hud_controller.gd index 7c209aa..a563583 100644 --- a/client/scripts/quest_hud_controller.gd +++ b/client/scripts/quest_hud_controller.gd @@ -268,6 +268,9 @@ func _request_accept(quest_id: String) -> bool: func _request_eligible_accept() -> void: + if not _progress_error.is_empty(): + _render_accept_feedback_label("Quest accept: progress error — try again") + return if _progress_error.is_empty() and _last_snapshot.is_empty(): _render_accept_feedback_label("Quest accept: progress loading — try again") return diff --git a/client/test/prototype_economy_hud_section_test.gd b/client/test/prototype_economy_hud_section_test.gd index df8e219..61ccb3d 100644 --- a/client/test/prototype_economy_hud_section_test.gd +++ b/client/test/prototype_economy_hud_section_test.gd @@ -95,6 +95,7 @@ func test_viewport_layout_clamps_hud_root_scroll() -> void: # Arrange await _set_test_viewport_size() var canvas := Control.new() + auto_free(canvas) canvas.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) var hud_scroll := ScrollContainer.new() hud_scroll.name = "HudRootScroll" @@ -110,4 +111,4 @@ func test_viewport_layout_clamps_hud_root_scroll() -> void: # Assert var panel_top: float = section.call("panel_top_y") assert_that(panel_top).is_greater(0.0) - assert_that(hud_scroll.size.y).is_less(panel_top) + assert_that(hud_scroll.size.y).is_less_equal(panel_top - 12.0) diff --git a/client/test/quest_hud_controller_test.gd b/client/test/quest_hud_controller_test.gd index a29a39a..83946f2 100644 --- a/client/test/quest_hud_controller_test.gd +++ b/client/test/quest_hud_controller_test.gd @@ -7,20 +7,104 @@ const QuestProgressClient := preload("res://scripts/quest_progress_client.gd") const QuestDefinitionsClient := preload("res://scripts/quest_definitions_client.gd") -func _build_controller() -> Node: +class MockHttpTransport: + extends Node + signal request_completed( + result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray + ) + var response_code: int = 200 + var body_json: String = "" + + func request( + _url: String, + _custom_headers: PackedStringArray = PackedStringArray(), + _method: HTTPClient.Method = HTTPClient.METHOD_GET, + _request_data: String = "" + ) -> Error: + request_completed.emit( + HTTPRequest.RESULT_SUCCESS, + response_code, + PackedStringArray(), + body_json.to_utf8_buffer() + ) + return OK + + +class NoopHttpTransport: + extends Node + signal request_completed( + result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray + ) + + func request( + _url: String, + _custom_headers: PackedStringArray = PackedStringArray(), + _method: HTTPClient.Method = HTTPClient.METHOD_GET, + _request_data: String = "" + ) -> Error: + return OK + + +class PendingAcceptHttpTransport: + extends Node + signal request_completed( + result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray + ) + + func request( + _url: String, + _custom_headers: PackedStringArray = PackedStringArray(), + _method: HTTPClient.Method = HTTPClient.METHOD_POST, + _request_data: String = "" + ) -> Error: + return OK + + +func _make_progress_client(sync_transport: Node, accept_transport: Node = null) -> Node: + var c: Node = QuestProgressClient.new() + c.set("injected_sync_http", sync_transport) + var accept_http: Node = accept_transport if accept_transport != null else sync_transport + c.set("injected_accept_http", accept_http) + auto_free(sync_transport) + if accept_transport != null and accept_transport != sync_transport: + auto_free(accept_transport) + auto_free(c) + add_child(c) + return c + + +func _make_defs_client(transport: Node) -> Node: + var c: Node = QuestDefinitionsClient.new() + c.set("injected_http", transport) + auto_free(transport) + auto_free(c) + add_child(c) + return c + + +func _build_controller( + sync_transport: Node, accept_transport: Node, defs_transport: Node +) -> Dictionary: var controller: Node = QuestHudController.new() - var progress: Node = QuestProgressClient.new() - var defs: Node = QuestDefinitionsClient.new() + var progress: Node = _make_progress_client(sync_transport, accept_transport) + var defs: Node = _make_defs_client(defs_transport) var progress_label := Label.new() var accept_label := Label.new() + auto_free(controller) + auto_free(progress_label) + auto_free(accept_label) add_child(controller) - add_child(progress) - add_child(defs) + await get_tree().process_frame controller.call( "setup", progress, defs, progress_label, accept_label, Callable(self, "_noop_http_config") ) await get_tree().process_frame - return controller + return { + "controller": controller, + "progress": progress, + "accept_label": accept_label, + "progress_label": progress_label, + } func _noop_http_config(_client: Node) -> void: @@ -29,32 +113,25 @@ func _noop_http_config(_client: Node) -> void: func test_try_accept_key_input_handles_q_key() -> void: # Arrange - var controller := await _build_controller() + var built: Dictionary = await _build_controller( + NoopHttpTransport.new(), PendingAcceptHttpTransport.new(), NoopHttpTransport.new() + ) var event := InputEventKey.new() event.pressed = true event.keycode = KEY_Q # Act - var handled: bool = bool(controller.call("try_accept_key_input", event)) + var handled: bool = bool(built["controller"].call("try_accept_key_input", event)) # Assert assert_bool(handled).is_true() func test_progress_label_shows_loading_before_snapshot() -> void: # Arrange - var controller: Node = QuestHudController.new() - var progress: Node = QuestProgressClient.new() - var defs: Node = QuestDefinitionsClient.new() - var progress_label := Label.new() - add_child(controller) - add_child(progress) - add_child(defs) - await get_tree().process_frame - controller.call( - "setup", progress, defs, progress_label, Label.new(), Callable(self, "_noop_http_config") + var built: Dictionary = await _build_controller( + NoopHttpTransport.new(), NoopHttpTransport.new(), NoopHttpTransport.new() ) - await get_tree().process_frame # Act - var text: String = progress_label.text + var text: String = str(built["progress_label"].text) # Assert assert_str(text).contains("Quests:") assert_str(text).contains("Loading") @@ -64,9 +141,15 @@ func test_shift_q_shows_progress_loading_before_snapshot() -> void: # Arrange var controller: Node = QuestHudController.new() var accept_label := Label.new() + var progress_client: Node = QuestProgressClient.new() + var defs_client: Node = QuestDefinitionsClient.new() + auto_free(controller) + auto_free(accept_label) + auto_free(progress_client) + auto_free(defs_client) add_child(controller) - controller.set("_progress_client", QuestProgressClient.new()) - controller.set("_defs_client", QuestDefinitionsClient.new()) + controller.set("_progress_client", progress_client) + controller.set("_defs_client", defs_client) controller.set("_accept_label", accept_label) controller.set("_last_snapshot", {}) controller.set("_progress_error", "") @@ -76,14 +159,40 @@ func test_shift_q_shows_progress_loading_before_snapshot() -> void: assert_str(accept_label.text).contains("progress loading") +func test_shift_q_blocks_when_progress_sync_failed() -> void: + # Arrange + var controller: Node = QuestHudController.new() + var accept_label := Label.new() + var progress_client: Node = QuestProgressClient.new() + var defs_client: Node = QuestDefinitionsClient.new() + auto_free(controller) + auto_free(accept_label) + auto_free(progress_client) + auto_free(defs_client) + add_child(controller) + controller.set("_progress_client", progress_client) + controller.set("_defs_client", defs_client) + controller.set("_accept_label", accept_label) + controller.set("_last_snapshot", {}) + controller.set("_progress_error", "HTTP 404 (player unknown)") + # Act + controller.call("_request_eligible_accept") + # Assert + assert_str(accept_label.text).contains("progress error") + + func test_request_accept_shows_busy_feedback() -> void: # Arrange - var controller := await _build_controller() - var progress: Node = controller.get("_progress_client") - progress.call("request_accept", QuestHudController.GATHER_QUEST_ID) - var accept_label: Label = controller.get("_accept_label") + var built: Dictionary = await _build_controller( + NoopHttpTransport.new(), PendingAcceptHttpTransport.new(), NoopHttpTransport.new() + ) + var controller: Node = built["controller"] + var accept_label: Label = built["accept_label"] + controller.call("_request_accept", QuestHudController.GATHER_QUEST_ID) # Act - var started: bool = bool(controller.call("_request_accept", QuestHudController.GATHER_QUEST_ID)) + var started: bool = bool( + controller.call("_request_accept", QuestHudController.GATHER_QUEST_ID) + ) # Assert assert_bool(started).is_false() assert_str(accept_label.text).contains("busy") diff --git a/docs/reviews/2026-06-07-NEO-122-follow-up.md b/docs/reviews/2026-06-07-NEO-122-follow-up.md index 0eb518a..6d9a431 100644 --- a/docs/reviews/2026-06-07-NEO-122-follow-up.md +++ b/docs/reviews/2026-06-07-NEO-122-follow-up.md @@ -45,14 +45,32 @@ Seven commits landed after the first review: code-review follow-up (quest-defini 3. Nit: Economy **`BodyScroll/Body`** child order is now **Recipes → Inventory → Skills → Gig XP** (was inventory-first). NEO-75 steps do not mandate order, but players accustomed to inventory-at-top may need a scroll on first open. +## Bugbot follow-up (PR #161) + +1. ~~**Shift+Q ignores progress sync failure** — After GET fails, `_request_eligible_accept` blocked accept when `_progress_error` is set; GdUnit **`test_shift_q_blocks_when_progress_sync_failed`**. **Done.** + +2. ~~**Hud scroll minimum causes panel overlap** — Removed 80px floor in `_apply_hud_root_scroll_bounds`; clamp to `maxf(0.0, available_height)`; layout test asserts scroll height ≤ panel top − gap. **Done.** + +3. ~~**GdUnit CI exit 101 (orphans)** — Rewrote **`quest_hud_controller_test.gd`** with `MockHttpTransport` / `auto_free()` on all nodes; full suite **247 tests, 0 orphans, exit 0**. **Done.** + ## Verification ```bash cd client +godot --headless --path . -s addons/gdUnit4/bin/GdUnitCmdTool.gd \ + --ignoreHeadlessMode \ + -a res://test +``` + +Full suite: **247** cases, **0** orphans (GdUnit exit **0**). + +```bash +# Focused NEO-122 / layout subset godot --headless --path . -s addons/gdUnit4/bin/GdUnitCmdTool.gd \ --ignoreHeadlessMode \ -a test/quest_progress_client_test.gd \ test/quest_definitions_client_test.gd \ + test/quest_hud_controller_test.gd \ test/craft_recipe_panel_test.gd \ test/prototype_economy_hud_section_test.gd ```