chore: point docs and rules at Linear NEO issue ids

Migrate Jira NEON-* references to Linear NEO-* and linear.app links in
docs, Cursor rules, READMEs, and traceability comments. Renamed
implementation plans and dated reviews to NEO filenames.

Branch is based on origin/main with this commit only (no NEON-29
gameplay chain). client/README follows main’s district/map copy;
player.gd matches main aside from NEO-14 debug trace prefixes.
pull/40/head
VinPropane 2026-04-13 12:58:53 -04:00
parent d0ed22f5d7
commit 7e17ff563b
60 changed files with 655 additions and 526 deletions

View File

@ -23,7 +23,7 @@ Canonical background: [`docs/architecture/tech_stack.md`](docs/architecture/tech
## Exceptions
- **Prototype-only, client-local** behavior is allowed when a Jira story or plan explicitly scopes it (e.g. **no server yet**, “client-only milestone”) and docs/README call out that it is **temporary** until authoritative sync exists. Do not silently expand client-only shortcuts into permanent architecture.
- **Prototype-only, client-local** behavior is allowed when a Linear issue or plan explicitly scopes it (e.g. **no server yet**, “client-only milestone”) and docs/README call out that it is **temporary** until authoritative sync exists. Do not silently expand client-only shortcuts into permanent architecture.
## When unsure

View File

@ -22,14 +22,14 @@ Align recommendations with repo rules and docs, including:
- [architecture-authority](architecture-authority.md) — server authority, client vs spike boundaries.
- [testing-expectations](testing-expectations.md) — when automated tests are required vs manual.
- [csharp-style](csharp-style.md) / [gdscript-style](gdscript-style.md) — naming, layout, primary constructors for C#.
- [jira-git-naming](jira-git-naming.md) — branch/commit prefixes when the work is ticketed.
- [linear-git-naming](linear-git-naming.md) — branch/commit prefixes when the work is ticketed.
- [git-workflow](git-workflow.md) — branch vs `main`, story-scoped plans.
### Plan and decomposition documentation (required to reference)
For every review, **identify and cite** the documentation that defines intent for the change, then **check the implementation against it** (not only style and correctness).
1. **Story / implementation plans** — If the work maps to a ticket or plan under `docs/plans/` (e.g. `NEON-*-implementation-plan.md`), treat that document as acceptance criteria. If the diff diverges without an updated plan or an explicit “out of scope” note, call it out (blocking or non-blocking by severity). Material **planning or implementation decisions** (options chosen, risks closed) should appear in that plan or related `docs/` per [planning-implementation-docs](planning-implementation-docs.md); if they exist only in PR/chat, note as a **should fix**.
1. **Story / implementation plans** — If the work maps to a ticket or plan under `docs/plans/` (e.g. `NEO-*-implementation-plan.md`), treat that document as acceptance criteria. If the diff diverges without an updated plan or an explicit “out of scope” note, call it out (blocking or non-blocking by severity). Material **planning or implementation decisions** (options chosen, risks closed) should appear in that plan or related `docs/` per [planning-implementation-docs](planning-implementation-docs.md); if they exist only in PR/chat, note as a **should fix**.
2. **Module docs** — Map the change to one or more modules in [`docs/decomposition/modules/module_dependency_register.md`](../../docs/decomposition/modules/module_dependency_register.md) and the corresponding `docs/decomposition/modules/E*_*.md` pages. Verify behavior matches **Purpose**, **Responsibilities**, **Key contracts**, and **Authority** / linked policy sections where relevant.
3. **Cross-cutting policies** — When applicable, align with `docs/decomposition/modules/` policy docs (e.g. [`contracts.md`](../../docs/decomposition/modules/contracts.md), [`client_server_authority.md`](../../docs/decomposition/modules/client_server_authority.md), [`pvp_combat_integration.md`](../../docs/decomposition/modules/pvp_combat_integration.md), [`data_and_ops_policy.md`](../../docs/decomposition/modules/data_and_ops_policy.md)).
4. **Implementation status** — If the register or [`documentation_and_implementation_alignment.md`](../../docs/decomposition/modules/documentation_and_implementation_alignment.md) tracks the module, note whether **Status** / the implementation tracking table should be updated after this merge.
@ -56,12 +56,12 @@ Work through what applies to the diff (skip irrelevant sections briefly).
**Every** invocation must produce a **saved markdown document** in the repo, not only a chat reply.
1. **Directory:** `docs/reviews/` (create the folder if it does not exist).
2. **Filename:** `YYYY-MM-DD-{slug}.md` using the **authoritative “Todays date”** from the session when known; otherwise use the actual calendar date. **Slug:** derive from the Jira key if the user named one (e.g. `NEON-3`), else the branch name, else a short topic (kebab-case, lowercase). Examples: `2026-03-29-NEON-3.md`, `2026-03-29-position-state-api.md`.
2. **Filename:** `YYYY-MM-DD-{slug}.md` using the **authoritative “Todays date”** from the session when known; otherwise use the actual calendar date. **Slug:** derive from the Linear issue id if the user named one (e.g. `NEO-6`), else the branch name, else a short topic (kebab-case, lowercase). Examples: `2026-03-29-NEO-6.md`, `2026-03-29-position-state-api.md`.
3. **Preamble** at the top of the file (after an optional `#` title):
- **Date** (same as in filename unless corrected)
- **Scope** — branch name, PR link, issue key, and/or `git` range the user asked to review (or “working tree / unstaged” if that was the scope)
- **Base** — e.g. `origin/main` or commit SHA if stated or inferable
4. **Body:** the full review using the **Output format** sections below (verdict through verification). This is the canonical copy; the chat response may be a short summary plus **path to the file** (e.g. `docs/reviews/2026-03-29-NEON-3.md`).
4. **Body:** the full review using the **Output format** sections below (verdict through verification). This is the canonical copy; the chat response may be a short summary plus **path to the file** (e.g. `docs/reviews/2026-03-29-NEO-6.md`).
5. **Commits:** Follow [commit-and-review](commit-and-review.md) — you may **commit** the review file at your discretion; **never** `git push`.
In the markdown file, use **normal fenced code blocks** for code snippets and **backtick file paths** (e.g. ``server/Program.cs``). Do **not** use IDE-only line-number code citations in the saved document—they do not render on GitHub or in plain Markdown viewers.

View File

@ -7,8 +7,8 @@ alwaysApply: true
## When the agent may commit
- You **may** run **`git commit`** at your **discretion** while **working on a Jira story** (or other ticketed work): logical checkpoints, end of a coherent change, plan-only commits on the story branch, implementation batches, test additions, etc.
- Use judgment: **small, coherent commits** are easier to review than one huge dump; match [jira-git-naming](jira-git-naming.md) for message format when a ticket applies.
- You **may** run **`git commit`** at your **discretion** while **working on a Linear issue** (or other ticketed work): logical checkpoints, end of a coherent change, plan-only commits on the story branch, implementation batches, test additions, etc.
- Use judgment: **small, coherent commits** are easier to review than one huge dump; match [linear-git-naming](linear-git-naming.md) for message format when a ticket applies.
- If the user is **not** on a story branch and the change is **exploratory** or **ambiguous**, prefer leaving the working tree uncommitted and summarizing until scope is clear—unless they asked you to commit.
## Never push
@ -21,10 +21,10 @@ alwaysApply: true
- Do **not** add **“Made-with: Cursor”**, **“Generated with Cursor”**, tool co-author lines, or similar AI/IDE boilerplate to **PR descriptions**, **GitHub merge/squash commit bodies** you draft, or other **remote-facing** narrative unless the user explicitly requests it.
- Keep PR text to scope, verification, and project-required contract snippets (e.g. from `docs/plans/`).
## Commit message format when a Jira story applies
## Commit message format when a Linear issue applies
- Any commit that is **part of implementing or delivering a Jira story or task** must put the **Jira issue key first** in the subject line, then **`:`**, then the summary (e.g. `NEON-5: persist position state in PostgreSQL`).
- Infer the key from the active branch name, the issue under discussion, or Jira context. Full rules (multi-issue commits, `chore:` when there is no ticket) are in [jira-git-naming](jira-git-naming.md).
- Any commit that is **part of implementing or delivering a Linear issue or task** must put the **Linear issue id first** in the subject line, then **`:`**, then the summary (e.g. `NEO-8: persist position state in PostgreSQL`).
- Infer the key from the active branch name, the issue under discussion, or Linear context. Full rules (multi-issue commits, `chore:` when there is no ticket) are in [linear-git-naming](linear-git-naming.md).
## Scope

View File

@ -1,5 +1,5 @@
---
description: New Jira story = new branch from kickoff; doc-only on main when not story-scoped
description: New Linear issue = new branch from kickoff; doc-only on main when not story-scoped
alwaysApply: true
---
@ -7,11 +7,11 @@ alwaysApply: true
**Agent:** You may **`git commit`** at your discretion while on story/ticket work; do **not** **`git push`**. See [commit-and-review](commit-and-review.md).
- **Beginning work on a new Jira story** — Create a **new branch** as soon as story work starts (planning, implementation, or both). **Branch names must start with the Jira key** (e.g. `NEON-3-position-state-api`); see [jira-git-naming](jira-git-naming.md). Stay on that branch for everything scoped to that issue, including **`docs/plans/{KEY}-implementation-plan.md`**, until the story is merged. Do not put ticketed story plans only on `main` while implementation lives on a branch.
- **Beginning work on a new Linear issue** — Create a **new branch** as soon as story work starts (planning, implementation, or both). **Branch names must start with the Linear issue id** (e.g. `NEO-6-position-state-api`); see [linear-git-naming](linear-git-naming.md). Stay on that branch for everything scoped to that issue, including **`docs/plans/{KEY}-implementation-plan.md`**, until the story is merged. Do not put ticketed story plans only on `main` while implementation lives on a branch.
- **Documentation-only work** — Commit directly on **`main`** when the change is **not** tied to an active Jira story branch: general Markdown under `docs/` (except per-story implementation plans for an issue you are working on that branch), root and nested `README.md` files, `neon_sprawl_vision.plan.md`, and other cross-cutting prose. No feature branch required.
- **Documentation-only work** — Commit directly on **`main`** when the change is **not** tied to an active Linear issue branch: general Markdown under `docs/` (except per-story implementation plans for an issue you are working on that branch), root and nested `README.md` files, `neon_sprawl_vision.plan.md`, and other cross-cutting prose. No feature branch required.
- **Code or implementation changes** — Use a **branch**, then merge to `main` when ready. **Commit subjects must start with the Jira key** when the work maps to an issue (e.g. `NEON-2: …`). See [jira-git-naming](jira-git-naming.md). This includes application source (**C#**, **GDScript**), Godot scenes and project files, `docker-compose.yml`, `.csproj` / build config, CI workflows, **JSON/YAML game data** under `content/`, and JSON Schema when it ships with data pipelines—not standalone doc prose.
- **Code or implementation changes** — Use a **branch**, then merge to `main` when ready. **Commit subjects must start with the Linear issue id** when the work maps to an issue (e.g. `NEO-5: …`). See [linear-git-naming](linear-git-naming.md). This includes application source (**C#**, **GDScript**), Godot scenes and project files, `docker-compose.yml`, `.csproj` / build config, CI workflows, **JSON/YAML game data** under `content/`, and JSON Schema when it ships with data pipelines—not standalone doc prose.
When a change mixes documentation and code, treat it as a **code change** (use a branch). Per-story implementation plans are part of the story branch, not an exception for `main`.

View File

@ -1,35 +0,0 @@
---
description: Branch names and commit messages must lead with the Jira issue key
alwaysApply: true
---
# Jira keys in branches and commits (Neon Sprawl)
Project key in Jira is **`NEON`** unless a different key is used for the issue (match the issue you are implementing).
## Issue hierarchy
Tracked work is **Epic → Story** (or Task/Bug). **Do not** create Jira **Feature** issues: module- or slice-level grouping below an epic is **labels on the story** only (prefer decomposition module ids such as **E1.M1** when they fit). Canonical wording: [decomposition README — Jira alignment](../../docs/decomposition/README.md#jira-alignment).
## Branch names
- Put the **Jira key first**, then a short **kebab-case** slug (and optional type). The key must be the **first path segment** of the branch name.
- **Good:** `NEON-2-click-to-move`, `NEON-13-fix-health-desync`, `NEON-2/feature/terrain-spike` (key still first segment if using slashes).
- **Avoid:** `feature/click-to-move` with no key, or `fix/neon-2-foo` with the key not at the start.
When suggesting or creating a branch for story **NEON-2**, use something like **`NEON-2-<slug>`**, not `feature/neon-2-…` with the key buried after a prefix.
## Commit messages
- For **any commit done as part of a Jira story, bug, or task** (work tracked under an issue), the **first token** of the subject line must be the **Jira key and number**, then **`:`** and the summary.
- **Good:** `NEON-2: add direct click-to-move steering`, `NEON-13: fix duplicate spawn on reconnect`
- **Avoid:** `fix(client): …` or `feat: …` **without** the Jira key at the front. If you use Conventional Commits, place them **after** the key: `NEON-2: feat(client): click-to-move prototype`
**Multi-issue commits:** Prefer one issue per commit; if unavoidable, list keys: `NEON-2, NEON-6: shared nav refactor`.
**No Jira issue (rare):** Maintenance or repo-only changes with no ticket — use **`chore:`** as the prefix (e.g. `chore: update .gitignore`). Do not invent fake keys.
## Agent behavior
- When creating a **branch** or **commit** for tracked work, **infer the key** from the current story (e.g. user says NEON-2), the branch already in use, or the Jira issue fetched via MCP.
- Follow [git workflow](git-workflow.md) (branch vs `main`) and [commit-and-review](commit-and-review.md) (commits at discretion on story work; **never** `git push`).

View File

@ -0,0 +1,34 @@
---
description: Branch names and commit messages must lead with the Linear issue id (team NEO)
alwaysApply: true
---
# Linear issue ids in branches and commits (Neon Sprawl)
The Neon Sprawl team in Linear uses issue ids **`NEO-*`** (see the **Neon Sprawl** team). Match the **issue you are implementing**—do not invent ids.
## Issue hierarchy
Tracked work is **Project → Issue** in Linear (epic/module **projects**, **stories** / **tasks** / **bugs** as issues). **Do not** recreate a Jira-style **Feature** middle tier: module- or slice-level grouping is **labels on the issue** only (prefer decomposition module ids such as **E1.M1** when they fit). Canonical wording: [decomposition README — Linear alignment](../../docs/decomposition/README.md#linear-alignment).
## Branch names
- Put the **Linear issue id** first (`NEO-123`), then a short **kebab-case** slug (and optional type). The id must be the **first path segment** of the branch name.
- **Good:** `NEO-5-click-to-move`, `NEO-13-fix-health-desync`, `NEO-5/feature/terrain-spike` (id still first segment if using slashes).
- **Avoid:** `feature/click-to-move` with no id, or `fix/neo-5-foo` with the id not at the start.
When suggesting or creating a branch for **NEO-5**, use something like **`NEO-5-<slug>`**, not `feature/neo-5-…` with the id buried after a prefix.
## Commit messages
- For **any commit done as part of a Linear issue** (work tracked under an issue), the **first token** of the subject line must be the **issue id** (`NEO-…`), then **`:`** and the summary.
- **Good:** `NEO-5: add direct click-to-move steering`, `NEO-13: fix duplicate spawn on reconnect`
- **Avoid:** `fix(client): …` or `feat: …` **without** the issue id at the front. If you use Conventional Commits, place them **after** the id: `NEO-5: feat(client): click-to-move prototype`
**Multi-issue commits:** Prefer one issue per commit; if unavoidable, list ids: `NEO-5, NEO-9: shared nav refactor`.
**No Linear issue (rare):** Maintenance or repo-only changes with no ticket — use **`chore:`** as the prefix (e.g. `chore: update .gitignore`). Do not invent fake ids.
## 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`).

View File

@ -1,5 +1,5 @@
---
description: Planning and implementation decisions must be written back into docs (plans, README, decomposition) so Jira chat and files stay aligned.
description: Planning and implementation decisions must be written back into docs (plans, README, decomposition) so Linear and the repo stay aligned.
alwaysApply: true
---
@ -11,7 +11,7 @@ Whenever you **decide** something during **story planning** or **implementation*
| Situation | Update |
|-----------|--------|
| Ticketed story work | **`docs/plans/{JIRA_KEY}-implementation-plan.md`** — add or edit a **Decisions** subsection, **resolve** former open questions, refresh **Technical approach** / **Acceptance criteria** checkboxes, and **Files to add/modify** if reality diverged from kickoff. |
| Ticketed story work | **`docs/plans/{LINEAR_ISSUE_ID}-implementation-plan.md`** (e.g. `NEO-7-…`) — add or edit a **Decisions** subsection, **resolve** former open questions, refresh **Technical approach** / **Acceptance criteria** checkboxes, and **Files to add/modify** if reality diverged from kickoff. |
| Cross-cutting or module contract | Relevant **`docs/decomposition/modules/*.md`**, **`docs/game-design/`**, or **`README`** / **`server/README.md`** / **`client/README.md`** when the decision affects how others integrate. |
| Review findings | **`docs/reviews/…`** when using the code-review agent; link back to the plan if the review changed direction. When feedback is fixed, **strike through + `Done.`** on the original bullets in that review file — see **Code review follow-up** below. |
@ -19,7 +19,7 @@ Whenever you **decide** something during **story planning** or **implementation*
When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`docs/reviews/YYYY-MM-DD-*.md`** are **fixed** in code or other docs, **edit that same review file** in the **same** change-set or the **next** commit—do **not** only fix the code and leave the review implying feedback is still open.
- **In place under the original headings** (`## Suggestions`, `## Nits`, `## Blocking issues`): strike through the resolved bullet with `~~…~~`, then add **`Done.`** / **`Addressed.`** / **`Deferred…`** and a one-line note (what changed, paths, optional commit SHA). Optionally add a one-line **Follow-up** note under the title (after **Scope**/**Base**) when all suggestions are done, e.g. “suggestions below are **done** (strikethrough + **Done.**)”. Same pattern as `docs/reviews/2026-03-30-NEON-5.md`, `2026-04-05-NEON-16.md`.
- **In place under the original headings** (`## Suggestions`, `## Nits`, `## Blocking issues`): strike through the resolved bullet with `~~…~~`, then add **`Done.`** / **`Addressed.`** / **`Deferred…`** and a one-line note (what changed, paths, optional commit SHA). Optionally add a one-line **Follow-up** note under the title (after **Scope**/**Base**) when all suggestions are done, e.g. “suggestions below are **done** (strikethrough + **Done.**)”. Same pattern as `docs/reviews/2026-03-30-NEO-8.md`, `2026-04-05-NEO-14.md`.
- Do **not** add a **separate** “Resolved suggestions” / “Follow-up implemented” section for items that already live under Suggestions or Nits—the strikethrough row **is** the audit trail.
- Items **declined** or **deferred** to a ticket: still strike through or annotate inline under the same bullet so the review stays honest.
- Applies to **any** agent pass that implements review feedback—see [code-review-agent](code-review-agent.md) **Resolved suggestions**.

View File

@ -1,11 +1,11 @@
---
description: When a Jira story is finished (merged or abandoned locally), return to main, pull, delete the local story branch
description: When a Linear-tracked story is finished (merged or abandoned locally), return to main, pull, delete the local story branch
alwaysApply: true
---
# Story end (Jira)
# Story end (Linear)
When the user **ends story work** on a ticketed branch—phrases like “end story”, “story is merged”, “clean up branch after NEON-…”, or they confirm the PR is **merged** to `main`—run this **git** sequence so local state matches the remote default branch.
When the user **ends story work** on a ticketed branch—phrases like “end story”, “story is merged”, “clean up branch after NEO-…”, or they confirm the PR is **merged** to `main`—run this **git** sequence so local state matches the remote default branch.
## 1. Preconditions
@ -16,7 +16,7 @@ When the user **ends story work** on a ticketed branch—phrases like “end sto
1. **`git checkout main`**
2. **`git pull origin main`** (or `git pull` if `main` already tracks `origin/main` and that is correct for this repo)
3. **`git branch -d <story-branch>`** — use the branch that was used for the issue (first path segment should be the Jira key, e.g. `NEON-7-move-validation`). If Git reports “not fully merged”, confirm whether **`origin/main`** already contains the story (e.g. squash merge or follow-up commits only cherry-picked to `main`). If yes, **`git branch -D <story-branch>`** is OK **only with user confirmation** so local unpushed tip is not lost by mistake.
3. **`git branch -d <story-branch>`** — use the branch that was used for the issue (first path segment should be the Linear issue id, e.g. `NEO-10-move-validation`). If Git reports “not fully merged”, confirm whether **`origin/main`** already contains the story (e.g. squash merge or follow-up commits only cherry-picked to `main`). If yes, **`git branch -D <story-branch>`** is OK **only with user confirmation** so local unpushed tip is not lost by mistake.
## 2a. When `main` is push-protected
@ -25,19 +25,19 @@ If the user asks to **push** changes that sit on **`main`** and **`git push orig
## 3. What not to do
- Do **not** delete **`main`** or **`origin/*`**.
- Do **not** assume the remote story branch should be deleted unless the user asks (many teams keep `origin/NEON-…` for history or delete via GitHub UI).
- Do **not** assume the remote story branch should be deleted unless the user asks (many teams keep `origin/NEO-…` for history or delete via GitHub UI).
## 4. Jira transition
## 4. Linear issue status
- **Do not** call Jira transition APIs (e.g. Atlassian MCP `transitionJiraIssue`) until the user has **explicitly** chosen the next status. Typical options here are **`In Test`** or **`Done`**; use `getTransitionsForJiraIssue` if names differ.
- If the user only says “end story” / “merge cleanup” / similar **without** naming a target status: run the **git** steps in §2, then **ask** “**In Test** or **Done**?” and **wait for their answer** before transitioning. **Never assume Done** (or any status) without that confirmation.
- **Exception:** If the **same user message** already names the status (e.g. “end story, move NEON-28 to **Done**”), you may transition after git cleanup without a second ask.
- Perform the transition with Atlassian MCP when available (**MCP server id `plugin-atlassian-atlassian`**, not `atlassian`).
- If MCP is unavailable or the transition fails, tell the user and ask them to update the board manually.
- **Do not** call Linear `save_issue` to change **state** until the user has **explicitly** chosen the target status. Typical names here are **`In Test`** or **`Done`**; use `list_issue_statuses` (Linear MCP, server **`plugin-linear-linear`**) if names differ on your team.
- If the user only says “end story” / “merge cleanup” / similar **without** naming a target status: run the **git** steps in §2, then **ask** which **Linear state** to set (e.g. **In Test** vs **Done**) and **wait for their answer** before updating the issue. **Never assume Done** without that confirmation.
- **Exception:** If the **same user message** already names the status (e.g. “end story, move NEO-18 to **Done**”), you may update Linear after git cleanup without a second ask.
- When Linear MCP is available, perform the update with **`save_issue`** (`id` = issue identifier, `state` = chosen status).
- If MCP is unavailable or the update fails, tell the user and ask them to update Linear manually.
## Related
- [story-kickoff](story-kickoff.md) — start of story workflow
- [git-workflow](git-workflow.md) — branch vs `main`
- [jira-git-naming](jira-git-naming.md) — branch names
- [linear-git-naming](linear-git-naming.md) — branch names
- [commit-and-review](commit-and-review.md) — push only when the user requests it

View File

@ -1,27 +1,27 @@
---
description: When starting Jira story work, move issue To Do → In Progress when applicable, create story branch, load issue + repo context; no implementation code until a plan exists under docs/plans/.
description: When starting Linear issue work, set status to In Progress when applicable, create story branch, load issue + repo context; no implementation code until a plan exists under docs/plans/.
alwaysApply: true
---
# Story kickoff (Jira)
# Story kickoff (Linear)
When the user starts work on a **Jira story** (e.g. issue key `NEON-2`, phrases like “begin work on story”, “implement NEON-…”), run this sequence **before** writing or changing application code.
When the user starts work on a **Linear issue** (e.g. issue key `NEO-5`, phrases like “begin work on story”, “implement NEO-…”), run this sequence **before** writing or changing application code.
## 1. Load Jira context
## 1. Load Linear context
- Fetch the issue when possible (e.g. Atlassian MCP `getJiraIssue`), or use a URL / pasted description the user provides.
- Fetch the issue when possible (Linear MCP `get_issue` / `list_issues`, server **`plugin-linear-linear`**), or use a URL / pasted description the user provides.
- Capture summary, description, acceptance criteria, and anything explicitly **out of scope**.
- **Hierarchy:** Expect **Epic → Story** (or Task/Bug). **Do not** add or assume a Jira **Feature** layer—**module / slice** grouping is **labels on the story** (see [Jira alignment](../../docs/decomposition/README.md#jira-alignment) in the decomposition README).
- **Hierarchy:** Expect **Project → Issue** (epics/modules map to **Linear projects**). **Do not** recreate a Jira-style **Feature** tier—**module / slice** grouping is **labels on the issue** (see [Linear alignment](../../docs/decomposition/README.md#linear-alignment) in the decomposition README).
## 1a. Board status (To Do → In Progress)
- After the issue is known, if its status is **To Do** (or your workflows equivalent “not started” state), **transition it to In Progress** so the board matches active kickoff.
- When Atlassian MCP is available: `getTransitionsForJiraIssue` to find the transition id for **In Progress**, then `transitionJiraIssue`; otherwise do the same in the Jira UI.
- When Linear MCP is available: `save_issue` with **`state`** set to **In Progress** (or your teams equivalent); otherwise update status in the Linear UI.
- If the issue is already **In Progress** or later, do not move it backward.
## 1b. Story branch
- **Create and use a new branch** for this issue as soon as story work begins (planning counts). Name it with the **Jira key first** and a short kebab-case slug (e.g. `NEON-3-position-state-api`); see [jira-git-naming](jira-git-naming.md) and [git workflow](git-workflow.md).
- **Create and use a new branch** for this issue as soon as story work begins (planning counts). Name it with the **Linear issue id first** and a short kebab-case slug (e.g. `NEO-6-position-state-api`); see [linear-git-naming](linear-git-naming.md) and [git workflow](git-workflow.md).
- If the user is already on a branch whose **first path segment** matches this issue key, continue on it unless they ask to rename.
- Do not wait for implementation to start before branching; the implementation plan and all story commits belong on this branch.
@ -36,14 +36,14 @@ When the user starts work on a **Jira story** (e.g. issue key `NEON-2`, phrases
## 4. Planning document
- Add **`docs/plans/{JIRA_KEY}-implementation-plan.md`** (example: `docs/plans/NEON-2-implementation-plan.md`). Create `docs/plans/` if it does not exist.
- Add **`docs/plans/{LINEAR_ISSUE_ID}-implementation-plan.md`** (example: `docs/plans/NEO-5-implementation-plan.md`). Create `docs/plans/` if it does not exist.
- **Commit** the plan on the **story branch** from step 1b when it is ready (at agent discretion per [commit-and-review](commit-and-review.md)); do not put ticketed plan-only work only on `main` while implementation stays on a branch—see [git workflow](git-workflow.md).
**Required sections** in that file (do **not** ship a plan that omits any of these):
- Story reference (key, title, link if available)
- Goal, scope, and out-of-scope (from Jira)
- Acceptance criteria checklist (from Jira)
- Goal, scope, and out-of-scope (from Linear)
- Acceptance criteria checklist (from Linear)
- Technical approach (concise)
- **Files to add****mandatory:** explicit list of new file paths (or state “none” with one line why)
- **Files to modify****mandatory:** explicit list of paths **with a one-line rationale each** (or state “none” with one line why)

View File

@ -1,5 +1,5 @@
---
description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot client unit tests with script changes (NEON-14 harness + gdscript.yml CI).
description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot client unit tests with script changes (NEO-12 harness + gdscript.yml CI).
alwaysApply: true
---
@ -23,7 +23,7 @@ alwaysApply: true
## Godot client (GDScript)
- **Harness (NEON-14):** **GdUnit4** lives under **`client/addons/gdUnit4/`**; suites under **`client/test/`**. **`client/README.md`** documents local and headless runs; **`.github/workflows/gdscript.yml`** runs **gdlint**, **gdformat**, and **headless GdUnit** when `client/scripts/`, `client/test/`, `client/addons/`, or `client/project.godot` change.
- **Harness (NEO-12):** **GdUnit4** lives under **`client/addons/gdUnit4/`**; suites under **`client/test/`**. **`client/README.md`** documents local and headless runs; **`.github/workflows/gdscript.yml`** runs **gdlint**, **gdformat**, and **headless GdUnit** when `client/scripts/`, `client/test/`, `client/addons/`, or `client/project.godot` change.
- **Production script changes:** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently.
- **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress.

View File

@ -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). **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). **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). **Jira:** ask **In Test** vs **Done** (or wait for the user to say which in the same message) **before** transitioning the 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). **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). **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:** ask which **state** to set (e.g. **In Test** vs **Done**), or wait for the user to say which in the same message, **before** updating the issue; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
**Commits tied to a Jira issue** must start the subject with the **issue key** and a colon (e.g. `NEON-5: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/jira-git-naming.md`](.cursor/rules/jira-git-naming.md).
**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).

View File

@ -11,49 +11,49 @@ Do not grow an all-in-one **`main.gd`**. The main scene root should **compose**
**Tests with script changes:** Any PR that adds or changes GDScript under **`scripts/`** should **add or update** tests under **`test/`** in the same change set (see [`.cursor/rules/testing-expectations.md`](../.cursor/rules/testing-expectations.md)). CI runs them via **`.github/workflows/gdscript.yml`**.
## Follow camera (NEON-25)
## Follow camera (NEO-15)
The main scene uses **`World/IsometricFollowCamera`** (`scripts/isometric_follow_camera.gd`): client-local **isometric follow** for **`Player`** (`NodePath` **`../Player`**), with damped eye motion (`follow_smoothness`, exponential lerp) and **teleport snap** when the eye is farther than **`snap_distance`** from the desired pose (e.g. after **`snap_to_server`**). Framing exports **`follow_distance`**, **`pitch_elevation_deg`**, and **`presentation_yaw_deg`** match the preNEON-25 static camera at spawn. **Orbit is off in the prototype:** **`allow_yaw`** is false, **`CameraState.yaw`** stays **0**, and no rotate input is read—enable **`allow_yaw`** later and drive **`_orbit_yaw_rad`** from input on the rig (see **`docs/decomposition/modules/E1_M2_IsometricCameraController.md`**). **`CameraState`** (`scripts/camera_state.gd`) is refreshed every **`_physics_process`** tick on the rig (**`process_physics_priority`** after **`Player`**) so framing tracks **`move_and_slide`** without display-vs-physics jitter. **`ground_pick`** uses the viewport camera; **`main.gd`** sets **`fallback_camera`** to **`World/IsometricFollowCamera/Camera3D`**.
The main scene uses **`World/IsometricFollowCamera`** (`scripts/isometric_follow_camera.gd`): client-local **isometric follow** for **`Player`** (`NodePath` **`../Player`**), with damped eye motion (`follow_smoothness`, exponential lerp) and **teleport snap** when the eye is farther than **`snap_distance`** from the desired pose (e.g. after **`snap_to_server`**). Framing exports **`follow_distance`**, **`pitch_elevation_deg`**, and **`presentation_yaw_deg`** match the preNEO-15 static camera at spawn. **Orbit is off in the prototype:** **`allow_yaw`** is false, **`CameraState.yaw`** stays **0**, and no rotate input is read—enable **`allow_yaw`** later and drive **`_orbit_yaw_rad`** from input on the rig (see **`docs/decomposition/modules/E1_M2_IsometricCameraController.md`**). **`CameraState`** (`scripts/camera_state.gd`) is refreshed every **`_physics_process`** tick on the rig (**`process_physics_priority`** after **`Player`**) so framing tracks **`move_and_slide`** without display-vs-physics jitter. **`ground_pick`** uses the viewport camera; **`main.gd`** sets **`fallback_camera`** to **`World/IsometricFollowCamera/Camera3D`**.
## Authoritative movement (NEON-4, NEON-8)
## Authoritative movement (NEO-7, NEO-11)
With the game server running ([`server/README.md`](../server/README.md)), each valid floor click sends a **`MoveCommand`** (**`POST`**) and a follow-up **`GET`** for **`PositionState`**. The server still **snaps** authority to the target (NEON-4/19); the client **moves** toward that verified position using **`NavigationAgent3D`** + a **baked mesh** instead of teleporting on the **`GET`** (NEON-8). **Boot** `sync_from_server()` **snaps** once so spawn matches the server.
With the game server running ([`server/README.md`](../server/README.md)), each valid floor click sends a **`MoveCommand`** (**`POST`**) and a follow-up **`GET`** for **`PositionState`**. The server still **snaps** authority to the target (NEO-7); the client **moves** toward that verified position using **`NavigationAgent3D`** + a **baked mesh** instead of teleporting on the **`GET`** (NEO-11). **Boot** `sync_from_server()` **snaps** once so spawn matches the server.
**Tradeoff (prototype):** `player.gd` only steers **straight in xz** toward the goal for real **descend** cases, where the picks **Y** is below the capsule **feet** by a small margin (smooth **stepped bumps**). Same-height floor clicks now stay on the baked **`NavigationAgent3D`** path instead of looking like a descend just because the capsule origin sits above the floor. **Automatic routing around tall obstacles on one click is still not guaranteed** for those true descend-bypass cases — use **several clicks** to go around e.g. the gray **`Obstacle`** when needed.
**NEON-7 / movement QA bumps:** On **run**, **`spawn_short_random_bumps`** adds **two** green cylinders, each on its own **`StaticBody3D`** **sibling** of **`Floor`** under **`NavigationRegion3D`** (**`walkable`** on bump roots — avoids compound **internal-edge** jitter vs floor+cylinder on one body). Bump meshes use Godot group **`random_floor_bump_mesh`**. **Collision radius** = mesh **+ `COLLISION_RADIUS_EXTRA`** (see **`scripts/random_floor_bump_collision_constants.gd`**, capped by **`COLLISION_RADIUS_MAX`**). **`bake_navigation_mesh(false)`** after spawn.
**NEO-10 / movement QA bumps:** On **run**, **`spawn_short_random_bumps`** adds **two** green cylinders, each on its own **`StaticBody3D`** **sibling** of **`Floor`** under **`NavigationRegion3D`** (**`walkable`** on bump roots — avoids compound **internal-edge** jitter vs floor+cylinder on one body). Bump meshes use Godot group **`random_floor_bump_mesh`**. **Collision radius** = mesh **+ `COLLISION_RADIUS_EXTRA`** (see **`scripts/random_floor_bump_collision_constants.gd`**, capped by **`COLLISION_RADIUS_MAX`**). **`bake_navigation_mesh(false)`** after spawn.
**Idle stability (NEON-16):** **Jolt Physics**; **TPS** **120**. **`physics/common/physics_interpolation`** is **off** — with **on**, small physics **position** changes on bump **edges** were **blended** across render frames and looked like **ghosting / extra jitter**. **`snap_to_server()`** still calls **`reset_physics_interpolation()`** for compatibility if you turn interpolation on later. **`floor_max_angle`** **~50°** walking / **~35°** idle; **loose** angle **~0.8 s** after walk stops. **Walk step assist**. **Idle rim / straddle:** **moving** `floor_max_angle` when floor normal is **shallow** or slide hits mix **floor + wall**. One idle **`move_and_slide()`**, rim **settle**, **`random_floor_bump_mesh`** **lip / rim / vertical-wall** escape (**`IDLE_BUMP_ESCAPE_STEP`**, **`PLAYER_CAPSULE_RADIUS`**, collider fudge from **`random_floor_bump_collision_constants.gd`**). **Flat idle hardening:** when support is already effectively level (floor normal almost **`Vector3.UP`** and no post-arrival loose-floor window), **`player.gd`** now skips the corrective idle slide / escape loop instead of nudging the capsule anyway, even if physics reports incidental extra contacts on an otherwise flat support. In that stable grounded state the player also keeps an **idle x/z anchor**, so any residual horizontal creep is clamped away until movement resumes or support stops looking stable. Goal arrival and descend-bypass checks both use the capsule **feet height** rather than the body origin, so flat floor clicks clear and route normally instead of lingering in a straight-line obstacle push. **`Player`:** interp **Off**, **`avoidance_enabled`** **false**, **`floor_block_on_wall`** **true** — do **not** rewrite **`global_transform`** in **`_process`** (can **ghost** **`CharacterBody3D`**). **`_snap_capsule_upright()`** now short-circuits when the basis is already identity. Idle **`FLOOR_SNAP_IDLE`** ~**11 cm**; walking **`FLOOR_SNAP_MOVING`** ~**0.32**. **Rendering:** **`Mat_player_capsule`**, **`cast_shadow = 0`**, capsule mesh **+Y ~3.4 cm** (visual-only vs **`CapsuleShape3D`** — less **z-fight** vs floor/bump tops), **`light_specular = 0`**, **`msaa_3d = 2`**. **`safe_margin`** **0.055**.
**Idle stability (NEO-14):** **Jolt Physics**; **TPS** **120**. **`physics/common/physics_interpolation`** is **off** — with **on**, small physics **position** changes on bump **edges** were **blended** across render frames and looked like **ghosting / extra jitter**. **`snap_to_server()`** still calls **`reset_physics_interpolation()`** for compatibility if you turn interpolation on later. **`floor_max_angle`** **~50°** walking / **~35°** idle; **loose** angle **~0.8 s** after walk stops. **Walk step assist**. **Idle rim / straddle:** **moving** `floor_max_angle` when floor normal is **shallow** or slide hits mix **floor + wall**. One idle **`move_and_slide()`**, rim **settle**, **`random_floor_bump_mesh`** **lip / rim / vertical-wall** escape (**`IDLE_BUMP_ESCAPE_STEP`**, **`PLAYER_CAPSULE_RADIUS`**, collider fudge from **`random_floor_bump_collision_constants.gd`**). **Flat idle hardening:** when support is already effectively level (floor normal almost **`Vector3.UP`** and no post-arrival loose-floor window), **`player.gd`** now skips the corrective idle slide / escape loop instead of nudging the capsule anyway, even if physics reports incidental extra contacts on an otherwise flat support. In that stable grounded state the player also keeps an **idle x/z anchor**, so any residual horizontal creep is clamped away until movement resumes or support stops looking stable. Goal arrival and descend-bypass checks both use the capsule **feet height** rather than the body origin, so flat floor clicks clear and route normally instead of lingering in a straight-line obstacle push. **`Player`:** interp **Off**, **`avoidance_enabled`** **false**, **`floor_block_on_wall`** **true** — do **not** rewrite **`global_transform`** in **`_process`** (can **ghost** **`CharacterBody3D`**). **`_snap_capsule_upright()`** now short-circuits when the basis is already identity. Idle **`FLOOR_SNAP_IDLE`** ~**11 cm**; walking **`FLOOR_SNAP_MOVING`** ~**0.32**. **Rendering:** **`Mat_player_capsule`**, **`cast_shadow = 0`**, capsule mesh **+Y ~3.4 cm** (visual-only vs **`CapsuleShape3D`** — less **z-fight** vs floor/bump tops), **`light_specular = 0`**, **`msaa_3d = 2`**. **`safe_margin`** **0.055**.
- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`; occluder bodies tagged `"occluder"` are passed through unconditionally so clicks reach the ground behind them — NEON-30), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify; second signal arg = boot snap vs nav goal), `scripts/player.gd` (path-follow), `scripts/isometric_follow_camera.gd` + `scripts/camera_state.gd` (NEON-25 follow), thin `scripts/main.gd` (nav bake on first frame, then wiring).
- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`; occluder bodies tagged `"occluder"` are passed through unconditionally so clicks reach the ground behind them — NEO-20), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify; second signal arg = boot snap vs nav goal), `scripts/player.gd` (path-follow), `scripts/isometric_follow_camera.gd` + `scripts/camera_state.gd` (NEO-15 follow), thin `scripts/main.gd` (nav bake on first frame, then wiring).
- **Scene:** `scenes/main.tscn` — walkable **`StaticBody3D`** geometry lives under **`World/NavigationRegion3D`**. `main.gd` waits one **`process_frame`**, spawns **random test bumps** on **`Floor`**, then **`bake_navigation_mesh(false)`** (main-thread bake), then waits two **`physics_frame`**s so **`NavigationServer3D`** has a map before agents query paths. The baked **`NavigationMesh`** asset in the scene file is **stale** until you run or re-bake in the editor.
- **Inspector:** select **`PositionAuthorityClient`** on the main scene to set **`base_url`** (default `http://127.0.0.1:5253`) and **`dev_player_id`** (default `dev-local-1`, must match server `Game:DevPlayerId`).
### Manual check (NEON-4 + NEON-8)
### Manual check (NEO-7 + NEO-11)
1. From repo root: `cd server/NeonSprawl.Server && dotnet run` (note the URL/port, usually `http://localhost:5253`).
2. If the port differs, set **`base_url`** on **`PositionAuthorityClient`** accordingly (e.g. `http://127.0.0.1:5253`).
3. Open the client in Godot and run the main scene (**F5**). The player should **snap** to the servers default position (e.g. **(-5, 0.9, -5)** per `Game:DefaultPosition` / NEON-6 walk demo).
4. **Left-click** the floor: the client **POST**s the target, then **GET**s position; the capsule **walks** toward the authoritative target (may follow nav waypoints or bee-line in xz per the tradeoff above). NEON-7 reject clicks still show the reject label and do **not** start a path.
3. Open the client in Godot and run the main scene (**F5**). The player should **snap** to the servers default position (e.g. **(-5, 0.9, -5)** per `Game:DefaultPosition` / NEO-9 walk demo).
4. **Left-click** the floor: the client **POST**s the target, then **GET**s position; the capsule **walks** toward the authoritative target (may follow nav waypoints or bee-line in xz per the tradeoff above). NEO-10 reject clicks still show the reject label and do **not** start a path.
5. Click the **far pad** or **pedestal top** (from spawn) to confirm **`horizontal_step_exceeded`** / **`vertical_step_exceeded`** behavior is unchanged.
6. After a move (or at spawn), **stand idle** with no click goal for **10+ seconds** and confirm the capsule does **not** visibly vibrate or drift in **x/z** on flat floor, then repeat on the **random green bumps** (positions change each run).
If the server is **down**, boot **`GET`** fails silently (check Output for warnings); clicks while a request is in flight are ignored until it finishes.
## Interaction + range preview (NEON-6)
## Interaction + range preview (NEO-9)
The main scene includes a **prototype terminal** at the map center (same world **X/Z** as the servers static registry) and **two glowing markers** driven by `scripts/interaction_radius_indicators.gd`. That script compares the **`CharacterBody3D`** **`global_position`** (after server snap) to the anchor in **`scripts/prototype_interaction_constants.gd`** using **horizontal distance on X/Z** and the same **`interaction_radius`** as **`PrototypeInteractableRegistry.cs`** — **preview only**; the server **`POST /game/players/{id}/interact`** is authoritative.
- **`InteractionRequestClient`** (child of the main scene root): press **E** to POST interact for the prototype id; **Output** prints `allowed` / `reasonCode` (or HTTP errors). While a request is in flight, further **E** presses are ignored (same pattern as **`PositionAuthorityClient`**).
- **Inspector:** match **`base_url`** / **`dev_player_id`** to **`PositionAuthorityClient`** and the server `Game:DevPlayerId`.
### Manual check (NEON-6)
### Manual check (NEO-9)
1. Run the game server and client as in NEON-4.
1. Run the game server and client as in NEO-7.
2. **F5** in Godot: default spawn is out of range of the terminal; markers should stay **dim**. **Click-move** toward the center until markers **brighten** (within **3** m on the floor plane).
3. Press **E** (input action **`interact`** in `project.godot`): Output should show **`allowed=true`** when markers glow, **`allowed=false`** with **`reasonCode=out_of_range`** when dim (if you walk back out). Interaction uses **`_input`**, not `_unhandled_input`, so keys register reliably in the embedded **Game** dock; click the game view if the editor had focus elsewhere.
### Manual check (NEON-30 — occluder click-through)
### Manual check (NEO-20 — occluder click-through)
1. Run the main scene (**F5**) with the server running.
2. **While `Obstacle` is opaque** (player not behind it): left-click the ground directly behind `World/NavigationRegion3D/Obstacle` (roughly `(6, 0, 5)` + a few units further). The player should receive a valid move target and walk toward that position — not stop or ignore the click.
@ -61,9 +61,9 @@ The main scene includes a **prototype terminal** at the map center (same world *
4. **Regression — normal floor:** click a clear area of the floor; confirm the player walks there as before.
5. **Regression — far pad / pedestal:** confirm click-rejection behavior on `MoveRejectPedestal` and `MoveRejectFarPad` is unchanged.
## Movement prototype (NEON-2 → NEON-8)
## Movement prototype (NEO-5 → NEO-11)
**`player.gd`** uses **`NavigationAgent3D.get_next_path_position()`** + **`move_and_slide()`** for horizontal motion when following the mesh; it may steer **directly at the goal** in xz when the descend bypass applies (NEON-8). **`snap_to_server()`** remains for **boot** (and would apply for any future hard reconcile).
**`player.gd`** uses **`NavigationAgent3D.get_next_path_position()`** + **`move_and_slide()`** for horizontal motion when following the mesh; it may steer **directly at the goal** in xz when the descend bypass applies (NEO-11). **`snap_to_server()`** remains for **boot** (and would apply for any future hard reconcile).
- The avatar is on **physics layer 2** with **mask 1** (floor + obstacle on layer 1); the pick ray uses **mask 1** so clicks pass through the avatar and hit the floor.
@ -97,7 +97,7 @@ godot --headless --path . --quit-after 5
A clean checkout has **`client/.godot/`** gitignored, so scripts intentionally avoid **`class_name`** global types that require a full editor import before headless can resolve them.
## Automated tests (GDScript, NEON-14)
## Automated tests (GDScript, NEO-12)
**Framework:** **[GdUnit4](https://github.com/godot-gdunit-labs/gdUnit4)** **v6.1.2** is vendored under **`addons/gdUnit4/`** (plugin enabled in `project.godot`). Test suites live in **`test/`** (`*_test.gd` files extending `GdUnitTestSuite`).
@ -118,7 +118,7 @@ On **Linux** (including GitHub Actions), the path must be **`gdUnit4`** with a c
**Camera zoom:** Input actions **`camera_zoom_in`** / **`camera_zoom_out`** (mouse wheel, **`=`** / **`-`**, keypad **+** / ****) are defined in **`project.godot`**. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed.
**Dev (NEON-28):** **`dev_toggle_occluder_obstacle`** (default **Ctrl+Shift+K**) toggles the prototype `Obstacle` visibility/collision in **`main.gd`**. **F9** / **F10** are used by the embedded debugger; use this action instead when the game is running in the editor.
**Dev (NEO-18):** **`dev_toggle_occluder_obstacle`** (default **Ctrl+Shift+K**) toggles the prototype `Obstacle` visibility/collision in **`main.gd`**. **F9** / **F10** are used by the embedded debugger; use this action instead when the game is running in the editor.
**Git hook (recommended):** install the repos local **pre-push** GDScript lint hook from the repo root (once per clone):

View File

@ -5,7 +5,7 @@ extends RefCounted
## the follow target has moved (rig uses `process_physics_priority` after `Player`).
## No `class_name` — see repo Godot headless / CI notes.
##
## **Consumer contract (NEON-28):** Risk UX / HUD may read this object from the rigs
## **Consumer contract (NEO-18):** Risk UX / HUD may read this object from the rigs
## `camera_state` getter. **Policy-style data** (occlusion enabled,
## zoom resource identity, pitch, presentation compass) is **not** duplicated here — read the
## rigs exports (`allow_yaw`, `presentation_yaw_deg`, resources) when needed. Future E6.M2 work

View File

@ -1,8 +1,8 @@
extends Node3D
## NEON-25: client-local isometric follow; updates [CameraState] each physics tick.
## NEON-26: discrete zoom bands via [member zoom_band_config]; wheel / `camera_zoom_*` actions.
## NEON-28: occluder overrides are keyed by [method Object.get_instance_id] only — never [Object]
## NEO-15: client-local isometric follow; updates [CameraState] each physics tick.
## NEO-16: discrete zoom bands via [member zoom_band_config]; wheel / `camera_zoom_*` actions.
## NEO-18: occluder overrides are keyed by [method Object.get_instance_id] only — never [Object]
## keys, or [method Dictionary.keys] / [method Dictionary.erase] can freeze after an occluder is
## [method Node.queue_free]'d (dev smoke: action [code]dev_toggle_occluder_obstacle[/code] in
## [code]main.gd[/code]).
@ -18,7 +18,7 @@ const CameraStateScript := preload("res://scripts/camera_state.gd")
const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd")
const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd")
## Tuned to match pre-NEON-25 static `Camera3D` at (12,10,12) vs player at (-5,0.9,-5).
## Tuned to match pre-NEO-15 static `Camera3D` at (12,10,12) vs player at (-5,0.9,-5).
## When [member zoom_band_config] is null or has no bands, this is the sole follow distance.
@export var follow_target_path: NodePath = NodePath("../Player")
@export var follow_distance: float = 25.709
@ -40,7 +40,7 @@ const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd")
## When eye-to-desired distance exceeds this, snap (server snap / huge teleports).
@export var snap_distance: float = 24.0
## Latest [CameraState] tick; same object each frame (see NEON-25 plan).
## Latest [CameraState] tick; same object each frame (see NEO-15 plan).
var camera_state:
get:
return _state
@ -231,7 +231,7 @@ static func occlusion_policy_is_valid(policy: Resource) -> bool:
## True if [param body] is a live [Node3D] suitable as an [member _occluder_overrides] key.
## Used before occlusion ray work and from tests (NEON-28).
## Used before occlusion ray work and from tests (NEO-18).
## [code]is_instance_valid[/code] first; [code]is Node3D[/code] on a freed ref errors at runtime.
static func occluder_override_key_is_valid(body: Variant) -> bool:
if not is_instance_valid(body):

View File

@ -3,7 +3,7 @@ extends Node3D
## NS-16: composes ground pick + server authority; see `ground_pick.gd` /
## `position_authority_client.gd`. NS-18: interaction POST + radius glow preview (see child nodes).
## NS-23: bakes `NavigationRegion3D` on startup; boot snap vs nav goal from authority signal.
## NEON-25: follow camera is `World/IsometricFollowCamera/Camera3D`
## NEO-15: follow camera is `World/IsometricFollowCamera/Camera3D`
## (see `isometric_follow_camera.gd`).
## Prototype: two random short bumps (sibling StaticBody3D under NavigationRegion3D; see
## `random_floor_bumps.gd`) before nav bake.
@ -80,7 +80,7 @@ func _on_move_rejected(reason_code: String) -> void:
)
## Dev smoke (NEON-28 occluder): action [code]dev_toggle_occluder_obstacle[/code]
## Dev smoke (NEO-18 occluder): action [code]dev_toggle_occluder_obstacle[/code]
## (default **Ctrl+Shift+K**). Toggles `Obstacle` **visibility + collision** —
## does not call [method Node.queue_free].
## Uses [member Node.process_mode] [code]PROCESS_MODE_DISABLED[/code] so the [StaticBody3D] is

View File

@ -1,7 +1,7 @@
extends Resource
## OcclusionPolicy — config for the RayCast-based per-surface material fade used by
## [IsometricFollowCamera] (NEON-27). No `class_name` — see repo Godot headless / CI notes.
## [IsometricFollowCamera] (NEO-17). No `class_name` — see repo Godot headless / CI notes.
## Master switch. When false, no rays are cast and any active overrides are restored.
@export var enabled: bool = true

View File

@ -260,7 +260,7 @@ func _debug_trace_idle_state(tag: String) -> void:
push_warning(
(
(
"NEON-16 idle trace [%s] pos=%s vel=%s on_floor=%s loose=%d "
"NEO-14 idle trace [%s] pos=%s vel=%s on_floor=%s loose=%d "
+ "stable=%s anchor=%s anchor_xz=%s floor=%s slides=%s"
)
% [
@ -296,7 +296,7 @@ func _debug_trace_transform(tag: String) -> void:
print(
(
(
"NEON-16 transform trace [%s] frame=%d pos=%s vel=%s has_goal=%s "
"NEO-14 transform trace [%s] frame=%d pos=%s vel=%s has_goal=%s "
+ "on_floor=%s loose=%d stable=%s anchor=%s anchor_xz=%s"
)
% [

View File

@ -1,4 +1,4 @@
# Static helpers for res://scripts/isometric_follow_camera.gd (NEON-25 framing, NEON-27/28 guards).
# Static helpers for res://scripts/isometric_follow_camera.gd (NEO-15 framing, NEO-17/28 guards).
extends GdUnitTestSuite
const IsoScript := preload("res://scripts/isometric_follow_camera.gd")

View File

@ -1,4 +1,4 @@
# Tests res://scripts/occlusion_policy.gd (NEON-27 occlusion config).
# Tests res://scripts/occlusion_policy.gd (NEO-17 occlusion config).
extends GdUnitTestSuite
const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd")

View File

@ -1,4 +1,4 @@
# Tests res://scripts/zoom_band_config.gd (NEON-26 discrete bands).
# Tests res://scripts/zoom_band_config.gd (NEO-16 discrete bands).
extends GdUnitTestSuite
const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd")

View File

@ -20,11 +20,11 @@ This workspace contains detailed planning artifacts derived from the finalized m
3. Use stable IDs (`E1`, `E1.M1`, etc.) for core game epics; use **`CT.M1`**-style IDs for [content tooling](tooling/internal_authoring.md) so tooling stays out of the E1E9 sequence.
4. Every new slice must reference telemetry and validation where applicable.
## Jira alignment
## Linear alignment
Engineering work is tracked as **Stories** (or **Tasks** / **Bugs** when appropriate) **under Epics**. **Do not** create Jira **Feature** issues for module- or slice-level grouping—that middle tier is not used.
Engineering work is tracked as **issues** in **Linear** (team **NEO**), grouped under **projects** that mirror epics and modules. **Do not** recreate a Jira-style **Feature** issue tier—module- or slice-level grouping is **labels on the issue** only.
Express that grouping with **labels on the story** instead (for example the decomposition **module id**`E1.M1`, `CT.M2`, etc. — or other labels the team maintains in Jira). The story still **belongs to the Epic** as today; labels carry what used to be “feature + labels on stories,” without a separate Feature work item.
Express that grouping with **labels** (for example the decomposition **module id**`E1.M1`, `CT.M2`, etc.). Issues belong to a **project** (e.g. Epic 1, E1.M1, Tech Debt); labels carry scope without a separate Feature work item.
## Epic files (E1E9, master plan)
@ -50,6 +50,6 @@ Template for new epics or major revisions: [epics/_epic_template.md](epics/_epic
## Next Actions
1. Expand slices into **stories** under epics; tag **module / slice** scope with **labels** (not Jira Features). Keep module IDs aligned with the master plan.
1. Expand slices into **Linear issues** under epic/module **projects**; tag **module / slice** scope with **labels** (not a duplicate Feature tier). Keep module IDs aligned with the master plan.
2. Populate `modules/module_dependency_register.md` as contracts stabilize.
3. Refine `milestones/phase_execution_plan.md` with dates and owners when execution starts.

View File

@ -22,7 +22,7 @@ Deliver the foundational client runtime: movement, locked isometric camera, worl
- Key contracts: `MoveCommand`, `PositionState`, `InteractionRequest`
- Dependencies: None
- Stage target: Prototype
- Status: **Ready** in the [module dependency register](../modules/module_dependency_register.md); Jira [NEON-9](https://neon-sprawl.atlassian.net/browse/NEON-9) **Done** for prototype scope — [module doc](../modules/E1_M1_InputAndMovementRuntime.md#implementation-snapshot) for snapshot and follow-on work.
- Status: **Ready** in the [module dependency register](../modules/module_dependency_register.md); Linear project [E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) **Done** for prototype scope — [module doc](../modules/E1_M1_InputAndMovementRuntime.md#implementation-snapshot) for snapshot and follow-on work.
### E1.M2 - IsometricCameraController

View File

@ -7,7 +7,7 @@
| **Module ID** | E1.M1 |
| **Epic** | [Epic 1 — Core Player Runtime](../epics/epic_01_core_player_runtime.md) |
| **Stage target** | Prototype |
| **Status** | Ready — prototype milestone complete ([NEON-9](https://neon-sprawl.atlassian.net/browse/NEON-9) Done); see [dependency register](module_dependency_register.md) and [implementation snapshot](#implementation-snapshot) |
| **Status** | Ready — prototype milestone complete ([E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) Done); see [dependency register](module_dependency_register.md) and [implementation snapshot](#implementation-snapshot) |
## Purpose
@ -33,7 +33,7 @@ Contract readiness is tracked in the [module dependency register](module_depende
## Implementation snapshot
- **Done (prototype):** Server-side authoritative **`PositionState`** read + **`MoveCommand`** apply (HTTP JSON v1, snap-to-target, `sequence` increments); **NEON-7** server-side step + optional district bounds validation with **`reasonCode`** rejections ([NEON-7](../../plans/NEON-7-implementation-plan.md), `MoveCommandValidation`, [server README — Move command](../../../server/README.md#move-command-neon-4-neon-7)); default **in-memory** store; optional **PostgreSQL** persistence when `ConnectionStrings:NeonSprawl` is set ([NEON-5](../../plans/NEON-5-implementation-plan.md)); shared **NpgsqlDataSource** disposed on application shutdown ([NEON-15](../../plans/NEON-15-implementation-plan.md)); Godot client **`POST`/`GET`** move flow ([NEON-3](../../plans/NEON-3-implementation-plan.md), [NEON-4](../../plans/NEON-4-implementation-plan.md), `server/NeonSprawl.Server/Game/PositionState/`, `client/scripts/`). **NEON-8** — client **`NavigationRegion3D` / `NavigationAgent3D`** for click-to-move visuals while server authority unchanged; **single-click obstacle detours not guaranteed** (see plan tradeoff) ([NEON-8](../../plans/NEON-8-implementation-plan.md); [client README](../../../client/README.md#authoritative-movement-neon-4-neon-8)). **`InteractionRequest`** + server-side horizontal range check ([NEON-6](../../plans/NEON-6-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`, [server README — Interaction](../../../server/README.md#interaction-neon-6)). See [server README — Position persistence](../../../server/README.md#position-persistence-neon-5). Jira **[NEON-9](https://neon-sprawl.atlassian.net/browse/NEON-9)** (E1.M1 Feature) is **Done** for this prototype scope.
- **Done (prototype):** Server-side authoritative **`PositionState`** read + **`MoveCommand`** apply (HTTP JSON v1, snap-to-target, `sequence` increments); **NEO-10** server-side step + optional district bounds validation with **`reasonCode`** rejections ([NEO-10](../../plans/NEO-10-implementation-plan.md), `MoveCommandValidation`, [server README — Move command](../../../server/README.md#move-command-neo-7-neo-10)); default **in-memory** store; optional **PostgreSQL** persistence when `ConnectionStrings:NeonSprawl` is set ([NEO-8](../../plans/NEO-8-implementation-plan.md)); shared **NpgsqlDataSource** disposed on application shutdown ([NEO-13](../../plans/NEO-13-implementation-plan.md)); Godot client **`POST`/`GET`** move flow ([NEO-6](../../plans/NEO-6-implementation-plan.md), [NEO-7](../../plans/NEO-7-implementation-plan.md), `server/NeonSprawl.Server/Game/PositionState/`, `client/scripts/`). **NEO-11** — client **`NavigationRegion3D` / `NavigationAgent3D`** for click-to-move visuals while server authority unchanged; **single-click obstacle detours not guaranteed** (see plan tradeoff) ([NEO-11](../../plans/NEO-11-implementation-plan.md); [client README](../../../client/README.md#authoritative-movement-neon-4-neon-8)). **`InteractionRequest`** + server-side horizontal range check ([NEO-9](../../plans/NEO-9-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`, [server README — Interaction](../../../server/README.md#interaction-neon-6)). See [server README — Position persistence](../../../server/README.md#position-persistence-neon-5). Jira **[E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4)** (E1.M1 Feature) is **Done** for this prototype scope.
- **Follow-on:** Client prediction/reconciliation; Epic 1 Slice 1 telemetry and movement-loop polish; optional **Protobuf** wire promotion for `MoveCommand` / `PositionState` per [contracts.md](contracts.md).
- **Alignment:** [Documentation and implementation alignment](documentation_and_implementation_alignment.md).

View File

@ -7,8 +7,8 @@
| **Module ID** | E1.M2 |
| **Epic** | [Epic 1 — Core Player Runtime](../epics/epic_01_core_player_runtime.md) |
| **Stage target** | Prototype |
| **Status** | **Ready** (prototype slice) — follow + `CameraState` + **discrete zoom bands** + **occlusion** + pick-through ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)[NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27), [NEON-30](https://neon-sprawl.atlassian.net/browse/NEON-30)); contracts + hardening ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)); see [dependency register](module_dependency_register.md) |
| **Jira** | Feature [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10); prototype backlog stories under parent [NEON-1](https://neon-sprawl.atlassian.net/browse/NEON-1) — [Jira backlog](#jira-backlog) |
| **Status** | **Ready** (prototype slice) — follow + `CameraState` + **discrete zoom bands** + **occlusion** + pick-through ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)[NEO-17](https://linear.app/neon-sprawl/issue/NEO-17), [NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)); see [dependency register](module_dependency_register.md) |
| **Linear** | Project [E1.M2 — IsometricCameraController](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395); prototype backlog issues under [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) — [Linear backlog](#linear-backlog) |
## Purpose
@ -39,11 +39,11 @@ Delivers an isometric follow camera that keeps the player readable during motion
| Contract | Role |
|----------|------|
| `CameraState` | Per-frame snapshot: follow path, effective distance, zoom band index, focus, **yaw** (orbit delta; prototype `0`). **Policy / presentation exports are not duplicated** in this type for the prototype — read the rig when needed ([Consumer contract](#consumer-contract-neon-28)). |
| `CameraState` | Per-frame snapshot: follow path, effective distance, zoom band index, focus, **yaw** (orbit delta; prototype `0`). **Policy / presentation exports are not duplicated** in this type for the prototype — read the rig when needed ([Consumer contract](#consumer-contract-neo-18)). |
| `ZoomBandConfig` | Data-driven min/max or discrete zoom steps. |
| `OcclusionPolicy` | Rules for fading, dithering, or offset when geometry blocks the view. |
## Consumer contract (NEON-28)
## Consumer contract (NEO-18)
Single place for dependents (e.g. **E6.M2**): what is on `CameraState` vs what stays on the rig. **Authority:** [E1.M2 — client vs server](client_server_authority.md#e1m2-isometriccameracontroller).
@ -82,33 +82,33 @@ Refreshed **every `_physics_process`** tick on `IsometricFollowCamera` after the
See Epic 1 **Slice 2 — Locked isometric camera**: follow-center, zoom bands, occlusion policy, **yaw fixed for players in prototype** (implementation still models yaw per [mid-project rotation policy](#mid-project-rotation-policy-practical-compromise)); optional telemetry such as throttled `camera_zoom_changed` and perf stress markers for occluders.
## Implementation snapshot (NEON-25 + NEON-26 + NEON-27 + NEON-30 + NEON-28, 2026-04-08 / 2026-04-10)
## Implementation snapshot (NEO-15 + NEO-16 + NEO-17 + NEO-20 + NEO-18, 2026-04-08 / 2026-04-10)
- **Godot:** `client/scripts/isometric_follow_camera.gd` on **`World/IsometricFollowCamera`**; child **`Camera3D`** (current). **`scripts/camera_state.gd`** — `RefCounted` tick snapshot (`yaw` = orbit component, **0** while **`allow_yaw`** is false; **`distance`** = effective follow distance; **`zoom_band_index`**; **`focus_world`**, **`follow_target_path`**).
- **Seam:** **`allow_yaw`** / **`max_yaw_deg`** on the rig; orbit would adjust **`_orbit_yaw_rad`** (no input bound yet). **Presentation** compass uses **`presentation_yaw_deg`** separately from orbit **`yaw`** in state.
- **Zoom:** **`ZoomBandConfig`** resource (`client/scripts/zoom_band_config.gd`); default **`client/resources/isometric_zoom_bands.tres`** on main rig. Bands are **distance-only** (ascending near→far); **pitch / FOV** unchanged vs NEON-25. Input: **`camera_zoom_in`** / **`camera_zoom_out`** in `client/project.godot` (mouse wheel + `=` / `-` + keypad **+** / ****). **`follow_distance`** remains **fallback** when config is null or has no bands; state **`zoom_band_index`** is **0** in that case. **`zoom_band_changed`** signal on rig; **TODO(E9.M1)** for throttled `camera_zoom_changed` telemetry.
- **Zoom:** **`ZoomBandConfig`** resource (`client/scripts/zoom_band_config.gd`); default **`client/resources/isometric_zoom_bands.tres`** on main rig. Bands are **distance-only** (ascending near→far); **pitch / FOV** unchanged vs NEO-15. Input: **`camera_zoom_in`** / **`camera_zoom_out`** in `client/project.godot` (mouse wheel + `=` / `-` + keypad **+** / ****). **`follow_distance`** remains **fallback** when config is null or has no bands; state **`zoom_band_index`** is **0** in that case. **`zoom_band_changed`** signal on rig; **TODO(E9.M1)** for throttled `camera_zoom_changed` telemetry.
- **Occlusion:** **`OcclusionPolicy`** resource (`client/scripts/occlusion_policy.gd`); default **`client/resources/isometric_occlusion_policy.tres`** on main rig. **Technique:** each **`_physics_process`** tick, iterative `intersect_ray` from `_smoothed_eye` to player focus (up to `max_occluder_cast_depth` calls). Bodies tagged `"occluder"` that intersect the ray have their `MeshInstance3D` surfaces overridden with a transparent `StandardMaterial3D` copy (`fade_alpha = 0.25`). Materials restored instantly when the body clears the ray. **Null-material surfaces** (no mesh or override material set) receive a plain transparent `StandardMaterial3D`; other non-`StandardMaterial3D` surfaces are skipped with `push_warning`. **Prototype district:** `Obstacle` node in `main.tscn` tagged `"occluder"`. **Perf marker:** `occluder_count_log_threshold` export (default 0 = disabled) emits `push_warning` for stress-test reference. See [risks and telemetry](#risks-and-telemetry) for readability gate.
> **Prototype demo readability gate:** before shipping the prototype demo, the PR for NEON-27 must include a before/after screenshot or clip demonstrating that the player is no longer fully hidden by the `Obstacle`. This serves as the first data point for the occlusion-hiding-telegraphs risk documented below.
> **Prototype demo readability gate:** before shipping the prototype demo, the PR for NEO-17 must include a before/after screenshot or clip demonstrating that the player is no longer fully hidden by the `Obstacle`. This serves as the first data point for the occlusion-hiding-telegraphs risk documented below.
- **Click-through input (NEON-30):** `scripts/ground_pick.gd` unconditionally skips bodies in the `"occluder"` group during click-to-move ground-pick raycasts. When a cast hits an occluder the ray origin advances `OCCLUDER_PICK_THROUGH` meters past the hit point and continues, independent of whether `OcclusionPolicy` is currently fading that body. This keeps the `"occluder"` group as the sole tagging convention shared between the camera occlusion system and ground-pick input — no dedicated collision layer is added for this case.
- **Click-through input (NEO-20):** `scripts/ground_pick.gd` unconditionally skips bodies in the `"occluder"` group during click-to-move ground-pick raycasts. When a cast hits an occluder the ray origin advances `OCCLUDER_PICK_THROUGH` meters past the hit point and continues, independent of whether `OcclusionPolicy` is currently fading that body. This keeps the `"occluder"` group as the sole tagging convention shared between the camera occlusion system and ground-pick input — no dedicated collision layer is added for this case.
- **Integration hardening (NEON-28):** [Consumer contract](#consumer-contract-neon-28) + `camera_state.gd` header; **E6.M2** adjacency notes ([E6_M2_ConsentAndRiskUxSignals.md](E6_M2_ConsentAndRiskUxSignals.md)); invalid / freed occluder bodies are **dropped** from `_occluder_overrides` before each occlusion ray pass and before full restore (`occluder_override_key_is_valid`, tests in `isometric_follow_camera_test.gd`) — **no** material restore on that path (freed subtree would error); live bodies still restore via `_restore_occluder` when they leave the ray. **Follow / state / occlusion** run in **`_physics_process`** with **`process_physics_priority = 1`** so the rig updates **after** `Player` (default **0**) and tracks **`move_and_slide`** without display-vs-physics jitter.
- **Integration hardening (NEO-18):** [Consumer contract](#consumer-contract-neo-18) + `camera_state.gd` header; **E6.M2** adjacency notes ([E6_M2_ConsentAndRiskUxSignals.md](E6_M2_ConsentAndRiskUxSignals.md)); invalid / freed occluder bodies are **dropped** from `_occluder_overrides` before each occlusion ray pass and before full restore (`occluder_override_key_is_valid`, tests in `isometric_follow_camera_test.gd`) — **no** material restore on that path (freed subtree would error); live bodies still restore via `_restore_occluder` when they leave the ray. **Follow / state / occlusion** run in **`_physics_process`** with **`process_physics_priority = 1`** so the rig updates **after** `Player` (default **0**) and tracks **`move_and_slide`** without display-vs-physics jitter.
## Jira backlog
## Linear backlog
Parent epic: [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1).
Parent epic: [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016).
| Key | Type | Summary |
|-----|------|---------|
| [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) | Feature | E1.M2 — IsometricCameraController (module umbrella) |
| [NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25) | Story | Isometric follow camera (fixed yaw prototype; CameraState seam) |
| [NEON-29](https://neon-sprawl.atlassian.net/browse/NEON-29) | Story | Expand prototype client district for camera/nav stress QA |
| [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26) | Story | ZoomBandConfig + discrete zoom input (clamped) |
| [NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27) | Story | OcclusionPolicy — keep player readable through geometry |
| [NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28) | Story | Camera integration hardening + dependent contract notes |
| [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) | Project | E1.M2 — IsometricCameraController (module umbrella) |
| [NEO-15](https://linear.app/neon-sprawl/issue/NEO-15) | Story | Isometric follow camera (fixed yaw prototype; CameraState seam) |
| [NEO-19](https://linear.app/neon-sprawl/issue/NEO-19) | Story | Expand prototype client district for camera/nav stress QA |
| [NEO-16](https://linear.app/neon-sprawl/issue/NEO-16) | Story | ZoomBandConfig + discrete zoom input (clamped) |
| [NEO-17](https://linear.app/neon-sprawl/issue/NEO-17) | Story | OcclusionPolicy — keep player readable through geometry |
| [NEO-18](https://linear.app/neon-sprawl/issue/NEO-18) | Story | Camera integration hardening + dependent contract notes |
Suggested order: NEON-25 → NEON-26 → NEON-27 → NEON-28. [NEON-29](https://neon-sprawl.atlassian.net/browse/NEON-29) may run in parallel with or after NEON-25 (scene/nav geography; does not require zoom/occlusion).
Suggested order: NEO-15 → NEO-16 → NEO-17 → NEO-18. [NEO-19](https://linear.app/neon-sprawl/issue/NEO-19) may run in parallel with or after NEO-15 (scene/nav geography; does not require zoom/occlusion).
## Risks and telemetry

View File

@ -13,11 +13,11 @@
Entry warnings, in-zone risk UI, and clear PvP state communication using eligibility from [E6.M1](E6_M1_PvPEligibilityAndFlagState.md) and camera/readability context from [E1.M2](E1_M2_IsometricCameraController.md).
## E1.M2 camera adjacency (NEON-28)
## E1.M2 camera adjacency (NEO-18)
When implementing zone-risk HUD and related **client** presentation, treat [E1.M2](E1_M2_IsometricCameraController.md) as the framing source:
- **May read** the follow rigs **`camera_state`** snapshot (`client/scripts/camera_state.gd`) — field meanings and refresh cadence: [Consumer contract](E1_M2_IsometricCameraController.md#consumer-contract-neon-28).
- **May read** the follow rigs **`camera_state`** snapshot (`client/scripts/camera_state.gd`) — field meanings and refresh cadence: [Consumer contract](E1_M2_IsometricCameraController.md#consumer-contract-neo-18).
- **Not duplicated in `CameraState` today:** occlusion on/off, bound zoom/occlusion resources, `presentation_yaw_deg`, `pitch_elevation_deg` — read **`IsometricFollowCamera`** exports (`client/scripts/isometric_follow_camera.gd`) when needed.
- **If `CameraState.yaw` becomes non-zero** (orbit enabled mid-project per E1.M2 rotation policy), screen-space layouts that assumed a fixed diagonal may need a compass / telegraph pass; keep **gameplay-adjacent** semantics **world-anchored** where possible.
- **Authority:** [Client vs server — E1.M2](client_server_authority.md#e1m2-isometriccameracontroller) — the server must **not** use client-reported camera pose for gameplay checks (targeting, LOS, zone rules).

View File

@ -10,8 +10,8 @@ The [module dependency register](module_dependency_register.md) **Status** colum
|--------|------|
| **Register + module markdown** | Intent, dependencies, contracts, policies (authority, PvP, quests, ops). |
| **Code + tests** | What actually runs; paths under `server/`, `client/`, etc. |
| **Plans** | `docs/plans/` (e.g. NEON-* implementation plans) tie stories to acceptance criteria. |
| **Jira** | Stories under epics; **module grouping = labels** on the story (no Jira **Feature** issues). [Decomposition README — Jira alignment](../README.md#jira-alignment). |
| **Plans** | `docs/plans/` (e.g. NEO-* implementation plans) tie stories to acceptance criteria. |
| **Linear** | Issues under projects; **module grouping = labels** on the issue (no duplicate **Feature** tier). [Decomposition README — Linear alignment](../README.md#linear-alignment). |
| **This docs tracking table** | Quick **inventory** of modules that have started implementation; expand as work lands. |
---
@ -46,8 +46,8 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
| Module | Register status | Snapshot | Plans / pointers |
|--------|-----------------|----------|-------------------|
| E1.M1 | Ready | Prototype milestone **Done** ([NEON-9](https://neon-sprawl.atlassian.net/browse/NEON-9)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEON-5](../../plans/NEON-5-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEON-15](../../plans/NEON-15-implementation-plan.md)); Godot sync + path-follow ([NEON-4](../../plans/NEON-4-implementation-plan.md), [NEON-8](../../plans/NEON-8-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEON-6](../../plans/NEON-6-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEON-3](../../plans/NEON-3-implementation-plan.md), [NEON-4](../../plans/NEON-4-implementation-plan.md), [NEON-5](../../plans/NEON-5-implementation-plan.md), [NEON-6](../../plans/NEON-6-implementation-plan.md), [NEON-7](../../plans/NEON-7-implementation-plan.md), [NEON-8](../../plans/NEON-8-implementation-plan.md), [NEON-15](../../plans/NEON-15-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` ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)); zoom bands ([NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)); occlusion ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)); occluder pick-through ([NEON-30](https://neon-sprawl.atlassian.net/browse/NEON-30)); contracts + hardening ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)). Client-local; no server use of camera pose. | [NEON-25](../../plans/NEON-25-implementation-plan.md), [NEON-26](../../plans/NEON-26-implementation-plan.md), [NEON-27](../../plans/NEON-27-implementation-plan.md), [NEON-28](../../plans/NEON-28-implementation-plan.md), [NEON-30](../../plans/NEON-30-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.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) |
---

View File

@ -17,7 +17,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
| E1.M3 | InteractionAndTargetingLayer | E1.M1 | TargetState, InteractableDescriptor, SelectionEvent | Prototype | Planned |
| E1.M4 | AbilityInputScaffold | E1.M3, E5.M1 | AbilityCastRequest, HotbarLoadout, CooldownSnapshot | Prototype | Planned |
**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`** ([NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)); **`OcclusionPolicy`** ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)); pick-through **`"occluder"`** convention ([NEON-30](https://neon-sprawl.atlassian.net/browse/NEON-30)); consumer contract + occluder lifecycle hardening ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)). See [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md).
**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).
### Epic 2 — Skills and Progression Framework

View File

@ -1,16 +1,16 @@
# NEON-7 — Implementation plan
# NEO-10 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-7 |
| **Key** | NEO-10 |
| **Title** | E1.M1: Server movement validation (speed + bounds) |
| **Jira** | [NEON-7](https://neon-sprawl.atlassian.net/browse/NEON-7) |
| **Parent context** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) · [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) |
| **Linear** | [NEO-10](https://linear.app/neon-sprawl/issue/NEO-10) |
| **Parent context** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) |
| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) (`MoveCommand` / `PositionState` authority) |
**Delivery:** Ship **`docs/plans/NEON-7-implementation-plan.md`** on the **same branch / PR** as the NEON-7 implementation (plan + code pushed together).
**Delivery:** Ship **`docs/plans/NEO-10-implementation-plan.md`** on the **same branch / PR** as the NEO-10 implementation (plan + code pushed together).
## Goal, scope, and out-of-scope
@ -18,7 +18,7 @@
**Design alignment — floor play (XZ emphasis)**
Match [NEON-6](NEON-6-implementation-plan.md): prototype gameplay is **on the floor**. **Primary** move cap should be **horizontal displacement on X/Z** (same spirit as `HorizontalReach`). Optionally cap **|ΔY|** separately so small floor bumps are allowed but vertical teleports are not. Document the exact rule in code and server README.
Match [NEO-9](NEO-9-implementation-plan.md): prototype gameplay is **on the floor**. **Primary** move cap should be **horizontal displacement on X/Z** (same spirit as `HorizontalReach`). Optionally cap **|ΔY|** separately so small floor bumps are allowed but vertical teleports are not. Document the exact rule in code and server README.
**In scope**
@ -28,17 +28,17 @@ Match [NEON-6](NEON-6-implementation-plan.md): prototype gameplay is **on the fl
- **`POST /game/players/{id}/move`:** after resolving the **current** authoritative position, run validation **before** `TryApplyMoveTarget`. On violation, **do not** mutate position or sequence.
- **Error payload:** JSON body with **`schemaVersion`** (`1`) and **`reasonCode`** (non-empty, stable string). **HTTP status** — prefer **400** for “valid request shape but move **rejected**” vs **404** for unknown player (keep **404** unchanged); finalize in PR if needed.
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Navmesh, full anti-cheat, client prediction / reconciliation.
**Dependencies**
- **`MoveCommand` endpoint** and stores ([NEON-3](NEON-3-implementation-plan.md), [NEON-4](NEON-4-implementation-plan.md), [NEON-5](NEON-5-implementation-plan.md)) — **done**.
- **`MoveCommand` endpoint** and stores ([NEO-6](NEO-6-implementation-plan.md), [NEO-7](NEO-7-implementation-plan.md), [NEO-8](NEO-8-implementation-plan.md)) — **done**.
## Policy — reject (locked)
Invalid moves are **rejected**: return the error payload; **do not** change authoritative position or sequence. Client can log **`reasonCode`** or show a short label. **Clamp** (silent or partial apply) is **out of scope** for NEON-7; a future story would need explicit product rules if we ever add it.
Invalid moves are **rejected**: return the error payload; **do not** change authoritative position or sequence. Client can log **`reasonCode`** or show a short label. **Clamp** (silent or partial apply) is **out of scope** for NEO-10; a future story would need explicit product rules if we ever add it.
## Acceptance criteria checklist
@ -105,8 +105,8 @@ Invalid moves are **rejected**: return the error payload; **do not** change auth
| `server/README.md` | Document move validation, config keys, rejection status/body, and `reasonCode` values. |
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Optional: small helper or documented pattern to override `Game` options with **strict** limits for HTTP tests without relying on production defaults. |
| `client/scripts/position_authority_client.gd` | Handle non-**200** from move POST: log **`reasonCode`** (and optional user-visible hint); do not emit authoritative snap as success. |
| `client/scenes/main.tscn` | Add **walkable** low bumps + tall **walkable** pedestal (and optional far pad) with meshes/materials for manual NEON-7 validation; keep floor/interactable layout coherent. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot: NEON-7 move validation + link to this plan. |
| `client/scenes/main.tscn` | Add **walkable** low bumps + tall **walkable** pedestal (and optional far pad) with meshes/materials for manual NEO-10 validation; keep floor/interactable layout coherent. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot: NEO-10 move validation + link to this plan. |
## Tests

View File

@ -1,16 +1,16 @@
# NEON-8 — Implementation plan
# NEO-11 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-8 |
| **Key** | NEO-11 |
| **Title** | E1.M1: Client path-follow baseline (navigation + visible locomotion) |
| **Jira** | [NEON-8](https://neon-sprawl.atlassian.net/browse/NEON-8) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) · [NEON-1 — Epic 1](https://neon-sprawl.atlassian.net/browse/NEON-1) · Slice 1 — Movement and position sync |
| **Linear** | [NEO-11](https://linear.app/neon-sprawl/issue/NEO-11) |
| **Parent context** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) · [Epic 1](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · Slice 1 — Movement and position sync |
| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) |
**Delivery:** Ship **`docs/plans/NEON-8-implementation-plan.md`** on the **same branch / PR** as the NEON-8 implementation (plan + code together), matching the NEON-7 pattern.
**Delivery:** Ship **`docs/plans/NEO-11-implementation-plan.md`** on the **same branch / PR** as the NEO-11 implementation (plan + code together), matching the NEO-10 pattern.
## Goal, scope, and out-of-scope
@ -20,9 +20,9 @@
- **Godot:** `NavigationRegion3D` (baked mesh) over the prototype walkable floor in `main.tscn` (obstacle geometry included in bake for future use); **`NavigationAgent3D`** on the player driving **`CharacterBody3D`** motion (`move_and_slide` toward next path position when the client is following the mesh — Godot 4.x pattern).
- **Pick integration:** Reuse **`ground_pick.gd`** (`walkable` group, upward normal); clicked point becomes **`NavigationAgent3D.target_position`** (after any needed Y / agent height adjustment consistent with current capsule).
- **Server authority (unchanged contract):** One **`MoveCommand`** per successful pick with the **same world target** as today ([NEON-4](NEON-4-implementation-plan.md)); server **v1 snap** + **[NEON-7](NEON-7-implementation-plan.md)** validation unchanged.
- **Server authority (unchanged contract):** One **`MoveCommand`** per successful pick with the **same world target** as today ([NEO-7](NEO-7-implementation-plan.md)); server **v1 snap** + **[NEO-10](NEO-10-implementation-plan.md)** validation unchanged.
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Full prediction/reconciliation, MMO-grade netcode, server-side nav mesh validation.
- Replacing **`InteractionRequest`** / range rules.
@ -33,9 +33,9 @@
|--------|----------|
| **When to POST** | **Once per click**, immediately with the **picked destination** (same JSON as today). No waypoint streaming to the server in this story. |
| **Server truth** | Still **snap to target** on **200** after validation; **`GET /position`** after POST remains the reconciliation read (existing `PositionAuthorityClient` flow). |
| **Straight-line vs path** | NEON-7 validates **displacement** from **current authoritative position** to **target** (horizontal + vertical limits). It does **not** check path clearance through geometry. **Client** may follow nav waypoints when the descend bypass is inactive; **automatic obstacle detours are not required** for this slice (see **Locked — prototype movement tradeoff**). |
| **Straight-line vs path** | NEO-10 validates **displacement** from **current authoritative position** to **target** (horizontal + vertical limits). It does **not** check path clearance through geometry. **Client** may follow nav waypoints when the descend bypass is inactive; **automatic obstacle detours are not required** for this slice (see **Locked — prototype movement tradeoff**). |
| **After successful move** | **Do not** teleport the avatar on the verifying **`GET`** when the player is expected to walk: apply the authoritative position as a **navigation goal** and let the agent complete the path; **initial boot** `sync_from_server` remains a **hard snap** so spawn matches the server. |
| **POST 400** | Existing behavior: emit **`move_rejected`**, show NEON-7 UX; **do not** start (or clear) a nav path to that target. |
| **POST 400** | Existing behavior: emit **`move_rejected`**, show NEO-10 UX; **do not** start (or clear) a nav path to that target. |
| **Distance limits** | Default **`MaxHorizontalStep`** (e.g. 18 m) remains compatible with **single-command** long clicks across the prototype; if a future scene needs shorter steps, chain commands or tune limits in a later story. |
## Locked — prototype movement tradeoff
@ -43,13 +43,13 @@
| Topic | Decision |
|--------|----------|
| **Uneven surfaces vs obstacles** | The client uses a **descend bypass** in `player.gd`: when the authoritative goal **Y** is below the body origin (typical for floor picks vs mid-capsule), steering goes **straight in xz** toward the goal instead of following nav waypoints first. This keeps **stepped bumps** smooth; it does **not** guarantee routing around tall **static obstacles** on a **single** click. |
| **Player expectation** | **Multi-click** (or chained moves) is acceptable to navigate around geometry the client would otherwise bee-line into. **No** automatic obstacle navigation is required for NEON-8 closure. |
| **Residual polish** | Visible idle jitter at rest is tracked as tech debt — [NEON-16](https://neon-sprawl.atlassian.net/browse/NEON-16) (not a blocker for this story). |
| **Player expectation** | **Multi-click** (or chained moves) is acceptable to navigate around geometry the client would otherwise bee-line into. **No** automatic obstacle navigation is required for NEO-11 closure. |
| **Residual polish** | Visible idle jitter at rest is tracked as tech debt — [NEO-14](https://linear.app/neon-sprawl/issue/NEO-14) (not a blocker for this story). |
## Acceptance criteria checklist
- [x] Clicking a valid floor target produces **continuous motion** toward the verified target without sliding through geometry in the common case; **navmesh waypoints** are used when the descend bypass does not apply. **Obstacle detours on one click are not required** (tradeoff above).
- [x] Authoritative position still **converges** with the server (`MoveCommand` + `GET`); no regression on NEON-7 rejection UX (`reasonCode` / label timeout).
- [x] Authoritative position still **converges** with the server (`MoveCommand` + `GET`); no regression on NEO-10 rejection UX (`reasonCode` / label timeout).
- [x] Short note in **`server/README.md`** and/or **client script header**: client nav vs server authority for this slice.
## Technical approach
@ -59,7 +59,7 @@
- Bake after layout changes; document any **editor step** in `client/README.md` if bake is not fully reproducible from scene alone.
2. **Player / agent**
- Attach **`NavigationAgent3D`**; replace NEON-2-style **direct steering toward `_goal`** with **path following** (velocity toward `get_next_path_position()` or equivalent, `move_and_slide`).
- Attach **`NavigationAgent3D`**; replace NEO-5-style **direct steering toward `_goal`** with **path following** (velocity toward `get_next_path_position()` or equivalent, `move_and_slide`).
- Keep **`snap_to_server`** for **boot** and any **hard reconcile** case you explicitly document (default: boot only).
3. **Wiring (`main.gd` / `PositionAuthorityClient`)**
@ -103,18 +103,18 @@ Prototype player collider in [`main.tscn`](../../client/scenes/main.tscn): **`Ca
| Path | Rationale |
|------|-----------|
| `client/scenes/main.tscn` | `NavigationRegion3D`, bake source meshes, agent node setup. |
| `client/scripts/player.gd` | Path-follow via `NavigationAgent3D`; clarify header (NEON-2 → NEON-8 evolution). |
| `client/scripts/player.gd` | Path-follow via `NavigationAgent3D`; clarify header (NEO-5 → NEO-11 evolution). |
| `client/scripts/main.gd` | Boot snap vs post-move nav goal wiring. |
| `client/scripts/position_authority_client.gd` | If needed: signal shape or phase so main can snap only on boot. |
| `client/README.md` | Manual check steps for path-follow + server. |
| `server/README.md` | Client nav vs authority note (AC). |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot when NEON-8 ships. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot when NEO-11 ships. |
## Tests
| Action | What to cover |
|--------|----------------|
| **Manual (required)** | Server running; click-move toward targets (including stepped bumps); NEON-7 reject targets — same rejection behavior as before; cold start — player matches server position. Optional: confirm multi-click can route around the prototype **`Obstacle`** when a single click bee-lines. |
| **Manual (required)** | Server running; click-move toward targets (including stepped bumps); NEO-10 reject targets — same rejection behavior as before; cold start — player matches server position. Optional: confirm multi-click can route around the prototype **`Obstacle`** when a single click bee-lines. |
| **Automated** | None required for this story unless an existing headless hook can assert nav baking (unlikely); prefer manual for Godot nav. |
## Open questions / risks
@ -123,4 +123,4 @@ Prototype player collider in [`main.tscn`](../../client/scenes/main.tscn): **`Ca
## PR / review
Cross-check [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), [client README](../../client/README.md), and [server README](../../server/README.md). Confirm NEON-7 props in `main.tscn` still behave after nav region changes.
Cross-check [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), [client README](../../client/README.md), and [server README](../../server/README.md). Confirm NEO-10 props in `main.tscn` still behave after nav region changes.

View File

@ -1,13 +1,13 @@
# NEON-14 — Implementation plan
# NEO-12 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-14 |
| **Key** | NEO-12 |
| **Title** | Set up GDScript unit testing and cover existing client scripts |
| **Jira** | [NEON-14](https://neon-sprawl.atlassian.net/browse/NEON-14) |
| **Parent** | [NEON-13 — Tech Debt](https://neon-sprawl.atlassian.net/browse/NEON-13) |
| **Linear** | [NEO-12](https://linear.app/neon-sprawl/issue/NEO-12) |
| **Parent** | [Tech Debt](https://linear.app/neon-sprawl/project/tech-debt-d2148715e875) |
## Goal, scope, and out-of-scope
@ -26,9 +26,9 @@
**Out of scope**
- **Full integration** tests of `main.gd` / `main.tscn` (navigation bake timing, label timers, full pick pipeline)—**intentionally deferred** here: thin composition + `await` chains are brittle without a dedicated scene test harness; README should state that explicitly (per Jira).
- **Full integration** tests of `main.gd` / `main.tscn` (navigation bake timing, label timers, full pick pipeline)—**intentionally deferred** here: thin composition + `await` chains are brittle without a dedicated scene test harness; README should state that explicitly (per Linear).
- Replacing **server** tests or adding **E2E** browser/gameplay automation.
- Changing **production** gameplay contracts (NEON-4/19/23) except where a test exposes a real bug (unlikely in this story).
- Changing **production** gameplay contracts (NEO-7/23) except where a test exposes a real bug (unlikely in this story).
**Ongoing policy (effective once this story merges):** Any change that **adds or modifies** production GDScript under **`client/scripts/`** (and any future app `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** unit tests under **`client/test/`** in the same change set, unless the PR or plan **explicitly documents** a justified exception (e.g. generated or no-logic glue). **`client/README.md`** should state this expectation for contributors; **[`.cursor/rules/testing-expectations.md`](../../.cursor/rules/testing-expectations.md)** is updated to match.

View File

@ -1,13 +1,13 @@
# NEON-15 — Implementation plan
# NEO-13 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-15 |
| **Key** | NEO-13 |
| **Title** | Dispose NpgsqlDataSource on application shutdown |
| **Jira** | [NEON-15](https://neon-sprawl.atlassian.net/browse/NEON-15) |
| **Parent** | [NEON-13 — Tech Debt](https://neon-sprawl.atlassian.net/browse/NEON-13) |
| **Linear** | [NEO-13](https://linear.app/neon-sprawl/issue/NEO-13) |
| **Parent** | [Tech Debt](https://linear.app/neon-sprawl/project/tech-debt-d2148715e875) |
| **Related** | NS-17 code review — [docs/reviews/2026-03-30-NS-17.md](../reviews/2026-03-30-NS-17.md) (suggestion #2) |
## Goal, scope, and out-of-scope
@ -76,4 +76,4 @@
**Merge / PR description:** Call out **two themes** so reviewers do not miss non-server files: (1) **Server**`NpgsqlDataSourceShutdownHostedService`, explicit `AddSingleton<NpgsqlDataSource>`, and `InMemoryWebApplicationFactory` stripping; (2) **Repo process** — [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md) and cross-links in `AGENTS.md`, `story-kickoff`, code-review, and docs-review agent rules.
Cross-check [decomposition — server / persistence](../decomposition/README.md) if referenced for lifecycle expectations; keep **NEON-15** in commit subjects per [jira-git-naming](../../.cursor/rules/jira-git-naming.md).
Cross-check [decomposition — server / persistence](../decomposition/README.md) if referenced for lifecycle expectations; keep **NEO-13** in commit subjects per [linear-git-naming](../../.cursor/rules/linear-git-naming.md).

View File

@ -1,24 +1,24 @@
# NEON-16 — Implementation plan
# NEO-14 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-16 |
| **Key** | NEO-14 |
| **Title** | Godot client: eliminate CharacterBody3D idle / rest vibration |
| **Jira** | [NEON-16](https://neon-sprawl.atlassian.net/browse/NEON-16) |
| **Parent** | [NEON-13 — Tech Debt](https://neon-sprawl.atlassian.net/browse/NEON-13) |
| **Related** | [NEON-8](https://neon-sprawl.atlassian.net/browse/NEON-8) (path-follow baseline; idle jitter explicitly deferred here) |
| **Linear** | [NEO-14](https://linear.app/neon-sprawl/issue/NEO-14) |
| **Parent** | [Tech Debt](https://linear.app/neon-sprawl/project/tech-debt-d2148715e875) |
| **Related** | [NEO-11](https://linear.app/neon-sprawl/issue/NEO-11) (path-follow baseline; idle jitter explicitly deferred here) |
## Goal, scope, and out-of-scope
**Goal:** Re-open **NEON-16** to restore a **stable rest pose** after later camera work made the prototype jitter worse: no perceptible idle vibration on flat walkable ground, and no continuous **x/z** drift while the avatar has no active walk goal.
**Goal:** Re-open **NEO-14** to restore a **stable rest pose** after later camera work made the prototype jitter worse: no perceptible idle vibration on flat walkable ground, and no continuous **x/z** drift while the avatar has no active walk goal.
**In scope**
- Reproduce the current regression in the prototype scene and separate **true player transform drift** from **camera presentation amplification**.
- Diagnose and fix idle jitter in the prototype **`CharacterBody3D`** player driven by **`player.gd`** and/or the follow rig in **`isometric_follow_camera.gd`** when evidence shows the camera is making tiny corrections read as larger visible jitter.
- Preserve **NEON-8** behavior: walking, arrival at goal, **`snap_to_server`** on boot, descend bypass tradeoff, and **NEON-7** validation UX on the server side (client must not regress those flows).
- Preserve **NEO-11** behavior: walking, arrival at goal, **`snap_to_server`** on boot, descend bypass tradeoff, and **NEO-10** validation UX on the server side (client must not regress those flows).
- Add or update focused **GdUnit** coverage for any new pure helper or guard logic introduced by the fix.
- Document non-obvious fixes in **`client/README.md`** and the living plan if the final solution changes the anti-jitter pattern or camera integration expectations.
@ -31,20 +31,20 @@
## Acceptance criteria checklist
- [ ] At rest (no click-to-move goal, feet on floor), **no visible vibration** in the default prototype scene for **several seconds** of observation on flat ground, and no measurable-looking **x/z** drift at idle.
- [ ] **Walking**, **arrival**, and **`snap_to_server`** still behave correctly; **no regression** on NEON-7 bumps or the **NEON-8** obstacle / descend-bypass tradeoff documented in **`client/README.md`**.
- [ ] **Walking**, **arrival**, and **`snap_to_server`** still behave correctly; **no regression** on NEO-10 bumps or the **NEO-11** obstacle / descend-bypass tradeoff documented in **`client/README.md`**.
- [ ] If camera-follow hardening is required, it does **not** introduce new visible lag, framing drift, or occlusion regressions in the current E1.M2 stack.
- [ ] If a **scene** or **project** setting change is required, it is **checked in** and called out for designers (README or short comment in scene notes).
## Resolution
**Re-opened 2026-04-09 after NEON-25/26/27/30 follow-camera work:** the earlier NEON-16 mitigation shipped, but testing now reports worse visible idle jitter even on a flat surface. The current branch treats that as **real idle correction noise first**, not purely a camera problem.
**Re-opened 2026-04-09 after NEO-15/26/27/30 follow-camera work:** the earlier NEO-14 mitigation shipped, but testing now reports worse visible idle jitter even on a flat surface. The current branch treats that as **real idle correction noise first**, not purely a camera problem.
**Implemented on this branch:** when the capsule already has **stable flat support** (floor normal effectively upright and no post-arrival loose-floor window), **`player.gd`** now skips the corrective idle **`move_and_slide()`** / rim-escape loop entirely and keeps the body parked. The flat-idle classification was broadened so incidental extra contacts on an otherwise flat support do not re-enable the old correction loop. Stable grounded idle also keeps an explicit **x/z anchor**, clamping away any residual horizontal creep until movement resumes or support is no longer classified as stable. Goal arrival and the descend-bypass check now both measure against the capsule **feet height** instead of the body origin, which stops same-height floor clicks near obstacles from dropping into the old straight-line wall push. **`_snap_capsule_upright()`** also avoids rewriting **`global_transform`** when the basis is already identity, reducing needless transform churn during idle.
## Technical approach
1. **Reproduce and isolate** in Godot 4.x editor: Remote **Transform** on the player and camera rig, **Visible Collision Shapes**, and short idle observation on flat ground first, then random bumps, to determine whether the player root is really moving in **x/z**.
2. **CharacterBody3D / physics** — inspect the current idle path in **`player.gd`**: idle **`move_and_slide()`**, rim settle, bump proximity escape, and upright snap. Prevent continuous idle correction on stable flat contacts while preserving the original bump / rim escape behavior that NEON-16 added for rougher geometry.
2. **CharacterBody3D / physics** — inspect the current idle path in **`player.gd`**: idle **`move_and_slide()`**, rim settle, bump proximity escape, and upright snap. Prevent continuous idle correction on stable flat contacts while preserving the original bump / rim escape behavior that NEO-14 added for rougher geometry.
3. **Camera / parenting** — if the player still only moves by tiny amounts, harden **`isometric_follow_camera.gd`** so the rig does not magnify sub-threshold target motion. Preferred direction: smooth or deadzone the **focus** used by **`desired_eye_world()`**, **`look_at()`**, and occlusion instead of smoothing only the eye position.
4. **Navigation agent** — confirm **`NavigationAgent3D`** rest targets (`set_target_position(global_position)` on clear/arrive) are not reintroducing path churn that feeds the idle correction loop.
5. **Ship** the smallest change that clears the AC, then update **`client/README.md`** and related docs with the final anti-jitter pattern and verification notes.
@ -53,7 +53,7 @@
| Topic | Choice | Rationale |
|------|---------|-----------|
| **Regression framing** | Treat this as a **re-opened NEON-16** fix, not a brand new camera story. | Jira already tracks idle vibration, and the new symptom is the same bug surface with later camera work as a likely amplifier. |
| **Regression framing** | Treat this as a **re-opened NEO-14** fix, not a brand new camera story. | Linear already tracks idle vibration (NEO-14), and the new symptom is the same bug surface with later camera work as a likely amplifier. |
| **Debug order** | Prove whether **player transform drift** exists before shipping a camera-only mitigation. | A camera-only band-aid could hide a real **`CharacterBody3D`** rest-state bug. |
| **Camera hardening scope** | Only harden follow behavior if player-side cleanup is insufficient or if raw focus tracking is clearly amplifying tiny motion. | Keeps the fix minimal and avoids reopening broader E1.M2 follow-camera design work. |
| **Flat idle strategy** | Detect **stable flat support** and skip corrective idle movement in that state. | The regression report is specifically about persistent idle **x/z** drift on a flat surface, where the old rim/bump correction loop should not be necessary. |
@ -80,7 +80,7 @@
| Action | What to cover |
|--------|----------------|
| **Manual (required)** | Run main scene with server as in **NEON-4/NEON-8** README steps: stand idle **10+ s** at spawn on flat ground, stand idle again after a move, idle on the random green bumps, click-move including **stepped bumps** (NEON-7 props), cold boot **`snap_to_server`**, far/vertical rejects, and NEON-30 occluder click-through sanity. |
| **Manual (required)** | Run main scene with server as in **NEO-7/NEO-11** README steps: stand idle **10+ s** at spawn on flat ground, stand idle again after a move, idle on the random green bumps, click-move including **stepped bumps** (NEO-10 props), cold boot **`snap_to_server`**, far/vertical rejects, and NEO-20 occluder click-through sanity. |
| **Automated** | Updated **`client/test/player_test.gd`** with stable-idle plus capsule-feet helper coverage. Headless GdUnit run: **52 tests, 0 errors, 0 failures** via Godot **4.6** and **`res://addons/gdUnit4/bin/GdUnitCmdTool.gd`**. One-off headless idle probe against **`main.tscn`** observed **`x_range=0`** and **`z_range=0`** across a **240-physics-frame** idle sample after settle. |
## Open questions / risks
@ -92,4 +92,4 @@
## PR / review
Cross-check [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), **`docs/plans/NEON-8-implementation-plan.md`** (residual polish pointer), and **`client/README.md`** after changes.
Cross-check [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), **`docs/plans/NEO-11-implementation-plan.md`** (residual polish pointer), and **`client/README.md`** after changes.

View File

@ -1,14 +1,14 @@
# NEON-25 — Implementation plan
# NEO-15 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-25 |
| **Key** | NEO-15 |
| **Title** | E1.M2: Isometric follow camera (fixed yaw prototype; CameraState seam) |
| **Jira** | [NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25) |
| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
| **Linear** | [NEO-15](https://linear.app/neon-sprawl/issue/NEO-15) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
## Goal, scope, and out-of-scope
@ -18,7 +18,7 @@
- Godot: `Camera3D` (on a small rig) for isometric presentation; follow the player / follow target each frame per **locked defaults** below (smooth follow + teleport snap).
- **Player-facing controls:** no camera rotate / orbit; **yaw stays at default** (`0` rad) for normal play.
- **`CameraState`** (dedicated script type): follow target reference, **yaw** (always `0` in prototype UX), default distance or zoom index (**single band** / scalar distance until NEON-26).
- **`CameraState`** (dedicated script type): follow target reference, **yaw** (always `0` in prototype UX), default distance or zoom index (**single band** / scalar distance until NEO-16).
- **Seam:** `@export` or config fields (e.g. `allow_yaw`, `max_yaw_deg`) default off; short comment where orbit input would apply.
- **Semantics:** movement and picking remain **world-anchored** (existing `ground_pick` + nav); camera does not redefine aim axes.
- Authority: camera pose stays **client-only** (see `docs/decomposition/modules/client_server_authority.md`).
@ -26,8 +26,8 @@
**Out of scope**
- Binding rotate input or shipping orbit UX.
- Multiple zoom bands (NEON-26).
- Occlusion policy (NEON-27).
- Multiple zoom bands (NEO-16).
- Occlusion policy (NEO-17).
- Server use of camera pose.
## Acceptance criteria checklist
@ -50,7 +50,7 @@
## Technical approach
1. **Introduce `CameraState`** as a small **RefCounted** (or equivalent) script under `res://scripts/` with explicit fields: follow target path (or resolved node identity in state for debug), **yaw** (`float`, default `0`), **distance** or zoom index placeholder for NEON-26, and room for future flags. Avoid `class_name` so headless CI matches [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md) guidance; tests can `preload` the script.
1. **Introduce `CameraState`** as a small **RefCounted** (or equivalent) script under `res://scripts/` with explicit fields: follow target path (or resolved node identity in state for debug), **yaw** (`float`, default `0`), **distance** or zoom index placeholder for NEO-16, and room for future flags. Avoid `class_name` so headless CI matches [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md) guidance; tests can `preload` the script.
2. **Isometric rig** — add a **`Node3D`** (e.g. `IsometricFollowCamera` or `CameraRig`) with script that:
- Owns or references the active **`Camera3D`** (`current = true`).
- Each **`_process`**: resolve **`NodePath`** follow target, read **`global_position`**, compute desired eye position from **fixed pitch** + **effective yaw** (forced to `0` when `allow_yaw` is false), **lerp** rig toward target+offset; **snap** rig/camera when separation exceeds a configured threshold.
@ -76,7 +76,7 @@ Godot may add companion `.uid` files when scripts are first touched in the edito
| `client/scenes/main.tscn` | Replace fixed `Camera3D` transform with rig + follow target wiring; keep `current` camera and layers consistent with picking. |
| `client/scripts/main.gd` | Point `_camera` (and `ground_pick.fallback_camera`) at the rigs **`Camera3D`**; minimal diff. |
| `client/README.md` | One short paragraph: follow camera behavior, fixed yaw prototype, where to tune distance/pitch. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Implementation snapshot** (date + NEON-25): rig script names, yaw seam, single-band distance until NEON-26. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Implementation snapshot** (date + NEO-15): rig script names, yaw seam, single-band distance until NEO-16. |
## Tests
@ -85,7 +85,7 @@ Godot may add companion `.uid` files when scripts are first touched in the edito
| `client/test/camera_state_test.gd` | **New** — default **yaw** is `0`; fields survive assign/read; optional: follow target path assignment (if exposed). |
| `client/test/isometric_follow_camera_test.gd` | **New** — if pure helpers are extracted (e.g. static offset from pitch/distance/yaw), unit-test them; otherwise minimal suite: instantiate rig + dummy `Node3D` target, advance one frame, assert camera global position moves toward expected quadrant (tolerance-based). If full follow is too scene-heavy, document **manual** verification as primary and keep **camera_state** tests as the automated baseline. |
**Manual (required):** Run main scene with server per README: click-move across floor and **NEON-7** bumps; confirm avatar stays framed, no user-driven rotation, ground pick still hits walkable surfaces from the moving camera.
**Manual (required):** Run main scene with server per README: click-move across floor and **NEO-10** bumps; confirm avatar stays framed, no user-driven rotation, ground pick still hits walkable surfaces from the moving camera.
## Open questions / risks

View File

@ -1,18 +1,18 @@
# NEON-26 — Implementation plan
# NEO-16 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-26 |
| **Key** | NEO-16 |
| **Title** | E1.M2: ZoomBandConfig + discrete zoom input (clamped) |
| **Jira** | [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26) |
| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
| **Linear** | [NEO-16](https://linear.app/neon-sprawl/issue/NEO-16) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
## Goal, scope, and out-of-scope
**Goal:** Add **data-driven discrete zoom bands** so the player can adjust framing within limits; **`CameraState`** reports the active band and **effective distance** while **preserving yaw** and the NEON-25 orbit seam.
**Goal:** Add **data-driven discrete zoom bands** so the player can adjust framing within limits; **`CameraState`** reports the active band and **effective distance** while **preserving yaw** and the NEO-15 orbit seam.
**In scope**
@ -23,24 +23,24 @@
- **Designer tuning:** one **Resource** asset (e.g. `res://resources/...tres` or inline default) editable in the inspector; rig points at it.
- **Optional:** stub signal / throttled `print` / `TODO(E9.M1)` for `camera_zoom_changed` telemetry when a schema exists.
**Out of scope** (per Jira)
**Out of scope** (per Linear)
- Smooth analog zoom between bands (unless a one-line lerp is explicitly chosen later — default **no**).
- Occlusion policy (**NEON-27**).
- Yaw orbit input (**NEON-25** seam stays; do not bind rotate).
- Occlusion policy (**NEO-17**).
- Yaw orbit input (**NEO-15** seam stays; do not bind rotate).
## Acceptance criteria checklist
- [x] Zoom input only moves between **defined bands** (clamped; no out-of-range index).
- [x] At each band, player remains **readably framed** during motion (manual playtest + distances chosen around current NEON-25 baseline).
- [x] At each band, player remains **readably framed** during motion (manual playtest + distances chosen around current NEO-15 baseline).
- [x] `CameraState` reflects **active band** and **effective distance**, and still exposes **yaw** unchanged in meaning.
- [x] Config lives in **one tunable place** for designers (`ZoomBandConfig` resource referenced by the rig).
## Technical approach
1. **`zoom_band_config.gd`** — `extends Resource` with e.g. `@export var band_distances: PackedFloat32Array` (length ≥ 1) and `@export var default_band_index: int`. Provide **`band_count`**, **`clamp_index(i: int) -> int`**, **`distance_at(index: int) -> float`** (uses clamped index). Validate in `_validate_property` or getter: if empty at runtime, treat as single band using rig fallback (see below).
2. **`isometric_follow_camera.gd`** — Add **`@export var zoom_band_config: Resource`** (no `class_name` on the config script); at runtime require **`get_script() == preload("…/zoom_band_config.gd")`** so headless CI and typed exports stay aligned. Conceptually this is **`ZoomBandConfig`**. Keep **`follow_distance`** as **fallback** when config is null, empty, or has nonpositive band distances (preserves NEON-25 scenes/tests). Track **`_zoom_band_index`** initialized from config default (clamped). On zoom-in/out actions: adjust index with `clamp_index`, no op at ends (or wrap — **clamp** per story). **`_process`:** compute effective distance from config + index or fallback export. Replace uses of raw `follow_distance` in `desired_eye_world` with that value. **`_sync_camera_state`:** pass effective distance into state (`zoom_band_index` + `distance`, plus existing yaw / path / focus).
3. **`project.godot`** — Add **`camera_zoom_in`** and **`camera_zoom_out`** (or one action with axis — prefer two actions for wheel + keys) with mouse wheel and key events as in Jira examples.
2. **`isometric_follow_camera.gd`** — Add **`@export var zoom_band_config: Resource`** (no `class_name` on the config script); at runtime require **`get_script() == preload("…/zoom_band_config.gd")`** so headless CI and typed exports stay aligned. Conceptually this is **`ZoomBandConfig`**. Keep **`follow_distance`** as **fallback** when config is null, empty, or has nonpositive band distances (preserves NEO-15 scenes/tests). Track **`_zoom_band_index`** initialized from config default (clamped). On zoom-in/out actions: adjust index with `clamp_index`, no op at ends (or wrap — **clamp** per story). **`_process`:** compute effective distance from config + index or fallback export. Replace uses of raw `follow_distance` in `desired_eye_world` with that value. **`_sync_camera_state`:** pass effective distance into state (`zoom_band_index` + `distance`, plus existing yaw / path / focus).
3. **`project.godot`** — Add **`camera_zoom_in`** and **`camera_zoom_out`** (or one action with axis — prefer two actions for wheel + keys) with mouse wheel and key events as in the Linear issue / product notes.
4. **Scene / resource** — Add a default **`ZoomBandConfig`** `.tres` under `res://resources/` (or co-located under `client/resources/`) with **≥ 3** bands bracketing **~25.709** m so readability can be checked; assign on **`IsometricFollowCamera`** in `main.tscn`.
5. **Telemetry stub** — Optional `signal zoom_band_changed(new_index: int, distance: float)` or `TODO` comment referencing E9.M1; no external schema required for this story.
@ -48,7 +48,7 @@
| Topic | Choice |
|--------|--------|
| **Band parameter** | **Distance-only** per step (same pitch / FOV as NEON-25); document in module snapshot. FOV-per-band deferred unless trivial. |
| **Band parameter** | **Distance-only** per step (same pitch / FOV as NEO-15); document in module snapshot. FOV-per-band deferred unless trivial. |
| **Bounds at ends** | **Clamp** (no wrap): input at min/max does nothing extra. |
| **Config null / empty** | Use **`follow_distance`** export only; **`zoom_band_index`** in state = `0`. |
| **Invalid band values** | Any **≤ 0** distance in **`band_distances`** treats the config as unusable (same fallback as null/empty). |
@ -68,7 +68,7 @@
| `client/scripts/camera_state.gd` | Tighten comments only if needed so **`distance`** = effective follow distance and **`zoom_band_index`** = active band are unambiguous (fields already exist). |
| `client/project.godot` | Register **`camera_zoom_in`** / **`camera_zoom_out`** input actions. |
| `client/scenes/main.tscn` | Assign **`zoom_band_config`** on the follow rig to the new `.tres` (or leave null to exercise fallback — prefer assigned for acceptance “designer tunable”). |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Update **implementation snapshot** for NEON-26: `ZoomBandConfig` path, input actions, discrete bands replace “single scalar until NEON-26”. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Update **implementation snapshot** for NEO-16: `ZoomBandConfig` path, input actions, discrete bands replace “single scalar until NEO-16”. |
## Tests

View File

@ -1,14 +1,14 @@
# NEON-27 — Implementation plan
# NEO-17 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-27 |
| **Key** | NEO-17 |
| **Title** | E1.M2: OcclusionPolicy — keep player readable through geometry |
| **Jira** | [NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27) |
| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
| **Linear** | [NEO-17](https://linear.app/neon-sprawl/issue/NEO-17) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
## Goal, scope, and out-of-scope
@ -23,7 +23,7 @@
- Optional perf marker: when `occluder_count_log_threshold > 0` and the active occluder count meets or exceeds the threshold, emit `push_warning` for later stress-test reference.
- Update `E1_M2_IsometricCameraController.md` with an implementation snapshot and the readability risk gate note required by acceptance criteria.
**Out of scope** (per Jira)
**Out of scope** (per Linear)
- Perfect transparency for all scene assets; full art pipeline integration.
- Server awareness of occlusion.
@ -84,7 +84,7 @@ Restore by setting `surface_override_material` back to the saved value (or `null
4. **`main.tscn`** — Add `"occluder"` group to the `Obstacle` node; assign `occlusion_policy` resource on `IsometricFollowCamera`.
5. **`E1_M2_IsometricCameraController.md`** — Add NEON-27 implementation snapshot; add readability risk gate note (links to acceptance criteria: before-/after screenshot in PR required at prototype demo gate).
5. **`E1_M2_IsometricCameraController.md`** — Add NEO-17 implementation snapshot; add readability risk gate note (links to acceptance criteria: before-/after screenshot in PR required at prototype demo gate).
## Decisions
@ -110,7 +110,7 @@ Restore by setting `surface_override_material` back to the saved value (or `null
| `client/scripts/isometric_follow_camera.gd` | Add `occlusion_policy` export, `_update_occlusion` pipeline, occluder override state, fade/restore helpers, `_occlusion_policy_valid` guard. |
| `client/scenes/main.tscn` | Tag `Obstacle` with `"occluder"` group; assign `isometric_occlusion_policy.tres` on `IsometricFollowCamera`. |
| `client/test/isometric_follow_camera_test.gd` | Add test for `_occlusion_policy_valid`-equivalent static path (null policy → false; wrong-script resource → false; valid resource → true) via a static helper if possible; otherwise document as manual-only. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Add NEON-27 implementation snapshot; add readability risk gate note per AC. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Add NEO-17 implementation snapshot; add readability risk gate note per AC. |
## Tests
@ -126,5 +126,5 @@ Restore by setting `surface_override_material` back to the saved value (or `null
- **Rapid toggle at wall edges:** with instant override, a player sidestepping at the edge of an occluder may cause per-frame toggle. This is visually acceptable for prototype; if it proves distracting, a one-frame hysteresis (require N consecutive frames clear before restoring) can be added without a structural change.
- **Shared vs. per-instance materials:** if the `Obstacle` in the future uses a shared `StandardMaterial3D` from the mesh resource directly (no override set), `duplicate()` ensures we don't mutate the shared asset. This is already handled by the override strategy above, but must be verified at runtime.
- **`max_occluder_cast_depth` performance:** each `intersect_ray` call with growing exclusion list is O(scene complexity). Cap of 4 is conservative for the prototype district; tune if the perf log fires.
- **Scene grows (NEON-29):** when the prototype district expands (NEON-29), add representative occluders to the `"occluder"` group and rerun the manual verification pass.
- **Scene grows (NEO-19):** when the prototype district expands (NEO-19), add representative occluders to the `"occluder"` group and rerun the manual verification pass.
- **Readability gate:** the risk documented in the module doc (occlusion hiding telegraphs) must be surfaced in the prototype demo checklist. Before-/after evidence in this PR's description serves as the first data point.

View File

@ -1,27 +1,27 @@
# NEON-28 — Implementation plan
# NEO-18 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-28 |
| **Key** | NEO-18 |
| **Title** | E1.M2: Camera integration hardening + dependent contract notes |
| **Jira** | [NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28) |
| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
| **Linear** | [NEO-18](https://linear.app/neon-sprawl/issue/NEO-18) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
**Jira source:** Description and acceptance criteria were pulled from Jira via Atlassian MCP (cloud `neon-sprawl`, issue **NEON-28**). This plan is the working implementation breakdown; if Jira edits after this date, re-sync the checklist.
**Linear source:** Description and acceptance criteria were pulled from Linear (issue **NEO-18**). This plan is the working implementation breakdown; if the Linear issue edits after this date, re-sync the checklist.
**Board:** Transitioned **To Do → In Progress** on kickoff (Atlassian MCP `transitionJiraIssue`, transition id `2` / name `prog`).
**Board:** Transitioned **To Do → In Progress** on kickoff (Linear MCP `save_issue` with **`state`** **In Progress**).
## Goal, scope, and out-of-scope
**Goal (from Jira):** Close the **E1.M2 prototype slice** with stable contracts, documentation, and hooks so **E6.M2** (and other dependents) can rely on `CameraState` and presentation assumptions without spelunking implementation.
**Goal (from Linear):** Close the **E1.M2 prototype slice** with stable contracts, documentation, and hooks so **E6.M2** (and other dependents) can rely on `CameraState` and presentation assumptions without spelunking implementation.
**In scope (from Jira)**
**In scope (from Linear)**
- **Finalize `CameraState`:** follow target, **yaw** (prototype `0`; document semantics if orbit is enabled mid-project), zoom band / effective framing, **policy flags as needed** (add fields only when the documented consumer contract requires them; otherwise state explicitly what is **not** in state and where to read it on the rig).
- **Implementation snapshot in module doc:** **Mid-project rotation policy** and **post-release freeze** from `E1_M2_IsometricCameraController.md` (prototype **fixed yaw for players**; optional `allow_yaw` / limits seam; world-anchored gameplay preferred; telegraph readability). **Cross-link [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10)** / module backlog for traceability.
- **Implementation snapshot in module doc:** **Mid-project rotation policy** and **post-release freeze** from `E1_M2_IsometricCameraController.md` (prototype **fixed yaw for players**; optional `allow_yaw` / limits seam; world-anchored gameplay preferred; telegraph readability). **Cross-link [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395)** / module backlog for traceability.
- **Client-local authority** and **fixed pitch/roll** for isometric presentation — align [client_server_authority.md](../decomposition/modules/client_server_authority.md) references in E1.M2 / E6.M2 notes.
- **Optional telemetry:** document hooks for throttled `camera_zoom_changed` and occlusion/perf counters **when E9.M1** schema exists (no requirement to implement telemetry in this story unless already trivial TODO cleanup).
- **Dependency register + alignment:** E1.M2 **Ready** (or agreed status) with pointers to scripts/scenes per acceptance criteria.
@ -30,21 +30,21 @@
- **`IsometricFollowCamera` occluder lifecycle:** `_occluder_overrides` must not **leak or fault** when occluder bodies are **freed** or invalid; **erase** stale dict keys without calling `set_surface_override_material` on a freed subtree (that errors); live bodies still restore when they leave the ray.
**Out of scope (from Jira)**
**Out of scope (from Linear)**
- New gameplay features; **E1.M3 / E1.M4** work.
- Committing to **yaw orbit in UX** without product sign-off (policy allows mid-project enablement only).
## Acceptance criteria checklist
Jira acceptance criteria (verbatim intent):
Acceptance criteria (verbatim intent):
- [x] **Module dependency register / module doc:** E1.M2 **Ready** (or agreed status) with pointers to scripts/scenes.
- [x] **`CameraState` contract** summarized in one place and matches module doc (**includes yaw**).
- [x] **E6.M2 note:** what consumers may read vs undefined; how **yaw** affects camera-adjacent UX if non-zero later.
- [x] **Epic 1 Slice 2** acceptance satisfied: prototype keeps **yaw fixed for players**, zoom within bands, readable motion, occlusion policy active; aligns with updated module wording (not “camera has no yaw,” but **yaw not exposed / remains default in UX**).
**Supporting engineering checks (not a substitute for Jira AC):**
**Supporting engineering checks (not a substitute for written acceptance criteria):**
- [x] Occluder invalid-key purge covered by tests or documented manual steps (see [Tests](#tests)).
- [x] [epic_01_core_player_runtime.md](../decomposition/epics/epic_01_core_player_runtime.md) **Slice 2** bullets use the same **yaw / presentation** language as the module (replace over-simplified “no rotation” if it contradicts the rotation policy).
@ -53,10 +53,10 @@ Jira acceptance criteria (verbatim intent):
1. **Contract documentation (primary)**
- **`camera_state.gd`:** Expand file header (and/or class-level comments) so it is a **concise, accurate** summary of fields: `yaw` = orbit delta vs presentation compass; `distance`; `zoom_band_index`; `focus_world`; `follow_target_path`; refresh **every `_physics_process`** tick on the rig (**`process_physics_priority`** after `Player` — see **Decisions**).
- **`E1_M2_IsometricCameraController.md`:** Single **Consumer contract** subsection: table linking each `CameraState` field + rig exports (`allow_yaw`, `presentation_yaw_deg`, resources). Include **mid-project rotation** + **post-release freeze** pointers; link **NEON-10**, **NEON-25NEON-27**, **NEON-30** as appropriate.
- **`E1_M2_IsometricCameraController.md`:** Single **Consumer contract** subsection: table linking each `CameraState` field + rig exports (`allow_yaw`, `presentation_yaw_deg`, resources). Include **mid-project rotation** + **post-release freeze** pointers; link **E1.M2**, **NEO-15NEO-17**, **NEO-20** as appropriate.
- **`E6_M2_ConsentAndRiskUxSignals.md`:** **E1.M2 camera adjacency** — allowed reads (`CameraState` fields), **undefined** (what stays on rig only), **yaw non-zero later** (presentation / HUD implications in one short paragraph); link authority (server must not use camera pose for gameplay).
- **`module_dependency_register.md`:** Set E1.M2 **Status** to **Ready** (or agreed label) and replace “NEON-28 remains” narrative with **shipped** wording + script/scene pointers.
- **`documentation_and_implementation_alignment.md`:** E1.M2 row matches register + NEON-28 completion.
- **`module_dependency_register.md`:** Set E1.M2 **Status** to **Ready** (or agreed label) and replace “NEO-18 remains” narrative with **shipped** wording + script/scene pointers.
- **`documentation_and_implementation_alignment.md`:** E1.M2 row matches register + NEO-18 completion.
- **`epic_01_core_player_runtime.md`:** Adjust **Slice 2** scope/acceptance lines so they reflect **fixed yaw in UX** and documented orbit seam, not flat “camera never rotates” if that misleads.
2. **`CameraState` “finalize”**
@ -75,15 +75,15 @@ Jira acceptance criteria (verbatim intent):
| Topic | Choice | Rationale |
|-------|--------|-----------|
| **`CameraState` policy flags** | **Doc-first** | Add fields only if Jira/E6.M2 contract text requires a snapshot; avoid speculative API. |
| **`CameraState` policy flags** | **Doc-first** | Add fields only if E6.M2 contract text requires a snapshot; avoid speculative API. |
| **Invalid occluder keys** | **Erase only** | Freed body ⇒ subtree is gone; restoring overrides hits freed `MeshInstance3D` / materials. Drop dict entry; live bodies restore when they leave the ray. |
| **Epic Slice 2 wording** | **Align with rotation policy** | Satisfies Jira AC on “yaw not exposed / default in UX” vs misleading “no rotation.” |
| **Epic Slice 2 wording** | **Align with rotation policy** | Satisfies acceptance criteria on “yaw not exposed / default in UX” vs misleading “no rotation.” |
| **Rig tick (jitter follow-up)** | **`_physics_process` + `process_physics_priority = 1`** | `Player` default **0** runs before **1**; camera reads post`move_and_slide`. Avoids `_process` vs 120Hz physics jitter (camera was above `Player` in scene order). |
| **Runtime “remove obstacle” + `NavigationAgent3D`** | **Reparent out of `NavigationRegion3D`, then rebake + sync agent** | Default `NavigationMesh.geometry_parsed_geometry_type` is **`PARSED_GEOMETRY_BOTH`**: **mesh instances** under the region are baked as source geometry alongside static colliders. Disabling collision (or rebaking alone) does **not** reopen nav if the bodys **`MeshInstance3D` remains a child of the region** — the hole persists. For any **gameplay mechanic that removes or opens geometry** while using baked nav, either **exclude that mesh from source** (project/settings), **reparent** the subtree **outside** the region before rebake, **`queue_free`** (if safe), or switch to an explicit geometry pipeline. Prototype dev toggle: `main.gd` reparents `Obstacle` to `World` when hidden, rebakes, `player.sync_navigation_agent_after_map_rebuild`. |
### Runtime navigation (lesson for future mechanics)
If the client uses **`NavigationRegion3D`** with default parsing (**colliders + meshes**), **hiding** or **disabling collision** on a prop is **not** enough for **click-to-move** to cross that tile: **`bake_navigation_mesh`** will still see **`MeshInstance3D`** children of the region. **Rebake + `NavigationAgent3D` map sync** only fixes the path once that geometry is **no longer under the region** (or no longer parsed). Plan any **destructible door**, **removed barricade**, or **temporary wall** with this in mind — same pitfall as NEON-28 dev smoke.
If the client uses **`NavigationRegion3D`** with default parsing (**colliders + meshes**), **hiding** or **disabling collision** on a prop is **not** enough for **click-to-move** to cross that tile: **`bake_navigation_mesh`** will still see **`MeshInstance3D`** children of the region. **Rebake + `NavigationAgent3D` map sync** only fixes the path once that geometry is **no longer under the region** (or no longer parsed). Plan any **destructible door**, **removed barricade**, or **temporary wall** with this in mind — same pitfall as NEO-18 dev smoke.
## Files to add
@ -94,8 +94,8 @@ If the client uses **`NavigationRegion3D`** with default parsing (**colliders +
| Path | Rationale |
|------|-----------|
| `client/scripts/camera_state.gd` | Finalize field documentation; optional policy flags if contract requires. |
| `client/scripts/isometric_follow_camera.gd` | Occluder invalid-key purge; keep TODO(E9.M1) aligned with Jira optional telemetry note. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Consumer contract, NEON-28 snapshot, NEON-10 / rotation policy cross-links. |
| `client/scripts/isometric_follow_camera.gd` | Occluder invalid-key purge; keep TODO(E9.M1) aligned with Linear optional telemetry note. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Consumer contract, NEO-18 snapshot, E1.M2 / rotation policy cross-links. |
| `docs/decomposition/modules/E6_M2_ConsentAndRiskUxSignals.md` | E1.M2 adjacency + yaw UX note + authority. |
| `docs/decomposition/modules/module_dependency_register.md` | E1.M2 Ready + pointers; remove “hardening remains.” |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E1.M2 row updated. |
@ -121,6 +121,6 @@ If the client uses **`NavigationRegion3D`** with default parsing (**colliders +
## Shipped (2026-04-10)
- `camera_state.gd` consumer contract header; `isometric_follow_camera.gd` invalid occluder purge + `occluder_override_key_is_valid`; tests in `isometric_follow_camera_test.gd`.
- Docs: `E1_M2_IsometricCameraController.md` (Ready, consumer contract, NEON-28 snapshot), `E6_M2_ConsentAndRiskUxSignals.md` (E1.M2 adjacency), `module_dependency_register.md`, `documentation_and_implementation_alignment.md`, `epic_01_core_player_runtime.md` Slice 2.
- Docs: `E1_M2_IsometricCameraController.md` (Ready, consumer contract, NEO-18 snapshot), `E6_M2_ConsentAndRiskUxSignals.md` (E1.M2 adjacency), `module_dependency_register.md`, `documentation_and_implementation_alignment.md`, `epic_01_core_player_runtime.md` Slice 2.
- Follow-up: rig **`_physics_process`** + priority (see **Decisions**).
- Client dev smoke (`main.gd`): obstacle toggle + **nav**: reparent out of `NavigationRegion3D`, rebake, agent sync — documents the **`PARSED_GEOMETRY_BOTH`** / mesh-as-source pitfall for future **runtime geometry removal** (see **Decisions** and **Runtime navigation**).

View File

@ -0,0 +1,130 @@
# NEO-19 Implementation Plan — Expand prototype client district for camera/nav stress QA
## Story reference
- **Key:** [NEO-19](https://linear.app/neon-sprawl/issue/NEO-19)
- **Title:** E1.M2: Expand prototype client district for camera/nav stress QA
- **Parent epic:** [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016)
- **Labels:** E1, E1.M2, client-qa-district, decomposition
---
## Goal, scope, and out-of-scope
### Goal
Provide a **~5× footprint** (order-of-magnitude) prototype play space versus the current district, with **more obstacles** and **height-changing regions** (steps, terraces, bumps) so day-to-day client work can stress **motion, framing, and readability** for the camera, navigation, ground pick, and interaction systems.
### Scope
- `client/scenes/main.tscn`: expand walkable floor / nav source geometry; add props that participate in `NavigationRegion3D` bake and `walkable` picking as today; include elevated terrace/step platforms for vertical variation.
- `client/README.md`: short runbook note covering district scale, intent, and any new designer-facing limits.
- Optional: pointer in `docs/decomposition/modules/E1_M2_IsometricCameraController.md` that this geography is the default stress playground for client stories.
### Out of scope
- New camera features (zoom bands NEO-16, occlusion NEO-17) — geography may help test them, but this story does not implement them.
- Server schema or MoveCommand contract changes unless existing NEO-10 step/bounds defaults visibly break on the larger map (in that case: document + minimal config tune only).
- Production art pass or zone graph / Epic 4 content.
---
## Acceptance criteria checklist
- [ ] Playable area is **materially larger** (~5× order of magnitude vs pre-change footprint) with **additional obstacles** and **vertical variation** usable with click-to-move.
- [ ] **NavigationRegion3D** bakes successfully at startup; long clicks behave per existing NEO-11 tradeoffs (document if new geometry exposes new edge cases).
- [ ] **Ground pick** and `walkable` grouping remain correct on new surfaces.
- [ ] README updated for anyone running F5 + server.
---
## Technical approach
### Current baseline
The current `main.tscn` district is a single **20 × 20** unit flat floor (`BoxMesh_floor`, placed at Y = -0.1), giving an approximate playable area of ~400 sq units. Static props:
- `PrototypeTerminal` (0.9 × 1 × 0.4) at origin
- `MoveRejectPedestal` (1.5 × 2.5 × 1.5) at (7.5, 0, -6.5)
- `MoveRejectFarPad` (2.5 × 0.12 × 2.5) at (9, 0.06, 9)
- `Obstacle` (2 × 2 × 2, `occluder` group) at (6, 0, 5)
- Player spawn at (-5, 0.9, -5)
The `NavigationMesh` stored in the scene file is intentionally stale; `main.gd` calls `bake_navigation_mesh(false)` at startup so the live mesh always reflects actual collision geometry.
### Expansion plan
**Floor size:** Replace the 20 × 20 floor with a **45 × 45** unit floor, giving ~2025 sq units (~5× of 400). Keep floor mesh center at origin (X=0, Z=0) so spawn position and existing props stay roughly central.
**Additional obstacles (nav stress + camera occlusion):** Add 34 extra `StaticBody3D` nodes in the `occluder` group, distributed across the expanded area — positioned so the follow camera must work around varying angles.
**Vertical variation (terraces / steps):** Add 23 raised terrace platforms as `StaticBody3D` nodes in the `walkable` group, elevated ~0.52.5 m above the main floor, with ramp/step approach geometry. These are `StaticBody3D` siblings of `Floor` under `NavigationRegion3D` so the runtime bake includes them as nav sources. Each terrace:
- Has its own `CollisionShape3D` (BoxShape3D).
- Is in the `walkable` group so ground-pick raycasts hit it correctly.
- Has a small `StaticBody3D` ramp or step approach (also `walkable`) so `NavigationAgent3D` can route onto it within the NEO-11 step/climb parameters (`agent_max_climb = 0.35` in existing NavMesh settings — ramp grade must respect this, or use multi-step approach).
**MoveReject props:** Keep `MoveRejectPedestal` and `MoveRejectFarPad` in place — they exercise NEO-10 reject behavior and must remain reachable. Shift `MoveRejectFarPad` outward if needed to stay within the expanded floor bounds and remain "far."
**NavigationMesh settings:** No changes to agent parameters (height=1.0, radius=0.4, max_climb=0.35, cell_size=0.15) — they remain aligned with the player capsule per NEO-11. The stale baked mesh in the scene will update at runtime. The `geometry_collision_mask = 1` stays, so all collision-layer-1 bodies under the region contribute.
**NEO-10 edge case check:** With a larger floor the server's `step_size` / `MaxDistance` defaults may now allow clicks that would previously have been out-of-range. Document in the README if this changes observed reject behavior; no server code change unless a new reject case is broken (not just shifted in range).
---
## Files to add
None. All geometry lives in the existing scene file; no new GDScript logic is required.
---
## Files to modify
| File | Rationale |
|------|-----------|
| `client/scenes/main.tscn` | Replace 20×20 floor with 45×45; add terrace/step platforms and extra obstacle props; update pre-baked `NavigationMesh` vertices (will be overwritten at runtime, but keeping a reasonable approximation avoids editor warnings). |
| `client/README.md` | Add district runbook note: new footprint, vertical variation, intent as stress playground, updated manual-check geometry references. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Optional lightweight pointer that this expanded geography is the default QA map for E1.M2 camera stories. |
---
## Tests
No new automated GDScript tests are added for this story — there is no new GDScript logic. This story is pure scene geometry.
**Manual verification replaces automated tests:**
1. **F5** with server running: confirm player spawns and nav bake completes without errors in Output.
2. Click-to-move across the expanded floor, up/down each terrace, and through obstacles — confirm `NavigationAgent3D` routes and `move_and_slide` behaves per NEO-11 tradeoffs.
3. Ground-pick regression: click behind each new `occluder`-tagged obstacle with player in and out of occlusion fade range (per NEO-20 manual check in README).
4. Confirm `MoveRejectPedestal` and `MoveRejectFarPad` still reject correctly.
5. Follow camera: walk to far corners of the expanded floor — confirm `IsometricFollowCamera` tracks smoothly without snapping or losing the player (snap_distance not tripped by longer walks).
6. Interaction radius: walk near `PrototypeTerminal` to confirm range indicators still work correctly.
---
## Decisions
- **No horizontal step limit (resolved):** The server's `MaxHorizontalStep = 18 m` was too restrictive for the expanded 45 × 45 m floor (diagonal ≈ 63 m). Added `HorizontalStepEnabled` bool to `MovementValidationOptions` (defaults `false`), matching the `DistrictBoundsEnabled` pattern. The horizontal check is now skipped by default; the property remains available to opt back in via config. `MoveRejectFarPad` no longer exercises a horizontal range reject; it retains its walkable surface for vertical-step regression only.
- **Step assist guard/lift bug fixed (resolved):** `_try_walk_step_assist` in `player.gd` compared the goal's surface Y against the **capsule centre** (`global_position.y ≈ 0.9`) instead of the actual capsule bottom (`global_position.y CAPSULE_HALF_HEIGHT PLAYER_CAPSULE_RADIUS = 0.0`). This disabled the assist for any surface below ≈ 0.925 m — including all three terrace platforms. The lift formula used offsets of 0.45/0.55 instead of the full total half-height (0.9), giving zero or negative lift. Fixed: guard now uses `actual_feet_y`; lift targets `goal.y + CAPSULE_HALF_HEIGHT + PLAYER_CAPSULE_RADIUS` via `clampf`.
- **Arrival check using wrong capsule reference (resolved):** `vertical_arrival_error` was called with `CAPSULE_HALF_HEIGHT = 0.5`, giving code-feet at `body_y 0.5 = 0.4` at floor level. Step surfaces (Y = 0.3) are only 0.1 m from that reference. When the Jolt physics engine nudges the capsule body down to ~0.8 m (bottom hemisphere contacts the step edge), code-feet becomes 0.3 — matching the step surface Y and making `vert_err = 0 ≤ VERT_ARRIVE_EPS`. If the player is also within `ARRIVE_EPS = 0.35 m` horizontally, the arrival check fires immediately, clearing the nav goal before the capsule has moved. Fixed: pass `CAPSULE_HALF_HEIGHT + PLAYER_CAPSULE_RADIUS` (= 0.9) to `vertical_arrival_error` so actual feet = `body_y 0.9`. Step goal vert_err is now 0.3 >> 0.055 from floor height; arrival fires correctly only once the step assist has lifted the capsule onto the surface (body_y = 1.2, vert_err ≈ 0). Two regression tests added.
- **snap_to_server places capsule underground (resolved):** The server stores the raw surface Y from the client's click (e.g. 0.6 for a terrace platform), not the capsule body-centre Y (which should be 0.6 + 0.9 = 1.5). `snap_to_server` was setting `global_position.y` directly to this surface Y, putting the capsule bottom at 0.3 m. With `_floor_angle_loose_ticks = 0` and a stale `is_on_floor() = true`, the idle path's `_stable_idle_support()` check passed and held the player without calling `move_and_slide()`, preventing Jolt from resolving the penetration. Fixed: clamp `global_position.y ≥ CAPSULE_HALF_HEIGHT + PLAYER_CAPSULE_RADIUS` (= 0.9 m) in `snap_to_server`; also set `_floor_angle_loose_ticks = FLOOR_ANGLE_LOOSE_TICKS_AFTER_STOP` (96 ticks) to force corrective idle physics before the next walk goal.
- **Step assist undone by floor snap (resolved):** Each `_try_walk_step_assist()` cycle lifts the capsule 0.11 m. The prior `floor_snap_length = FLOOR_SNAP_MOVING = 0.32 m` meant the snap always reached the lower floor (0.11 m below the lifted bottom), pulling the capsule straight back down every tick. The player "nudged and stopped" because each lift was immediately cancelled. Fixed: added `WALK_STEP_ASSIST_SNAP = 0.09 m` constant (below 0.11 m so the snap cannot reach the previous floor, but above 0.03 m so it catches the step surface once cleared) and a `_step_assist_active` boolean flag. During active climbing, `floor_snap_length` is set to `WALK_STEP_ASSIST_SNAP` in both `_after_walk_move_and_slide()` and the main physics loop. The flag is cleared when the player lands cleanly on a floor without wall contact (`is_on_floor() and not is_on_wall()`), or when the nav goal is cleared/snapped.
- **Terrace B descend / climb (resolved, iterated):** Scene lifts and **descend-bypass** bee-lines caused regressions (stuck climbs, lip oscillation, wrong-direction slides). **Removed the descend bypass:** vertical moves (e.g. platform ↔ floor) use the **baked nav path + step assist** only. **Walk gravity** is **`!is_on_floor()`** again — never “goal below feet” while still walking on an upper surface (that applied gravity across the whole **TerracePlatformB** deck and destroyed horizontal motion). **`velocity.y`** is cleared by the horizontal steer each tick as before.
- **Lip stuck (floor→platform / platform→floor):** QA screenshots for this were on **`TerracePlatformA`** (10 × 10 large SE pad, **0.3 m** single step — not the NW **Terrace B** step/platform pair). Step assist required **`vel_h.dot(want) ≤ 0.52`**, so a **head-on** push into the vertical face (dot ≈ 1) **never** triggered the lift. **Removed** that gate. For **platform→floor**, **`floor_block_on_wall = false`** only when the nav goal is **below** the feet **and** **`is_on_wall()`** or a **wall-ish slide normal** exists, so open deck walking stays stable.
- **Teal pad still stuck (follow-up):** Step assist used **slide normals only**; Jolt often exposes the lip via **`is_on_wall()`** / **`get_wall_normal()`** instead — re-added a **directional** wall check (`wnh.dot(want) < -0.02`). Descend relax now also triggers on **shallow floor normal** (`get_floor_normal().dot(UP) < 0.992`) and slightly wider vertical-ish slide band. **`TerracePlatformA`** root **Y = 0.012 m** breaks **coplanarity** with the main floor without the larger gap that hurt **TerraceStepB** approach.
- **Nav waypoint steering dropped (movement):** Following **`get_next_path_position()`** by default caused **near-zero** horizontal progress under **Jolt** + **120 Hz** (waypoint advance / finished edge cases), read as universal **stuck** movement even with the server up. **`player.gd`** now **bee-lines in XZ** toward **`_auth_walk_goal`** and uses path-based steer only when **`needs_vertical_routing`** (Schmitt **`WALK_VERT_ROUTE_LATCH_*`**) **and** **`NAV_COLUMN_STEER_*`** hysteresis **and** **`is_on_floor()`** (mid-air column steer caused **180° XZ flips** while falling). **`_set_horizontal_velocity_from_nav_path_or_goal`** scans **`get_current_navigation_path()`** for the first waypoint **`NAV_PATH_STEER_MIN_LOOKAHEAD` (0.22 m)** in XZ, with a **goal-direction dot** guard — raw **`get_next_path_position()`** and a **5 cm** scan both sat inside Jolts per-tick slide and could flip velocity **180°** each tick.
- **Walk ledge / void gravity (iterated):** **`is_on_floor()`** + **`floor_snap_length`** could keep the capsule **glued** while crossing void to a lower click. **Fix:** **multi-sample** down rays under the **foot disk** (offsets along **move direction**; depth ~**0.32**); apply gravity when **none** hit; **`floor_snap_length = 0`** that tick so snap does not cancel **`velocity.y`**. Dropped a **floor-normal** gate that skipped the probe on **lip** contacts.
- **Step assist vs `agent_max_climb`:** Nav **`agent_max_climb = 0.35`** only constrained mesh links; **bee-line + chained step assist** could still climb **TerracePlatformB** (~**0.6 m** from floor) in one click. **`_try_walk_step_assist`** now returns false when **`_auth_walk_goal.y > feet + WALK_STEP_ASSIST_MAX_SURFACE_DELTA`** (**0.35**, same as the mesh).
- **Teal cardinal / tread shake (rollback):** Aggressive tweaks (**0.16** lift, **1**-tick cooldown, **`descending_stall`** toggling **`floor_block_on_wall`**, **seam-based** step-assist clear, **walk-stall nav replan**) caused **oscillation** on the small approach treads. **Reverted** to **`WALK_STEP_ASSIST_DELTA = 0.11`**, **`WALK_STEP_ASSIST_COOLDOWN_TICKS = 8`**, **no** stall replan, **no** `descending_stall` branch, **no** seam-based assist clear — only the original clear (clean floor without wall, or airborne with no slides). **Climb** wallish snap cap applies only when **`goal.y > feet_y + 0.12`** so shallow tread-to-tread motion is not capped to assist snap every frame.
- **Teal pad — geometry wins:** Script-only mitigations still failed in practice; **`TerracePlatformA_Approach`** adds **twelve** `walkable` **`StaticBody3D`** treads (three per cardinal) with **~0.104 m** rise each, **darker teal** albedo, flush to the platform lip — nav + `CharacterBody3D` use **sloped contact** instead of fighting a **single vertical face**. **Follow-up:** tread **run** was **0.45 m** along the climb axis while the capsule **diameter is 0.8 m** — the body was wider than each tread, causing straddle / stuck motion when **entering** and blocking horizontal progress toward the lip when **exiting**. Treads widened to **1.0 m** run with positions re-centered. **`NavigationAgent3D`** `path_desired_distance` / `target_desired_distance` were briefly lowered to **0.22** but that caused **floor** movement to **stall**; **reverted to 0.35**.
## Approach treads (district convention)
- **Shipped in scene:** **`TerracePlatformC_Approach`** (`PCS_*`, teal-style **3×4** cardinals, **7.2 m** NS / **WE** span), **`TerraceStepB_Approach`** (`TSB_*`, **S/E/W** only — north meets violet), **`TerracePlatformB_Approach`** (`TPB_*`, **N/E/W**, **six** ~**0.104 m** risers for **0.6 m** deck height; south remains **gold step**). **Smooth ramps** for other props can follow the same rise/run idea without new `player.gd` logic.
- **Idle jitter on thin treads / flat stops:** (1) **`idle_support_is_stable`**: post-stop **`loose_ticks` + ridged** blocks stable idle only when **`floor_normal.dot(UP) < IDLE_RIM_MIN_FLOOR_UP_DOT`** — flat tread + vertical riser (e.g. **TerraceStepB_Approach** corner) no longer spends ~0.8 s in corrective idle. (2) **`use_loose_floor_angle`** not tied to **`loose_ticks` alone**. (3) **`STABLE_IDLE_FLOOR_MIN_UP_DOT` 0.998** (was 0.999 — Jolt flat-floor normals often ~0.9991+ so the anchor rarely latched and **`move_and_slide`** + nudges drifted HUD XZ); **`STABLE_IDLE_ENTER_STREAK_FRAMES` 2**; **`STABLE_IDLE_UNLATCH_TICKS` 10**; ridged **wallish ny < 0.35**; **stable latch + Y anchor**. (4) **Arrival** stable shortcut (immediate latch, no enter streak). (5) **Nav column steer** hysteresis. (6) **`needs_vertical_routing` Schmitt latch**; **`debug_idle_trace`** walking extras.
- **Not in this pass:** **`PrototypeTerminal`**, **MoveReject** props, **runtime random bumps** — add geometry when those need climbable access.
## Open questions / risks
- **NavMesh bake time:** A 45×45 floor with several terraces will bake more polygons than the 20×20 baseline. Expected to be negligible for `bake_navigation_mesh(false)` on desktop, but monitor Output for a perceptible pause.
- **Agent climb on terraces:** `agent_max_climb = 0.35` limits single-step height. Each terrace transition is 0.30 m; verify in Godot editor nav mesh debug overlay before shipping.

View File

@ -1,14 +1,14 @@
# NEON-30 — Implementation plan
# NEO-20 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-30 |
| **Key** | NEO-20 |
| **Title** | E1.M2: Click-through input — occluder geometry must not block ground-pick raycasts |
| **Jira** | [NEON-30](https://neon-sprawl.atlassian.net/browse/NEON-30) |
| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
| **Linear** | [NEO-20](https://linear.app/neon-sprawl/issue/NEO-20) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
## Goal, scope, and out-of-scope
@ -17,11 +17,11 @@
**In scope**
- Inspect `client/scripts/ground_pick.gd` and preserve its current walkable-surface rules while making occluders transparent to picking.
- Make the exclusion unconditional: the click ray must ignore occluders whether or not NEON-27's `OcclusionPolicy` is currently fading them.
- Make the exclusion unconditional: the click ray must ignore occluders whether or not NEO-17's `OcclusionPolicy` is currently fading them.
- Keep non-occluder click targeting behavior unchanged on walkable surfaces.
- Document the chosen approach in this implementation plan without introducing unexplained collision-layer constants.
**Out of scope** (per Jira)
**Out of scope** (per Linear)
- Changing the movement or navigation systems.
- Server-side awareness of occluder state.
@ -37,11 +37,11 @@
### Preferred approach: explicit occluder skip in `ground_pick.gd`
Keep the existing stepped raycast loop in `ground_pick.gd`, but treat `"occluder"` hits as a first-class skip case before the walkable-surface check. When the ray hits an occluder, advance the ray origin a small distance past the hit point along the ray direction and continue the loop, similar to the existing steep-surface pass-through behavior. This keeps the fix local to input picking, preserves NEON-27's scene convention (`"occluder"` group), and avoids changing the collision-layer contract that the occlusion system already uses.
Keep the existing stepped raycast loop in `ground_pick.gd`, but treat `"occluder"` hits as a first-class skip case before the walkable-surface check. When the ray hits an occluder, advance the ray origin a small distance past the hit point along the ray direction and continue the loop, similar to the existing steep-surface pass-through behavior. This keeps the fix local to input picking, preserves NEO-17's scene convention (`"occluder"` group), and avoids changing the collision-layer contract that the occlusion system already uses.
### Why not a dedicated occluder collision layer first
Moving occluders to a new layer would also require updating the occlusion raycast mask in `isometric_follow_camera.gd`, plus scene data changes for every occluder. That is a broader contract change than this story needs and would add a new physics-layer convention that the acceptance criteria explicitly ask us to document carefully. Group-based exclusion in the pick loop is smaller, easier to verify, and reuses the convention already established by NEON-27.
Moving occluders to a new layer would also require updating the occlusion raycast mask in `isometric_follow_camera.gd`, plus scene data changes for every occluder. That is a broader contract change than this story needs and would add a new physics-layer convention that the acceptance criteria explicitly ask us to document carefully. Group-based exclusion in the pick loop is smaller, easier to verify, and reuses the convention already established by NEO-17.
### Step-by-step implementation
@ -55,9 +55,9 @@ Moving occluders to a new layer would also require updating the occlusion raycas
| Topic | Choice | Rationale |
|-------|--------|-----------|
| **Occluder exclusion mechanism** | Skip `"occluder"` hits in the ground-pick loop | Smallest change set; preserves NEON-27's established group tag; no new collision-layer convention needed. |
| **Occluder exclusion mechanism** | Skip `"occluder"` hits in the ground-pick loop | Smallest change set; preserves NEO-17's established group tag; no new collision-layer convention needed. |
| **Collision-layer strategy** | Do not add or repurpose layers for this story | Avoids extra scene churn and occlusion-ray regressions; satisfies the AC against unexplained magic numbers. |
| **Scope boundary** | Fix only click picking, not movement or nav | Matches Jira scope and keeps this story focused on input selection correctness. |
| **Scope boundary** | Fix only click picking, not movement or nav | Matches the Linear issue scope and keeps this story focused on input selection correctness. |
| **Debug warning on occluder skip** | No `push_warning` — silent pass-through | An occluder skip is intended, non-exceptional behavior; a warning would flood logs during normal play. Unlike the occlusion policy's skip of non-`StandardMaterial3D` surfaces (which signals a content mismatch), advancing past an occluder is the correct outcome. |
## Files to add
@ -87,4 +87,4 @@ None. The expected change is a focused update to existing pick logic, tests, and
- **Segment cap:** `MAX_PICK_SEGMENTS = 24` leaves ample headroom in the prototype with a single `Obstacle`. Each occluder skip consumes one iteration; the remaining segments are still available for steep-surface pass-through. No cap change needed.
- **Pass-through epsilon:** Named constant `OCCLUDER_PICK_THROUGH = 0.14` introduced (same value as `STEEP_PICK_THROUGH`; separate name for self-documentation since the two cases are conceptually distinct). Chosen to match the already-validated steep-surface nudge distance.
- **Module doc update:** E1_M2_IsometricCameraController.md updated with the NEON-30 click-through contract (group-based exclusion, no new collision layer) to record the shared `"occluder"` group convention for future readers.
- **Module doc update:** E1_M2_IsometricCameraController.md updated with the NEO-20 click-through contract (group-based exclusion, no new collision layer) to record the shared `"occluder"` group convention for future readers.

View File

@ -1,13 +1,13 @@
# NEON-2 — Implementation plan
# NEO-5 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-2 |
| **Key** | NEO-5 |
| **Title** | E1.M1: Click-to-move prototype (client-only) |
| **Jira** | [NEON-2](https://neon-sprawl.atlassian.net/browse/NEON-2) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) |
| **Linear** | [NEO-5](https://linear.app/neon-sprawl/issue/NEO-5) |
| **Parent context** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) |
## Goal, scope, and out-of-scope
@ -20,7 +20,7 @@
- Flat prototype terrain and simple obstacles sufficient to validate stopping behavior.
- Brief note in README and/or scene that this is **temporary** until authoritative movement sync exists.
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Game server, persistence, `MoveCommand` / wire protocol, Protobuf contracts.
@ -46,8 +46,8 @@
- `_unhandled_input` or `_input`: on left mouse button, raycast from camera through cursor into the physics world; if the collider is the ground (or a dedicated “walkable” layer/mask), set the navigation target.
4. **Documentation**
- Update `client/README.md` with a short “Movement prototype (NEON-2)” subsection: mouse click-to-move, navigation-based obstacle avoidance, **not** server-authoritative.
- Optional: `Editable Children` note on root node in scene or a comment in `main.gd` pointing to NEON-2 / temporariness.
- Update `client/README.md` with a short “Movement prototype (NEO-5)” subsection: mouse click-to-move, navigation-based obstacle avoidance, **not** server-authoritative.
- Optional: `Editable Children` note on root node in scene or a comment in `main.gd` pointing to NEO-5 / temporariness.
## Files to add
@ -79,4 +79,4 @@
- **Navigation baking:** First-time contributors need to bake the navigation mesh in the editor after pulling scene changes; document that in README if it is not obvious.
- **Isometric later:** Camera rig may be replaced for isometric; keep movement logic independent of camera style where possible (raycast from current camera).
- **Performance:** Single-agent prototype is sufficient; no need to optimize for crowds in NEON-2.
- **Performance:** Single-agent prototype is sufficient; no need to optimize for crowds in NEO-5.

View File

@ -1,14 +1,14 @@
# NEON-3 — Implementation plan
# NEO-6 — Implementation plan
## Story reference
| Field | Value |
| ------------------ | --------------------------------------------------------------------------------------- |
| **Key** | NEON-3 |
| **Key** | NEO-6 |
| **Title** | E1.M1: Authoritative PositionState API (in-memory) |
| **Jira** | [NEON-3](https://neon-sprawl.atlassian.net/browse/NEON-3) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) |
| **Linear** | [NEO-6](https://linear.app/neon-sprawl/issue/NEO-6) |
| **Parent context** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) |
## Goal, scope, and out-of-scope
@ -21,7 +21,7 @@
- Seed one logical player id for local development (configuration entry or documented constant; prefer **config** so environments stay obvious).
- **In-memory** store only (dictionary or small service holding positions); no PostgreSQL.
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Godot or other client integration, move submission, database persistence.
@ -54,7 +54,7 @@
- One integration test: `GET` for the configured dev player returns **200**, body parses, **`schemaVersion`** present, **`position.x`**, **`position.y`**, **`position.z`** present.
- Optional second test: unknown id returns **404**.
6. **Documentation**
- Extend `server/README.md` with “Position state (NEON-3)”: endpoint path, example `curl`, note that state is in-memory and authoritative only at the HTTP spike layer.
- Extend `server/README.md` with “Position state (NEO-6)”: endpoint path, example `curl`, note that state is in-memory and authoritative only at the HTTP spike layer.
## Files to add
@ -86,9 +86,9 @@
- **Automated:** Integration test(s) via `WebApplicationFactory<Program>` hitting `GET /game/players/{id}/position` — satisfies AC and [testing expectations](../../.cursor/rules/testing-expectations.md) (first testable server module gets a test project).
- **Manual:** `curl` snippet in README against `dotnet run` (default port from `launchSettings.json`, typically `http://localhost:5253`).
## Pull request description (Jira AC)
## Pull request description (Linear AC)
Jira NEON-3 asks for stable JSON **field names** documented for reviewers outside the repo. **When you open the GitHub PR** (and optionally comment on [NEON-3](https://neon-sprawl.atlassian.net/browse/NEON-3)), paste the following so AC is satisfied without hunting the README.
NEO-6 asks for stable JSON **field names** documented for reviewers outside the repo. **When you open the GitHub PR** (and optionally comment on [NEO-6](https://linear.app/neon-sprawl/issue/NEO-6)), paste the following so AC is satisfied without hunting the README.
**Endpoint:** `GET /game/players/{id}/position`**200** when the player exists, **404** when unknown.
@ -115,12 +115,12 @@ Jira NEON-3 asks for stable JSON **field names** documented for reviewers outsid
| `position.x` | number | X coordinate. |
| `position.y` | number | Y coordinate. |
| `position.z` | number | Z coordinate. |
| `sequence` | number (integer) | Increments on each successful move apply ([NEON-4](NEON-4-implementation-plan.md)); `0` until first move for the seeded dev player. |
| `sequence` | number (integer) | Increments on each successful move apply ([NEO-7](NEO-7-implementation-plan.md)); `0` until first move for the seeded dev player. |
**v1 omissions:** `facing` is not included (see Open questions). JSON uses **camelCase** property names (ASP.NET Core default serialization).
## Open questions / risks
- **Facing convention:** If `facing` is included, confirm units (radians vs degrees) with NEON-9 / future sync design; if unclear, omit v1 `facing` and document “reserved for v2” or add `schemaVersion` bump when added.
- **Facing convention:** If `facing` is included, confirm units (radians vs degrees) with E1.M1 / future sync design; if unclear, omit v1 `facing` and document “reserved for v2” or add `schemaVersion` bump when added.
- **Player id format:** Use a simple string (e.g. `dev-local-1`); path segment must be URL-safe.
- **CI:** If no workflow runs `dotnet test` yet, local `dotnet test` still satisfies AC; consider a follow-up chore to wire CI when available.

View File

@ -1,13 +1,13 @@
# NEON-4 — Implementation plan
# NEO-7 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-4 |
| **Key** | NEO-7 |
| **Title** | E1.M1: Submit MoveCommand → server updates PositionState |
| **Jira** | [NEON-4](https://neon-sprawl.atlassian.net/browse/NEON-4) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) |
| **Linear** | [NEO-7](https://linear.app/neon-sprawl/issue/NEO-7) |
| **Parent context** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) |
## Goal, scope, and out-of-scope
@ -16,18 +16,18 @@
**In scope**
- **Server:** HTTP endpoint accepting a **MoveCommand** JSON payload: at minimum **target position** + **player id** (route and/or body; dev session using configured `Game:DevPlayerId` is sufficient).
- **Server rule (document in plan + XML on types):** apply movement with a single clear rule. **Planned v1 rule:** **snap** authoritative position to the commands target immediately (no fixed-step simulation). Increment **`sequence`** on each successful apply so consumers can detect updates (NEON-3 left `sequence` at `0`; this story advances semantics).
- **Client:** Build on [NEON-2](NEON-2-implementation-plan.md) click-to-move: add a minimal **HTTP** client to **submit** the command and **confirm** state via **`GET`** (poll or follow-up GET after POST — both satisfy “read back”).
- **Server rule (document in plan + XML on types):** apply movement with a single clear rule. **Planned v1 rule:** **snap** authoritative position to the commands target immediately (no fixed-step simulation). Increment **`sequence`** on each successful apply so consumers can detect updates (NEO-6 left `sequence` at `0`; this story advances semantics).
- **Client:** Build on [NEO-5](NEO-5-implementation-plan.md) click-to-move: add a minimal **HTTP** client to **submit** the command and **confirm** state via **`GET`** (poll or follow-up GET after POST — both satisfy “read back”).
- **Wire format:** JSON for the prototype (explicit throwaway per [`docs/decomposition/modules/contracts.md`](../decomposition/modules/contracts.md) until protobuf).
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Prediction / reconciliation, PostgreSQL, speed anti-cheat.
**Dependencies**
- [NEON-3](NEON-3-implementation-plan.md): in-memory `PositionState` **read** API and store.
- [NEON-2](NEON-2-implementation-plan.md): click-to-move scaffold.
- [NEO-6](NEO-6-implementation-plan.md): in-memory `PositionState` **read** API and store.
- [NEO-5](NEO-5-implementation-plan.md): click-to-move scaffold.
## Acceptance criteria checklist
@ -39,7 +39,7 @@
1. **MoveCommand (v1 JSON)**
- New request DTO (e.g. `MoveCommandRequest`) with top-level **`schemaVersion`** (`1`) and **`target`**: object with **`x`**, **`y`**, **`z`** (same numeric style as `PositionVector`).
- **Endpoint:** `POST /game/players/{id}/move` with JSON body; `{id}` identifies the player (trimmed, case-insensitive match to store, consistent with NEON-3 `GET`).
- **Endpoint:** `POST /game/players/{id}/move` with JSON body; `{id}` identifies the player (trimmed, case-insensitive match to store, consistent with NEO-6 `GET`).
- **Responses:** **200** + body same shape as **`PositionStateResponse`** after apply; **404** if player unknown; **400** if `schemaVersion` ≠ 1 or payload invalid / missing `target`.
2. **Store mutation**
@ -53,14 +53,14 @@
- Unknown player → **404**; bad `schemaVersion`**400**.
5. **Godot client****Split scripts by concern** (required repo policy: [godot-client-script-organization.md](../../.cursor/rules/godot-client-script-organization.md)); do **not** grow a monolithic `main.gd` with pick + HTTP + wiring.
- **`ground_pick.gd`** on a new **`Node3D`** child (e.g. `GroundPick`): owns walkable **ray pick** (logic moved out of NEON-2 `main.gd`). Exposes **`target_chosen(Vector3)`** when the user clicks valid ground. **`main.gd`** sets **`fallback_camera`** (or equivalent) in `_ready`.
- **`position_authority_client.gd`** on a new **`Node`** child: `@export` **base URL** (default `http://127.0.0.1:5253`, match `launchSettings.json`) and **dev player id** (`dev-local-1`). Creates **`HTTPRequest`** in **`_ready`** via **`add_child`** (not as a node hand-placed in `main.tscn`). Flow: **`POST`** `MoveCommand`, then **`GET`** position (tests must show **GET** reflects apply). Emits **`authoritative_position_received(Vector3, bool)`** (NEON-8: second arg = boot snap vs nav goal) so **`main`** snaps on boot and path-follows after move verify.
- **`ground_pick.gd`** on a new **`Node3D`** child (e.g. `GroundPick`): owns walkable **ray pick** (logic moved out of NEO-5 `main.gd`). Exposes **`target_chosen(Vector3)`** when the user clicks valid ground. **`main.gd`** sets **`fallback_camera`** (or equivalent) in `_ready`.
- **`position_authority_client.gd`** on a new **`Node`** child: `@export` **base URL** (default `http://127.0.0.1:5253`, match `launchSettings.json`) and **dev player id** (`dev-local-1`). Creates **`HTTPRequest`** in **`_ready`** via **`add_child`** (not as a node hand-placed in `main.tscn`). Flow: **`POST`** `MoveCommand`, then **`GET`** position (tests must show **GET** reflects apply). Emits **`authoritative_position_received(Vector3, bool)`** (NEO-11: second arg = boot snap vs nav goal) so **`main`** snaps on boot and path-follows after move verify.
- **`main.gd`:** Thin composer only—connect signals, call **`sync_from_server()`** on run, delegate pick and HTTP to the two child scripts.
- Align **client README** with NEON-2 note: server authority path replaces pure local steering for this demo.
- Align **client README** with NEO-5 note: server authority path replaces pure local steering for this demo.
6. **Documentation**
- **`server/README.md`:** new subsection for move submission: method, path, sample **`curl`**, v1 snap rule, link to DTO XML.
- **`client/README.md`:** how to run server + client for NEON-4 demo, exported vars, expectation of snap-to-server.
- **`client/README.md`:** how to run server + client for NEO-7 demo, exported vars, expectation of snap-to-server.
## Files to add
@ -77,22 +77,22 @@
| `server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs` | Declare authoritative apply API. |
| `server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs` | Implement snap + sequence increment. |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | Map `POST /game/players/{id}/move`. |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs` | XML: clarify **`sequence`** increments after moves (postNEON-3). |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs` | XML: clarify **`sequence`** increments after moves (postNEO-6). |
| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | `POST` + `GET` integration and error cases. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs` | Existing `GET` integration tests (same feature folder). |
| `client/scripts/main.gd` | **Thin composer:** add/wire `GroundPick` + `PositionAuthorityClient`, connect signals, snap player to server (remove inlined pick + HTTP). |
| `client/scenes/main.tscn` | Add child nodes that attach **`ground_pick.gd`** and **`position_authority_client.gd`** only. **Do not** add **`HTTPRequest`** to the scene—the authority script owns and instantiates it. |
| `client/scripts/player.gd` | Only if needed (e.g. **`velocity = ZERO`** on snap, or helper **`snap_to(world_pos)`**); prefer minimal changes. |
| `server/README.md` | Move command + curl + v1 behavior. |
| `client/README.md` | NEON-4 runbook and authority note. |
| `client/README.md` | NEO-7 runbook and authority note. |
## Tests
- **Automated:** xUnit integration tests via `WebApplicationFactory<Program>``POST` then `GET` equality, `404` / `400` as above.
- **Manual:** Godot F5 with server running; click floor; avatar snaps to server-reported position.
- **No new client automated test harness** in scope unless one already exists (none planned for NEON-4).
- **No new client automated test harness** in scope unless one already exists (none planned for NEO-7).
## Pull request description (Jira AC) — draft for merge
## Pull request description (Linear AC) — draft for merge
Paste when opening the PR (adjust if shapes differ slightly in implementation).
@ -114,9 +114,9 @@ Paste when opening the PR (adjust if shapes differ slightly in implementation).
| `target` | object | Destination world position. |
| `target.x` / `y` / `z` | number | Coordinates (snap applied server-side). |
**PositionState v1 — response** (unchanged from NEON-3; **`sequence`** increments after each successful move)
**PositionState v1 — response** (unchanged from NEO-6; **`sequence`** increments after each successful move)
- Same as NEON-3 **`GET`** response body; successful **`POST`** returns the updated snapshot.
- Same as NEO-6 **`GET`** response body; successful **`POST`** returns the updated snapshot.
**Server rule:** position is set to **`target`** immediately (**snap**); **`sequence`** increases by one per successful apply.

View File

@ -1,13 +1,13 @@
# NEON-5 — Implementation plan
# NEO-8 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-5 |
| **Key** | NEO-8 |
| **Title** | E1.M1: Persist character position (PostgreSQL) |
| **Jira** | [NEON-5](https://neon-sprawl.atlassian.net/browse/NEON-5) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) · [NEON-4](NEON-4-implementation-plan.md) (MoveCommand → `PositionState`) |
| **Linear** | [NEO-8](https://linear.app/neon-sprawl/issue/NEO-8) |
| **Parent context** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) · [NEO-7](NEO-7-implementation-plan.md) (MoveCommand → `PositionState`) |
## Goal, scope, and out-of-scope
@ -20,7 +20,7 @@
- Reuse **[`docker-compose.yml`](../../docker-compose.yml)** Postgres from repo root / [root README](../../README.md).
- Document how the server picks the database (configuration / env vars).
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Full character creation flow, multiple zones, ORM/migration framework polish beyond what is needed for this slice.
@ -32,7 +32,7 @@
- [ ] Restart **`dotnet run`** with the same Postgres volume: last committed position is still returned from **`GET /game/players/{id}/position`** (after a prior **`POST …/move`**).
- [ ] Connection / database configuration documented (env vars or `ConnectionStrings`, aligned with ASP.NET conventions).
- [ ] No regression to the **in-memory** happy path: default **`dotnet test`** / CI without Postgres continues to behave as NEON-3/NEON-4 (see [Tests](#tests)).
- [ ] No regression to the **in-memory** happy path: default **`dotnet test`** / CI without Postgres continues to behave as NEO-6/NEO-7 (see [Tests](#tests)).
## Technical approach
@ -79,12 +79,12 @@
- **Existing** [`MoveCommandApiTests`](../../server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs) and [`PositionStateApiTests`](../../server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs): unchanged setup — **no** connection string so the host keeps **`InMemoryPositionStateStore`** — satisfies “no regression to in-memory happy path.”
- **New (required):** **PostgreSQL integration tests** per [`.cursor/rules/testing-expectations.md`](../../.cursor/rules/testing-expectations.md): use **`WebApplicationFactory<Program>`** with configuration (or environment) that sets the same **connection string** the app uses for the durable store, pointing at real Postgres.
- Cover at least: **POST move** then **GET** sees stored position and **`sequence`**; **persistence across host restarts** simulated by **disposing** the first factory and creating a **second** `WebApplicationFactory` with the same connection string, then **GET** still returns the last committed state (matches Jira “restart server + DB” without a literal process kill).
- Cover at least: **POST move** then **GET** sees stored position and **`sequence`**; **persistence across host restarts** simulated by **disposing** the first factory and creating a **second** `WebApplicationFactory` with the same connection string, then **GET** still returns the last committed state (matches the acceptance-criteria “restart server + DB” scenario without a literal process kill).
- Optionally add a **failure-path** case (e.g. unknown player **404** on move when row absent) if the implementation keeps that behavior for Postgres.
- **CI:** [.github/workflows/dotnet.yml](../../.github/workflows/dotnet.yml) must run these tests **always** in the existing job (service container + env), not as an optional manual job — so the durable path is gated like the rest of the suite.
- **Local:** document in **`server/README.md`**: run **`docker compose up -d`** (or any Postgres with matching credentials) and set the same connection string env vars the workflow uses, then `dotnet test`.
**Manual verification (Jira AC, optional double-check):** `docker compose up -d``dotnet run` with Postgres enabled → `POST …/move` → stop server → `dotnet run` again → `GET …/position` matches last move.
**Manual verification (acceptance criteria, optional double-check):** `docker compose up -d``dotnet run` with Postgres enabled → `POST …/move` → stop server → `dotnet run` again → `GET …/position` matches last move.
## Open questions / risks
@ -93,4 +93,4 @@
## Pull request description (draft)
**NEON-5:** Persist `PositionState` in PostgreSQL when a connection string is configured; keep in-memory store when not. Adds DDL + Npgsql-backed `IPositionStateStore`, **PostgreSQL integration tests** (including simulating restart via a second web factory), **Postgres service in `dotnet.yml`**, and README notes for env vars and local test runs.
**NEO-8:** Persist `PositionState` in PostgreSQL when a connection string is configured; keep in-memory store when not. Adds DDL + Npgsql-backed `IPositionStateStore`, **PostgreSQL integration tests** (including simulating restart via a second web factory), **Postgres service in `dotnet.yml`**, and README notes for env vars and local test runs.

View File

@ -1,20 +1,20 @@
# NEON-6 — Implementation plan
# NEO-9 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-6 |
| **Key** | NEO-9 |
| **Title** | E1.M1: InteractionRequest with server-side range check |
| **Jira** | [NEON-6](https://neon-sprawl.atlassian.net/browse/NEON-6) |
| **Parent context** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) · [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) |
| **Linear** | [NEO-9](https://linear.app/neon-sprawl/issue/NEO-9) |
| **Parent context** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) |
| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) (range / authority); seeds [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md) contracts |
**Delivery:** Ship **`docs/plans/NEON-6-implementation-plan.md`** on the **same branch / PR** as the NEON-6 implementation (plan + code pushed together).
**Delivery:** Ship **`docs/plans/NEO-9-implementation-plan.md`** on the **same branch / PR** as the NEO-9 implementation (plan + code pushed together).
## Goal, scope, and out-of-scope
**Goal:** **Server-authoritative** answer to “can this player use this interactable **now**?” using the same **position source of truth** as movement ([NEON-3](NEON-3-implementation-plan.md) / [NEON-4](NEON-4-implementation-plan.md) / [NEON-5](NEON-5-implementation-plan.md)).
**Goal:** **Server-authoritative** answer to “can this player use this interactable **now**?” using the same **position source of truth** as movement ([NEO-6](NEO-6-implementation-plan.md) / [NEO-7](NEO-7-implementation-plan.md) / [NEO-8](NEO-8-implementation-plan.md)).
**Design decision — floor plane only (Y ignored for reach)**
@ -27,13 +27,13 @@ The game is **3D**, but **prototype interaction and movement tuning** treat all
- **HTTP API:** versioned **`InteractionRequest`** JSON; server loads **authoritative** `PositionState` for `{id}` from `IPositionStateStore`, compares to interactable position via the helper, returns **allowed** + optional small payload, or **denied** + **stable `reasonCode`** string.
- **Godot:** one **placeholder** interactable in the world (visible mesh + collision optional); minimal **client** path that POSTs an interaction for a **fixed id** after a key press (e.g. **E**) so QA can stand in range vs out of range without building full targeting.
**Out of scope (per Jira)**
**Out of scope (per Linear)**
- Inventory, rich UI prompts, full **E1.M3** selection / hover / target-lock polish, multiple interactable **types** or data-driven defs beyond the static table.
**Dependencies**
- Authoritative **`PositionState`** + **move** path ([NEON-3](NEON-3-implementation-plan.md), [NEON-4](NEON-4-implementation-plan.md)); optional Postgres ([NEON-5](NEON-5-implementation-plan.md)) unchanged—interaction reads current store only.
- Authoritative **`PositionState`** + **move** path ([NEO-6](NEO-6-implementation-plan.md), [NEO-7](NEO-7-implementation-plan.md)); optional Postgres ([NEO-8](NEO-8-implementation-plan.md)) unchanged—interaction reads current store only.
## Acceptance criteria checklist
@ -111,8 +111,8 @@ Authoritative values live in **`PrototypeInteractableRegistry.cs`**. **Godot** a
### 7. Documentation
- **`server/README.md`:** subsection **Interaction (NEON-6)** — endpoint, curl example, `reasonCode` table, note on horizontal distance.
- **`docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md`** (and optionally **`documentation_and_implementation_alignment.md`**): one-line snapshot that **InteractionRequest** + range check landed (NEON-6).
- **`server/README.md`:** subsection **Interaction (NEO-9)** — endpoint, curl example, `reasonCode` table, note on horizontal distance.
- **`docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md`** (and optionally **`documentation_and_implementation_alignment.md`**): one-line snapshot that **InteractionRequest** + range check landed (NEO-9).
## Files to add
@ -136,9 +136,9 @@ Authoritative values live in **`PrototypeInteractableRegistry.cs`**. **Godot** a
| `server/NeonSprawl.Server/Program.cs` | Register `MapInteractionApi()`. |
| `client/scenes/main.tscn` | Add prototype interactable + **2+ glow marker meshes** (no collision); materials suitable for emission toggle. |
| `client/scripts/main.gd` | Compose interaction client (signals/exports). |
| `client/README.md` | NEON-6 manual check: markers **glow** in-range / **dim** out-of-range (preview = same XZ+radius as server); walk + **E**; compare **Output** to glow. |
| `client/README.md` | NEO-9 manual check: markers **glow** in-range / **dim** out-of-range (preview = same XZ+radius as server); walk + **E**; compare **Output** to glow. |
| `server/README.md` | Interaction API + reason codes + distance rule. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot for NEON-6. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot for NEO-9. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Optional tracking row update for E1.M1. |
## Tests
@ -151,7 +151,7 @@ Authoritative values live in **`PrototypeInteractableRegistry.cs`**. **Godot** a
- **Multi-floor / vertical placement:** Revisit distance math, interactable placement, and any **Y**-aware rules when stacked levels matter; until then **floor-plane reach only**.
## Pull request description (Jira AC) — draft for merge
## Pull request description (Linear AC) — draft for merge
**InteractionRequest v1**

View File

@ -1,13 +1,13 @@
# Code review — NEON-3 (authoritative position read API)
# Code review — NEO-6 (authoritative position read API)
## Review metadata
| Field | Value |
|--------|--------|
| **Date** | 2026-03-29 |
| **Scope** | Branch `NEON-3-position-state-api` vs `origin/main`, plus **working tree**: modified tracked files and **untracked** implementation/tests/agent files (full change set as present locally) |
| **Scope** | Branch `NEO-6-position-state-api` vs `origin/main`, plus **working tree**: modified tracked files and **untracked** implementation/tests/agent files (full change set as present locally) |
| **Base** | `origin/main` |
| **Issue** | NEON-3 — E1.M1: Authoritative PositionState API (in-memory) |
| **Issue** | NEO-6 — E1.M1: Authoritative PositionState API (in-memory) |
---
@ -17,11 +17,11 @@
## Summary
The NEON-3 work adds an in-memory `IPositionStateStore`, `Game` configuration for a dev player id and default position, `PositionStateApi.MapPositionStateApi()` for `GET /game/players/{id}/position`, XML-documented DTOs with `schemaVersion`, integration tests via `WebApplicationFactory<Program>`, and `Program.Waf.cs` for the test host. `Program.cs` stays thin (DI + map extension calls only), matching `docs/plans/NEON-3-implementation-plan.md`. README documents `curl` and sample JSON. Separately, `.cursor/rules/csharp-style.md` gains primary-constructor guidance, and the code-review agent + `AGENTS.md` add process docs. Overall risk is **low** for a read-only spike; no persistence or client integration.
The NEO-6 work adds an in-memory `IPositionStateStore`, `Game` configuration for a dev player id and default position, `PositionStateApi.MapPositionStateApi()` for `GET /game/players/{id}/position`, XML-documented DTOs with `schemaVersion`, integration tests via `WebApplicationFactory<Program>`, and `Program.Waf.cs` for the test host. `Program.cs` stays thin (DI + map extension calls only), matching `docs/plans/NEO-6-implementation-plan.md`. README documents `curl` and sample JSON. Separately, `.cursor/rules/csharp-style.md` gains primary-constructor guidance, and the code-review agent + `AGENTS.md` add process docs. Overall risk is **low** for a read-only spike; no persistence or client integration.
## Blocking issues
1. **Version control gap** — As of this review, `server/NeonSprawl.Server/Game/`, `server/NeonSprawl.Server.Tests/`, `server/NeonSprawl.Server/Program.Waf.cs`, `.cursor/rules/code-review-agent.md`, and `AGENTS.md` are **untracked**, while `Program.cs`, `appsettings.json`, `server/README.md`, `NeonSprawl.sln`, `docs/plans/NEON-3-implementation-plan.md`, and `.cursor/rules/csharp-style.md` are **modified but uncommitted**. **Before merge:** `git add` all intended files and push so CI/reviewers see the real diff.
1. **Version control gap** — As of this review, `server/NeonSprawl.Server/Game/`, `server/NeonSprawl.Server.Tests/`, `server/NeonSprawl.Server/Program.Waf.cs`, `.cursor/rules/code-review-agent.md`, and `AGENTS.md` are **untracked**, while `Program.cs`, `appsettings.json`, `server/README.md`, `NeonSprawl.sln`, `docs/plans/NEO-6-implementation-plan.md`, and `.cursor/rules/csharp-style.md` are **modified but uncommitted**. **Before merge:** `git add` all intended files and push so CI/reviewers see the real diff.
## Suggestions
@ -29,16 +29,16 @@ The NEON-3 work adds an in-memory `IPositionStateStore`, `Game` configuration fo
2. **Case sensitivity** — Lookups use `StringComparer.Ordinal`. Document that player ids in URLs are case-sensitive, or normalize route id and stored keys the same way if product expects case-insensitivity.
3. **Implementation plan markdown** — In `docs/plans/NEON-3-implementation-plan.md`, the acceptance section mixes raw `**` markers with bullets (e.g. `` `**dotnet test**` ``) and no longer uses `- [ ]` checkboxes. Restoring valid Markdown and checkboxes improves scanability.
3. **Implementation plan markdown** — In `docs/plans/NEO-6-implementation-plan.md`, the acceptance section mixes raw `**` markers with bullets (e.g. `` `**dotnet test**` ``) and no longer uses `- [ ]` checkboxes. Restoring valid Markdown and checkboxes improves scanability.
4. **Solution file noise** — If `NeonSprawl.sln` picks up a UTF-8 BOM or Windows-style path separators, consider normalizing to reduce cross-platform diff churn (non-functional).
5. **PR description** — When opening the PR, paste the sample JSON contract (as in README) so Jira AC (“document field names”) is satisfied outside the repo alone.
5. **PR description** — When opening the PR, paste the sample JSON contract (as in README) so acceptance criteria (“document field names”) is satisfied outside the repo alone (e.g. in the Linear issue description or a PR comment).
## Nits
- **Nit:** `SchemaVersion` is set to `1` in both `PositionStateResponse` and the handler; a single named constant could prevent drift.
- **Nit:** No logging on 404 vs success—acceptable for NEON-3; revisit when debugging multi-player seeds.
- **Nit:** No logging on 404 vs success—acceptable for NEO-6; revisit when debugging multi-player seeds.
## Verification
@ -62,13 +62,13 @@ Expect HTTP 200 and JSON including `schemaVersion`, `playerId`, `position` with
## Files touched (review scope)
**Modified (tracked):** `.cursor/rules/csharp-style.md`, `NeonSprawl.sln`, `docs/plans/NEON-3-implementation-plan.md`, `server/NeonSprawl.Server/Program.cs`, `server/NeonSprawl.Server/appsettings.json`, `server/README.md`
**Modified (tracked):** `.cursor/rules/csharp-style.md`, `NeonSprawl.sln`, `docs/plans/NEO-6-implementation-plan.md`, `server/NeonSprawl.Server/Program.cs`, `server/NeonSprawl.Server/appsettings.json`, `server/README.md`
**Untracked (must be added for a complete PR):** `.cursor/rules/code-review-agent.md`, `AGENTS.md`, `server/NeonSprawl.Server/Program.Waf.cs`, `server/NeonSprawl.Server/Game/PositionState/*.cs`, `server/NeonSprawl.Server.Tests/*`
## Conformance notes
- **architecture-authority:** Server-held in-memory state, HTTP read spike; no client or move submission—matches NEON-3 out-of-scope.
- **architecture-authority:** Server-held in-memory state, HTTP read spike; no client or move submission—matches NEO-6 out-of-scope.
- **testing-expectations:** First server module gets `NeonSprawl.Server.Tests` with integration-style coverage for happy path and 404—appropriate.
- **csharp-style:** Test class uses primary constructor and `using Xunit;`—aligned with updated rule.
- **Plan alignment:** Separate `PositionStateApi`, `PositionStateServiceCollectionExtensions`, no position routes inlined in `Program.cs`—matches plan.

View File

@ -1,8 +1,8 @@
# Code review: NEON-4 (MoveCommand → PositionState)
# Code review: NEO-7 (MoveCommand → PositionState)
**Date:** 2026-03-30
**Scope:** Branch `NEON-4-move-command-position-state` vs `origin/main` (working tree / uncommitted changes included at review time).
**Issue:** [NEON-4](https://neon-sprawl.atlassian.net/browse/NEON-4) — E1.M1: Submit MoveCommand → server updates PositionState
**Scope:** Branch `NEO-7-move-command-position-state` vs `origin/main` (working tree / uncommitted changes included at review time).
**Issue:** [NEO-7](https://linear.app/neon-sprawl/issue/NEO-7) — E1.M1: Submit MoveCommand → server updates PositionState
**Base:** `origin/main`
## Verdict
@ -11,15 +11,15 @@
## Summary
The branch delivers NEON-4 end-to-end: **`POST /game/players/{id}/move`** with versioned **`MoveCommandRequest`**, thread-safe snap + **`sequence`** in **`InMemoryPositionStateStore`**, **`PositionStateResponse`** on success, integration tests under **`Game/PositionState/`** with **`MethodName_ShouldExpectedOutcome_WhenScenario`** naming and AAA layout, and a Godot client split into **`ground_pick.gd`**, **`position_authority_client.gd`**, and a thin **`main.gd`**. **`UseAppHost=false`** for Debug avoids Linux apphost probing **`/usr/share/dotnet`** when .NET 10 lives under **`~/.dotnet`**. Overall risk is **low** for a prototype slice; there is **no auth** on move (acceptable for the story). Repo rules and READMEs were extended for tests, Godot layout, and Linux/Rider notes.
The branch delivers NEO-7 end-to-end: **`POST /game/players/{id}/move`** with versioned **`MoveCommandRequest`**, thread-safe snap + **`sequence`** in **`InMemoryPositionStateStore`**, **`PositionStateResponse`** on success, integration tests under **`Game/PositionState/`** with **`MethodName_ShouldExpectedOutcome_WhenScenario`** naming and AAA layout, and a Godot client split into **`ground_pick.gd`**, **`position_authority_client.gd`**, and a thin **`main.gd`**. **`UseAppHost=false`** for Debug avoids Linux apphost probing **`/usr/share/dotnet`** when .NET 10 lives under **`~/.dotnet`**. Overall risk is **low** for a prototype slice; there is **no auth** on move (acceptable for the story). Repo rules and READMEs were extended for tests, Godot layout, and Linux/Rider notes.
## Documentation checked
| Document | Result |
|----------|--------|
| `docs/plans/NEON-4-implementation-plan.md` | **Matches** — endpoint, snap rule, client split, tests, README expectations align with implementation. |
| `docs/plans/NEON-3-implementation-plan.md` | **Partially matches** — PR table row for **`sequence`** was stale (“v1 is 0” only); **updated** in this review pass to reference NEON-4 semantics. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Partially matched → updated** — snapshot still said “Not yet: MoveCommand”; **updated** to reflect NEON-4. |
| `docs/plans/NEO-7-implementation-plan.md` | **Matches** — endpoint, snap rule, client split, tests, README expectations align with implementation. |
| `docs/plans/NEO-6-implementation-plan.md` | **Partially matches** — PR table row for **`sequence`** was stale (“v1 is 0” only); **updated** in this review pass to reference NEO-7 semantics. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Partially matched → updated** — snapshot still said “Not yet: MoveCommand”; **updated** to reflect NEO-7. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matched → updated** — E1.M1 row still said no MoveCommand/client sync; **updated**. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no row text change required for this review beyond alignment table (register Status unchanged **In Progress**). |
| `docs/decomposition/modules/contracts.md` | **Matches** — JSON spike / throwaway until protobuf acknowledged in code comments. |
@ -33,7 +33,7 @@ The branch delivers NEON-4 end-to-end: **`POST /game/players/{id}/move`** with v
## Suggestions
1. **Jira / plan checkboxes** — `docs/plans/NEON-4-implementation-plan.md` acceptance bullets are still unchecked; tick them when you close NEON-4 or paste PR proof in Jira.
1. **Linear / plan checkboxes** — `docs/plans/NEO-7-implementation-plan.md` acceptance bullets are still unchecked; tick them when you close NEO-7 or paste PR proof on the Linear issue.
2. **`PositionAuthorityClient` boot failure** — if the initial GET fails, the client fails quietly (warnings only); acceptable for the spike; consider a single user-visible error later.
3. **Observability** — server does not log failed moves or bad payloads; fine for prototype; add when debugging multiplayer.
@ -43,7 +43,7 @@ The branch delivers NEON-4 end-to-end: **`POST /game/players/{id}/move`** with v
**Resolved after review:**
- NEON-4 plan now states explicitly that scripts are **split per client policy**, and **`HTTPRequest`** is created in **`position_authority_client._ready`** (not placed in **`main.tscn`**).
- NEO-7 plan now states explicitly that scripts are **split per client policy**, and **`HTTPRequest`** is created in **`position_authority_client._ready`** (not placed in **`main.tscn`**).
- **`PositionAuthorityClient`** reuses **`_request_get()`** for the post-POST verify GET instead of duplicating the position URL.
## Verification

View File

@ -1,16 +1,16 @@
# Code review — NEON-5 (PostgreSQL position persistence)
# Code review — NEO-8 (PostgreSQL position persistence)
**Date:** 2026-03-30
**Scope:** Branch `NEON-5-persist-position-postgres`; Jira [NEON-5](https://neon-sprawl.atlassian.net/browse/NEON-5). Working tree includes **modified** and **untracked** files (e.g. `PostgresPositionStateStore.cs`, `server/db/`, Postgres test types are **untracked** at review time).
**Scope:** Branch `NEO-8-persist-position-postgres`; Linear [NEO-8](https://linear.app/neon-sprawl/issue/NEO-8). Working tree includes **modified** and **untracked** files (e.g. `PostgresPositionStateStore.cs`, `server/db/`, Postgres test types are **untracked** at review time).
**Base:** `main` (comparison: `git diff main --stat` plus untracked NEON-5 assets).
**Base:** `main` (comparison: `git diff main --stat` plus untracked NEO-8 assets).
---
## Verdict
**Approve with nits** — Persistence wiring matches the NEON-5 plan and authority docs; tests and CI cover the durable path. Decomposition **implementation snapshot** docs updated for NEON-5 (see **Author follow-up** below).
**Approve with nits** — Persistence wiring matches the NEO-8 plan and authority docs; tests and CI cover the durable path. Decomposition **implementation snapshot** docs updated for NEO-8 (see **Author follow-up** below).
---
@ -18,7 +18,7 @@
This work adds a PostgreSQL-backed `IPositionStateStore`, a versioned DDL file under `server/db/migrations/`, configuration via `ConnectionStrings:NeonSprawl`, idempotent schema application once per process, and per-request idempotent dev-row seeding so behavior stays consistent with TRUNCATE-heavy tests. GitHub Actions gains a Postgres service and passes the connection string into `dotnet test`. In-memory API tests are isolated via `InMemoryWebApplicationFactory` + `ConfigureTestServices` so a global CI env var does not poison the in-memory suite. Overall risk is **lowmedium**: prototype-grade lifecycle (no explicit `NpgsqlDataSource` disposal), runtime DDL from the app, and every request paying a seed `INSERT` (no-op when row exists).
The diff also includes **repo-wide C# brace style** updates in `.cursor/rules/csharp-style.md` and touched server files — acceptable but **mixed scope** vs NEON-5; consider separate commit or callout in PR.
The diff also includes **repo-wide C# brace style** updates in `.cursor/rules/csharp-style.md` and touched server files — acceptable but **mixed scope** vs NEO-8; consider separate commit or callout in PR.
---
@ -26,12 +26,12 @@ The diff also includes **repo-wide C# brace style** updates in `.cursor/rules/cs
| Document | Assessment |
|----------|------------|
| `docs/plans/NEON-5-implementation-plan.md` | **Matches** — columns not JSONB, `player_position`, `IPositionStateStore`, connection string toggle, CI service + env, integration tests (POST→GET, second factory, unknown player), README/env docs. **Partially matches** — plan listed optional `appsettings.Development.json`; implementation correctly leaves connection string unset by default (still within plan). |
| `docs/plans/NEO-8-implementation-plan.md` | **Matches** — columns not JSONB, `player_position`, `IPositionStateStore`, connection string toggle, CI service + env, integration tests (POST→GET, second factory, unknown player), README/env docs. **Partially matches** — plan listed optional `appsettings.Development.json`; implementation correctly leaves connection string unset by default (still within plan). |
| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server remains authoritative for `PositionState`; persistence does not move truth to the client. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches (post-review update)** — Snapshot describes optional Postgres persistence + NEON-5; “Not yet” trimmed to prediction / full slice / telemetry. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches (post-review update)** — E1.M1 tracking row includes NEON-5 and Postgres-when-configured. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches (post-review update)** — Snapshot describes optional Postgres persistence + NEO-8; “Not yet” trimmed to prediction / full slice / telemetry. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches (post-review update)** — E1.M1 tracking row includes NEO-8 and Postgres-when-configured. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M1 remains **In Progress**; no contract version promotion required for this slice. |
| `docs/decomposition/modules/contracts.md` | **Matches** — HTTP JSON shapes unchanged; NEON-5 is durability, not a breaking wire change. |
| `docs/decomposition/modules/contracts.md` | **Matches** — HTTP JSON shapes unchanged; NEO-8 is durability, not a breaking wire change. |
| `server/README.md` | **Matches** — persistence, env vars, local Postgres test instructions. |
---
@ -44,8 +44,8 @@ The diff also includes **repo-wide C# brace style** updates in `.cursor/rules/cs
## Suggestions
1. ~~**Docs alignment (recommended before merge):**~~ **Done**`E1_M1_InputAndMovementRuntime.md` and `documentation_and_implementation_alignment.md` updated for NEON-5 / optional Postgres (planned follow-up; separate from **Nit 1** below).
2. ~~**Lifecycle:**~~ **Tracked:** [NEON-15](https://neon-sprawl.atlassian.net/browse/NEON-15) under epic [NEON-13 — Tech Debt](https://neon-sprawl.atlassian.net/browse/NEON-13) — dispose `NpgsqlDataSource` on host shutdown (was: singleton never disposed; acceptable for prototype until addressed).
1. ~~**Docs alignment (recommended before merge):**~~ **Done**`E1_M1_InputAndMovementRuntime.md` and `documentation_and_implementation_alignment.md` updated for NEO-8 / optional Postgres (planned follow-up; separate from **Nit 1** below).
2. ~~**Lifecycle:**~~ **Tracked:** [NEO-13](https://linear.app/neon-sprawl/issue/NEO-13) under epic [Tech Debt](https://linear.app/neon-sprawl/project/tech-debt-d2148715e875) — dispose `NpgsqlDataSource` on host shutdown (was: singleton never disposed; acceptable for prototype until addressed).
3. ~~**Naming clarity:**~~ **Done**`EnsureSchema()` XML summary in `PostgresPositionStateStore.cs` now states the heavy path (read migration + DDL) runs at most once per process; later calls are the fast volatile-read path. No rename.
---
@ -68,4 +68,4 @@ Before merge, the author should run:
and `dotnet test NeonSprawl.sln -c Release` — all tests including `PostgresPositionStateIntegrationTests` should pass.
3. **Manual AC (optional):** `dotnet run` with connection string set, `POST …/move`, restart server, `GET …/position` unchanged.
Ensure **all** NEON-5 files (including `server/db/` and new test types) are **staged and committed** — at review time much of the implementation was still untracked relative to `git status`.
Ensure **all** NEO-8 files (including `server/db/` and new test types) are **staged and committed** — at review time much of the implementation was still untracked relative to `git status`.

View File

@ -1,12 +1,12 @@
# Code review — NEON-6 (InteractionRequest + related)
# Code review — NEO-9 (InteractionRequest + related)
**Date:** 2026-04-04
**Scope:** Branch `NEON-6-interaction-request-range-check` and **current working tree** (large NEON-6 implementation + Postgres test ergonomics + dev HTTP logging + Godot fixes are **modified/untracked** vs `origin/main`; only the plan file is committed ahead of main in this clone).
**Scope:** Branch `NEO-9-interaction-request-range-check` and **current working tree** (large NEO-9 implementation + Postgres test ergonomics + dev HTTP logging + Godot fixes are **modified/untracked** vs `origin/main`; only the plan file is committed ahead of main in this clone).
**Base:** `origin/main` (merge-base `a2015dd4bfe2e6554d677507499ed33e6a9e50b5` where available).
## Verdict
**Approve with nits** — Behavior matches NEON-6 and E1.M1 intent; tests and docs are in good shape. **Commit and push the full implementation** before opening/merging the PR so review and CI run on the real diff.
**Approve with nits** — Behavior matches NEO-9 and E1.M1 intent; tests and docs are in good shape. **Commit and push the full implementation** before opening/merging the PR so review and CI run on the real diff.
## Summary
@ -16,25 +16,25 @@ The work delivers **server-authoritative** `POST /game/players/{id}/interact` wi
| Document | Assessment |
|----------|------------|
| `docs/plans/NEON-6-implementation-plan.md` | **Matches** — Endpoint, HTTP semantics, horizontal distance, registry defaults, DTO rules, client glow + POST, file list, and AC checklist align with the implementation. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches** — Snapshot documents InteractionRequest + range check and links to NEON-6 / server README. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M1 row references NEON-6 and server paths. |
| `docs/plans/NEO-9-implementation-plan.md` | **Matches** — Endpoint, HTTP semantics, horizontal distance, registry defaults, DTO rules, client glow + POST, file list, and AC checklist align with the implementation. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches** — Snapshot documents InteractionRequest + range check and links to NEO-9 / server README. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M1 row references NEO-9 and server paths. |
| `docs/decomposition/modules/client_server_authority.md` (E1.M1) | **Matches** — Position-derived rules stay server-side; client glow is explicitly non-authoritative. |
| `server/README.md` | **Matches** — Interaction subsection, Postgres + `postgres.runsettings`, Rider xUnit note, Docker automation note, dev logging implied via Development config. |
| `client/README.md` | **Matches** — NEON-4/NEON-6 manual checks, `interact` / `_input` note. |
| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M1 status remains “In Progress”; no conflict. Optional follow-up: bump or refine **register** row if you track NEON-6 explicitly there (not required if table stays high-level). |
| `client/README.md` | **Matches** — NEO-7/NEO-9 manual checks, `interact` / `_input` note. |
| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M1 status remains “In Progress”; no conflict. Optional follow-up: bump or refine **register** row if you track NEO-9 explicitly there (not required if table stays high-level). |
## Blocking issues
_None identified in the design or code paths reviewed._
**Process (merge gate):** Ensure **all** server/client/doc files for NEON-6 and the test-harness/logging changes are **committed** on the branch so GitHub CI and reviewers see one coherent diff.
**Process (merge gate):** Ensure **all** server/client/doc files for NEO-9 and the test-harness/logging changes are **committed** on the branch so GitHub CI and reviewers see one coherent diff.
## Suggestions
1. **`HorizontalReach`:** ~~Consider guarding **`radius < 0`**~~ **Done**`ArgumentOutOfRangeException.ThrowIfNegative` + unit test.
2. **`InteractionRequestClient`:** ~~Optional **`_busy`** flag~~ **Done** — ignores **E** while HTTP in flight.
3. **Postgres + `postgres.runsettings`:** ~~Call out in PR description~~ **Done**`server/README.md` (**Contributors / PRs**) and `docs/plans/NEON-6-implementation-plan.md` (reviewer note under PR draft).
3. **Postgres + `postgres.runsettings`:** ~~Call out in PR description~~ **Done**`server/README.md` (**Contributors / PRs**) and `docs/plans/NEO-9-implementation-plan.md` (reviewer note under PR draft).
## Nits

View File

@ -1,7 +1,7 @@
# Code review: NEON-8 client path-follow (`NEON-8-client-path-follow`)
# Code review: NEO-11 client path-follow (`NEO-11-client-path-follow`)
**Date:** 2026-04-05
**Scope:** Branch `NEON-8-client-path-follow` vs `origin/main` (full NEON-8 slice: client nav, wiring, docs, small server remark).
**Scope:** Branch `NEO-11-client-path-follow` vs `origin/main` (full NEO-11 slice: client nav, wiring, docs, small server remark).
**Base:** `origin/main`
**Follow-up:** Design and docs were aligned after the initial review; blocking items and suggestions below are **done** unless marked otherwise.
@ -16,17 +16,17 @@ The branch adds **`NavigationRegion3D`** / **`NavigationAgent3D`**, sync nav bak
**`player.gd`** intentionally skips nav waypoints when **`_auth_walk_goal.y < global_position.y - DESCEND_GOAL_Y_MARGIN`**, so for typical picks (floor **Y** vs mid-capsule origin) the client often **bee-lines in xz** toward the goal. That matches the **locked prototype tradeoff** (smooth bumps; multi-click around obstacles when needed).
**Idle optimization:** skipping **`move_and_slide()`** when there is no walk goal, velocity is zero, and **`is_on_floor()`** is a reasonable jitter mitigation; residual vibration is tracked in **[NEON-16](https://neon-sprawl.atlassian.net/browse/NEON-16)** (Tech Debt).
**Idle optimization:** skipping **`move_and_slide()`** when there is no walk goal, velocity is zero, and **`is_on_floor()`** is a reasonable jitter mitigation; residual vibration is tracked in **[NEO-14](https://linear.app/neon-sprawl/issue/NEO-14)** (Tech Debt).
## Documentation checked
| Document | Result (at closure) |
|----------|---------------------|
| `docs/plans/NEON-8-implementation-plan.md` | **Matches** after **Locked — prototype movement tradeoff**, updated AC, tests, and **NEON-16** link. |
| `docs/plans/NEO-11-implementation-plan.md` | **Matches** after **Locked — prototype movement tradeoff**, updated AC, tests, and **NEO-14** link. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches** — snapshot notes single-click obstacle detours not guaranteed. |
| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server owns **`PositionState`**; client presentation path valid. |
| `docs/decomposition/modules/module_dependency_register.md` / `documentation_and_implementation_alignment.md` | **N/A** deep diff. |
| `client/README.md` | **Matches** — tradeoff, manual steps, **Known issue** + **NEON-16** link. |
| `client/README.md` | **Matches** — tradeoff, manual steps, **Known issue** + **NEO-14** link. |
| `server/README.md` | **Matches** — client nav presentation; detours not guaranteed on one click. |
## Blocking issues
@ -41,7 +41,7 @@ The branch adds **`NavigationRegion3D`** / **`NavigationAgent3D`**, sync nav bak
1. ~~**`DESCEND_GOAL_Y_MARGIN`:**~~ **Done.** Comment added in `client/scripts/player.gd` (surface pick Y vs mid-capsule; bumps vs obstacles).
2. ~~**`_skip_move_when_idle_on_floor`:**~~ **Done.** Doc comment above the helper (`is_on_floor()` / last `move_and_slide()`; static floors).
3. ~~**NEON-16:**~~ **Done.** Out of NEON-8 scope in plan; **[NEON-16](https://neon-sprawl.atlassian.net/browse/NEON-16)** linked from **`NEON-8-implementation-plan.md`** and **`client/README.md`**.
3. ~~**NEO-14:**~~ **Done.** Out of NEO-11 scope in plan; **[NEO-14](https://linear.app/neon-sprawl/issue/NEO-14)** linked from **`NEO-11-implementation-plan.md`** and **`client/README.md`**.
## Nits
@ -51,5 +51,5 @@ The branch adds **`NavigationRegion3D`** / **`NavigationAgent3D`**, sync nav bak
- `cd server/NeonSprawl.Server && dotnet test` (no logic change expected, sanity).
- Godot **F5** with server: boot snap; click floor; bump flow; gray **Obstacle** may need **multiple clicks** per docs.
- Confirm NEON-7 reject targets still show rejection UX (`reasonCode` / label).
- Confirm NEO-10 reject targets still show rejection UX (`reasonCode` / label).
- `gdlint` / `gdformat` on touched GDScript if CI does not already run on the branch.

View File

@ -0,0 +1,55 @@
# Code review — NEO-14 idle / rest stability (Godot client)
**Date:** 2026-04-05
**Scope:** Branch `NEO-14-idle-vibration-fix` vs merge-base `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61` (tracked as `origin/main` at review time). Issue **NEO-14**; no PR URL supplied.
**Base:** `origin/main` @ `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61`
**Follow-up:** Review **suggestions** below are **done** (strikethrough + **Done.**); **nits** remain optional.
## Verdict
**Approve with nits**
## Summary
This branch delivers **NEO-14** by hardening the prototype **`CharacterBody3D`** idle path (`player.gd`): dual **`floor_max_angle`** behavior, rim/straddle detection, idle nudges, bump proximity escape, **`floor_block_on_wall`**, **`NavigationAgent3D.avoidance_enabled = false`**, and **physics interpolation off** at project + player node level. It fixes **compound internal-edge** issues by spawning each QA bump as its **own `StaticBody3D`** (`random_floor_bumps.gd`), adds **Jolt** in **`project.godot`**, and reduces **z-fighting** via capsule **mesh Y lift** (visual-only vs collision shape). **`client/README.md`** and **`server/README.md`** (bump height copy) are updated. **Server contracts** (`MoveCommand` / `PositionState`) are not changed; risk is localized to client feel and QA geometry. Manual QA by the author reports **no remaining jitter** in exercised poses.
## Documentation checked
| Document | Result |
|----------|--------|
| `docs/plans/NEO-14-implementation-plan.md` | **Matches****Resolution**, checked AC, **Shipped note**, updated **Open questions** (`015ca13`). |
| `docs/plans/NEO-11-implementation-plan.md` | **Matches** at intent level (client nav visual, server authority unchanged); no contract drift observed in the diff. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches.** Prototype client locomotion and reconciliation surface remain consistent; changes are polish under “movement-loop” / QA, not authority or wire shape. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** for code behavior; cited as the hub for module IDs. No register **Status** change required for this client-only jitter fix. |
| `docs/decomposition/modules/client_server_authority.md` | **N/A** (no change to who owns `PositionState` or move validation). |
| `client/README.md` | **Matches** NEO-14 call to document the pattern and manual steps (idle 10+ s, bumps). |
## Blocking issues
*(none)*
## Suggestions
**All done.**
1. ~~**Close the loop in `docs/plans/NEO-14-implementation-plan.md`:**~~ **Done.** Check off acceptance criteria; add **Resolution** (shipped mitigations) and **Shipped note** under technical approach — `015ca13`.
2. ~~**DRY bump collision constants** (`BUMP_COLLISION_*` aligned across `player.gd` / `random_floor_bumps.gd`):~~ **Done.** `client/scripts/random_floor_bump_collision_constants.gd` + preload from both scripts; README references — `015ca13`, doc sync `87d8ee0` (file later renamed off **`ns19_*`** identifiers).
3. ~~**Future character yaw** (`_snap_capsule_upright()` full identity basis):~~ **Done.** `TODO(NEO-14 follow-on)` on `_snap_capsule_upright()` in `player.gd`; plan **Open questions**`015ca13`.
## Nits
- **`get_tree().get_nodes_in_group(...)`** on **`random_floor_bump_mesh`** each idle tick is fine at **two** bumps; if the group grows, cache references after spawn or guard cost.
- ~~**Commit history:**~~ **Done.** Branch history squashed to a **single** **`NEO-14:`** commit before merge; no stray **`fix:`** / **`docs:`** subjects on the slice.
## Verification
Per **`NEO-14-implementation-plan.md`** and **`client/README.md`**:
1. Run **`main.tscn`** with server as in NEO-7/NEO-11 README steps.
2. **Idle 10+ s** at spawn and after a move (no click goal); confirm **no visible vibration** on slab and on **random green bumps** (new positions each run).
3. **Walking**, **arrival**, **`snap_to_server`** on cold boot; **NEO-10** reject UX (pedestal / far pad) unchanged.
4. **Obstacle / descend** flows per NEO-11 tradeoff (multi-click around tall obstacle if needed).
5. Spot-check **bump rim**, **cylinder side**, and **floorbump** contact (prior jitter loci).
**Automated:** None required for NEO-14 (no Godot harness in repo).

View File

@ -1,55 +0,0 @@
# Code review — NEON-16 idle / rest stability (Godot client)
**Date:** 2026-04-05
**Scope:** Branch `NEON-16-idle-vibration-fix` vs merge-base `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61` (tracked as `origin/main` at review time). Issue **NEON-16**; no PR URL supplied.
**Base:** `origin/main` @ `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61`
**Follow-up:** Review **suggestions** below are **done** (strikethrough + **Done.**); **nits** remain optional.
## Verdict
**Approve with nits**
## Summary
This branch delivers **NEON-16** by hardening the prototype **`CharacterBody3D`** idle path (`player.gd`): dual **`floor_max_angle`** behavior, rim/straddle detection, idle nudges, bump proximity escape, **`floor_block_on_wall`**, **`NavigationAgent3D.avoidance_enabled = false`**, and **physics interpolation off** at project + player node level. It fixes **compound internal-edge** issues by spawning each QA bump as its **own `StaticBody3D`** (`random_floor_bumps.gd`), adds **Jolt** in **`project.godot`**, and reduces **z-fighting** via capsule **mesh Y lift** (visual-only vs collision shape). **`client/README.md`** and **`server/README.md`** (bump height copy) are updated. **Server contracts** (`MoveCommand` / `PositionState`) are not changed; risk is localized to client feel and QA geometry. Manual QA by the author reports **no remaining jitter** in exercised poses.
## Documentation checked
| Document | Result |
|----------|--------|
| `docs/plans/NEON-16-implementation-plan.md` | **Matches****Resolution**, checked AC, **Shipped note**, updated **Open questions** (`015ca13`). |
| `docs/plans/NEON-8-implementation-plan.md` | **Matches** at intent level (client nav visual, server authority unchanged); no contract drift observed in the diff. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches.** Prototype client locomotion and reconciliation surface remain consistent; changes are polish under “movement-loop” / QA, not authority or wire shape. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** for code behavior; cited as the hub for module IDs. No register **Status** change required for this client-only jitter fix. |
| `docs/decomposition/modules/client_server_authority.md` | **N/A** (no change to who owns `PositionState` or move validation). |
| `client/README.md` | **Matches** NEON-16 call to document the pattern and manual steps (idle 10+ s, bumps). |
## Blocking issues
*(none)*
## Suggestions
**All done.**
1. ~~**Close the loop in `docs/plans/NEON-16-implementation-plan.md`:**~~ **Done.** Check off acceptance criteria; add **Resolution** (shipped mitigations) and **Shipped note** under technical approach — `015ca13`.
2. ~~**DRY bump collision constants** (`BUMP_COLLISION_*` aligned across `player.gd` / `random_floor_bumps.gd`):~~ **Done.** `client/scripts/random_floor_bump_collision_constants.gd` + preload from both scripts; README references — `015ca13`, doc sync `87d8ee0` (file later renamed off **`ns19_*`** identifiers).
3. ~~**Future character yaw** (`_snap_capsule_upright()` full identity basis):~~ **Done.** `TODO(NEON-16 follow-on)` on `_snap_capsule_upright()` in `player.gd`; plan **Open questions**`015ca13`.
## Nits
- **`get_tree().get_nodes_in_group(...)`** on **`random_floor_bump_mesh`** each idle tick is fine at **two** bumps; if the group grows, cache references after spawn or guard cost.
- ~~**Commit history:**~~ **Done.** Branch history squashed to a **single** **`NEON-16:`** commit before merge; no stray **`fix:`** / **`docs:`** subjects on the slice.
## Verification
Per **`NEON-16-implementation-plan.md`** and **`client/README.md`**:
1. Run **`main.tscn`** with server as in NEON-4/NEON-8 README steps.
2. **Idle 10+ s** at spawn and after a move (no click goal); confirm **no visible vibration** on slab and on **random green bumps** (new positions each run).
3. **Walking**, **arrival**, **`snap_to_server`** on cold boot; **NEON-7** reject UX (pedestal / far pad) unchanged.
4. **Obstacle / descend** flows per NEON-8 tradeoff (multi-click around tall obstacle if needed).
5. Spot-check **bump rim**, **cylinder side**, and **floorbump** contact (prior jitter loci).
**Automated:** None required for NEON-16 (no Godot harness in repo).

View File

@ -1,8 +1,8 @@
# Code review: NEON-25 (isometric follow camera)
# Code review: NEO-15 (isometric follow camera)
**Date:** 2026-04-07
**Scope:** Branch `NEON-25-isometric-follow-camera` vs `main`; issue [NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25). Commits on branch include implementation plan, locked defaults, **NEON-29** backlog row in `E1_M2_IsometricCameraController.md`, and camera implementation + tests + docs. Manual testing reported OK by author.
**Base:** `main` (compare: `main...NEON-25-isometric-follow-camera`).
**Scope:** Branch `NEO-15-isometric-follow-camera` vs `main`; issue [NEO-15](https://linear.app/neon-sprawl/issue/NEO-15). Commits on branch include implementation plan, locked defaults, **NEO-19** backlog row in `E1_M2_IsometricCameraController.md`, and camera implementation + tests + docs. Manual testing reported OK by author.
**Base:** `main` (compare: `main...NEO-15-isometric-follow-camera`).
## Verdict
@ -16,10 +16,10 @@ The change replaces a static `Camera3D` with an `IsometricFollowCamera` rig that
| Document | Result |
|----------|--------|
| `docs/plans/NEON-25-implementation-plan.md` | **Matches** — locked defaults (`_process`, damped follow + snap, `NodePath` follow, per-frame `CameraState`), file list, tests, and README/module snapshot are reflected in the diff. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot documents rig scripts, yaw seam, single-band distance; Jira table includes NEON-29 (separate ticket; see nits). |
| `docs/plans/NEO-15-implementation-plan.md` | **Matches** — locked defaults (`_process`, damped follow + snap, `NodePath` follow, per-frame `CameraState`), file list, tests, and README/module snapshot are reflected in the diff. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot documents rig scripts, yaw seam, single-band distance; Linear backlog table includes NEO-19 (separate ticket; see nits). |
| `docs/decomposition/modules/client_server_authority.md` (E1.M2) | **Matches** — camera and `CameraState` remain client-local; no server use of camera pose. |
| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M2 row still **Planned** with full contract set; NEON-25 delivers a slice (follow + `CameraState` seam), not zoom/occlusion. Updating register or module **Status** after merge is optional hygiene, not required for this PRs correctness. |
| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M2 row still **Planned** with full contract set; NEO-15 delivers a slice (follow + `CameraState` seam), not zoom/occlusion. Updating register or module **Status** after merge is optional hygiene, not required for this PRs correctness. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** — no E1.M2-specific row found; no conflict. |
## Blocking issues
@ -34,10 +34,10 @@ None.
## Nits
- **Nit:** `camera_state.gd` header references `[member presentation_yaw_deg]`; that property lives on `isometric_follow_camera.gd`, not on `CameraState`—minor doc confusion for Godot help tooltips.
- **Nit:** Same branch carries **NEON-29** (prototype district) backlog registration; harmless, but reviewers merging a “NEON-25 only” PR should expect that small doc delta.
- **Nit:** Same branch carries **NEO-19** (prototype district) backlog registration; harmless, but reviewers merging a “NEO-15 only” PR should expect that small doc delta.
- **Nit:** Plan wording “set state yaw to `0` when `allow_yaw` is false” is satisfied via `_orbit_yaw_rad = 0` and `_state.yaw = _orbit_yaw_rad`; presentation yaw remains in exports—consistent with module intent but worth remembering for readers who expect a single “total yaw” on state.
## Verification
- **Automated:** From `client/`, run GdUnit headless per `client/README.md` (Godot 4.6 import + `GdUnitCmdTool` on `res://test`).
- **Manual:** (Author confirmed OK.) Re-run: server + F5, click-move floor and NEON-7 bumps, ground pick from moving camera, idle observation; optional inspector tweak of `follow_smoothness` / `snap_distance` if feel regresses on different hardware.
- **Manual:** (Author confirmed OK.) Re-run: server + F5, click-move floor and NEO-10 bumps, ground pick from moving camera, idle observation; optional inspector tweak of `follow_smoothness` / `snap_distance` if feel regresses on different hardware.

View File

@ -1,7 +1,7 @@
# Code review: NEON-15
# Code review: NEO-13
**Date:** 2026-04-08
**Scope:** Branch `NEON-15-dispose-npgsql-datasource` vs `main` — [NEON-15](https://neon-sprawl.atlassian.net/browse/NEON-15) (dispose `NpgsqlDataSource` on shutdown), plus planning-doc rule cross-links.
**Scope:** Branch `NEO-13-dispose-npgsql-datasource` vs `main` — [NEO-13](https://linear.app/neon-sprawl/issue/NEO-13) (dispose `NpgsqlDataSource` on shutdown), plus planning-doc rule cross-links.
**Base:** `main` (range `main...HEAD`, commits `abffd29``62ff210`).
**Follow-up:** **Suggestions** and **nits** below are **done** (strikethrough + **Done.**).
@ -20,9 +20,9 @@ Residual risk is low and already acknowledged in the plan: the root `ServiceProv
| Document | Result |
|----------|--------|
| `docs/plans/NEON-15-implementation-plan.md` | **Matches** — Decisions (Option 2, ConfigureAwait), AC checked, technical approach and file lists align with the diff; open questions resolved per [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md). |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches (post-review update)** — Snapshot links NEON-15 for shared `NpgsqlDataSource` shutdown — `8c3e8c0`. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches (post-review update)** — E1.M1 row snapshot + plans column include NEON-15`8c3e8c0`. |
| `docs/plans/NEO-13-implementation-plan.md` | **Matches** — Decisions (Option 2, ConfigureAwait), AC checked, technical approach and file lists align with the diff; open questions resolved per [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md). |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches (post-review update)** — Snapshot links NEO-13 for shared `NpgsqlDataSource` shutdown — `8c3e8c0`. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches (post-review update)** — E1.M1 row snapshot + plans column include NEO-13`8c3e8c0`. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no status or dependency change required for a shutdown-only fix. |
| `docs/decomposition/modules/client_server_authority.md`, `contracts.md` | **N/A** — no authority or wire-shape change. |
| `.cursor/rules/planning-implementation-docs.md`, `story-kickoff.md`, `code-review-agent.md`, `docs-review-agent.md`, `AGENTS.md` | **Matches** — rule text is clear; relative links in agent rules remain valid. |
@ -35,8 +35,8 @@ _None._
**All done.**
1. ~~**Traceability (optional):** Add `NEON-15` / `docs/plans/NEON-15-implementation-plan.md` to the E1.M1 **Plans / pointers** column in `documentation_and_implementation_alignment.md` (or a one-line note under Postgres in `E1_M1_InputAndMovementRuntime.md`) so future readers find shutdown lifecycle work next to NEON-5.~~ **Done.** E1.M1 snapshot + plans list + module implementation snapshot updated — `8c3e8c0`.
2. ~~**PR narrative:** Call out the **two themes** in the branch (server disposal + planning-doc rule) so reviewers do not miss the `.cursor/` and `AGENTS.md` changes if they only skim `server/`.~~ **Done.** `docs/plans/NEON-15-implementation-plan.md` **PR / review** section lists server vs repo-process themes — `8c3e8c0`.
1. ~~**Traceability (optional):** Add `NEO-13` / `docs/plans/NEO-13-implementation-plan.md` to the E1.M1 **Plans / pointers** column in `documentation_and_implementation_alignment.md` (or a one-line note under Postgres in `E1_M1_InputAndMovementRuntime.md`) so future readers find shutdown lifecycle work next to NEO-8.~~ **Done.** E1.M1 snapshot + plans list + module implementation snapshot updated — `8c3e8c0`.
2. ~~**PR narrative:** Call out the **two themes** in the branch (server disposal + planning-doc rule) so reviewers do not miss the `.cursor/` and `AGENTS.md` changes if they only skim `server/`.~~ **Done.** `docs/plans/NEO-13-implementation-plan.md` **PR / review** section lists server vs repo-process themes — `8c3e8c0`.
## Nits

View File

@ -1,7 +1,7 @@
# Code review — NEON-26 (ZoomBandConfig + discrete zoom)
# Code review — NEO-16 (ZoomBandConfig + discrete zoom)
**Date:** 2026-04-08
**Scope:** Branch `NEON-26-zoom-band-config-discrete-input` vs `origin/main` (NEON-26 zoom bands + follow-up `.tres` tuning commits). Issue **NEON-26**; no PR URL supplied.
**Scope:** Branch `NEO-16-zoom-band-config-discrete-input` vs `origin/main` (NEO-16 zoom bands + follow-up `.tres` tuning commits). Issue **NEO-16**; no PR URL supplied.
**Base:** `origin/main` @ `88653292104eddfc1db702f97b02a088fb80c1c7` (merge-base with HEAD at review time).
**Follow-up:** Suggestions and nits below were implemented in-repo (README, plan typing note, positive-distance validation, `_sync_camera_state(effective_distance)`, telemetry TODO wording) and re-reviewed after `ff83081`.
@ -17,11 +17,11 @@ The branch delivers **data-driven discrete zoom** via `ZoomBandConfig`, **InputM
| Document | Result |
|----------|--------|
| `docs/plans/NEON-26-implementation-plan.md` | **Matches** — technical approach updated to document **`Resource` export + `get_script()`** and positive-distance fallback; aligns with implementation. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot updated for NEON-26 (config path, input actions, distance-only bands, fallback, signal + E9.M1 TODO). |
| `docs/plans/NEO-16-implementation-plan.md` | **Matches** — technical approach updated to document **`Resource` export + `get_script()`** and positive-distance fallback; aligns with implementation. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot updated for NEO-16 (config path, input actions, distance-only bands, fallback, signal + E9.M1 TODO). |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 note reflects zoom bands shipped. |
| `docs/decomposition/modules/client_server_authority.md` (camera client-local) | **Matches** — no server use of camera pose introduced. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** for mandatory table edits; E1.M2 status remains “In progress” appropriately (NEON-27/28 open). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** for mandatory table edits; E1.M2 status remains “In progress” appropriately (NEO-17/28 open). |
## Blocking issues
@ -33,7 +33,7 @@ None.
2. ~~**`ZoomBandConfig` validation** — Designer-editable **`band_distances`** could theoretically include **non-positive** values, which would produce odd framing. Optional: `_validate_property` or a short runtime guard in the rig when loading config (non-blocking for prototype).~~ **Done.** **`all_band_distances_positive()`** on the resource; **`_zoom_config_valid()`** and **`effective_follow_distance`** fall back when any distance is ≤ 0; tests added.
3. ~~**Plan vs implementation (typing)** — As above, add a brief note to **`NEON-26-implementation-plan.md`** that **`zoom_band_config` is exported as `Resource`** (or untyped) with script identity checked at runtime, so future readers are not confused by the plans “ZoomBandConfig” type mention.~~ **Done.** Technical approach step 2 rewritten accordingly.
3. ~~**Plan vs implementation (typing)** — As above, add a brief note to **`NEO-16-implementation-plan.md`** that **`zoom_band_config` is exported as `Resource`** (or untyped) with script identity checked at runtime, so future readers are not confused by the plans “ZoomBandConfig” type mention.~~ **Done.** Technical approach step 2 rewritten accordingly.
## Nits

View File

@ -1,7 +1,7 @@
# Code review — NEON-27 (OcclusionPolicy)
# Code review — NEO-17 (OcclusionPolicy)
**Date:** 2026-04-08
**Scope:** Branch `NEON-27-occlusion-policy` vs `origin/main` (NEON-27 occlusion policy resource, camera integration, scene wiring, tests, and docs). Issue **NEON-27**; no PR URL supplied.
**Scope:** Branch `NEO-17-occlusion-policy` vs `origin/main` (NEO-17 occlusion policy resource, camera integration, scene wiring, tests, and docs). Issue **NEO-17**; no PR URL supplied.
**Base:** `origin/main` @ `1d891eb8bb42ccc5e8a74b4fcdb349b9165f3454` (merge-base with HEAD at review time).
**Follow-up:** Previously requested changes below are done and were re-reviewed against the updated branch state.
@ -17,9 +17,9 @@ The branch adds a data-driven `OcclusionPolicy` resource, wires it into `Isometr
| Document | Result |
|----------|--------|
| `docs/plans/NEON-27-implementation-plan.md` | **Matches** — technical approach now reflects the implemented three-way material handling, including the explicit null-material fallback. |
| `docs/plans/NEO-17-implementation-plan.md` | **Matches** — technical approach now reflects the implemented three-way material handling, including the explicit null-material fallback. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot and readability gate note align with the code and scene changes. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 still correctly reads **In progress**, and the note now reflects follow + zoom + occlusion shipped with NEON-28 remaining. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 still correctly reads **In progress**, and the note now reflects follow + zoom + occlusion shipped with NEO-18 remaining. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — implementation tracking now includes `E1.M2` with current shipped scope and pointers. |
| `docs/decomposition/modules/client_server_authority.md` | **Matches** — the change stays client-local and does not introduce any server use of camera pose. |
@ -29,9 +29,9 @@ The branch adds a data-driven `OcclusionPolicy` resource, wires it into `Isometr
## Suggestions
1. ~~**`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth.~~ **Done.** The “Material override strategy” section now documents the three explicit cases: `StandardMaterial3D`, null material, and non-`StandardMaterial3D`.
1. ~~**`docs/plans/NEO-17-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth.~~ **Done.** The “Material override strategy” section now documents the three explicit cases: `StandardMaterial3D`, null material, and non-`StandardMaterial3D`.
2. ~~**`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module.~~ **Done.** The register note now says NEON-25 through NEON-27 are in repo, and the implementation alignment table now includes an `E1.M2` row with current scope and pointers.
2. ~~**`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module.~~ **Done.** The register note now says NEO-15 through NEO-17 are in repo, and the implementation alignment table now includes an `E1.M2` row with current scope and pointers.
## Nits

View File

@ -0,0 +1,38 @@
# NEO-20 review
**Date:** 2026-04-09
**Scope:** Branch `NEO-20-click-through-input` reviewed against `origin/main...HEAD`
**Base:** `origin/main`
## Verdict
Approve with nits
## Summary
This branch keeps the fix narrowly scoped to `client/scripts/ground_pick.gd`: occluder-tagged bodies are now skipped before the existing walkable-surface logic runs, which matches the intended NEO-20 behavior and avoids introducing a new collision-layer contract. The supporting unit tests and module/README updates are directionally correct and consistent with the existing `"occluder"` group convention established in NEO-17. I did not find a correctness issue in the shipped code path from the diff alone. I could not run the Godot/GdUnit verification commands in this environment because `godot` is not installed locally, so the full click-through behavior still needs author-side verification.
## Documentation checked
- `docs/plans/NEO-20-implementation-plan.md` — partially matches. The implementation and recorded decisions line up with the diff, but the acceptance-criteria checklist is still entirely unchecked even though the branch is presented as implemented; per the plan/doc workflow, that checklist should be refreshed before merge.
- `docs/decomposition/modules/E1_M2_IsometricCameraController.md` — matches. The new click-through input note accurately records the shared `"occluder"` group contract and the absence of a new collision-layer convention.
- `docs/decomposition/modules/module_dependency_register.md` — matches / no update needed. `E1.M2` remains `In progress`; this story does not appear to change the module readiness state.
- `.cursor/rules/testing-expectations.md` — matches. The branch updates `client/test/ground_pick_test.gd` alongside the production GDScript change and still relies on manual verification for the full pick flow, which is acceptable for this scope.
## Blocking issues
None.
## Suggestions
1. ~~Refresh the acceptance-criteria checklist in `docs/plans/NEO-20-implementation-plan.md` before merge. The current file still shows every box unchecked, which leaves the canonical story artifact out of sync with the implemented branch and weakens the plan-as-acceptance-criteria workflow this repo is using.~~ Done. All three AC boxes checked in `docs/plans/NEO-20-implementation-plan.md`.
## Nits
- ~~Nit: Consider adding a short NEO-20-specific manual check bullet to `client/README.md` near the existing movement verification steps so the expected "click behind the obstacle while opaque/faded" regression check is visible where contributors already look for client runtime checks.~~ Done. Added `### Manual check (NEO-20 — occluder click-through)` section in `client/README.md` covering opaque/faded occluder clicks and regression cases.
## Verification
- Install Godot 4.6 on the author machine, then from `client/` run `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test`.
- Run the manual NEO-20 scenario from `client/scenes/main.tscn`: click the ground behind `World/NavigationRegion3D/Obstacle` while it is opaque and while it is faded by the occlusion system; confirm the player still receives a valid move target behind it.
- Re-check the existing non-occluder cases called out in `client/README.md`, especially normal floor clicks and the far pad / pedestal rejection behavior, to confirm the added occluder skip did not regress earlier ground-pick rules.

View File

@ -1,38 +0,0 @@
# NEON-30 review
**Date:** 2026-04-09
**Scope:** Branch `NEON-30-click-through-input` reviewed against `origin/main...HEAD`
**Base:** `origin/main`
## Verdict
Approve with nits
## Summary
This branch keeps the fix narrowly scoped to `client/scripts/ground_pick.gd`: occluder-tagged bodies are now skipped before the existing walkable-surface logic runs, which matches the intended NEON-30 behavior and avoids introducing a new collision-layer contract. The supporting unit tests and module/README updates are directionally correct and consistent with the existing `"occluder"` group convention established in NEON-27. I did not find a correctness issue in the shipped code path from the diff alone. I could not run the Godot/GdUnit verification commands in this environment because `godot` is not installed locally, so the full click-through behavior still needs author-side verification.
## Documentation checked
- `docs/plans/NEON-30-implementation-plan.md` — partially matches. The implementation and recorded decisions line up with the diff, but the acceptance-criteria checklist is still entirely unchecked even though the branch is presented as implemented; per the plan/doc workflow, that checklist should be refreshed before merge.
- `docs/decomposition/modules/E1_M2_IsometricCameraController.md` — matches. The new click-through input note accurately records the shared `"occluder"` group contract and the absence of a new collision-layer convention.
- `docs/decomposition/modules/module_dependency_register.md` — matches / no update needed. `E1.M2` remains `In progress`; this story does not appear to change the module readiness state.
- `.cursor/rules/testing-expectations.md` — matches. The branch updates `client/test/ground_pick_test.gd` alongside the production GDScript change and still relies on manual verification for the full pick flow, which is acceptable for this scope.
## Blocking issues
None.
## Suggestions
1. ~~Refresh the acceptance-criteria checklist in `docs/plans/NEON-30-implementation-plan.md` before merge. The current file still shows every box unchecked, which leaves the canonical story artifact out of sync with the implemented branch and weakens the plan-as-acceptance-criteria workflow this repo is using.~~ Done. All three AC boxes checked in `docs/plans/NEON-30-implementation-plan.md`.
## Nits
- ~~Nit: Consider adding a short NEON-30-specific manual check bullet to `client/README.md` near the existing movement verification steps so the expected "click behind the obstacle while opaque/faded" regression check is visible where contributors already look for client runtime checks.~~ Done. Added `### Manual check (NEON-30 — occluder click-through)` section in `client/README.md` covering opaque/faded occluder clicks and regression cases.
## Verification
- Install Godot 4.6 on the author machine, then from `client/` run `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test`.
- Run the manual NEON-30 scenario from `client/scenes/main.tscn`: click the ground behind `World/NavigationRegion3D/Obstacle` while it is opaque and while it is faded by the occlusion system; confirm the player still receives a valid move target behind it.
- Re-check the existing non-occluder cases called out in `client/README.md`, especially normal floor clicks and the far pad / pedestal rejection behavior, to confirm the added occluder skip did not regress earlier ground-pick rules.

View File

@ -1,8 +1,8 @@
# Code review — NEON-16 idle jitter follow-up (Godot client)
# Code review — NEO-14 idle jitter follow-up (Godot client)
**Date:** 2026-04-10
**Scope:** Branch `NEON-16-idle-jitter-followup` vs `origin/main` @ `2aff663`. Issue **NEON-16** (re-opened). No PR URL supplied; working-tree diff reviewed.
**Base:** `origin/main` @ `2aff663` (post-NEON-30 merge)
**Scope:** Branch `NEO-14-idle-jitter-followup` vs `origin/main` @ `2aff663`. Issue **NEO-14** (re-opened). No PR URL supplied; working-tree diff reviewed.
**Base:** `origin/main` @ `2aff663` (post-NEO-20 merge)
---
@ -14,7 +14,7 @@
## Summary
This branch re-opens NEON-16 to eliminate residual idle x/z drift on flat ground that became worse after the NEON-25/26/27/30 follow-camera work. The fix is localized entirely to `player.gd`: a stable-flat-support fast-path that skips the corrective `move_and_slide()` / rim-escape loop, an explicit x/z anchor to clamp away residual horizontal creep while the player is classified as stable, a correction-budget cap (`IDLE_MANUAL_CORRECTION_MAX_TICKS`) to prevent edge contacts from running the loop indefinitely, and a capsule-feet reference point for both goal-arrival and descend-bypass checks. `_snap_capsule_upright()` gains an early-out when the basis is already identity. Debug tracing is gated behind `@export var debug_idle_trace`. Thirteen new unit tests cover the new static helpers and state invariants. Server contracts are untouched; risk is client-only.
This branch re-opens NEO-14 to eliminate residual idle x/z drift on flat ground that became worse after the NEO-15/26/27/30 follow-camera work. The fix is localized entirely to `player.gd`: a stable-flat-support fast-path that skips the corrective `move_and_slide()` / rim-escape loop, an explicit x/z anchor to clamp away residual horizontal creep while the player is classified as stable, a correction-budget cap (`IDLE_MANUAL_CORRECTION_MAX_TICKS`) to prevent edge contacts from running the loop indefinitely, and a capsule-feet reference point for both goal-arrival and descend-bypass checks. `_snap_capsule_upright()` gains an early-out when the basis is already identity. Debug tracing is gated behind `@export var debug_idle_trace`. Thirteen new unit tests cover the new static helpers and state invariants. Server contracts are untouched; risk is client-only.
---
@ -22,8 +22,8 @@ This branch re-opens NEON-16 to eliminate residual idle x/z drift on flat ground
| Document | Result |
|----------|--------|
| `docs/plans/NEON-16-implementation-plan.md` | **Matches** — Resolution section updated, Decisions table present, Files to modify and Tests sections reflect what shipped. AC boxes not yet checked (branch not merged). |
| `docs/reviews/2026-04-05-NEON-16.md` | **Matches** — existing review is the prior-story approval; this is a follow-up branch against the same issue. No bullets to strike through in the old file from this diff. |
| `docs/plans/NEO-14-implementation-plan.md` | **Matches** — Resolution section updated, Decisions table present, Files to modify and Tests sections reflect what shipped. AC boxes not yet checked (branch not merged). |
| `docs/reviews/2026-04-05-NEO-14.md` | **Matches** — existing review is the prior-story approval; this is a follow-up branch against the same issue. No bullets to strike through in the old file from this diff. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches** — change is client movement-loop polish; server authority (`PositionState`, `MoveCommand`) and all contracts unchanged. Module status ("Ready — prototype") does not need updating for this fix. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no new modules or dependencies introduced. |
| `docs/decomposition/modules/client_server_authority.md` | **N/A** — no authority boundary changes. |
@ -115,12 +115,12 @@ This branch re-opens NEON-16 to eliminate residual idle x/z drift on flat ground
## Verification
Per `NEON-16-implementation-plan.md` and updated `client/README.md` step 6:
Per `NEO-14-implementation-plan.md` and updated `client/README.md` step 6:
1. Fix the two blocking lint issues and run `gdlint client/scripts client/test` + `gdformat --check client/scripts client/test` locally to confirm clean.
2. Run headless GdUnit: `godot --headless --script res://addons/gdUnit4/bin/GdUnitCmdTool.gd --add res://test/` — expect **52+ tests, 0 errors, 0 failures**.
3. Run `main.tscn` with server: stand idle **10+ s** at spawn on flat ground; confirm no visible vibration and no x/z drift.
4. Walk to a destination, stop, idle again — same stability check.
5. Idle on the random green bumps for **10+ s** — capsule should not drift radially or oscillate.
6. Click-move including stepped bumps (NEON-7 props); confirm walking, arrival, and `snap_to_server` on cold boot are unaffected.
7. NEON-30 occluder click-through sanity (click behind an occluder body — ground pick still registers).
6. Click-move including stepped bumps (NEO-10 props); confirm walking, arrival, and `snap_to_server` on cold boot are unaffected.
7. NEO-20 occluder click-through sanity (click behind an occluder body — ground pick still registers).

View File

@ -1,26 +1,26 @@
# Code review — NEON-28 (Camera integration hardening)
# Code review — NEO-18 (Camera integration hardening)
**Date:** 2026-04-10
**Scope:** Branch `NEON-28-camera-integration-hardening` against `origin/main`.
**Scope:** Branch `NEO-18-camera-integration-hardening` against `origin/main`.
**Base:** `origin/main` (merge-base `01cfd4ec4d8f31a15bbaa0429e46ef0d2ddea4c8`).
## Verdict
**Approve with nits** — The camera hardening and contract/doc updates line up well with NEON-28 and E1.M2 intent, and I did not find a correctness break in the reviewed camera path. ~~Clean up the remaining plan/scope mismatches before merge so the branch tells one consistent story.~~ **Plan Technical approach + manual occluder note addressed post-review; HttpLogging chore left on branch per author (suggestion 2 declined).**
**Approve with nits** — The camera hardening and contract/doc updates line up well with NEO-18 and E1.M2 intent, and I did not find a correctness break in the reviewed camera path. ~~Clean up the remaining plan/scope mismatches before merge so the branch tells one consistent story.~~ **Plan Technical approach + manual occluder note addressed post-review; HttpLogging chore left on branch per author (suggestion 2 declined).**
## Summary
This branch closes the E1.M2 prototype slice in a coherent way: `CameraState` is documented as a consumer-facing snapshot, E1.M2/E6.M2/module-register docs now agree on what is and is not part of that contract, and the occluder hardening avoids touching freed keys before restore. Moving the rig update to `_physics_process` with `process_physics_priority = 1` is a sensible follow-up to the jitter concern and matches the updated module documentation. Overall risk is low in the camera code itself. ~~The main remaining concerns are documentation drift inside the NEON-28 plan and an extra server observability change that is not tied back to the story scope.~~ **Resolved / noted:** plan drift fixed; server logging change acknowledged as intentionally bundled.
This branch closes the E1.M2 prototype slice in a coherent way: `CameraState` is documented as a consumer-facing snapshot, E1.M2/E6.M2/module-register docs now agree on what is and is not part of that contract, and the occluder hardening avoids touching freed keys before restore. Moving the rig update to `_physics_process` with `process_physics_priority = 1` is a sensible follow-up to the jitter concern and matches the updated module documentation. Overall risk is low in the camera code itself. ~~The main remaining concerns are documentation drift inside the NEO-18 plan and an extra server observability change that is not tied back to the story scope.~~ **Resolved / noted:** plan drift fixed; server logging change acknowledged as intentionally bundled.
## Documentation checked
| Document | Assessment |
|----------|------------|
| `docs/plans/NEON-28-implementation-plan.md` | **Matches** — Scope, decisions, shipped notes, acceptance checklist, and Technical approach (`_physics_process` + priority) align with the branch. **Supplement:** plan now records **runtime nav mesh** behavior (`PARSED_GEOMETRY_BOTH`, reparent + rebake for dev smoke); see plan **Decisions** and **Runtime navigation (lesson for future mechanics)**. |
| `docs/plans/NEO-18-implementation-plan.md` | **Matches** — Scope, decisions, shipped notes, acceptance checklist, and Technical approach (`_physics_process` + priority) align with the branch. **Supplement:** plan now records **runtime nav mesh** behavior (`PARSED_GEOMETRY_BOTH`, reparent + rebake for dev smoke); see plan **Decisions** and **Runtime navigation (lesson for future mechanics)**. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — Consumer contract, E1.M2 Ready status, yaw semantics, occlusion hardening, and the `_physics_process` update all reflect the implementation. |
| `docs/decomposition/modules/E6_M2_ConsentAndRiskUxSignals.md` | **Matches** — Camera-adjacent consumer guidance and authority split are consistent with the code and E1.M2 doc. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 is marked Ready with appropriate script/scene pointers. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M2 tracking row reflects NEON-28 completion; no extra tracking-table update appears needed after this merge. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M2 tracking row reflects NEO-18 completion; no extra tracking-table update appears needed after this merge. |
| `docs/decomposition/epics/epic_01_core_player_runtime.md` | **Matches** — Slice 2 language now reflects “yaw fixed in UX” rather than “camera never rotates,” which is the right contract. |
| `docs/decomposition/modules/client_server_authority.md` | **Matches** — Camera remains client-local presentation only; no server gameplay authority was added. |
| `docs/decomposition/modules/contracts.md` | **Matches** — E1.M2s Ready status is reasonable for this prototype slice given the documented contract and consumer guidance. |
@ -31,19 +31,19 @@ _None identified in the implementation reviewed._
## Suggestions
1. ~~`docs/plans/NEON-28-implementation-plan.md`: update the Technical approach bullet that still says `CameraState` is refreshed every `_process`. The same document now records the shipped behavior as `_physics_process` plus `process_physics_priority = 1`, so leaving both versions in the plan makes the acceptance source internally inconsistent.~~ **Done.**
2. ~~`server/NeonSprawl.Server/Program.cs` and `server/NeonSprawl.Server/appsettings.Development.json`: either split the ASP.NET HttpLogging removal into a separate chore or record the reason in the NEON-28 plan/PR. It is not part of the documented camera-hardening scope, and it reduces local request-path observability during the manual client/server checks described elsewhere in the repo.~~ **Intentionally not applied** (author keeps HttpLogging removal on this branch without plan/PR carve-out).
1. ~~`docs/plans/NEO-18-implementation-plan.md`: update the Technical approach bullet that still says `CameraState` is refreshed every `_process`. The same document now records the shipped behavior as `_physics_process` plus `process_physics_priority = 1`, so leaving both versions in the plan makes the acceptance source internally inconsistent.~~ **Done.**
2. ~~`server/NeonSprawl.Server/Program.cs` and `server/NeonSprawl.Server/appsettings.Development.json`: either split the ASP.NET HttpLogging removal into a separate chore or record the reason in the NEO-18 plan/PR. It is not part of the documented camera-hardening scope, and it reduces local request-path observability during the manual client/server checks described elsewhere in the repo.~~ **Intentionally not applied** (author keeps HttpLogging removal on this branch without plan/PR carve-out).
## Nits
- ~~`client/test/isometric_follow_camera_test.gd`: the new helper tests are worthwhile, but the actual purge/restore path is still effectively manual-only. If that editor verification has already been done, note it explicitly in the PR/story handoff so readers know the freed-occluder scenario was exercised end-to-end.~~ **Done.** — See `docs/plans/NEON-28-implementation-plan.md` §Tests manual verification (freed occluder, 2026-04-10).
- ~~`client/test/isometric_follow_camera_test.gd`: the new helper tests are worthwhile, but the actual purge/restore path is still effectively manual-only. If that editor verification has already been done, note it explicitly in the PR/story handoff so readers know the freed-occluder scenario was exercised end-to-end.~~ **Done.** — See `docs/plans/NEO-18-implementation-plan.md` §Tests manual verification (freed occluder, 2026-04-10).
## Verification
- Reviewed `git diff origin/main...HEAD` for the branch, with focused checks on `client/scripts/isometric_follow_camera.gd`, `client/scripts/camera_state.gd`, `client/test/isometric_follow_camera_test.gd`, and the updated E1.M2/E6.M2/module docs.
- `dotnet build NeonSprawl.sln` could not complete in this session because `server/NeonSprawl.Server/bin/Debug/net10.0/NeonSprawl.Server.dll` was locked by a running `dotnet.exe` process.
- Godot / GdUnit client tests were not run in this session.
- Before merge, I would still run: `dotnet build NeonSprawl.sln` with the dev server stopped, then the documented camera manual checks in `client/README.md` plus the NEON-28 plan step that frees an occluder while it is faded.
- Before merge, I would still run: `dotnet build NeonSprawl.sln` with the dev server stopped, then the documented camera manual checks in `client/README.md` plus the NEO-18 plan step that frees an occluder while it is faded.
## Supplement — runtime navigation (2026-04-10)
@ -51,7 +51,7 @@ _None identified in the implementation reviewed._
**Lesson:** With Godots default **`NavigationMesh.geometry_parsed_geometry_type`** (**`PARSED_GEOMETRY_BOTH`**), **`MeshInstance3D` nodes under `NavigationRegion3D`** still contribute to bakes. **Disabling collision and rebaking** can leave the **nav hole** unchanged. **`main.gd`** fixes dev smoke by **reparenting** the obstacle **out of the region**, **rebaking**, and **`sync_navigation_agent_after_map_rebuild`**.
**Product impact:** Any future mechanic that **removes or opens world geometry** during play (barricades, doors, destroyed cover) must account for **nav source geometry**, not just **collision** — see `docs/plans/NEON-28-implementation-plan.md` (**Decisions** table row and **Runtime navigation** subsection).
**Product impact:** Any future mechanic that **removes or opens world geometry** during play (barricades, doors, destroyed cover) must account for **nav source geometry**, not just **collision** — see `docs/plans/NEO-18-implementation-plan.md` (**Decisions** table row and **Runtime navigation** subsection).
---

View File

@ -3,7 +3,7 @@ using Npgsql;
namespace NeonSprawl.Server.Game.PositionState;
/// <summary>Disposes the shared <see cref="NpgsqlDataSource"/> when the host stops. Registered before <see cref="PostgresDevPlayerSeedHostedService"/> so <see cref="IHostedService.StopAsync"/> runs last (NEON-15).</summary>
/// <summary>Disposes the shared <see cref="NpgsqlDataSource"/> when the host stops. Registered before <see cref="PostgresDevPlayerSeedHostedService"/> so <see cref="IHostedService.StopAsync"/> runs last (NEO-13).</summary>
internal sealed class NpgsqlDataSourceShutdownHostedService(NpgsqlDataSource dataSource) : IHostedService
{
public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask;

View File

@ -16,11 +16,11 @@ dotnet run
- Default URL is printed by Kestrel (see `Properties/launchSettings.json`, typically `http://localhost:5253`).
- Check `GET /health` for a JSON heartbeat.
## Position persistence (NEON-5)
## Position persistence (NEO-8)
When **`ConnectionStrings:NeonSprawl`** is set to a valid PostgreSQL connection string, the server uses the **`player_position`** table (relational columns `pos_x``sequence`, not JSONB). DDL lives in [`server/db/migrations/V001__player_position.sql`](../db/migrations/V001__player_position.sql); the app applies that script on startup and on first store use (idempotent `CREATE TABLE IF NOT EXISTS`). The configured dev player (`Game:DevPlayerId` / `Game:DefaultPosition`) is seeded once at host startup, matching the in-memory stores constructor seed (not on every HTTP request).
If **`ConnectionStrings:NeonSprawl`** is missing or blank, behavior matches NEON-3/NEON-4 (**in-memory** only). The **test** projects `postgres.runsettings` sets `ConnectionStrings__NeonSprawl` for the test host so Postgres integration tests run in CI/Rider/`dotnet test`; non-Postgres tests use `InMemoryWebApplicationFactory` and do not require a live DB. Remove or rename `postgres.runsettings` (and the `RunSettingsFilePath` property) if you want those three tests to **skip** again when the variable is unset.
If **`ConnectionStrings:NeonSprawl`** is missing or blank, behavior matches NEO-6/NEO-7 (**in-memory** only). The **test** projects `postgres.runsettings` sets `ConnectionStrings__NeonSprawl` for the test host so Postgres integration tests run in CI/Rider/`dotnet test`; non-Postgres tests use `InMemoryWebApplicationFactory` and do not require a live DB. Remove or rename `postgres.runsettings` (and the `RunSettingsFilePath` property) if you want those three tests to **skip** again when the variable is unset.
**Environment variables** (typical — same mapping as other ASP.NET Core config):
@ -31,7 +31,7 @@ If **`ConnectionStrings:NeonSprawl`** is missing or blank, behavior matches NEON
Match credentials with [root `docker-compose.yml`](../../docker-compose.yml) for local development.
**Restart check (Jira AC):** start Postgres (`docker compose up -d` from repo root), set the connection string, `dotnet run`, `POST …/move`, stop the server, `dotnet run` again, `GET …/position` should return the last committed state.
**Restart check (acceptance criteria):** start Postgres (`docker compose up -d` from repo root), set the connection string, `dotnet run`, `POST …/move`, stop the server, `dotnet run` again, `GET …/position` should return the last committed state.
**Postgres integration tests** require `ConnectionStrings__NeonSprawl` (set automatically in [`.github/workflows/dotnet.yml`](../../.github/workflows/dotnet.yml)). The test project sets this via **`NeonSprawl.Server.Tests/postgres.runsettings`** (`RunSettingsFilePath` in the `.csproj`) so **`dotnet test`** and **Rider** pick it up without shell `export`. **`launchSettings.json`** in the same project is for IDE launch profiles and is **not** the xUnit “config file” in Rider settings.
@ -41,9 +41,9 @@ Match credentials with [root `docker-compose.yml`](../../docker-compose.yml) for
**Local Docker (optional automation):** When **not** in CI (`CI`, `GITHUB_ACTIONS`, `GITLAB_CI`, `TF_BUILD`), the Postgres test harness tries to connect first. If the server is unreachable, it runs **`docker compose up -d`** from the repo root (same compose file as [root `docker-compose.yml`](../../docker-compose.yml)), waits for the DB, runs tests, then runs **`docker compose down`** **only if** it started Compose itself. If Postgres was already listening (existing container or native install), tests do **not** stop your database afterward. Initialization is **async** (no sync-over-async on the xUnit sync context) so **Rider** / **Visual Studio** do not leave Postgres tests stuck **Pending** after Compose starts.
## Position state (NEON-3)
## Position state (NEO-6)
Authoritative player position is served over HTTP whether the backing store is **in memory** or **PostgreSQL** (NEON-5). The HTTP JSON shape is versioned with top-level `schemaVersion` (see XML docs on `PositionStateResponse` in the server project). Path `{id}` is **trimmed** and matched **case-insensitively** against stored players; `playerId` in the JSON body echoes the path segment from the request. Stored player ids use **invariant lowercase + trim** in Postgres for lookup parity.
Authoritative player position is served over HTTP whether the backing store is **in memory** or **PostgreSQL** (NEO-8). The HTTP JSON shape is versioned with top-level `schemaVersion` (see XML docs on `PositionStateResponse` in the server project). Path `{id}` is **trimmed** and matched **case-insensitively** against stored players; `playerId` in the JSON body echoes the path segment from the request. Stored player ids use **invariant lowercase + trim** in Postgres for lookup parity.
Example (dev player id defaults to `dev-local-1` in `appsettings.json`):
@ -51,7 +51,7 @@ Example (dev player id defaults to `dev-local-1` in `appsettings.json`):
curl -s http://localhost:5253/game/players/dev-local-1/position
```
Sample response (default spawn matches Godot capsule and NEON-6 walk-in range demo):
Sample response (default spawn matches Godot capsule and NEO-9 walk-in range demo):
```json
{"schemaVersion":1,"playerId":"dev-local-1","position":{"x":-5,"y":0.9,"z":-5},"sequence":0}
@ -59,13 +59,13 @@ Sample response (default spawn matches Godot capsule and NEON-6 walk-in range de
Unknown player ids return **404**. Full zone sync / replication is still out of scope for these spikes.
## Move command (NEON-4, NEON-7)
## Move command (NEO-7, NEO-10)
**`POST /game/players/{id}/move`** with JSON body applies a v1 **MoveCommand**: authoritative position **snaps** to `target` immediately; `sequence` in responses increases by one per successful apply.
**Client navigation (NEON-8):** The Godot prototype uses a **baked navigation mesh** only for **presentation**; the client may follow waypoints when active, but **automatic obstacle detours on a single click are not guaranteed** (tradeoff for smooth movement on stepped geometry). The server does **not** simulate navmesh; it validates **straight-line** step limits (NEON-7) from the **last authoritative position** to the **requested target**. Cheating or divergent paths are out of scope for this slice.
**Client navigation (NEO-11):** The Godot prototype uses a **baked navigation mesh** only for **presentation**; the client may follow waypoints when active, but **automatic obstacle detours on a single click are not guaranteed** (tradeoff for smooth movement on stepped geometry). The server does **not** simulate navmesh; it validates **straight-line** step limits (NEO-10) from the **last authoritative position** to the **requested target**. Cheating or divergent paths are out of scope for this slice.
**NEON-7 validation (reject-only):** Before applying, the server checks the move against **`Game:MovementValidation`**. Limits use **horizontal** distance on **X/Z** only and **|ΔY|** separately (see `MoveCommandValidation`). Unknown players return **404** before validation.
**NEO-10 validation (reject-only):** Before applying, the server checks the move against **`Game:MovementValidation`**. Limits use **horizontal** distance on **X/Z** only and **|ΔY|** separately (see `MoveCommandValidation`). Unknown players return **404** before validation.
| Config key | Meaning | Default (see `appsettings.json`) |
|------------|---------|-----------------------------------|
@ -85,14 +85,14 @@ curl -s -X POST http://localhost:5253/game/players/dev-local-1/move \
```
- **200** — body matches **`PositionStateResponse`** (same shape as `GET``/position`).
- **400** — malformed command (missing/invalid body or wrong `schemaVersion`) **or** move rejected by NEON-7 rules. Rejection responses are JSON **`MoveCommandRejectedResponse`**: `schemaVersion` (`1`) + **`reasonCode`** (`horizontal_step_exceeded`, `vertical_step_exceeded`, `out_of_bounds`). Malformed requests return **400** with **no** rejection body.
- **400** — malformed command (missing/invalid body or wrong `schemaVersion`) **or** move rejected by NEO-10 rules. Rejection responses are JSON **`MoveCommandRejectedResponse`**: `schemaVersion` (`1`) + **`reasonCode`** (`horizontal_step_exceeded`, `vertical_step_exceeded`, `out_of_bounds`). Malformed requests return **400** with **no** rejection body.
- **404** — unknown player id.
See XML on `MoveCommandRequest`, `PositionStateResponse`, and `MoveCommandRejectedResponse` in the server project.
For a **PR/Jira-ready** contract blurb (formatted JSON + field table), see the [NEON-3 implementation plan — Pull request description](../../docs/plans/NEON-3-implementation-plan.md#pull-request-description-jira-ac) (GET shape) and [NEON-4 implementation plan](../../docs/plans/NEON-4-implementation-plan.md#pull-request-description-jira-ac--draft-for-merge) (MoveCommand + sequence semantics). NEON-7: [NEON-7 implementation plan](../../docs/plans/NEON-7-implementation-plan.md).
For a **PR-ready** contract blurb (formatted JSON + field table), see the [NEO-6 implementation plan — Pull request description](../../docs/plans/NEO-6-implementation-plan.md#pull-request-description-linear-ac) (GET shape) and [NEO-7 implementation plan](../../docs/plans/NEO-7-implementation-plan.md#pull-request-description-linear-ac--draft-for-merge) (MoveCommand + sequence semantics). NEO-10: [NEO-10 implementation plan](../../docs/plans/NEO-10-implementation-plan.md).
## Interaction (NEON-6)
## Interaction (NEO-9)
**`POST /game/players/{id}/interact`** with a versioned **InteractionRequest** body asks whether the given player may use a prototype interactable **now**. The server reads **authoritative** `PositionState` from `IPositionStateStore` (same id rules as position APIs: trim + ordinal case-insensitive match). **Horizontal reach** uses **X and Z only**; **Y is ignored** for distance (floor-plane prototype). See `HorizontalReach` in `NeonSprawl.Server/Game/World/` and `PrototypeInteractableRegistry` in `Game/Interaction/`.
@ -121,7 +121,7 @@ curl -s -X POST http://localhost:5253/game/players/dev-local-1/interact \
When **`allowed` is `true`**, **`reasonCode` is omitted** (clients must not require it on success). Unknown player never returns this JSON — use **404**.
Contract details and PR blurb: [NEON-6 implementation plan](../../docs/plans/NEON-6-implementation-plan.md).
Contract details and PR blurb: [NEO-9 implementation plan](../../docs/plans/NEO-9-implementation-plan.md).
## Solution