Merge pull request #51 from ViPro-Technologies/NEO-26-selectionevent-surface-debug-consumer
NEO-26: SelectionEvent surface on TargetSelectionClientpull/53/head
GitHub
commit
f9d730749b
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
description: Commits at agent discretion; push only when user asks; gh pr create with NEO-* title when opening PRs; PR text without tool boilerplate
|
||||
description: Commits at agent discretion; push only when user asks; GitHub MCP create_pull_request with NEO-* title when opening PRs; PR text without tool boilerplate
|
||||
alwaysApply: true
|
||||
---
|
||||
|
||||
|
|
@ -25,12 +25,14 @@ alwaysApply: true
|
|||
|
||||
When the user asks to **open**, **create**, or **file** a pull request (or clearly wants one opened for the current branch):
|
||||
|
||||
1. **Use the GitHub CLI** — Run **`gh pr create`** (not only a browser **`/pull/new/...`** link). That way you control the **title** and avoid GitHub’s default, which title-cases the branch name and turns **`NEO-25`** into **`Neo 25`**.
|
||||
2. **Title** — **`--title`** must **begin** with the **Linear issue key** in canonical form: **`NEO-123:`** (see [linear-git-naming](linear-git-naming.md): uppercase team prefix, hyphen, issue number), then a short human summary. Example: **`NEO-25: InteractableDescriptor list / fetch-driven client`**.
|
||||
3. **Infer the key** — In order: the **current branch**’s leading segment (e.g. `NEO-25-my-slug` → `NEO-25`); the matching **`docs/plans/{KEY}-implementation-plan.md`** on the branch; Linear MCP / issue in chat; or **ask** the user if ambiguous.
|
||||
4. **Body** — Pass **`--body`** or **`--body-file`** with scope, how to verify (`dotnet test`, manual QA path, etc.), and pointers to `docs/plans/` when useful.
|
||||
5. **Upstream** — If **`gh pr create`** fails because the branch is not on `origin`, push first (**only** as allowed under **Never push** above—i.e. the same user message asked to open the PR or explicitly to push).
|
||||
6. **Fallback** — If **`gh`** is missing or not authenticated (`gh auth status`), tell the user and optionally still give the **`pull/new/...`** URL, with an explicit reminder to **edit the title** so it starts with **`NEO-123:`** (not `Neo 123`).
|
||||
1. **Use the GitHub MCP** — Call **`create_pull_request`** on the **`user-github`** MCP server (read the tool descriptor under the workspace MCP folder first, same as other MCP tools). Pass **`owner`**, **`repo`**, **`head`**, **`base`**, **`title`**, and **`body`**. You set the **title** explicitly—do not rely only on GitHub’s **Compare & pull request** default, which title-cases the branch and can turn **`NEO-25`** into **`Neo 25`**.
|
||||
2. **Repository** — Set **`owner`** and **`repo`** from **`git remote get-url origin`** (e.g. `git@github.com:ViPro-Technologies/neon-sprawl.git` → `ViPro-Technologies`, `neon-sprawl`). For this repo they are almost always **`ViPro-Technologies`** / **`neon-sprawl`**.
|
||||
3. **Branches** — **`head`** is the branch that contains the work (e.g. `NEO-26-selectionevent-surface-debug-consumer`); **`base`** is usually **`main`** unless the user names another target.
|
||||
4. **Title** — Must **begin** with the **Linear issue key** in canonical form: **`NEO-123:`** (see [linear-git-naming](linear-git-naming.md): uppercase team prefix, hyphen, issue number), then a short human summary. Example: **`NEO-25: InteractableDescriptor list / fetch-driven client`**.
|
||||
5. **Infer the key** — In order: the **current branch**’s leading segment (e.g. `NEO-25-my-slug` → `NEO-25`); the matching **`docs/plans/{KEY}-implementation-plan.md`** on the branch; Linear MCP / issue in chat; or **ask** the user if ambiguous.
|
||||
6. **Body** — Pass **`body`** with scope, how to verify (`dotnet test`, manual QA path, etc.), and pointers to `docs/plans/` when useful. When building JSON for the MCP tool, use **real newlines** in markdown strings (not literal `\n` escape sequences), per MCP guidance for this server.
|
||||
7. **Upstream** — If **`create_pull_request`** fails because **`head`** is not on **`origin`**, push first (**only** as allowed under **Never push** above—i.e. the same user message asked to open the PR or explicitly to push).
|
||||
8. **Fallback** — If the GitHub MCP is unavailable or the call fails after push, give **`https://github.com/{owner}/{repo}/pull/new/{head}`** and tell the user to set the PR title to **`NEO-123:`** … . They may use **`gh pr create`** locally if they prefer the CLI.
|
||||
|
||||
## Commit message format when a Linear issue applies
|
||||
|
||||
|
|
|
|||
|
|
@ -17,4 +17,4 @@ When a change mixes documentation and code, treat it as a **code change** (use a
|
|||
|
||||
After the PR for a story branch is **merged**, clean up locally per [story-end](story-end.md) (`checkout main`, `pull`, `branch -d`).
|
||||
|
||||
When the user asks the agent to **open a PR**, use **`gh pr create`** with a **`NEO-*:`** title prefix per [commit-and-review](commit-and-review.md) **Opening a PR when the user asks**—not only a GitHub **Compare & pull request** link (the web UI mangles issue-key casing in the default title).
|
||||
When the user asks the agent to **open a PR**, use the **GitHub MCP** **`create_pull_request`** tool with a **`NEO-*:`** title prefix per [commit-and-review](commit-and-review.md) **Opening a PR when the user asks**—not only a GitHub **Compare & pull request** link (the web UI mangles issue-key casing in the default title).
|
||||
|
|
|
|||
|
|
@ -32,4 +32,4 @@ When suggesting or creating a branch for **NEO-5**, use something like **`NEO-5-
|
|||
## Agent behavior
|
||||
|
||||
- When creating a **branch** or **commit** for tracked work, **infer the id** from the current story (e.g. user says NEO-5), the branch already in use, or the Linear issue fetched via MCP (`plugin-linear-linear`, e.g. `get_issue` / `list_issues`).
|
||||
- When the user asks the agent to **open a PR**, the PR **title** must start with that same **`NEO-*:`** form (use **`gh pr create --title`** per [commit-and-review](commit-and-review.md)); do not ship a default web-only title that lowercases the key to **`Neo`**.
|
||||
- When the user asks the agent to **open a PR**, the PR **title** must start with that same **`NEO-*:`** form (set **`title`** on the GitHub MCP **`create_pull_request`** call per [commit-and-review](commit-and-review.md)); do not ship a default web-only title that lowercases the key to **`Neo`**.
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ When the user **ends story work** on a ticketed branch—phrases like “end sto
|
|||
|
||||
## 2a. When `main` is push-protected
|
||||
|
||||
If the user asks to **push** changes that sit on **`main`** and **`git push origin main` fails** (required PR / status checks), create a branch from current `main` (e.g. `chore/…`), **`git push -u origin`** that branch, then open the PR with **`gh pr create`** and a title keyed per [commit-and-review](commit-and-review.md) **Opening a PR when the user asks** (or give the **`pull/new/...`** URL **and** remind them to fix the title to **`NEO-123:`**). **`git reset --hard origin/main`** if you had temporarily advanced local `main` so local `main` matches remote until the PR merges.
|
||||
If the user asks to **push** changes that sit on **`main`** and **`git push origin main` fails** (required PR / status checks), create a branch from current `main` (e.g. `chore/…`), **`git push -u origin`** that branch, then open the PR with the **GitHub MCP** **`create_pull_request`** tool and a title keyed per [commit-and-review](commit-and-review.md) **Opening a PR when the user asks** (or give the **`pull/new/...`** URL **and** remind them to fix the title to **`NEO-123:`**). **`git reset --hard origin/main`** if you had temporarily advanced local `main` so local `main` matches remote until the PR merges.
|
||||
|
||||
## 3. What not to do
|
||||
|
||||
|
|
@ -58,4 +58,4 @@ Then give any **git** details (branch deleted, `main` ahead, etc.) **after** the
|
|||
- [story-kickoff](story-kickoff.md) — start of story workflow
|
||||
- [git-workflow](git-workflow.md) — branch vs `main`
|
||||
- [linear-git-naming](linear-git-naming.md) — branch names
|
||||
- [commit-and-review](commit-and-review.md) — push and **`gh pr create`** only when the user requests it; PR title **`NEO-*:`** prefix
|
||||
- [commit-and-review](commit-and-review.md) — push and **GitHub MCP `create_pull_request`** only when the user requests it; PR title **`NEO-*:`** prefix
|
||||
|
|
|
|||
|
|
@ -67,4 +67,4 @@ These three lists (**files to add**, **files to modify**, **tests**) must **alwa
|
|||
|
||||
When the story is **done** and merged to `main`, follow [story-end](story-end.md) to return to `main`, pull, and remove the local story branch.
|
||||
|
||||
When the user asks to **open or create a PR** for this story, follow [commit-and-review](commit-and-review.md) **Opening a PR when the user asks**: use **`gh pr create`** with a title that **starts with the Linear key** (e.g. **`NEO-25:`**), not GitHub’s default title from the web UI alone.
|
||||
When the user asks to **open or create a PR** for this story, follow [commit-and-review](commit-and-review.md) **Opening a PR when the user asks**: use the **GitHub MCP** **`create_pull_request`** tool with a title that **starts with the Linear key** (e.g. **`NEO-25:`**), not GitHub’s default title from the web UI alone.
|
||||
|
|
|
|||
|
|
@ -7,6 +7,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or
|
|||
| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer |
|
||||
| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention |
|
||||
|
||||
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use **`gh pr create`** with a title starting **`NEO-123:`** (same rule file). **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
|
||||
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)). **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
|
||||
|
||||
**Commits tied to a Linear issue** must start the subject with the **issue id** and a colon (e.g. `NEO-8: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/linear-git-naming.md`](.cursor/rules/linear-git-naming.md).
|
||||
|
|
|
|||
|
|
@ -107,6 +107,15 @@ Full checklist: [`docs/manual-qa/NEO-25.md`](../manual-qa/NEO-25.md).
|
|||
- **Soft lock refresh:** the client does **not** poll on a timer. It refreshes via `GET …/target` on boot, on every `POST …/target/select` response (body already carries `targetState`), and — while a lock is currently held — on **`authoritative_ack`** from `PositionAuthorityClient`, throttled to at most one GET per **250 ms**. `authoritative_ack` fires on every server-confirmed position (boot `GET` 200 **and** successful `move-stream` 200), so a held lock revalidates during normal WASD locomotion without re-introducing snap rubber-banding. Boot is naturally skipped because no lock exists yet. Purely server-driven state flips (future PvP / combat categories) would require revisiting this policy; today NEO-23 has none.
|
||||
- **Inspector:** match **`base_url`** / **`dev_player_id`** on the `TargetSelectionClient` node to the other HTTP clients / `Game:DevPlayerId`.
|
||||
|
||||
### SelectionEvent (NEO-26)
|
||||
|
||||
- **`TargetSelectionClient`** emits **`selection_event(event: Dictionary)`** only when the server-acknowledged **`lockedTargetId`** changes (not on **`validity`-only** updates; those stay on **`target_state_changed`**).
|
||||
- Payload keys: **`previous`** ([String] or `null`), **`next`** (same), **`cause`** (`tab` \| `clear` \| `server_correction`), **`sequence`** (int from the new state). **`POST …/target/select`** from **`request_select_target_id`** (including the path used by Tab) uses cause **`tab`**; **`request_clear_target`** uses **`clear`**; every **`GET …/target`** completion uses **`server_correction`** (including boot sync).
|
||||
- **Dev logging:** enable **`log_selection_events`** on the `TargetSelectionClient` node to mirror each event to the Godot **Output** (`print`).
|
||||
- **E1.M4+:** connect to **`$TargetSelectionClient.selection_event`** (or your scene’s path to that node) without reshaping the payload.
|
||||
|
||||
Full checklist: [`docs/manual-qa/NEO-26.md`](../docs/manual-qa/NEO-26.md).
|
||||
|
||||
### Manual check (NEO-24)
|
||||
|
||||
1. Run the server and client as in NEO-7 / NEO-9; confirm spawn snap at `(-5, 0.9, -5)`.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
extends Node
|
||||
|
||||
## NEO-24 (E1M3-02): client tab-target + lock UI synced to server (NEO-23 `TargetState` v1).
|
||||
## NEO-26 (E1M3-04): emits [signal selection_event] when server-acknowledged
|
||||
## [code]lockedTargetId[/code] changes (see [member log_selection_events]).
|
||||
##
|
||||
## Flow:
|
||||
## - `request_sync_from_server()` → `GET …/target` returns `PlayerTargetStateResponse` v1.
|
||||
|
|
@ -19,6 +21,13 @@ extends Node
|
|||
|
||||
signal target_state_changed(state: Dictionary)
|
||||
|
||||
## NEO-26: fired only when normalized [code]lockedTargetId[/code] changes after a GET/POST.
|
||||
## Payload: [code]previous[/code] ([String] or [code]null[/code]), [code]next[/code] (same),
|
||||
## [code]cause[/code] ([code]"tab"[/code] | [code]"clear"[/code] |
|
||||
## [code]"server_correction"[/code]), [code]sequence[/code] ([int], from new state).
|
||||
## E1.M4+ may [code]connect[/code] here without reshaping.
|
||||
signal selection_event(event: Dictionary)
|
||||
|
||||
enum Phase { IDLE, GET, POST_SELECT }
|
||||
|
||||
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
|
||||
|
|
@ -30,6 +39,10 @@ const _REFRESH_COOLDOWN_MSEC: int = 250
|
|||
@export var base_url: String = "http://127.0.0.1:5253"
|
||||
@export var dev_player_id: String = "dev-local-1"
|
||||
|
||||
## When true, each [signal selection_event] is also printed to the Godot output
|
||||
## (dev aid only).
|
||||
@export var log_selection_events: bool = false
|
||||
|
||||
var _http: Node
|
||||
var _busy: bool = false
|
||||
var _phase: Phase = Phase.IDLE
|
||||
|
|
@ -54,6 +67,11 @@ var _state: Dictionary = {}
|
|||
## `-_REFRESH_COOLDOWN_MSEC` so the first snap is never blocked by the window on boot.
|
||||
var _last_refresh_msec: int = -_REFRESH_COOLDOWN_MSEC
|
||||
|
||||
## Cause for the in-flight HTTP request (GET → [code]server_correction[/code];
|
||||
## POST → [code]tab[/code] / [code]clear[/code]).
|
||||
## Set immediately before a request starts; cleared when the response is applied or dropped.
|
||||
var _http_selection_cause: String = ""
|
||||
|
||||
|
||||
func _create_http_request() -> Node:
|
||||
return HTTPRequest.new()
|
||||
|
|
@ -81,6 +99,7 @@ func _input(event: InputEvent) -> void:
|
|||
func request_sync_from_server() -> void:
|
||||
if _busy:
|
||||
return
|
||||
_http_selection_cause = "server_correction"
|
||||
_busy = true
|
||||
_phase = Phase.GET
|
||||
_last_refresh_msec = Time.get_ticks_msec()
|
||||
|
|
@ -90,6 +109,7 @@ func request_sync_from_server() -> void:
|
|||
push_warning("TargetSelectionClient: GET failed to start (%s)" % err)
|
||||
_busy = false
|
||||
_phase = Phase.IDLE
|
||||
_http_selection_cause = ""
|
||||
|
||||
|
||||
func request_tab_next() -> void:
|
||||
|
|
@ -117,12 +137,12 @@ func request_select_target_id(target_id: String) -> void:
|
|||
return
|
||||
var payload: Dictionary = {"schemaVersion": 1, "targetId": trimmed}
|
||||
_maybe_attach_position_hint(payload)
|
||||
_post_select(payload)
|
||||
_post_select(payload, "tab")
|
||||
|
||||
|
||||
func request_clear_target() -> void:
|
||||
# NEO-23: omit `targetId` to clear. JSON `null` also works; omission keeps the body minimal.
|
||||
_post_select({"schemaVersion": 1})
|
||||
_post_select({"schemaVersion": 1}, "clear")
|
||||
|
||||
|
||||
## NEO-24 follow-up #5: attach the live capsule position as an advisory `positionHint` so the
|
||||
|
|
@ -172,9 +192,10 @@ func _has_lock() -> bool:
|
|||
return v is String and not (v as String).is_empty()
|
||||
|
||||
|
||||
func _post_select(payload: Dictionary) -> void:
|
||||
func _post_select(payload: Dictionary, selection_cause: String) -> void:
|
||||
if _busy:
|
||||
return
|
||||
_http_selection_cause = selection_cause
|
||||
_kick_freshness_stream()
|
||||
_busy = true
|
||||
_phase = Phase.POST_SELECT
|
||||
|
|
@ -186,6 +207,7 @@ func _post_select(payload: Dictionary) -> void:
|
|||
push_warning("TargetSelectionClient: POST failed to start (%s)" % err)
|
||||
_busy = false
|
||||
_phase = Phase.IDLE
|
||||
_http_selection_cause = ""
|
||||
|
||||
|
||||
## Nudges the authority with the current capsule position so the server's stored snapshot
|
||||
|
|
@ -220,6 +242,7 @@ func _on_request_completed(
|
|||
|
||||
if _result != HTTPRequest.RESULT_SUCCESS:
|
||||
push_warning("TargetSelectionClient: HTTP failed (result=%s)" % _result)
|
||||
_http_selection_cause = ""
|
||||
return
|
||||
|
||||
var text := body.get_string_from_utf8()
|
||||
|
|
@ -227,6 +250,7 @@ func _on_request_completed(
|
|||
if not parsed is Dictionary:
|
||||
if response_code != 200:
|
||||
push_warning("TargetSelectionClient: HTTP %s non-JSON body" % response_code)
|
||||
_http_selection_cause = ""
|
||||
return
|
||||
var data: Dictionary = parsed
|
||||
|
||||
|
|
@ -234,37 +258,51 @@ func _on_request_completed(
|
|||
Phase.GET:
|
||||
if response_code == 200:
|
||||
_update_state_from_get(data)
|
||||
else:
|
||||
_http_selection_cause = ""
|
||||
Phase.POST_SELECT:
|
||||
# Server includes authoritative `targetState` on 200 (apply and denial). 400 / 404
|
||||
# bodies may lack it — swallow quietly; `_state` keeps last-good.
|
||||
if response_code == 200:
|
||||
_update_state_from_post(data)
|
||||
else:
|
||||
_http_selection_cause = ""
|
||||
_:
|
||||
pass
|
||||
_http_selection_cause = ""
|
||||
|
||||
|
||||
func _update_state_from_get(data: Dictionary) -> void:
|
||||
var state := _extract_target_state_fields(data)
|
||||
var cause := _http_selection_cause
|
||||
_http_selection_cause = ""
|
||||
if state.is_empty():
|
||||
return
|
||||
var prev: Dictionary = _state.duplicate()
|
||||
_state = state
|
||||
target_state_changed.emit(_state.duplicate())
|
||||
_maybe_emit_selection_event(prev, state, cause)
|
||||
|
||||
|
||||
func _update_state_from_post(data: Dictionary) -> void:
|
||||
var target_state_variant: Variant = data.get("targetState", null)
|
||||
if not target_state_variant is Dictionary:
|
||||
_http_selection_cause = ""
|
||||
push_warning("TargetSelectionClient: POST 200 missing targetState")
|
||||
return
|
||||
var state := _extract_target_state_fields(target_state_variant as Dictionary)
|
||||
if state.is_empty():
|
||||
_http_selection_cause = ""
|
||||
return
|
||||
var cause := _http_selection_cause
|
||||
_http_selection_cause = ""
|
||||
state["selectionApplied"] = bool(data.get("selectionApplied", false))
|
||||
var reason_variant: Variant = data.get("reasonCode", null)
|
||||
if reason_variant is String and not (reason_variant as String).is_empty():
|
||||
state["reasonCode"] = reason_variant as String
|
||||
var prev: Dictionary = _state.duplicate()
|
||||
_state = state
|
||||
target_state_changed.emit(_state.duplicate())
|
||||
_maybe_emit_selection_event(prev, state, cause)
|
||||
|
||||
|
||||
## Shape-only parse of the v1 target state payload (works for GET body and POST echo).
|
||||
|
|
@ -285,3 +323,32 @@ func _extract_target_state_fields(data: Dictionary) -> Dictionary:
|
|||
out["validity"] = (validity_variant as String) if validity_variant is String else "none"
|
||||
out["sequence"] = int(data.get("sequence", 0))
|
||||
return out
|
||||
|
||||
|
||||
func _normalized_lock_id(state: Dictionary) -> Variant:
|
||||
if state.is_empty():
|
||||
return null
|
||||
var v: Variant = state.get("lockedTargetId", null)
|
||||
if v is String and not (v as String).is_empty():
|
||||
return v as String
|
||||
return null
|
||||
|
||||
|
||||
func _maybe_emit_selection_event(
|
||||
prev_state: Dictionary, new_state: Dictionary, cause: String
|
||||
) -> void:
|
||||
var prev_id: Variant = _normalized_lock_id(prev_state)
|
||||
var next_id: Variant = _normalized_lock_id(new_state)
|
||||
if prev_id == next_id:
|
||||
return
|
||||
if cause.is_empty():
|
||||
return
|
||||
var ev: Dictionary = {
|
||||
"previous": prev_id,
|
||||
"next": next_id,
|
||||
"cause": cause,
|
||||
"sequence": int(new_state.get("sequence", 0)),
|
||||
}
|
||||
selection_event.emit(ev)
|
||||
if log_selection_events:
|
||||
print("[TargetSelectionClient] SelectionEvent ", ev)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,157 @@
|
|||
extends GdUnitTestSuite
|
||||
|
||||
## NEO-26: `TargetSelectionClient.selection_event` — cause strings and lock-id-only rule.
|
||||
|
||||
const TargetSelectionTestDouble := preload("res://test/target_selection_test_double.gd")
|
||||
|
||||
const ALPHA_ID := "prototype_target_alpha"
|
||||
const BETA_ID := "prototype_target_beta"
|
||||
|
||||
|
||||
class MockHttpTransport:
|
||||
extends Node
|
||||
signal request_completed(
|
||||
result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray
|
||||
)
|
||||
var _queue: Array[Dictionary] = []
|
||||
|
||||
func enqueue(result: int, code: int, body: String) -> void:
|
||||
_queue.append({"result": result, "code": code, "body": body})
|
||||
|
||||
func request(
|
||||
_url: String,
|
||||
_custom_headers: PackedStringArray = PackedStringArray(),
|
||||
_method: HTTPClient.Method = HTTPClient.METHOD_GET,
|
||||
_request_data: String = ""
|
||||
) -> Error:
|
||||
if _queue.is_empty():
|
||||
return ERR_UNAVAILABLE
|
||||
var r: Dictionary = _queue.pop_front()
|
||||
var body_bytes: PackedByteArray = str(r["body"]).to_utf8_buffer()
|
||||
request_completed.emit(r["result"], r["code"], PackedStringArray(), body_bytes)
|
||||
return OK
|
||||
|
||||
|
||||
func _make_client(http_transport: Node) -> Node:
|
||||
var c: Node = TargetSelectionTestDouble.new()
|
||||
c.set("injected_http", http_transport)
|
||||
auto_free(http_transport)
|
||||
auto_free(c)
|
||||
add_child(c)
|
||||
return c
|
||||
|
||||
|
||||
func _target_state_json(locked_id_or_null: Variant, validity: String, sequence: int = 0) -> String:
|
||||
var locked := "null"
|
||||
if locked_id_or_null is String and not (locked_id_or_null as String).is_empty():
|
||||
locked = '"%s"' % (locked_id_or_null as String)
|
||||
return (
|
||||
'{"schemaVersion":1,"playerId":"dev-local-1","lockedTargetId":%s,' % locked
|
||||
+ '"validity":"%s","sequence":%d}' % [validity, sequence]
|
||||
)
|
||||
|
||||
|
||||
func _select_response_json(
|
||||
applied: bool,
|
||||
locked_id_or_null: Variant,
|
||||
validity: String,
|
||||
sequence: int = 0,
|
||||
reason: String = ""
|
||||
) -> String:
|
||||
var target_state := _target_state_json(locked_id_or_null, validity, sequence)
|
||||
if applied:
|
||||
return '{"schemaVersion":1,"selectionApplied":true,"targetState":%s}' % target_state
|
||||
var reason_segment := ""
|
||||
if not reason.is_empty():
|
||||
reason_segment = ',"reasonCode":"%s"' % reason
|
||||
return (
|
||||
'{"schemaVersion":1,"selectionApplied":false%s,"targetState":%s}'
|
||||
% [reason_segment, target_state]
|
||||
)
|
||||
|
||||
|
||||
func test_tab_lock_change_emits_tab() -> void:
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
|
||||
)
|
||||
var c := _make_client(transport)
|
||||
var events: Array = []
|
||||
c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate()))
|
||||
c.request_tab_next()
|
||||
assert_that(events.size()).is_equal(1)
|
||||
var last: Dictionary = events[0] as Dictionary
|
||||
assert_that(last.get("cause", "")).is_equal("tab")
|
||||
assert_that(last.get("previous")).is_null()
|
||||
assert_that(last.get("next")).is_equal(ALPHA_ID)
|
||||
assert_that(int(last.get("sequence", -1))).is_equal(1)
|
||||
|
||||
|
||||
func test_clear_lock_change_emits_clear() -> void:
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
|
||||
)
|
||||
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, null, "none", 2))
|
||||
var c := _make_client(transport)
|
||||
var events: Array = []
|
||||
c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate()))
|
||||
c.request_tab_next()
|
||||
c.request_clear_target()
|
||||
assert_that(events.size()).is_equal(2)
|
||||
assert_that(int((events[0] as Dictionary).get("sequence", -1))).is_equal(1)
|
||||
assert_that((events[1] as Dictionary).get("cause", "")).is_equal("clear")
|
||||
assert_that((events[1] as Dictionary).get("previous")).is_equal(ALPHA_ID)
|
||||
assert_that((events[1] as Dictionary).get("next")).is_null()
|
||||
assert_that(int((events[1] as Dictionary).get("sequence", -1))).is_equal(2)
|
||||
|
||||
|
||||
func test_get_lock_id_change_emits_server_correction() -> void:
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
|
||||
)
|
||||
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(BETA_ID, "ok", 2))
|
||||
var c := _make_client(transport)
|
||||
var events: Array = []
|
||||
c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate()))
|
||||
c.request_tab_next()
|
||||
c.request_sync_from_server()
|
||||
assert_that(events.size()).is_equal(2)
|
||||
var last: Dictionary = events[1] as Dictionary
|
||||
assert_that(last.get("cause", "")).is_equal("server_correction")
|
||||
assert_that(last.get("previous")).is_equal(ALPHA_ID)
|
||||
assert_that(last.get("next")).is_equal(BETA_ID)
|
||||
assert_that(int(last.get("sequence", -1))).is_equal(2)
|
||||
|
||||
|
||||
func test_get_validity_only_change_emits_nothing() -> void:
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
|
||||
)
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1)
|
||||
)
|
||||
var c := _make_client(transport)
|
||||
var events: Array = []
|
||||
c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate()))
|
||||
c.request_tab_next()
|
||||
c.request_sync_from_server()
|
||||
assert_that(events.size()).is_equal(1)
|
||||
assert_that((events[0] as Dictionary).get("cause", "")).is_equal("tab")
|
||||
assert_that(int((events[0] as Dictionary).get("sequence", -1))).is_equal(1)
|
||||
|
||||
|
||||
func test_denial_without_lock_id_change_emits_nothing() -> void:
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.enqueue(
|
||||
HTTPRequest.RESULT_SUCCESS,
|
||||
200,
|
||||
_select_response_json(false, null, "none", 0, "out_of_range")
|
||||
)
|
||||
var c := _make_client(transport)
|
||||
var events: Array = []
|
||||
c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate()))
|
||||
c.request_select_target_id(ALPHA_ID)
|
||||
assert_that(events.size()).is_equal(0)
|
||||
|
|
@ -0,0 +1 @@
|
|||
uid://ksj15nsjl6ph
|
||||
|
|
@ -100,7 +100,7 @@ flowchart LR
|
|||
|
||||
## Implementation snapshot
|
||||
|
||||
- **`TargetState` (prototype JSON v1 — NEO-23):** server **`GET /game/players/{id}/target`** and **`POST /game/players/{id}/target/select`** with versioned **`PlayerTargetStateResponse`** / **`TargetSelectRequest`** / **`TargetSelectResponse`**; stub registry + in-memory lock under `server/NeonSprawl.Server/Game/Targeting/`; horizontal reach + soft lock semantics — [NEO-23](../../plans/NEO-23-implementation-plan.md), [server README — Targeting](../../../server/README.md#targeting-neo-23). Godot tab-target: [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24). **`InteractableDescriptor` list:** **`GET /game/world/interactables`** (NEO-25). **Still open on later issues:** **`SelectionEvent`**. **Story backlog:** [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md).
|
||||
- **`TargetState` (prototype JSON v1 — NEO-23):** server **`GET /game/players/{id}/target`** and **`POST /game/players/{id}/target/select`** with versioned **`PlayerTargetStateResponse`** / **`TargetSelectRequest`** / **`TargetSelectResponse`**; stub registry + in-memory lock under `server/NeonSprawl.Server/Game/Targeting/`; horizontal reach + soft lock semantics — [NEO-23](../../plans/NEO-23-implementation-plan.md), [server README — Targeting](../../../server/README.md#targeting-neo-23). Godot tab-target: [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24). **`InteractableDescriptor` list:** **`GET /game/world/interactables`** (NEO-25). **`SelectionEvent` (prototype):** Godot **`TargetSelectionClient.selection_event`** when **`lockedTargetId`** changes — [NEO-26](../../plans/NEO-26-implementation-plan.md). **Story backlog:** [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md).
|
||||
- **Precursor under E1.M1 (NEO-9 + NEO-25):** server **`POST /game/players/{id}/interact`** with **`InteractionRequest`** / **`InteractionResponse`**; **`GET /game/world/interactables`** (versioned descriptor list); **`HorizontalReach`** (`server/NeonSprawl.Server/Game/World/HorizontalReach.cs`); prototype registry under `server/NeonSprawl.Server/Game/Interaction/`; client **`interactables_catalog_client.gd`**, **`interactable_world_builder.gd`**, **`interaction_request_client.gd`**, **`interaction_radius_indicators.gd`** — see [NEO-9](../../plans/NEO-9-implementation-plan.md), [NEO-25](../../plans/NEO-25-implementation-plan.md), and [server README — Interaction](../../../server/README.md#interaction-neo-9).
|
||||
- **Alignment:** [Documentation and implementation alignment](documentation_and_implementation_alignment.md).
|
||||
|
||||
|
|
|
|||
|
|
@ -48,7 +48,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
|
|||
|--------|-----------------|----------|-------------------|
|
||||
| E1.M1 | Ready | Prototype milestone **Done** ([E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEO-8](../../plans/NEO-8-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEO-13](../../plans/NEO-13-implementation-plan.md)); Godot sync + path-follow ([NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEO-9](../../plans/NEO-9-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEO-6](../../plans/NEO-6-implementation-plan.md), [NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-8](../../plans/NEO-8-implementation-plan.md), [NEO-9](../../plans/NEO-9-implementation-plan.md), [NEO-10](../../plans/NEO-10-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md), [NEO-13](../../plans/NEO-13-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`, `Game/Interaction/`; [server README](../../../server/README.md) |
|
||||
| E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) |
|
||||
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **Still open:** **`SelectionEvent`** and richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact` (see [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md)). **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) |
|
||||
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`, Slice 3 telemetry (see [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md)). **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
|
|||
|
||||
**E1.M2 note:** Prototype slice **Ready**: `client/scripts/isometric_follow_camera.gd` + child `Camera3D` on **`World/IsometricFollowCamera`** in `client/scenes/main.tscn`; per-tick **`CameraState`** (`client/scripts/camera_state.gd`); **`ZoomBandConfig`** ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); **`OcclusionPolicy`** ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); pick-through **`"occluder"`** convention ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); consumer contract + occluder lifecycle hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). See [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md).
|
||||
|
||||
**E1.M3 note:** Decomposition expanded in [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md) (E1.M1 vs E1.M3 boundary, contract sketches, Slice 3 alignment). **Partial (NEO-23):** versioned JSON v1 **`PlayerTargetStateResponse`** + **`POST …/target/select`** under `server/NeonSprawl.Server/Game/Targeting/` ([NEO-23](../../plans/NEO-23-implementation-plan.md); [server README — Targeting](../../../server/README.md#targeting-neo-23)) — throwaway HTTP spike per [contracts.md](contracts.md). **Partial (NEO-25):** versioned **`GET /game/world/interactables`** + `InteractablesListDtos` / `InteractablesWorldApi` in `Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` ([NEO-25](../../plans/NEO-25-implementation-plan.md); [server README — Interactable descriptors](../../../server/README.md#interactable-descriptors-neo-25)). **`SelectionEvent`** and richer multi-consumer **`InteractableDescriptor`** work remain **in progress** on later Linear issues. **Precursor (E1.M1):** **`InteractionRequest`** + horizontal reach ([NEO-9](../../plans/NEO-9-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`).
|
||||
**E1.M3 note:** Decomposition expanded in [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md) (E1.M1 vs E1.M3 boundary, contract sketches, Slice 3 alignment). **Partial (NEO-23):** versioned JSON v1 **`PlayerTargetStateResponse`** + **`POST …/target/select`** under `server/NeonSprawl.Server/Game/Targeting/` ([NEO-23](../../plans/NEO-23-implementation-plan.md); [server README — Targeting](../../../server/README.md#targeting-neo-23)) — throwaway HTTP spike per [contracts.md](contracts.md). **Partial (NEO-25):** versioned **`GET /game/world/interactables`** + `InteractablesListDtos` / `InteractablesWorldApi` in `Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` ([NEO-25](../../plans/NEO-25-implementation-plan.md); [server README — Interactable descriptors](../../../server/README.md#interactable-descriptors-neo-25)). **NEO-26 (prototype `SelectionEvent`):** Godot **`TargetSelectionClient.selection_event`** on **`lockedTargetId`** changes ([NEO-26](../../plans/NEO-26-implementation-plan.md)). **Follow-on / in progress:** richer multi-consumer **`InteractableDescriptor`** targeting on later Linear issues. **Precursor (E1.M1):** **`InteractionRequest`** + horizontal reach ([NEO-9](../../plans/NEO-9-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`).
|
||||
|
||||
### Epic 2 — Skills and Progression Framework
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,25 @@
|
|||
# NEO-26 — Manual QA checklist
|
||||
|
||||
| Field | Value |
|
||||
|--------|--------|
|
||||
| **Key** | NEO-26 |
|
||||
| **Title** | E1.M3: SelectionEvent surface + debug or HUD consumer |
|
||||
| **Linear** | [NEO-26](https://linear.app/neon-sprawl/issue/NEO-26/e1m3-selectionevent-surface-debug-or-hud-consumer) |
|
||||
| **Plan** | [`docs/plans/NEO-26-implementation-plan.md`](../plans/NEO-26-implementation-plan.md) |
|
||||
|
||||
Pair client (**F5**) with the server. Open the Godot **Output** dock.
|
||||
|
||||
## 1. `log_selection_events`
|
||||
|
||||
- [ ] Select **`TargetSelectionClient`** in the scene tree; set **`log_selection_events`** to **On** in the Inspector.
|
||||
- [ ] Run with server. Press **Tab** to lock a target: Output shows a line **`[TargetSelectionClient] SelectionEvent`** with `cause=tab`, `previous` null or prior id, `next` = locked id.
|
||||
- [ ] Press **Esc** to clear: next line shows **`cause=clear`**, `next` null when lock cleared.
|
||||
- [ ] Turn **`log_selection_events`** **Off**; Tab / Esc no longer print selection lines (HUD from NEO-24 unchanged).
|
||||
|
||||
## 2. Movement GET (validity-only)
|
||||
|
||||
- [ ] With logging **off**, lock **alpha**, **WASD** until HUD shows **`Validity: out_of_range`** (NEO-24 soft lock). No requirement for extra **Output** lines from NEO-26 here — **id-only** events suppress **`validity`-only** GET updates.
|
||||
|
||||
## 3. Subscription sanity (optional)
|
||||
|
||||
- [ ] In a throwaway local script or remote inspector, **`connect`** to **`selection_event`** and confirm the same payloads as printed when logging is on.
|
||||
|
|
@ -0,0 +1,97 @@
|
|||
# NEO-26 — Implementation plan
|
||||
|
||||
## Story reference
|
||||
|
||||
| Field | Value |
|
||||
|--------|--------|
|
||||
| **Key** | NEO-26 |
|
||||
| **Title** | E1.M3: SelectionEvent surface + debug or HUD consumer |
|
||||
| **Linear** | [NEO-26](https://linear.app/neon-sprawl/issue/NEO-26/e1m3-selectionevent-surface-debug-or-hud-consumer) |
|
||||
| **Slug** | E1M3-04 |
|
||||
| **Git branch** | `NEO-26-selectionevent-surface-debug-consumer` |
|
||||
| **Parent context** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · [E1M3 prototype backlog](E1M3-prototype-backlog.md) |
|
||||
| **Depends on** | [NEO-23](NEO-23-implementation-plan.md) (`TargetState` v1), [NEO-24](NEO-24-implementation-plan.md) (tab/clear + `TargetSelectionClient`) — code paths exist on `main`; Linear may still list relations until those issues are closed. |
|
||||
| **Decomposition** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md) (`SelectionEvent` contract); [E1.M4 — AbilityInputScaffold](../decomposition/modules/E1_M4_AbilityInputScaffold.md) (future subscriber). |
|
||||
|
||||
## Goal, scope, and out-of-scope
|
||||
|
||||
**Goal:** Every **material change** to the client’s **server-acknowledged combat target lock id** (`lockedTargetId` in the cached `TargetState` mirror) is observable as a **`SelectionEvent`** carrying **previous** id, **next** id, and **cause** (`tab`, `clear`, `server_correction`, …). Ship **one** built-in consumer: **dev-only `print`** behind an `@export` flag (planning choice 2026-04-25) so the bus is exercised without new HUD nodes.
|
||||
|
||||
**In scope**
|
||||
|
||||
- Minimal **GDScript** event shape (Dictionary or small typed helper) aligned with the illustrative table in E1.M3: `previous` (Variant `String` or `null`), `next` (same), `cause` (`String` enum-like literals).
|
||||
- **Cause attribution** at the authoritative update boundary inside the existing selection client (see Technical approach) so we do not guess causes from state diffs alone.
|
||||
- **Consumer:** optional **`print`** when `@export var log_selection_events` (name TBD) is true on **`TargetSelectionClient`** (or tiny child node); document the **subscription point** for E1.M4 (`signal` name + emitter node path pattern).
|
||||
- **Tests:** tab vs clear vs movement-driven GET produce **distinct** `cause` strings when they each produce a lock-id transition (or documented no-op when id unchanged).
|
||||
|
||||
**Out of scope (per Linear / backlog)**
|
||||
|
||||
- Full prompt / party UI; wire protobuf events.
|
||||
- **Interactable** focus / primary selection as `SelectionEvent` — **target lock** only for this ticket (see D6); catalog exists regardless of NEO-25 lifecycle.
|
||||
- Changing server **`TargetState`** contracts (NEO-23); new HTTP routes.
|
||||
|
||||
## Acceptance criteria checklist
|
||||
|
||||
- [x] **Tab** path: after a successful selection cycle that **changes** `lockedTargetId`, a `SelectionEvent` is emitted with `cause` **`tab`** (and correct previous/next ids).
|
||||
- [x] **Clear** path: clearing the lock emits **`clear`** when `lockedTargetId` transitions from non-null to `null`.
|
||||
- [x] **Server correction:** a **GET**-driven authoritative update that **changes** `lockedTargetId` emits **`server_correction`**. **Id-unchanged** GETs (e.g. **`validity`** only) do **not** emit `SelectionEvent` for v1 — `test_get_validity_only_change_emits_nothing`; id change via mock in `test_get_lock_id_change_emits_server_correction`.
|
||||
- [x] **E1.M4 hook:** this plan + **`client/README.md`** (“SelectionEvent (NEO-26)”) document **`TargetSelectionClient.selection_event`** and stable payload keys.
|
||||
|
||||
## Technical approach
|
||||
|
||||
1. **Single emitter:** Extend **`TargetSelectionClient`** (`client/scripts/target_selection_client.gd`) rather than diffing `target_state_changed` in a second node — only this class knows whether the in-flight HTTP phase was **GET** (refresh) vs **POST** from **tab**, **clear**, or direct **`request_select_target_id`** (if used).
|
||||
|
||||
2. **Intent / phase tags:** Before starting HTTP, set a private enum or String tag, e.g. `_pending_selection_cause`, cleared after handling the response:
|
||||
- `request_tab_next` → `tab` (even though it calls `request_select_target_id` internally, set cause at the tab entrypoint before POST).
|
||||
- `request_clear_target` → `clear`.
|
||||
- `request_select_target_id` when **not** entered from `request_tab_next` → use cause **`tab`** (same string as Tab path — planning choice 2026-04-25: no separate `direct_select`).
|
||||
- `request_sync_from_server` when invoked from **`on_authoritative_ack`** → mark refresh as **`server_correction`** for the next GET completion.
|
||||
- Boot / one-shot sync from `main.gd` → **Decision (2026-04-25):** first and later **GET** completions use cause **`server_correction`**; **only emit** if normalized `lockedTargetId` differs from prior snapshot (avoids spam on identical boot GET).
|
||||
|
||||
3. **Emit rule:** After `_state` is updated in `_update_state_from_get` / `_update_state_from_post`, compare **previous** normalized `lockedTargetId` to **new**; if equal, **no** `SelectionEvent` (validity-only changes stay on `target_state_changed` only unless product later expands the contract).
|
||||
|
||||
4. **Signal:** Add e.g. **`selection_event(event: Dictionary)`** with keys `previous`, `next`, `cause` (and optional `sequence` int passthrough for debugging). Keep **`target_state_changed`** unchanged for existing HUD.
|
||||
|
||||
5. **Consumer:** No new scene nodes — **`TargetSelectionClient.log_selection_events`** mirrors each **`selection_event`** to **Output** via **`print`**.
|
||||
|
||||
6. **Docs:** Update **`client/README.md`** “Target lock + tab cycle” area with a short **“SelectionEvent (NEO-26)”** subsection: signal name, payload keys, print flag, E1.M4 subscription note.
|
||||
|
||||
## Files to add
|
||||
|
||||
| Path | Purpose |
|
||||
|------|---------|
|
||||
| `client/test/selection_event_client_test.gd` | GdUnit4: assert causes for tab POST, clear POST, and GET-with-mock-body changing lock id (`TargetSelectionTestDouble` pattern). |
|
||||
| `docs/manual-qa/NEO-26.md` | Manual QA for `log_selection_events` + optional subscription check. |
|
||||
|
||||
## Files to modify
|
||||
|
||||
| Path | Rationale |
|
||||
|------|-----------|
|
||||
| `client/scripts/target_selection_client.gd` | Track pending cause through GET/POST; compare old/new `lockedTargetId`; emit `selection_event`; optional `print` behind `@export`; document next to `target_state_changed`. |
|
||||
| `client/README.md` | Document subscription point + payload for E1.M4. |
|
||||
| `docs/decomposition/modules/E1_M3_InteractionAndTargetingLayer.md` | **Implementation snapshot** bullet: replace “Still open — `SelectionEvent`” with pointer to NEO-26 + signal name (keeps decomposition aligned with shipped code). |
|
||||
|
||||
## Tests
|
||||
|
||||
| Test file | What it covers |
|
||||
|-----------|------------------|
|
||||
| `client/test/selection_event_client_test.gd` | **Add:** Tab mock POST changes lock → event `cause == "tab"`. Clear mock → `"clear"`. Simulated GET completion after flagging movement refresh → `"server_correction"` when body changes `lockedTargetId`. Assert **no** event when id unchanged. |
|
||||
| `client/test/target_selection_client_test.gd` | **Change (optional):** If new behavior duplicates setup, keep NEO-24 tests focused on HTTP/state; only touch this file when sharing helpers avoids copy-paste. |
|
||||
|
||||
**Manual verification:** [`docs/manual-qa/NEO-26.md`](../../manual-qa/NEO-26.md).
|
||||
|
||||
## Open questions / risks
|
||||
|
||||
- **Validity-only updates:** If v1 stays **lock-id-only** (recommended), extend later with `kind` or dual fields if telemetry needs **`validity`** transitions on the same bus.
|
||||
- **Linear blocked-by:** Relations may still show NEO-23/NEO-24 until closed; implementation assumes their client/server surfaces on `main` are present.
|
||||
|
||||
## Decisions
|
||||
|
||||
| Topic | Decision |
|
||||
|--------|----------|
|
||||
| D1 — Emitter location | **`TargetSelectionClient`** owns `selection_event` (intent-aware). |
|
||||
| D2 — Boot GET | Use cause **`server_correction`** for all GET completions (including first sync); **suppress** event if normalized `lockedTargetId` unchanged vs prior snapshot. |
|
||||
| D3 — Non-tab `request_select_target_id` | Use cause **`tab`** (same literal as Tab path; no `direct_select`). |
|
||||
| D4 — Consumer | **Print only** behind `@export` flag on emitter; no HUD panel in NEO-26. |
|
||||
| D5 — Emit rule (v1) | **Lock id only:** emit `SelectionEvent` only when normalized **`lockedTargetId`** changes; **`validity`** / sequence updates without id change → no event (HUD keeps using `target_state_changed`). *Rationale (2026-04-25):* matches backlog “previous / next” id story, smallest payload, clear E1.M4 hook for cast target; avoids conflating soft-lock range telemetry with selection changes. Override if you want validity on this signal too. |
|
||||
| D6 — Interactables | **Out of scope:** target lock only for NEO-26. *Rationale:* Linear / E1M3-04 acceptance is about **combat target** lock changes (`SelectionEvent` for tab-target), not interactable focus or “use now” flow ([NEO-9](NEO-9-implementation-plan.md) / catalog). Descriptor work ([NEO-25](NEO-25-implementation-plan.md)) can be **done** and this story still stays narrow: a unified “all selection kinds” bus would be **extra contract design** (payload + causes + wiring) that this ticket never asked for—not a dependency on NEO-25 being open. |
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
# Code Review — NEO-26 SelectionEvent
|
||||
|
||||
- **Date:** 2026-04-25
|
||||
- **Scope:** Branch `NEO-26-selectionevent-surface-debug-consumer`; diff `origin/main...HEAD` (NEO-26 `SelectionEvent` surface + debug consumer)
|
||||
- **Base:** `origin/main`
|
||||
|
||||
## Verdict
|
||||
|
||||
Approve with nits.
|
||||
|
||||
## Summary
|
||||
|
||||
This branch adds a focused `SelectionEvent` surface in `TargetSelectionClient` and keeps it intentionally narrow to `lockedTargetId` transitions, with explicit `cause` attribution (`tab`, `clear`, `server_correction`). The implementation is coherent with the NEO-26 plan: cause tagging is set at request start, stale/failed paths clear the pending cause, and emit is suppressed for id-unchanged updates. Coverage in `client/test/selection_event_client_test.gd` exercises both positive and no-op rules, and the targeted suite passes locally. Risk is low and mostly around documentation tracking consistency rather than runtime behavior.
|
||||
|
||||
## Documentation checked
|
||||
|
||||
- `docs/plans/NEO-26-implementation-plan.md` — **matches** (signal shape, cause rules, lock-id-only emit rule, print consumer, QA artifact all implemented).
|
||||
- `docs/decomposition/modules/E1_M3_InteractionAndTargetingLayer.md` — **matches** (implementation snapshot updated to include NEO-26 `selection_event` prototype).
|
||||
- ~~`docs/decomposition/modules/module_dependency_register.md` — **partially matches** (E1.M3 note still says "`SelectionEvent` ... remain in progress"; …).~~ **Addressed:** E1.M3 note now records **NEO-26** prototype `SelectionEvent` and separates follow-on `InteractableDescriptor` / multi-consumer work.
|
||||
- ~~`docs/decomposition/modules/documentation_and_implementation_alignment.md` — **partially matches** (E1.M3 row still says "`SelectionEvent` ... still open"; …).~~ **Addressed:** E1.M3 snapshot row records **NEO-26** landed vs follow-on scope.
|
||||
|
||||
~~Implementation tracking update needed after merge: update the E1.M3 row wording in `documentation_and_implementation_alignment.md` (and optionally the E1.M3 note in `module_dependency_register.md`) so the docs reflect that prototype `SelectionEvent` now exists while richer downstream consumers remain open.~~ **Done.**
|
||||
|
||||
## Blocking issues
|
||||
|
||||
None.
|
||||
|
||||
## Suggestions
|
||||
|
||||
1. ~~Update the E1.M3 tracking row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` to explicitly record NEO-26 as landed, replacing the blanket "`SelectionEvent` still open" phrasing with "prototype landed; richer consumers follow-on."~~ **Done.**
|
||||
|
||||
## Nits
|
||||
|
||||
- ~~Nit: Consider adding one assertion for `sequence` in `client/test/selection_event_client_test.gd` to lock payload stability for downstream subscribers.~~ **Done.** (`sequence` asserted on tab, clear, GET id-change, and validity-only cases.)
|
||||
|
||||
## Verification
|
||||
|
||||
- `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/selection_event_client_test.gd` (from `client/`) — passed (`5 test cases`, `0 failures`).
|
||||
- Manual checks documented in `docs/manual-qa/NEO-26.md`.
|
||||
Loading…
Reference in New Issue