NEO-97: address code review — helpers, tests, doc fixes
Extract npc_combat_hud_helpers.gd for shared HUD formatters; hide telegraph at zero windup; surface runtime sync errors; npc_row last-snapshot fallback; expand GdUnit coverage; align E5.M2 module docs and strike review findings.pull/136/head
parent
39ec50efa2
commit
fd0b6d3db8
|
|
@ -45,6 +45,7 @@ const CooldownStateScript := preload("res://scripts/cooldown_state.gd")
|
|||
const NpcRuntimeClientScript := preload("res://scripts/npc_runtime_client.gd")
|
||||
const PlayerCombatHealthClientScript := preload("res://scripts/player_combat_health_client.gd")
|
||||
const NpcRuntimeHudStateScript := preload("res://scripts/npc_runtime_hud_state.gd")
|
||||
const NpcCombatHudHelpers := preload("res://scripts/npc_combat_hud_helpers.gd")
|
||||
const COMBAT_POLL_INTERVAL_SEC: float = 1.0
|
||||
const COOLDOWN_REASON_ON_CD := "on_cooldown"
|
||||
const DEV_BOOTSTRAP_CAST_ABILITY_ID := "prototype_pulse"
|
||||
|
|
@ -87,6 +88,7 @@ var _npc_runtime_client: Node = null
|
|||
var _player_combat_health_client: Node = null
|
||||
var _npc_runtime_hud_state: RefCounted = null
|
||||
var _npc_runtime_snapshot: Dictionary = {}
|
||||
var _npc_runtime_sync_error: String = ""
|
||||
var _player_combat_health_snapshot: Dictionary = {}
|
||||
var _player_combat_health_error: String = ""
|
||||
var _cooldown_state: RefCounted = null
|
||||
|
|
@ -1066,35 +1068,16 @@ func _dev_player_id() -> String:
|
|||
return "dev-local-1"
|
||||
|
||||
|
||||
func _is_known_npc_id(target_id: String) -> bool:
|
||||
return PrototypeTargetConstants.ANCHORS.has(target_id.strip_edges())
|
||||
|
||||
|
||||
func _is_combat_active() -> bool:
|
||||
var lock_id := _locked_target_id_from_state(_last_target_state)
|
||||
if _is_known_npc_id(lock_id):
|
||||
return true
|
||||
return not _incoming_aggro_row().is_empty()
|
||||
|
||||
|
||||
func _incoming_aggro_row() -> Dictionary:
|
||||
if _npc_runtime_snapshot.is_empty():
|
||||
return {}
|
||||
return NpcRuntimeClientScript.row_for_aggro_holder(_dev_player_id(), _npc_runtime_snapshot)
|
||||
return NpcCombatHudHelpers.is_combat_active(
|
||||
_last_target_state, _npc_runtime_snapshot, _dev_player_id()
|
||||
)
|
||||
|
||||
|
||||
func _resolve_hud_npc_rows() -> Dictionary:
|
||||
var lock_id := _locked_target_id_from_state(_last_target_state)
|
||||
var locked_row: Dictionary = {}
|
||||
if _is_known_npc_id(lock_id):
|
||||
locked_row = NpcRuntimeClientScript.npc_row(lock_id, _npc_runtime_snapshot)
|
||||
var incoming_row: Dictionary = {}
|
||||
var aggro_row: Dictionary = _incoming_aggro_row()
|
||||
if not aggro_row.is_empty():
|
||||
var aggro_id: String = str(aggro_row.get("npcInstanceId", "")).strip_edges()
|
||||
if aggro_id != lock_id:
|
||||
incoming_row = aggro_row
|
||||
return {"locked": locked_row, "incoming": incoming_row}
|
||||
return NpcCombatHudHelpers.resolve_hud_npc_rows(
|
||||
_last_target_state, _npc_runtime_snapshot, _dev_player_id()
|
||||
)
|
||||
|
||||
|
||||
func _request_npc_combat_sync() -> void:
|
||||
|
|
@ -1126,6 +1109,7 @@ func _on_combat_poll_timeout() -> void:
|
|||
|
||||
|
||||
func _on_npc_runtime_received(snapshot: Dictionary) -> void:
|
||||
_npc_runtime_sync_error = ""
|
||||
_npc_runtime_snapshot = snapshot.duplicate(true)
|
||||
if _npc_runtime_hud_state != null and _npc_runtime_hud_state.has_method("apply_snapshot"):
|
||||
_npc_runtime_hud_state.call("apply_snapshot", _npc_runtime_snapshot)
|
||||
|
|
@ -1133,7 +1117,8 @@ func _on_npc_runtime_received(snapshot: Dictionary) -> void:
|
|||
_render_npc_combat_hud_labels()
|
||||
|
||||
|
||||
func _on_npc_runtime_sync_failed(_reason: String) -> void:
|
||||
func _on_npc_runtime_sync_failed(reason: String) -> void:
|
||||
_npc_runtime_sync_error = reason
|
||||
_render_npc_combat_hud_labels()
|
||||
|
||||
|
||||
|
|
@ -1171,75 +1156,25 @@ func _render_npc_combat_hud_labels() -> void:
|
|||
|
||||
|
||||
func _has_interpolated_telegraph_display() -> bool:
|
||||
if _npc_runtime_hud_state == null:
|
||||
return false
|
||||
var rows: Dictionary = _resolve_hud_npc_rows()
|
||||
for key: String in ["locked", "incoming"]:
|
||||
var row: Dictionary = rows.get(key, {}) as Dictionary
|
||||
if row.is_empty():
|
||||
continue
|
||||
var instance_id: String = str(row.get("npcInstanceId", "")).strip_edges()
|
||||
if instance_id.is_empty():
|
||||
continue
|
||||
var telegraph: Variant = row.get("activeTelegraph", null)
|
||||
if telegraph == null or not telegraph is Dictionary:
|
||||
continue
|
||||
if _npc_runtime_hud_state.has_method("has_active_windup"):
|
||||
if bool(_npc_runtime_hud_state.call("has_active_windup", instance_id)):
|
||||
return true
|
||||
return false
|
||||
return NpcCombatHudHelpers.has_interpolated_telegraph_display(
|
||||
_resolve_hud_npc_rows(), _npc_runtime_hud_state
|
||||
)
|
||||
|
||||
|
||||
func _render_npc_state_label() -> void:
|
||||
if not is_instance_valid(_npc_state_label):
|
||||
return
|
||||
var rows: Dictionary = _resolve_hud_npc_rows()
|
||||
var lines: PackedStringArray = PackedStringArray()
|
||||
var locked_row: Dictionary = rows.get("locked", {}) as Dictionary
|
||||
if not locked_row.is_empty():
|
||||
var locked_id: String = str(locked_row.get("npcInstanceId", ""))
|
||||
var locked_name: String = PrototypeTargetConstants.display_short_name(locked_id)
|
||||
var locked_state: String = str(locked_row.get("state", "unknown"))
|
||||
lines.append("NPC state: %s → %s" % [locked_name, locked_state])
|
||||
var incoming_row: Dictionary = rows.get("incoming", {}) as Dictionary
|
||||
if not incoming_row.is_empty():
|
||||
var incoming_id: String = str(incoming_row.get("npcInstanceId", ""))
|
||||
var incoming_name: String = PrototypeTargetConstants.display_short_name(incoming_id)
|
||||
var incoming_state: String = str(incoming_row.get("state", "unknown"))
|
||||
lines.append("Incoming: %s → %s" % [incoming_name, incoming_state])
|
||||
if lines.is_empty():
|
||||
_npc_state_label.text = "NPC state: —"
|
||||
return
|
||||
_npc_state_label.text = "\n".join(lines)
|
||||
|
||||
|
||||
func _format_telegraph_line(row: Dictionary) -> String:
|
||||
var instance_id: String = str(row.get("npcInstanceId", "")).strip_edges()
|
||||
var telegraph: Variant = row.get("activeTelegraph", null)
|
||||
if telegraph == null or not telegraph is Dictionary:
|
||||
return ""
|
||||
var telegraph_dict: Dictionary = telegraph as Dictionary
|
||||
var short_name: String = PrototypeTargetConstants.display_short_name(instance_id)
|
||||
var telegraph_id: String = str(telegraph_dict.get("telegraphId", ""))
|
||||
var archetype: String = str(telegraph_dict.get("archetypeKind", ""))
|
||||
var remaining: float = float(telegraph_dict.get("windupRemainingSeconds", 0.0))
|
||||
if _npc_runtime_hud_state != null and _npc_runtime_hud_state.has_method("display_windup_remaining"):
|
||||
remaining = float(_npc_runtime_hud_state.call("display_windup_remaining", instance_id))
|
||||
return "Telegraph: %s %s · %.1fs (%s)" % [short_name, telegraph_id, remaining, archetype]
|
||||
_npc_state_label.text = NpcCombatHudHelpers.build_npc_state_label(
|
||||
_resolve_hud_npc_rows(), _npc_runtime_sync_error
|
||||
)
|
||||
|
||||
|
||||
func _render_telegraph_label() -> void:
|
||||
if not is_instance_valid(_telegraph_label):
|
||||
return
|
||||
var rows: Dictionary = _resolve_hud_npc_rows()
|
||||
var locked_row: Dictionary = rows.get("locked", {}) as Dictionary
|
||||
var line: String = _format_telegraph_line(locked_row)
|
||||
if line.is_empty():
|
||||
line = _format_telegraph_line(rows.get("incoming", {}) as Dictionary)
|
||||
if line.is_empty():
|
||||
_telegraph_label.text = "Telegraph: —"
|
||||
return
|
||||
_telegraph_label.text = line
|
||||
_telegraph_label.text = NpcCombatHudHelpers.build_telegraph_label(
|
||||
_resolve_hud_npc_rows(), _npc_runtime_hud_state, _npc_runtime_sync_error
|
||||
)
|
||||
|
||||
|
||||
func _on_move_rejected(reason_code: String) -> void:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,133 @@
|
|||
extends RefCounted
|
||||
|
||||
## NEO-97: shared combat-active gate + NPC/telegraph HUD copy for [code]main.gd[/code] and tests.
|
||||
|
||||
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
|
||||
const NpcRuntimeClient := preload("res://scripts/npc_runtime_client.gd")
|
||||
|
||||
|
||||
static func locked_target_id_from_state(state: Dictionary) -> String:
|
||||
var locked_variant: Variant = state.get("lockedTargetId", null)
|
||||
if locked_variant is String:
|
||||
return (locked_variant as String).strip_edges()
|
||||
return ""
|
||||
|
||||
|
||||
static func is_known_npc_id(target_id: String) -> bool:
|
||||
return PrototypeTargetConstants.ANCHORS.has(target_id.strip_edges())
|
||||
|
||||
|
||||
static func incoming_aggro_row(dev_player_id: String, npc_runtime_snapshot: Dictionary) -> Dictionary:
|
||||
if npc_runtime_snapshot.is_empty():
|
||||
return {}
|
||||
return NpcRuntimeClient.row_for_aggro_holder(dev_player_id, npc_runtime_snapshot)
|
||||
|
||||
|
||||
static func is_combat_active(
|
||||
last_target_state: Dictionary, npc_runtime_snapshot: Dictionary, dev_player_id: String
|
||||
) -> bool:
|
||||
var lock_id := locked_target_id_from_state(last_target_state)
|
||||
if is_known_npc_id(lock_id):
|
||||
return true
|
||||
return not incoming_aggro_row(dev_player_id, npc_runtime_snapshot).is_empty()
|
||||
|
||||
|
||||
static func resolve_hud_npc_rows(
|
||||
last_target_state: Dictionary, npc_runtime_snapshot: Dictionary, dev_player_id: String
|
||||
) -> Dictionary:
|
||||
var lock_id := locked_target_id_from_state(last_target_state)
|
||||
var locked_row: Dictionary = {}
|
||||
if is_known_npc_id(lock_id):
|
||||
locked_row = NpcRuntimeClient.npc_row(lock_id, npc_runtime_snapshot)
|
||||
var incoming_row: Dictionary = {}
|
||||
var aggro_row: Dictionary = incoming_aggro_row(dev_player_id, npc_runtime_snapshot)
|
||||
if not aggro_row.is_empty():
|
||||
var aggro_id: String = str(aggro_row.get("npcInstanceId", "")).strip_edges()
|
||||
if aggro_id != lock_id:
|
||||
incoming_row = aggro_row
|
||||
return {"locked": locked_row, "incoming": incoming_row}
|
||||
|
||||
|
||||
static func build_npc_state_label(
|
||||
rows: Dictionary, runtime_sync_error: String = ""
|
||||
) -> String:
|
||||
var lines: PackedStringArray = PackedStringArray()
|
||||
var locked_row: Dictionary = rows.get("locked", {}) as Dictionary
|
||||
if not locked_row.is_empty():
|
||||
var locked_id: String = str(locked_row.get("npcInstanceId", ""))
|
||||
lines.append(
|
||||
"NPC state: %s → %s"
|
||||
% [
|
||||
PrototypeTargetConstants.display_short_name(locked_id),
|
||||
str(locked_row.get("state", "unknown")),
|
||||
]
|
||||
)
|
||||
var incoming_row: Dictionary = rows.get("incoming", {}) as Dictionary
|
||||
if not incoming_row.is_empty():
|
||||
var incoming_id: String = str(incoming_row.get("npcInstanceId", ""))
|
||||
lines.append(
|
||||
"Incoming: %s → %s"
|
||||
% [
|
||||
PrototypeTargetConstants.display_short_name(incoming_id),
|
||||
str(incoming_row.get("state", "unknown")),
|
||||
]
|
||||
)
|
||||
if lines.is_empty():
|
||||
if not runtime_sync_error.is_empty():
|
||||
return "NPC state: — (%s)" % runtime_sync_error
|
||||
return "NPC state: —"
|
||||
if not runtime_sync_error.is_empty():
|
||||
lines.append("(runtime sync: %s)" % runtime_sync_error)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
static func format_telegraph_line(row: Dictionary, hud_state: RefCounted = null) -> String:
|
||||
var instance_id: String = str(row.get("npcInstanceId", "")).strip_edges()
|
||||
var telegraph: Variant = row.get("activeTelegraph", null)
|
||||
if telegraph == null or not telegraph is Dictionary:
|
||||
return ""
|
||||
var telegraph_dict: Dictionary = telegraph as Dictionary
|
||||
var remaining: float = float(telegraph_dict.get("windupRemainingSeconds", 0.0))
|
||||
if hud_state != null and hud_state.has_method("display_windup_remaining"):
|
||||
remaining = float(hud_state.call("display_windup_remaining", instance_id))
|
||||
if remaining <= 0.0:
|
||||
return ""
|
||||
var short_name: String = PrototypeTargetConstants.display_short_name(instance_id)
|
||||
var telegraph_id: String = str(telegraph_dict.get("telegraphId", ""))
|
||||
var archetype: String = str(telegraph_dict.get("archetypeKind", ""))
|
||||
return "Telegraph: %s %s · %.1fs (%s)" % [short_name, telegraph_id, remaining, archetype]
|
||||
|
||||
|
||||
static func build_telegraph_label(
|
||||
rows: Dictionary, hud_state: RefCounted = null, runtime_sync_error: String = ""
|
||||
) -> String:
|
||||
var locked_row: Dictionary = rows.get("locked", {}) as Dictionary
|
||||
var line: String = format_telegraph_line(locked_row, hud_state)
|
||||
if line.is_empty():
|
||||
line = format_telegraph_line(rows.get("incoming", {}) as Dictionary, hud_state)
|
||||
if line.is_empty():
|
||||
if not runtime_sync_error.is_empty():
|
||||
return "Telegraph: — (%s)" % runtime_sync_error
|
||||
return "Telegraph: —"
|
||||
return line
|
||||
|
||||
|
||||
static func has_interpolated_telegraph_display(
|
||||
rows: Dictionary, hud_state: RefCounted
|
||||
) -> bool:
|
||||
if hud_state == null:
|
||||
return false
|
||||
for key: String in ["locked", "incoming"]:
|
||||
var row: Dictionary = rows.get(key, {}) as Dictionary
|
||||
if row.is_empty():
|
||||
continue
|
||||
var instance_id: String = str(row.get("npcInstanceId", "")).strip_edges()
|
||||
if instance_id.is_empty():
|
||||
continue
|
||||
var telegraph: Variant = row.get("activeTelegraph", null)
|
||||
if telegraph == null or not telegraph is Dictionary:
|
||||
continue
|
||||
if hud_state.has_method("has_active_windup"):
|
||||
if bool(hud_state.call("has_active_windup", instance_id)):
|
||||
return true
|
||||
return false
|
||||
|
|
@ -0,0 +1 @@
|
|||
uid://bneo97hudhlpr1
|
||||
|
|
@ -53,7 +53,18 @@ static func parse_runtime_json(text: String) -> Variant:
|
|||
return root
|
||||
|
||||
|
||||
func npc_row(instance_id: String, snapshot: Dictionary = {}) -> Dictionary:
|
||||
var source: Dictionary = snapshot
|
||||
if source.is_empty():
|
||||
source = _last_snapshot
|
||||
return npc_row_in_snapshot(instance_id, source)
|
||||
|
||||
|
||||
static func npc_row(instance_id: String, snapshot: Dictionary = {}) -> Dictionary:
|
||||
return npc_row_in_snapshot(instance_id, snapshot)
|
||||
|
||||
|
||||
static func npc_row_in_snapshot(instance_id: String, snapshot: Dictionary) -> Dictionary:
|
||||
var source: Dictionary = snapshot
|
||||
if source.is_empty():
|
||||
return {}
|
||||
|
|
|
|||
|
|
@ -3,71 +3,8 @@ extends GdUnitTestSuite
|
|||
## NEO-97: combat-active gate + HUD label copy from snapshot fixtures.
|
||||
|
||||
const NpcRuntimeClient := preload("res://scripts/npc_runtime_client.gd")
|
||||
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
|
||||
|
||||
|
||||
class NpcCombatHudHarness:
|
||||
extends RefCounted
|
||||
var last_target_state: Dictionary = {}
|
||||
var npc_runtime_snapshot: Dictionary = {}
|
||||
var dev_player_id: String = "dev-local-1"
|
||||
|
||||
func is_combat_active() -> bool:
|
||||
var lock_id := locked_target_id()
|
||||
if PrototypeTargetConstants.ANCHORS.has(lock_id):
|
||||
return true
|
||||
return not incoming_aggro_row().is_empty()
|
||||
|
||||
func locked_target_id() -> String:
|
||||
var locked_variant: Variant = last_target_state.get("lockedTargetId", null)
|
||||
if locked_variant is String:
|
||||
return (locked_variant as String).strip_edges()
|
||||
return ""
|
||||
|
||||
func incoming_aggro_row() -> Dictionary:
|
||||
if npc_runtime_snapshot.is_empty():
|
||||
return {}
|
||||
return NpcRuntimeClient.row_for_aggro_holder(dev_player_id, npc_runtime_snapshot)
|
||||
|
||||
func resolve_hud_npc_rows() -> Dictionary:
|
||||
var lock_id := locked_target_id()
|
||||
var locked_row: Dictionary = {}
|
||||
if PrototypeTargetConstants.ANCHORS.has(lock_id):
|
||||
locked_row = NpcRuntimeClient.npc_row(lock_id, npc_runtime_snapshot)
|
||||
var incoming_row: Dictionary = {}
|
||||
var aggro_row: Dictionary = incoming_aggro_row()
|
||||
if not aggro_row.is_empty():
|
||||
var aggro_id: String = str(aggro_row.get("npcInstanceId", "")).strip_edges()
|
||||
if aggro_id != lock_id:
|
||||
incoming_row = aggro_row
|
||||
return {"locked": locked_row, "incoming": incoming_row}
|
||||
|
||||
func build_npc_state_label() -> String:
|
||||
var rows: Dictionary = resolve_hud_npc_rows()
|
||||
var lines: PackedStringArray = PackedStringArray()
|
||||
var locked_row: Dictionary = rows.get("locked", {}) as Dictionary
|
||||
if not locked_row.is_empty():
|
||||
var locked_id: String = str(locked_row.get("npcInstanceId", ""))
|
||||
lines.append(
|
||||
"NPC state: %s → %s"
|
||||
% [
|
||||
PrototypeTargetConstants.display_short_name(locked_id),
|
||||
str(locked_row.get("state", "unknown")),
|
||||
]
|
||||
)
|
||||
var incoming_row: Dictionary = rows.get("incoming", {}) as Dictionary
|
||||
if not incoming_row.is_empty():
|
||||
var incoming_id: String = str(incoming_row.get("npcInstanceId", ""))
|
||||
lines.append(
|
||||
"Incoming: %s → %s"
|
||||
% [
|
||||
PrototypeTargetConstants.display_short_name(incoming_id),
|
||||
str(incoming_row.get("state", "unknown")),
|
||||
]
|
||||
)
|
||||
if lines.is_empty():
|
||||
return "NPC state: —"
|
||||
return "\n".join(lines)
|
||||
const NpcRuntimeHudState := preload("res://scripts/npc_runtime_hud_state.gd")
|
||||
const NpcCombatHudHelpers := preload("res://scripts/npc_combat_hud_helpers.gd")
|
||||
|
||||
|
||||
static func _elite_aggro_snapshot_json() -> String:
|
||||
|
|
@ -81,45 +18,131 @@ static func _elite_aggro_snapshot_json() -> String:
|
|||
)
|
||||
|
||||
|
||||
static func _elite_telegraph_snapshot_json() -> String:
|
||||
return (
|
||||
'{"schemaVersion":1,"serverTimeUtc":"2026-05-29T12:00:00Z","npcInstances":['
|
||||
+ '{"npcInstanceId":"prototype_npc_elite","behaviorDefId":"prototype_elite_boss",'
|
||||
+ '"state":"telegraph_windup","aggroHolderPlayerId":"dev-local-1",'
|
||||
+ '"activeTelegraph":{"telegraphId":"prototype_elite_slam","npcInstanceId":'
|
||||
+ '"prototype_npc_elite","windupRemainingSeconds":2.5,"archetypeKind":"elite"}},'
|
||||
+ '{"npcInstanceId":"prototype_npc_melee","behaviorDefId":"prototype_melee_rush",'
|
||||
+ '"state":"idle","aggroHolderPlayerId":null,"activeTelegraph":null}'
|
||||
+ "]}"
|
||||
)
|
||||
|
||||
|
||||
func test_npc_lock_makes_combat_active_without_aggro_snapshot() -> void:
|
||||
# Arrange
|
||||
var harness := NpcCombatHudHarness.new()
|
||||
harness.last_target_state = {"lockedTargetId": "prototype_npc_melee", "validity": "ok"}
|
||||
var target_state := {"lockedTargetId": "prototype_npc_melee", "validity": "ok"}
|
||||
# Act
|
||||
var active: bool = harness.is_combat_active()
|
||||
var active: bool = NpcCombatHudHelpers.is_combat_active(target_state, {}, "dev-local-1")
|
||||
# Assert
|
||||
assert_that(active).is_true()
|
||||
|
||||
|
||||
func test_aggro_holder_makes_combat_active_without_lock() -> void:
|
||||
# Arrange
|
||||
var harness := NpcCombatHudHarness.new()
|
||||
var parsed: Variant = NpcRuntimeClient.parse_runtime_json(_elite_aggro_snapshot_json())
|
||||
harness.npc_runtime_snapshot = parsed as Dictionary
|
||||
# Act
|
||||
var active: bool = harness.is_combat_active()
|
||||
var active: bool = NpcCombatHudHelpers.is_combat_active({}, parsed as Dictionary, "dev-local-1")
|
||||
# Assert
|
||||
assert_that(active).is_true()
|
||||
|
||||
|
||||
func test_idle_without_lock_is_not_combat_active() -> void:
|
||||
# Arrange
|
||||
var harness := NpcCombatHudHarness.new()
|
||||
harness.npc_runtime_snapshot = {}
|
||||
# Act
|
||||
var active: bool = harness.is_combat_active()
|
||||
var active: bool = NpcCombatHudHelpers.is_combat_active({}, {}, "dev-local-1")
|
||||
# Assert
|
||||
assert_that(active).is_false()
|
||||
|
||||
|
||||
func test_combat_poll_gate_stops_when_idle() -> void:
|
||||
# Arrange
|
||||
var timer := Timer.new()
|
||||
timer.one_shot = false
|
||||
add_child(timer)
|
||||
timer.start()
|
||||
# Act
|
||||
var should_poll: bool = NpcCombatHudHelpers.is_combat_active({}, {}, "dev-local-1")
|
||||
if not should_poll:
|
||||
timer.stop()
|
||||
# Assert
|
||||
assert_that(should_poll).is_false()
|
||||
assert_that(timer.is_stopped()).is_true()
|
||||
|
||||
|
||||
func test_combat_poll_gate_starts_when_lock_held() -> void:
|
||||
# Arrange
|
||||
var timer := Timer.new()
|
||||
timer.one_shot = false
|
||||
add_child(timer)
|
||||
var target_state := {"lockedTargetId": "prototype_npc_elite", "validity": "ok"}
|
||||
# Act
|
||||
var should_poll: bool = NpcCombatHudHelpers.is_combat_active(target_state, {}, "dev-local-1")
|
||||
if should_poll and timer.is_stopped():
|
||||
timer.start()
|
||||
# Assert
|
||||
assert_that(should_poll).is_true()
|
||||
assert_that(timer.is_stopped()).is_false()
|
||||
|
||||
|
||||
func test_locked_plus_incoming_state_label() -> void:
|
||||
# Arrange
|
||||
var harness := NpcCombatHudHarness.new()
|
||||
harness.last_target_state = {"lockedTargetId": "prototype_npc_melee", "validity": "ok"}
|
||||
var target_state := {"lockedTargetId": "prototype_npc_melee", "validity": "ok"}
|
||||
var parsed: Variant = NpcRuntimeClient.parse_runtime_json(_elite_aggro_snapshot_json())
|
||||
harness.npc_runtime_snapshot = parsed as Dictionary
|
||||
var rows: Dictionary = NpcCombatHudHelpers.resolve_hud_npc_rows(
|
||||
target_state, parsed as Dictionary, "dev-local-1"
|
||||
)
|
||||
# Act
|
||||
var label: String = harness.build_npc_state_label()
|
||||
var label: String = NpcCombatHudHelpers.build_npc_state_label(rows)
|
||||
# Assert
|
||||
assert_that(label).contains("NPC state: Melee → idle")
|
||||
assert_that(label).contains("Incoming: Elite → aggro")
|
||||
|
||||
|
||||
func test_telegraph_label_falls_back_to_incoming_row() -> void:
|
||||
# Arrange
|
||||
var target_state := {"lockedTargetId": "prototype_npc_melee", "validity": "ok"}
|
||||
var parsed: Variant = NpcRuntimeClient.parse_runtime_json(_elite_telegraph_snapshot_json())
|
||||
var rows: Dictionary = NpcCombatHudHelpers.resolve_hud_npc_rows(
|
||||
target_state, parsed as Dictionary, "dev-local-1"
|
||||
)
|
||||
# Act
|
||||
var label: String = NpcCombatHudHelpers.build_telegraph_label(rows)
|
||||
# Assert
|
||||
assert_that(label).contains("Telegraph: Elite prototype_elite_slam")
|
||||
assert_that(label).contains("(elite)")
|
||||
|
||||
|
||||
func test_telegraph_label_empty_when_local_windup_expired() -> void:
|
||||
# Arrange
|
||||
var short_windup_json := (
|
||||
'{"schemaVersion":1,"serverTimeUtc":"2026-05-29T12:00:00Z","npcInstances":['
|
||||
+ '{"npcInstanceId":"prototype_npc_elite","behaviorDefId":"prototype_elite_boss",'
|
||||
+ '"state":"telegraph_windup","aggroHolderPlayerId":"dev-local-1",'
|
||||
+ '"activeTelegraph":{"telegraphId":"prototype_elite_slam","npcInstanceId":'
|
||||
+ '"prototype_npc_elite","windupRemainingSeconds":0.1,"archetypeKind":"elite"}}'
|
||||
+ "]}"
|
||||
)
|
||||
var parsed: Variant = NpcRuntimeClient.parse_runtime_json(short_windup_json)
|
||||
var rows: Dictionary = NpcCombatHudHelpers.resolve_hud_npc_rows(
|
||||
{"lockedTargetId": "prototype_npc_elite", "validity": "ok"},
|
||||
parsed as Dictionary,
|
||||
"dev-local-1"
|
||||
)
|
||||
var hud_state: RefCounted = NpcRuntimeHudState.new()
|
||||
hud_state.call("apply_snapshot", parsed as Dictionary)
|
||||
await (Engine.get_main_loop() as SceneTree).create_timer(0.15).timeout
|
||||
# Act
|
||||
var label: String = NpcCombatHudHelpers.build_telegraph_label(rows, hud_state)
|
||||
# Assert
|
||||
assert_that(label).is_equal("Telegraph: —")
|
||||
|
||||
|
||||
func test_runtime_sync_error_surfaces_on_empty_npc_state_label() -> void:
|
||||
# Arrange
|
||||
# Act
|
||||
var label: String = NpcCombatHudHelpers.build_npc_state_label({}, "HTTP 500")
|
||||
# Assert
|
||||
assert_that(label).is_equal("NPC state: — (HTTP 500)")
|
||||
|
|
|
|||
|
|
@ -104,6 +104,20 @@ func test_row_for_aggro_holder_returns_elite_row() -> void:
|
|||
assert_that(str(row.get("npcInstanceId", ""))).is_equal("prototype_npc_elite")
|
||||
|
||||
|
||||
func test_npc_row_falls_back_to_last_snapshot_on_instance() -> void:
|
||||
# Arrange
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.body_json = _runtime_snapshot_json()
|
||||
var c := _make_client(transport)
|
||||
var got: Array = []
|
||||
c.connect("runtime_received", func(snapshot: Dictionary) -> void: got.append(snapshot))
|
||||
c.call("request_sync_from_server")
|
||||
# Act
|
||||
var row: Dictionary = c.call("npc_row", "prototype_npc_elite") as Dictionary
|
||||
# Assert
|
||||
assert_that(str(row.get("state", ""))).is_equal("telegraph_windup")
|
||||
|
||||
|
||||
func test_invalid_schema_emits_sync_failed() -> void:
|
||||
# Arrange
|
||||
var transport := MockHttpTransport.new()
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@ The **first shipped three-archetype NPC spine** under `content/npc-behaviors/*.j
|
|||
|
||||
**NPC behavior definition registry (NEO-89):** **`INpcBehaviorDefinitionRegistry`** in `server/NeonSprawl.Server/Game/Npc/` wraps the startup-loaded catalog for read-only lookup by stable behavior `id` (`TryGetDefinition`), trim/lowercase validation (`TryNormalizeKnown`), and ordered enumeration (`GetDefinitionsInIdOrder`). Stable prototype id constants: **`PrototypeNpcBehaviorRegistry`**. **NEO-91+** (instance registry, runtime tick) should inject the interface rather than `NpcBehaviorDefinitionCatalog`. Plan: [NEO-89 implementation plan](../../plans/NEO-89-implementation-plan.md).
|
||||
|
||||
**NPC instance registry (NEO-91):** **`PrototypeNpcRegistry`** in `server/NeonSprawl.Server/Game/Npc/` — instance ids **`prototype_npc_melee`**, **`prototype_npc_ranged`**, **`prototype_npc_elite`** with behavior bindings + world anchors; **`ICombatEntityHealthStore`** uses catalog **`maxHp`** per instance. Plan: [NEO-91 implementation plan](../../plans/NEO-91-implementation-plan.md). **Breaking:** Godot constants migrate in [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97).
|
||||
**NPC instance registry (NEO-91):** **`PrototypeNpcRegistry`** in `server/NeonSprawl.Server/Game/Npc/` — instance ids **`prototype_npc_melee`**, **`prototype_npc_ranged`**, **`prototype_npc_elite`** with behavior bindings + world anchors; **`ICombatEntityHealthStore`** uses catalog **`maxHp`** per instance. Plan: [NEO-91 implementation plan](../../plans/NEO-91-implementation-plan.md). Godot **`prototype_target_constants.gd`** migration **landed in NEO-91**; [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) adds HUD poll clients only.
|
||||
|
||||
**Threat / aggro store (NEO-92):** **`IThreatStateStore`** + **`AggroOperations`** in `server/NeonSprawl.Server/Game/Npc/` — first damaging cast acquires holder; leash break clears holder (move + cast hooks). HTTP projection: **`aggroHolderPlayerId`** on **`GET /game/world/npc-runtime-snapshot`** ([NEO-94](../../plans/NEO-94-implementation-plan.md)). Plan: [NEO-92 implementation plan](../../plans/NEO-92-implementation-plan.md).
|
||||
|
||||
|
|
@ -99,7 +99,7 @@ The **first shipped three-archetype NPC spine** under `content/npc-behaviors/*.j
|
|||
|
||||
## Risks and telemetry
|
||||
|
||||
- Telegraph desync: single **`npc-runtime-snapshot`** poll surface; no client windup math.
|
||||
- Telegraph desync: single **`npc-runtime-snapshot`** poll surface; client display-only windup tick-down between polls (reconciled on each GET via **`npc_runtime_hud_state.gd`**).
|
||||
- Aggro confusion: deterministic first-hit + leash rules; instrument `npc_state_transition` when E9.M1 lands.
|
||||
|
||||
## Source anchors
|
||||
|
|
|
|||
|
|
@ -56,10 +56,10 @@
|
|||
|
||||
## Implementation reconciliation (shipped)
|
||||
|
||||
- **`npc_runtime_client.gd`**, **`player_combat_health_client.gd`**, **`npc_runtime_hud_state.gd`** — poll + parse + windup interpolation.
|
||||
- **`npc_runtime_client.gd`**, **`player_combat_health_client.gd`**, **`npc_runtime_hud_state.gd`**, **`npc_combat_hud_helpers.gd`** — poll + parse + windup interpolation + shared HUD formatters.
|
||||
- **`main.gd`** — combat-active ~1 Hz poll timer; **`TelegraphLabel`**, **`NpcStateLabel`**, **`PlayerCombatHpLabel`** render helpers (locked + incoming threat rows).
|
||||
- **`main.tscn`** — three HUD labels; downstream label offsets shifted.
|
||||
- GdUnit: **`npc_runtime_client_test.gd`**, **`player_combat_health_client_test.gd`**, **`npc_runtime_hud_state_test.gd`**, **`npc_combat_hud_refresh_test.gd`**.
|
||||
- GdUnit: **`npc_runtime_client_test.gd`**, **`player_combat_health_client_test.gd`**, **`npc_runtime_hud_state_test.gd`**, **`npc_combat_hud_refresh_test.gd`**; shared formatters in **`npc_combat_hud_helpers.gd`**.
|
||||
- **`docs/manual-qa/NEO-97.md`**, **`client/README.md`**, E5M2 backlog + module + alignment register updated.
|
||||
- **`prototype_target_constants.gd`** / **`prototype_target_markers.gd`** — verified on `main` (NEO-91); no rename to `npc_archetype_markers.gd`.
|
||||
|
||||
|
|
@ -88,7 +88,7 @@
|
|||
|
||||
### Local windup mirror (`npc_runtime_hud_state.gd`)
|
||||
|
||||
- On **`runtime_received`**: store **`serverTimeUtc`** anchor + receive tick (same pattern as **`cooldown_state.gd`**).
|
||||
- On **`runtime_received`**: store per-row **`windupRemainingSeconds`** at receive tick (no `serverTimeUtc` anchor — display smoothing only).
|
||||
- For each row with nested **`activeTelegraph`**: record **`windupRemainingSeconds`** at receive time.
|
||||
- **`display_windup_remaining(instance_id) -> float`**: anchor value minus elapsed local seconds since receive (floor at 0).
|
||||
- Reconcile on every poll — no client windup *scheduling*; only display smoothing between polls.
|
||||
|
|
@ -147,31 +147,36 @@ Use server snake_case **`state`** as-is for prototype debug readability (`idle`,
|
|||
| `client/scripts/player_combat_health_client.gd.uid` | Godot uid companion. |
|
||||
| `client/scripts/npc_runtime_hud_state.gd` | Snapshot cache + windup tick-down between polls. |
|
||||
| `client/scripts/npc_runtime_hud_state.gd.uid` | Godot uid companion. |
|
||||
| `client/scripts/npc_combat_hud_helpers.gd` | Shared combat-active gate + NPC/telegraph label formatters. |
|
||||
| `client/scripts/npc_combat_hud_helpers.gd.uid` | Godot uid companion. |
|
||||
| `client/test/npc_runtime_client_test.gd` | HTTP double: URL, parse, row lookup, aggro-holder helper. |
|
||||
| `client/test/player_combat_health_client_test.gd` | HTTP double: URL, parse, schema guard. |
|
||||
| `client/test/npc_runtime_hud_state_test.gd` | Windup interpolation + reconcile on new snapshot. |
|
||||
| `client/test/npc_combat_hud_refresh_test.gd` | Combat-active gate + label render harness (mock clients + snapshot fixtures). |
|
||||
| `client/test/npc_combat_hud_refresh_test.gd` | Combat-active gate + timer mirror; telegraph fallback/expiry; sync error copy. |
|
||||
| `docs/manual-qa/NEO-97.md` | Godot manual QA — lock elite, cast, observe telegraph countdown + player HP drop. |
|
||||
|
||||
## Files to modify
|
||||
|
||||
| Path | Rationale |
|
||||
|------|-----------|
|
||||
| `client/scripts/main.gd` | Wire npc-runtime + combat-health clients, combat poll timer, HUD render helpers, combat-active gate. |
|
||||
| `client/scripts/main.gd` | Wire npc-runtime + combat-health clients, combat poll timer, HUD render via helpers. |
|
||||
| `client/scenes/main.tscn` | Add `TelegraphLabel`, `NpcStateLabel`, `PlayerCombatHpLabel`; adjust downstream label offsets. |
|
||||
| `client/README.md` | NEO-97 NPC/telegraph HUD section; fix stale alpha/beta targeting/combat copy. |
|
||||
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-11 acceptance checkboxes + landed note when complete. |
|
||||
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | NEO-97 client landed note under Related implementation slices. |
|
||||
| `client/scripts/npc_runtime_client.gd` | Instance **`npc_row`** falls back to **`_last_snapshot`**. |
|
||||
| `client/scripts/npc_combat_hud_helpers.gd` | Code-review follow-up: shared HUD helpers extracted from `main.gd`. |
|
||||
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | Risks + NPC instance row aligned with NEO-97 kickoff. |
|
||||
| `docs/reviews/2026-05-29-NEO-97.md` | Strike-through review suggestions when fixed. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M2 alignment register — E5M2-11 complete when shipped. |
|
||||
|
||||
## Tests
|
||||
|
||||
| File | Coverage |
|
||||
|------|----------|
|
||||
| `client/test/npc_runtime_client_test.gd` | GET URL; **`parse_runtime_json`** schema v1; **`npc_row`**; **`row_for_aggro_holder`**. |
|
||||
| `client/test/npc_runtime_client_test.gd` | GET URL; parse; **`row_for_aggro_holder`**; instance **`npc_row`** **`_last_snapshot`** fallback. |
|
||||
| `client/test/player_combat_health_client_test.gd` | GET URL with player id; parse success/fail; **`defeated`** flag. |
|
||||
| `client/test/npc_runtime_hud_state_test.gd` | Apply snapshot → interpolated windup decreases; new poll reconciles to server value. |
|
||||
| `client/test/npc_combat_hud_refresh_test.gd` | Combat-active gate starts/stops timer; locked + incoming label copy from fixture snapshots. |
|
||||
| `client/test/npc_combat_hud_refresh_test.gd` | Combat-active gate + timer mirror; telegraph fallback/expiry; sync error on labels. |
|
||||
| *(existing unchanged)* | **`prototype_target_constants_test.gd`**, **`target_selection_client_test.gd`**, **`prototype_target_markers_test.gd`** — three-NPC tab/marker behavior already covered. |
|
||||
|
||||
## Open questions / risks
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ NEO-97 adds the Epic 5 Slice 2 **client half**: thin HTTP clients for **`GET /ga
|
|||
|------|--------|
|
||||
| `docs/plans/NEO-97-implementation-plan.md` | **Matches** — kickoff decisions (combat-active gate, locked + incoming HUD, interpolate windup, keep `prototype_target_markers.gd`) reflected in code; reconciliation section accurate; AC checked. |
|
||||
| `docs/plans/E5M2-prototype-backlog.md` (E5M2-11) | **Matches** — AC checked, landed note present; script name in backlog still says `npc_archetype_markers.gd` but plan kickoff explicitly adopted `prototype_target_markers.gd` (documented divergence). |
|
||||
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Partially matches** — NEO-97 landed line under Related implementation slices is correct; **Risks** bullet still says “no client windup math” (conflicts with kickoff interpolate decision); NPC instance row still says “Breaking: Godot constants migrate in NEO-97” though constants landed in NEO-91. |
|
||||
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — NEO-97 landed line correct; Risks + NPC instance row updated for windup display + NEO-91 constants migration. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row includes **NEO-97 landed** with README/plan/manual QA links. |
|
||||
| `docs/manual-qa/NEO-97.md` | **Matches** — Godot steps cover lock, cast, telegraph countdown, player HP drop, incoming threat row, regressions. |
|
||||
| `client/README.md` | **Matches** — NPC runtime + telegraph HUD section documents poll gate, labels, tab order, script paths. |
|
||||
|
|
@ -30,27 +30,27 @@ None.
|
|||
|
||||
## Suggestions
|
||||
|
||||
1. **E5.M2 Risks bullet** — Update “Telegraph desync: … no client windup math” to “display-only windup tick-down between polls; reconciled on each `npc-runtime-snapshot` GET” so module docs align with the NEO-97 kickoff decision and `npc_runtime_hud_state.gd`.
|
||||
1. ~~**E5.M2 Risks bullet** — Update “Telegraph desync: … no client windup math” to “display-only windup tick-down between polls; reconciled on each `npc-runtime-snapshot` GET” so module docs align with the NEO-97 kickoff decision and `npc_runtime_hud_state.gd`.~~ **Done.**
|
||||
|
||||
2. **E5.M2 NPC instance row** — Change “Breaking: Godot constants migrate in NEO-97” to note migration **landed in NEO-91**; NEO-97 adds HUD poll clients only.
|
||||
2. ~~**E5.M2 NPC instance row** — Change “Breaking: Godot constants migrate in NEO-97” to note migration **landed in NEO-91**; NEO-97 adds HUD poll clients only.~~ **Done.**
|
||||
|
||||
3. **Test coverage vs plan** — `npc_combat_hud_refresh_test.gd` covers combat-active bool + `NpcStateLabel` copy but not **`TelegraphLabel`** formatting or **timer start/stop** (called out in the implementation plan test table). Add at least one telegraph line assertion (locked vs incoming fallback) and, if feasible without heavy scene wiring, a timer-gate test mirroring `_update_combat_poll_gate()`.
|
||||
3. ~~**Test coverage vs plan** — `npc_combat_hud_refresh_test.gd` covers combat-active bool + `NpcStateLabel` copy but not **`TelegraphLabel`** formatting or **timer start/stop** (called out in the implementation plan test table). Add at least one telegraph line assertion (locked vs incoming fallback) and, if feasible without heavy scene wiring, a timer-gate test mirroring `_update_combat_poll_gate()`.~~ **Done.** Telegraph fallback + local windup expiry + timer mirror tests added.
|
||||
|
||||
4. **Shared HUD helpers** — Combat-active gate, row resolution, and label formatters are duplicated in `main.gd` (~180 lines) and `NpcCombatHudHarness` in tests. Extract to a small RefCounted (e.g. `npc_combat_hud_helpers.gd`) used by both to prevent drift; optional follow-up given NEO-85 precedent of wiring in `main.gd`.
|
||||
4. ~~**Shared HUD helpers** — Combat-active gate, row resolution, and label formatters are duplicated in `main.gd` (~180 lines) and `NpcCombatHudHarness` in tests. Extract to a small RefCounted (e.g. `npc_combat_hud_helpers.gd`) used by both to prevent drift; optional follow-up given NEO-85 precedent of wiring in `main.gd`.~~ **Done.** `npc_combat_hud_helpers.gd` shared by `main.gd` and tests.
|
||||
|
||||
5. **Telegraph at zero** — `_format_telegraph_line` can show **`· 0.0s`** while the snapshot row still carries `activeTelegraph` until the next poll. Consider returning empty when `display_windup_remaining` ≤ 0 so `TelegraphLabel` shows **`—`** between local expiry and server reconcile.
|
||||
5. ~~**Telegraph at zero** — `_format_telegraph_line` can show **`· 0.0s`** while the snapshot row still carries `activeTelegraph` until the next poll. Consider returning empty when `display_windup_remaining` ≤ 0 so `TelegraphLabel` shows **`—`** between local expiry and server reconcile.~~ **Done.** `format_telegraph_line` returns empty when remaining ≤ 0.
|
||||
|
||||
6. **Runtime sync failure** — `_on_npc_runtime_sync_failed` re-renders from the last good snapshot without surfacing failure on `NpcStateLabel` / `TelegraphLabel` (player HP already shows error). Acceptable for prototype; optionally dash NPC/telegraph lines or append a sync-failed hint for QA clarity.
|
||||
6. ~~**Runtime sync failure** — `_on_npc_runtime_sync_failed` re-renders from the last good snapshot without surfacing failure on `NpcStateLabel` / `TelegraphLabel` (player HP already shows error). Acceptable for prototype; optionally dash NPC/telegraph lines or append a sync-failed hint for QA clarity.~~ **Done.** `_npc_runtime_sync_error` appended to NPC/telegraph labels when empty or alongside stale rows.
|
||||
|
||||
## Nits
|
||||
|
||||
- Nit: `npc_runtime_client.gd` stores `_last_snapshot` but static `npc_row()` does not fall back to it (unlike `combat_targets_client.target_row()`); field is write-only today.
|
||||
- ~~Nit: `npc_runtime_client.gd` stores `_last_snapshot` but static `npc_row()` does not fall back to it (unlike `combat_targets_client.target_row()`); field is write-only today.~~ **Done.** Instance `npc_row()` falls back to `_last_snapshot`.
|
||||
|
||||
- Nit: Duplicate `MockHttpTransport` inner classes in `npc_runtime_client_test.gd` and `player_combat_health_client_test.gd` — fine for now; a shared test helper would reduce copy-paste.
|
||||
|
||||
- Nit: `main.gd` grows ~250 lines for NEO-97; repo policy prefers thin `main.gd` — track for a future extract when the next HUD story lands.
|
||||
- Nit: `main.gd` grows ~250 lines for NEO-97; repo policy prefers thin `main.gd` — track for a future extract when the next HUD story lands. **Partially addressed** — helpers extracted; poll wiring remains in `main.gd`.
|
||||
|
||||
- Nit: Plan text references `serverTimeUtc` anchor in `npc_runtime_hud_state.gd`; implementation uses receive tick + server `windupRemainingSeconds` only — sufficient for display smoothing; plan wording could be tightened.
|
||||
- ~~Nit: Plan text references `serverTimeUtc` anchor in `npc_runtime_hud_state.gd`; implementation uses receive tick + server `windupRemainingSeconds` only — sufficient for display smoothing; plan wording could be tightened.~~ **Done.**
|
||||
|
||||
## Verification
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue