NEO-122: Fix bugbot items and GdUnit orphan leaks.
Block Shift+Q when progress sync failed; remove HudRootScroll 80px min that overlapped the economy panel; rewrite quest_hud_controller tests with HTTP mocks and auto_free for CI exit 0.pull/161/head
parent
3606d0ba57
commit
46e24d35d6
|
|
@ -85,7 +85,7 @@ func _apply_hud_root_scroll_bounds() -> void:
|
||||||
return
|
return
|
||||||
var available_height: float = position.y - HUD_TOP_MARGIN - HUD_ECONOMY_GAP
|
var available_height: float = position.y - HUD_TOP_MARGIN - HUD_ECONOMY_GAP
|
||||||
hud_scroll.position = Vector2(SIDE_MARGIN, HUD_TOP_MARGIN)
|
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:
|
func _measure_header_height() -> float:
|
||||||
|
|
|
||||||
|
|
@ -268,6 +268,9 @@ func _request_accept(quest_id: String) -> bool:
|
||||||
|
|
||||||
|
|
||||||
func _request_eligible_accept() -> void:
|
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():
|
if _progress_error.is_empty() and _last_snapshot.is_empty():
|
||||||
_render_accept_feedback_label("Quest accept: progress loading — try again")
|
_render_accept_feedback_label("Quest accept: progress loading — try again")
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -95,6 +95,7 @@ func test_viewport_layout_clamps_hud_root_scroll() -> void:
|
||||||
# Arrange
|
# Arrange
|
||||||
await _set_test_viewport_size()
|
await _set_test_viewport_size()
|
||||||
var canvas := Control.new()
|
var canvas := Control.new()
|
||||||
|
auto_free(canvas)
|
||||||
canvas.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT)
|
canvas.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT)
|
||||||
var hud_scroll := ScrollContainer.new()
|
var hud_scroll := ScrollContainer.new()
|
||||||
hud_scroll.name = "HudRootScroll"
|
hud_scroll.name = "HudRootScroll"
|
||||||
|
|
@ -110,4 +111,4 @@ func test_viewport_layout_clamps_hud_root_scroll() -> void:
|
||||||
# Assert
|
# Assert
|
||||||
var panel_top: float = section.call("panel_top_y")
|
var panel_top: float = section.call("panel_top_y")
|
||||||
assert_that(panel_top).is_greater(0.0)
|
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)
|
||||||
|
|
|
||||||
|
|
@ -7,20 +7,104 @@ const QuestProgressClient := preload("res://scripts/quest_progress_client.gd")
|
||||||
const QuestDefinitionsClient := preload("res://scripts/quest_definitions_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 controller: Node = QuestHudController.new()
|
||||||
var progress: Node = QuestProgressClient.new()
|
var progress: Node = _make_progress_client(sync_transport, accept_transport)
|
||||||
var defs: Node = QuestDefinitionsClient.new()
|
var defs: Node = _make_defs_client(defs_transport)
|
||||||
var progress_label := Label.new()
|
var progress_label := Label.new()
|
||||||
var accept_label := Label.new()
|
var accept_label := Label.new()
|
||||||
|
auto_free(controller)
|
||||||
|
auto_free(progress_label)
|
||||||
|
auto_free(accept_label)
|
||||||
add_child(controller)
|
add_child(controller)
|
||||||
add_child(progress)
|
await get_tree().process_frame
|
||||||
add_child(defs)
|
|
||||||
controller.call(
|
controller.call(
|
||||||
"setup", progress, defs, progress_label, accept_label, Callable(self, "_noop_http_config")
|
"setup", progress, defs, progress_label, accept_label, Callable(self, "_noop_http_config")
|
||||||
)
|
)
|
||||||
await get_tree().process_frame
|
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:
|
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:
|
func test_try_accept_key_input_handles_q_key() -> void:
|
||||||
# Arrange
|
# Arrange
|
||||||
var controller := await _build_controller()
|
var built: Dictionary = await _build_controller(
|
||||||
|
NoopHttpTransport.new(), PendingAcceptHttpTransport.new(), NoopHttpTransport.new()
|
||||||
|
)
|
||||||
var event := InputEventKey.new()
|
var event := InputEventKey.new()
|
||||||
event.pressed = true
|
event.pressed = true
|
||||||
event.keycode = KEY_Q
|
event.keycode = KEY_Q
|
||||||
# Act
|
# 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
|
||||||
assert_bool(handled).is_true()
|
assert_bool(handled).is_true()
|
||||||
|
|
||||||
|
|
||||||
func test_progress_label_shows_loading_before_snapshot() -> void:
|
func test_progress_label_shows_loading_before_snapshot() -> void:
|
||||||
# Arrange
|
# Arrange
|
||||||
var controller: Node = QuestHudController.new()
|
var built: Dictionary = await _build_controller(
|
||||||
var progress: Node = QuestProgressClient.new()
|
NoopHttpTransport.new(), NoopHttpTransport.new(), NoopHttpTransport.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")
|
|
||||||
)
|
)
|
||||||
await get_tree().process_frame
|
|
||||||
# Act
|
# Act
|
||||||
var text: String = progress_label.text
|
var text: String = str(built["progress_label"].text)
|
||||||
# Assert
|
# Assert
|
||||||
assert_str(text).contains("Quests:")
|
assert_str(text).contains("Quests:")
|
||||||
assert_str(text).contains("Loading")
|
assert_str(text).contains("Loading")
|
||||||
|
|
@ -64,9 +141,15 @@ func test_shift_q_shows_progress_loading_before_snapshot() -> void:
|
||||||
# Arrange
|
# Arrange
|
||||||
var controller: Node = QuestHudController.new()
|
var controller: Node = QuestHudController.new()
|
||||||
var accept_label := Label.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)
|
add_child(controller)
|
||||||
controller.set("_progress_client", QuestProgressClient.new())
|
controller.set("_progress_client", progress_client)
|
||||||
controller.set("_defs_client", QuestDefinitionsClient.new())
|
controller.set("_defs_client", defs_client)
|
||||||
controller.set("_accept_label", accept_label)
|
controller.set("_accept_label", accept_label)
|
||||||
controller.set("_last_snapshot", {})
|
controller.set("_last_snapshot", {})
|
||||||
controller.set("_progress_error", "")
|
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")
|
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:
|
func test_request_accept_shows_busy_feedback() -> void:
|
||||||
# Arrange
|
# Arrange
|
||||||
var controller := await _build_controller()
|
var built: Dictionary = await _build_controller(
|
||||||
var progress: Node = controller.get("_progress_client")
|
NoopHttpTransport.new(), PendingAcceptHttpTransport.new(), NoopHttpTransport.new()
|
||||||
progress.call("request_accept", QuestHudController.GATHER_QUEST_ID)
|
)
|
||||||
var accept_label: Label = controller.get("_accept_label")
|
var controller: Node = built["controller"]
|
||||||
|
var accept_label: Label = built["accept_label"]
|
||||||
|
controller.call("_request_accept", QuestHudController.GATHER_QUEST_ID)
|
||||||
# Act
|
# 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
|
||||||
assert_bool(started).is_false()
|
assert_bool(started).is_false()
|
||||||
assert_str(accept_label.text).contains("busy")
|
assert_str(accept_label.text).contains("busy")
|
||||||
|
|
|
||||||
|
|
@ -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.
|
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
|
## Verification
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
cd client
|
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 \
|
godot --headless --path . -s addons/gdUnit4/bin/GdUnitCmdTool.gd \
|
||||||
--ignoreHeadlessMode \
|
--ignoreHeadlessMode \
|
||||||
-a test/quest_progress_client_test.gd \
|
-a test/quest_progress_client_test.gd \
|
||||||
test/quest_definitions_client_test.gd \
|
test/quest_definitions_client_test.gd \
|
||||||
|
test/quest_hud_controller_test.gd \
|
||||||
test/craft_recipe_panel_test.gd \
|
test/craft_recipe_panel_test.gd \
|
||||||
test/prototype_economy_hud_section_test.gd
|
test/prototype_economy_hud_section_test.gd
|
||||||
```
|
```
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue