From f7bfd7cb32087c111f259c7e12c9172af37c6460 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 29 Mar 2026 23:08:17 -0400 Subject: [PATCH] NS-15 implement position state api --- .cursor/rules/code-review-agent.md | 75 +++++++++++++ .cursor/rules/csharp-style.md | 17 ++- AGENTS.md | 9 ++ NeonSprawl.sln | 7 ++ docs/plans/NS-15-implementation-plan.md | 105 ++++++++++++------ docs/reviews/2026-03-29-NS-15.md | 74 ++++++++++++ .../NeonSprawl.Server.Tests.csproj | 25 +++++ .../PositionStateApiTests.cs | 49 ++++++++ .../Game/PositionState/GamePositionOptions.cs | 23 ++++ .../Game/PositionState/IPositionStateStore.cs | 8 ++ .../InMemoryPositionStateStore.cs | 33 ++++++ .../Game/PositionState/PositionSnapshot.cs | 4 + .../Game/PositionState/PositionStateApi.cs | 27 +++++ .../PositionState/PositionStateResponse.cs | 29 +++++ ...ositionStateServiceCollectionExtensions.cs | 12 ++ .../Game/PositionState/PositionVector.cs | 11 ++ server/NeonSprawl.Server/Program.Waf.cs | 2 + server/NeonSprawl.Server/Program.cs | 6 + server/NeonSprawl.Server/appsettings.json | 10 +- server/README.md | 24 +++- 20 files changed, 513 insertions(+), 37 deletions(-) create mode 100644 .cursor/rules/code-review-agent.md create mode 100644 AGENTS.md create mode 100644 docs/reviews/2026-03-29-NS-15.md create mode 100644 server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj create mode 100644 server/NeonSprawl.Server.Tests/PositionStateApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/PositionSnapshot.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs create mode 100644 server/NeonSprawl.Server/Game/PositionState/PositionVector.cs create mode 100644 server/NeonSprawl.Server/Program.Waf.cs diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md new file mode 100644 index 0000000..c42b2eb --- /dev/null +++ b/.cursor/rules/code-review-agent.md @@ -0,0 +1,75 @@ +--- +description: >- + Code review agent — PR/diff review before merge. @code-review-agent or ask for a + “code review” / “review this PR” using this persona. +alwaysApply: false +--- + +# Code review agent (Neon Sprawl) + +Use this rule when the user wants a **structured review** of a branch, PR, diff, or set of changed files—not when they are only asking for a quick one-line opinion. + +## Role + +- Act as a **senior engineer** reviewing for merge: correctness and safety first, then maintainability, tests, and project conventions. +- Be **direct and specific** (file, symbol, or line when possible). Prefer **actionable** feedback over vague style opinions. +- Separate **must fix** (blocking), **should fix** (non-blocking but valuable), and **nits** (optional). Do not inflate nits into blockers. + +## Ground truth + +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. +- [git-workflow](git-workflow.md) — branch vs `main`, story-scoped plans. + +If the change contradicts an adopted plan under `docs/plans/`, call that out. + +## Review checklist + +Work through what applies to the diff (skip irrelevant sections briefly). + +1. **Correctness** — Logic, edge cases, null/error paths, concurrency where relevant. +2. **APIs & contracts** — Breaking changes, versioning, serialization shapes, documented public surface. +3. **Security** — Injection, secrets in repo, authz/authn assumptions, unsafe defaults. +4. **Performance** — Obvious hot-path allocations or N+1 patterns in new code only; avoid speculative micro-optimization. +5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. +6. **Observability** — Logging/metrics where failures would be hard to diagnose (server/runtime code). +7. **Docs** — README or plan updates when behavior or run instructions change. + +## Code review document (required) + +**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 “Today’s 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. `NS-15`), else the branch name, else a short topic (kebab-case, lowercase). Examples: `2026-03-29-NS-15.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-NS-15.md`). +5. **Commits:** follow [commit-and-review](commit-and-review.md)—write the file to disk **uncommitted** unless the user explicitly asks to commit it. + +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. + +## Output format (required) + +Use this structure in **both** the saved document and (abbreviated if you want) chat: + +1. **Verdict** — One line: `Approve` / `Approve with nits` / `Request changes`. +2. **Summary** — 2–5 sentences on what the change does and overall risk. +3. **Blocking issues** — Numbered list; empty section if none. +4. **Suggestions** — Non-blocking improvements. +5. **Nits** — Optional; prefix with “Nit:”. +6. **Verification** — Commands or manual steps the author should run before merge (e.g. `dotnet test`, Godot scenario). + +In **chat only**, you may use Cursor line-number **code citations** when referencing existing code. For hypothetical fixes, use normal fenced code blocks everywhere. + +## Boundaries + +- Do **not** skip writing `docs/reviews/…`; the document is mandatory output of this agent. +- Do **not** rewrite the whole change unless asked; review is the default. +- Do **not** request large refactors unrelated to the diff without strong justification labeled as non-blocking. +- If context is insufficient (missing diff, wrong branch), still create the document with a **“Blocked”** verdict section explaining what is missing, then ask in chat for the missing context. diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index c802a62..e41ea2c 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -1,5 +1,5 @@ --- -description: C# naming and layout aligned with Microsoft coding conventions and .NET idioms. +description: C# naming, layout, and primary constructors; Microsoft conventions and .NET idioms. globs: "**/*.cs" alwaysApply: true --- @@ -18,6 +18,21 @@ Follow Microsoft’s **[C# Coding Conventions](https://learn.microsoft.com/en-us - **Static fields:** `camelCase` for private/internal static fields; `PascalCase` for `public static` members (including `readonly`/constants) per Microsoft guidance; stay consistent within a project. - **Async methods:** suffix with `Async` (e.g. `LoadProfileAsync`). +## Primary constructors + +- Prefer **primary constructor** syntax for classes, structs, and records when it fits: dependency injection, `IClassFixture<>` test classes, small services, and any type that mainly captures parameters into fields or base calls. +- **Skip** primary constructors when they hurt clarity: heavy logic in the body that belongs in a conventional constructor, `this` references before the implicit constructor runs in odd ways, or a file already uses a consistent legacy style—then match the file. + +```csharp +// Prefer +public sealed class OrderService(IOrderStore store, ILogger log) +{ + public Task GetAsync(Guid id) => store.FindAsync(id); +} + +// Avoid when you would only reassign into mutable fields with non-trivial validation—use an explicit constructor instead. +``` + ## Layout and syntax - **Braces:** opening brace `{` on a **new line** for types and members (Allman-style), per common Microsoft examples. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..1838faf --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,9 @@ +# Agents (Neon Sprawl) + +Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or by asking explicitly (e.g. “review this as the code review agent”). + +| Agent | Rule file | When to use | +|--------|-----------|-------------| +| **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` plus a short chat pointer | + +Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). diff --git a/NeonSprawl.sln b/NeonSprawl.sln index 1ae92d7..b4cfda4 100644 --- a/NeonSprawl.sln +++ b/NeonSprawl.sln @@ -7,6 +7,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "server", "server", "{8683E7 EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NeonSprawl.Server", "server/NeonSprawl.Server/NeonSprawl.Server.csproj", "{098FCB1C-4A31-4E36-8D9C-1F1CEECDFF50}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NeonSprawl.Server.Tests", "server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj", "{9249FE6F-28EF-46E6-89AB-D23C48A664B0}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -20,8 +22,13 @@ Global {098FCB1C-4A31-4E36-8D9C-1F1CEECDFF50}.Debug|Any CPU.Build.0 = Debug|Any CPU {098FCB1C-4A31-4E36-8D9C-1F1CEECDFF50}.Release|Any CPU.ActiveCfg = Release|Any CPU {098FCB1C-4A31-4E36-8D9C-1F1CEECDFF50}.Release|Any CPU.Build.0 = Release|Any CPU + {9249FE6F-28EF-46E6-89AB-D23C48A664B0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {9249FE6F-28EF-46E6-89AB-D23C48A664B0}.Debug|Any CPU.Build.0 = Debug|Any CPU + {9249FE6F-28EF-46E6-89AB-D23C48A664B0}.Release|Any CPU.ActiveCfg = Release|Any CPU + {9249FE6F-28EF-46E6-89AB-D23C48A664B0}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(NestedProjects) = postSolution {098FCB1C-4A31-4E36-8D9C-1F1CEECDFF50} = {8683E7E4-B4D9-4001-8E99-DA987A03A73D} + {9249FE6F-28EF-46E6-89AB-D23C48A664B0} = {8683E7E4-B4D9-4001-8E99-DA987A03A73D} EndGlobalSection EndGlobal diff --git a/docs/plans/NS-15-implementation-plan.md b/docs/plans/NS-15-implementation-plan.md index 05d48a3..e8f5dbe 100644 --- a/docs/plans/NS-15-implementation-plan.md +++ b/docs/plans/NS-15-implementation-plan.md @@ -2,13 +2,15 @@ ## Story reference -| Field | Value | -|--------|--------| -| **Key** | NS-15 | -| **Title** | E1.M1: Authoritative PositionState API (in-memory) | -| **Jira** | [NS-15](https://neon-sprawl.atlassian.net/browse/NS-15) | + +| Field | Value | +| ------------------ | --------------------------------------------------------------------------------------- | +| **Key** | NS-15 | +| **Title** | E1.M1: Authoritative PositionState API (in-memory) | +| **Jira** | [NS-15](https://neon-sprawl.atlassian.net/browse/NS-15) | | **Parent context** | [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) | + ## Goal, scope, and out-of-scope **Goal:** The game server is the source of truth for a single development **PositionState**, exposed read-only over HTTP as JSON (spike alignment with [tech stack](../../docs/architecture/tech_stack.md)). @@ -31,55 +33,92 @@ ## Technical approach -1. **Response contract** - - Define a small set of types (e.g. record/DTO) for the HTTP JSON body. - - **Suggested v1 shape** (adjust names in implementation if parent epic standardizes differently): - - `schemaVersion`: `1` - - `playerId`: string (echo path id for sanity) - - `position`: object with `x`, `y`, `z` as doubles (or floats serialized as JSON numbers) - - Optional: `facing` (e.g. yaw radians or degrees — pick one and document), `sequence` (ulong/int for future tick ordering) - - XML doc comments on public API types document each property; PR description repeats the sample JSON. - -2. **In-memory authority** - - Register a singleton service (e.g. `IPositionStateStore`) backed by `ConcurrentDictionary` or immutable snapshots. - - On startup, ensure the **dev player id** from config exists with a default position (e.g. origin); other ids may return **404** until a later story seeds them. - -3. **Minimal API** - - `MapGet("/game/players/{id}/position", …)` resolves the store, returns `Results.Json(...)` with correct content type, or `Results.NotFound()` for unknown players. - -4. **Configuration** - - Add section in `appsettings.json` / `appsettings.Development.json` (e.g. `Game:DevPlayerId`, optional `Game:DefaultPosition` or nested object). Bind with `IOptions` or read once at startup when seeding the store. - -5. **Tests** - - Add **`NeonSprawl.Server.Tests`** (xUnit), reference **`Microsoft.AspNetCore.Mvc.Testing`**, and add **`public partial class Program { }`** to the server assembly (empty partial) so **`WebApplicationFactory`** works with top-level statements. - - One integration test: `GET` for the configured dev player returns **200**, body parses, **`schemaVersion`** present, **`position.x/y/z`** present. - - Optional second test: unknown id returns **404**. - -6. **Documentation** - - Extend `server/README.md` with “Position state (NS-15)”: endpoint path, example `curl`, note that state is in-memory and authoritative only at the HTTP spike layer. +1. **Response contract** + - Define a small set of types (e.g. record/DTO) for the HTTP JSON body. + - **Suggested v1 shape** (adjust names in implementation if parent epic standardizes differently): + - `schemaVersion`: `1` + - `playerId`: string (echo path id for sanity) + - `position`: object with `x`, `y`, `z` as doubles (or floats serialized as JSON numbers) + - Optional: `facing` (e.g. yaw radians or degrees — pick one and document), `sequence` (ulong/int for future tick ordering) + - XML doc comments on public API types document each property; PR description repeats the sample JSON. +2. **In-memory authority** + - Implement `IPositionStateStore` (and concrete in-memory type) backed by `ConcurrentDictionary` or immutable snapshots. + - On startup, ensure the **dev player id** from config exists with a default position (e.g. origin); other ids may return **404** until a later story seeds them. + - **Do not** register the store in `Program.cs`. Use an `IServiceCollection` extension (e.g. `PositionStateServiceCollectionExtensions.AddPositionStateStore`) that binds options and registers the singleton. +3. **HTTP API (dedicated class)** + - **Do not** define routes inline in `Program.cs`. Add a **separate class** (e.g. `PositionStateApi` or `PositionStateEndpoints`) whose responsibility is mapping this feature’s routes—typically a static class with an extension method on `IEndpointRouteBuilder` or `WebApplication`, e.g. `MapPositionStateApi()`, containing the `MapGet("/game/players/{id}/position", …)` and handler logic (or thin delegates to a small handler type). Handlers resolve `IPositionStateStore` from DI, return `Results.Json(...)` or `Results.NotFound()`. +4. **Configuration** + - Add section in `appsettings.json` / `appsettings.Development.json` (e.g. `Game:DevPlayerId`, optional `Game:DefaultPosition` or nested object). Bind with `IOptions` or read once at startup when seeding the store. +5. **Tests** + - Add **`NeonSprawl.Server.Tests`** (xUnit), reference **`Microsoft.AspNetCore.Mvc.Testing`**, and add **`public partial class Program { }`** to the server assembly (empty partial in a **small** file—e.g. `Program.Waf.cs` or the tail of `Program.cs`—only for the test host, not for endpoint definitions) so **`WebApplicationFactory`** works with top-level statements. + - 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 (NS-15)”: endpoint path, example `curl`, note that state is in-memory and authoritative only at the HTTP spike layer. ## Files to add + | Path | Purpose | |------|--------| | `server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` | xUnit + `Microsoft.AspNetCore.Mvc.Testing` test project. | | `server/NeonSprawl.Server.Tests/PositionStateApiTests.cs` (or similar) | Integration test(s) for `GET /game/players/{id}/position`. | -| New C# files under `server/NeonSprawl.Server/` as needed | DTOs/contracts, `IPositionStateStore` + implementation, optional mapping extension methods — keep `Program.cs` thin via `AddSingleton` + `MapGet` only if small; otherwise extract to `GameEndpoints.cs` or `ServiceCollectionExtensions.cs` per existing repo style (currently single-file host). | +| `server/NeonSprawl.Server/...` DTOs / contracts | e.g. `PositionStateResponse.cs` (or folder `Game/PositionState/`) — versioned JSON shape. | +| `server/NeonSprawl.Server/...` `IPositionStateStore` + in-memory implementation | Authority + seeding from options. | +| `server/NeonSprawl.Server/...PositionStateServiceCollectionExtensions.cs` (or `Game/DependencyInjection/…`) | `AddPositionStateStore(IServiceCollection, IConfiguration)` (or options binding); **all** registration for this feature, not `Program.cs`. | +| `server/NeonSprawl.Server/...PositionStateApi.cs` (or `PositionStateEndpoints.cs`) | **Own class** mapping `GET /game/players/{id}/position` (extension on `WebApplication` / `IEndpointRouteBuilder`); no route definitions in `Program.cs`. | +| Optional: `server/NeonSprawl.Server/Program.Waf.cs` | Empty `public partial class Program { }` for `WebApplicationFactory` only. | + ## Files to modify + | Path | Rationale | |------|-----------| -| `server/NeonSprawl.Server/Program.cs` | Register store, map `GET /game/players/{id}/position`, expose `partial class Program` for test host. | +| `server/NeonSprawl.Server/Program.cs` | Host bootstrap only: `WebApplication.CreateBuilder`, call service + endpoint extension methods (one line each or similarly minimal), `app.Run()`. **No** store registration body and **no** `MapGet` for position state here. | | `server/NeonSprawl.Server/appsettings.json` and/or `appsettings.Development.json` | Dev player id and default position seed. | | `server/README.md` | curl example + short contract note. | | `NeonSprawl.sln` | Include test project in solution. | + ## Tests - **Automated:** Integration test(s) via `WebApplicationFactory` 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) + +Jira NS-15 asks for stable JSON **field names** documented for reviewers outside the repo. **When you open the GitHub PR** (and optionally comment on [NS-15](https://neon-sprawl.atlassian.net/browse/NS-15)), 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. + +**Example response body (v1, formatted for readability):** + +```json +{ + "schemaVersion": 1, + "playerId": "dev-local-1", + "position": { + "x": 0, + "y": 0, + "z": 0 + }, + "sequence": 0 +} +``` + +| JSON property | Type (v1) | Meaning | +|---------------|-----------|---------| +| `schemaVersion` | number (integer) | Contract version; increment when the payload shape or semantics change. | +| `playerId` | string | Echo of the `{id}` path segment from the request (casing preserved). | +| `position` | object | Authoritative world position. | +| `position.x` | number | X coordinate. | +| `position.y` | number | Y coordinate. | +| `position.z` | number | Z coordinate. | +| `sequence` | number (integer) | Ordering hint for future sync; v1 is `0`. | + +**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 NS-10 / future sync design; if unclear, omit v1 `facing` and document “reserved for v2” or add `schemaVersion` bump when added. diff --git a/docs/reviews/2026-03-29-NS-15.md b/docs/reviews/2026-03-29-NS-15.md new file mode 100644 index 0000000..aa223c7 --- /dev/null +++ b/docs/reviews/2026-03-29-NS-15.md @@ -0,0 +1,74 @@ +# Code review — NS-15 (authoritative position read API) + +## Review metadata + +| Field | Value | +|--------|--------| +| **Date** | 2026-03-29 | +| **Scope** | Branch `NS-15-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** | NS-15 — E1.M1: Authoritative PositionState API (in-memory) | + +--- + +## Verdict + +**Request changes** (for merge readiness). Implementation quality is **Approve with nits**; the **request changes** is because the feature and tests are largely **not yet in git** relative to `origin/main` (untracked paths), so a PR built only from pushed commits would be incomplete. + +## Summary + +The NS-15 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`, and `Program.Waf.cs` for the test host. `Program.cs` stays thin (DI + map extension calls only), matching `docs/plans/NS-15-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/NS-15-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 + +1. **Null-safe `DefaultPosition`** — In `InMemoryPositionStateStore`, configuration binding could theoretically set `GamePositionOptions.DefaultPosition` to `null`. Use `var p = o.DefaultPosition ?? new DefaultPositionOptions();` before reading `p.X` / `p.Y` / `p.Z` to avoid a startup `NullReferenceException`. + +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/NS-15-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. + +## 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 NS-15; revisit when debugging multi-player seeds. + +## Verification + +Run from repo root (**.NET 10 SDK** on `PATH`): + +```bash +git status # expect clean after add/commit, no missing Game/ or Tests/ +dotnet build NeonSprawl.sln +dotnet test NeonSprawl.sln +``` + +Manual: + +```bash +cd server/NeonSprawl.Server && dotnet run +# elsewhere: +curl -s http://localhost:5253/game/players/dev-local-1/position +``` + +Expect HTTP 200 and JSON including `schemaVersion`, `playerId`, `position` with `x`/`y`/`z`, and `sequence`. Unknown id should return 404. + +## Files touched (review scope) + +**Modified (tracked):** `.cursor/rules/csharp-style.md`, `NeonSprawl.sln`, `docs/plans/NS-15-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 NS-15 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. diff --git a/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj b/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj new file mode 100644 index 0000000..2c0dea6 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj @@ -0,0 +1,25 @@ + + + + net10.0 + enable + enable + NeonSprawl.Server.Tests + false + + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + + diff --git a/server/NeonSprawl.Server.Tests/PositionStateApiTests.cs b/server/NeonSprawl.Server.Tests/PositionStateApiTests.cs new file mode 100644 index 0000000..6386375 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/PositionStateApiTests.cs @@ -0,0 +1,49 @@ +using System.Net; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Mvc.Testing; +using NeonSprawl.Server.Game.PositionState; +using Xunit; + +namespace NeonSprawl.Server.Tests; + +public class PositionStateApiTests(WebApplicationFactory factory) + : IClassFixture> +{ + private readonly HttpClient _client = factory.CreateClient(); + + [Fact] + public async Task GetPosition_dev_player_returns_versioned_json_with_xyz() + { + var response = await _client.GetAsync("/game/players/dev-local-1/position"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(PositionStateResponse.CurrentSchemaVersion, body.SchemaVersion); + Assert.Equal("dev-local-1", body.PlayerId); + Assert.NotNull(body.Position); + Assert.Equal(0, body.Position.X); + Assert.Equal(0, body.Position.Y); + Assert.Equal(0, body.Position.Z); + } + + [Fact] + public async Task GetPosition_dev_player_case_insensitive_path_returns_200() + { + var response = await _client.GetAsync("/game/players/DEV-LOCAL-1/position"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(PositionStateResponse.CurrentSchemaVersion, body!.SchemaVersion); + Assert.Equal("DEV-LOCAL-1", body.PlayerId); + Assert.Equal(0, body.Position!.X); + } + + [Fact] + public async Task GetPosition_unknown_player_returns_404() + { + var response = await _client.GetAsync("/game/players/unknown-player/position"); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs new file mode 100644 index 0000000..a4642cc --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs @@ -0,0 +1,23 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// +/// Binds the Game configuration section (dev player seed for local authority). +/// +public sealed class GamePositionOptions +{ + public const string SectionName = "Game"; + + /// Logical player id seeded in the in-memory store (URL-safe string). + public string DevPlayerId { get; set; } = "dev-local-1"; + + /// World position for the dev player at process start. + public DefaultPositionOptions DefaultPosition { get; set; } = new(); +} + +/// Default spawn coordinates for the configured dev player. +public sealed class DefaultPositionOptions +{ + public double X { get; set; } + public double Y { get; set; } + public double Z { get; set; } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs new file mode 100644 index 0000000..055c623 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs @@ -0,0 +1,8 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// In-memory source of truth for player positions (NS-15 spike). +public interface IPositionStateStore +{ + /// Returns false if the player is unknown (HTTP 404). is trimmed; matching is ordinal case-insensitive. + bool TryGetPosition(string playerId, out PositionSnapshot snapshot); +} diff --git a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs new file mode 100644 index 0000000..ba69008 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs @@ -0,0 +1,33 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Options; + +namespace NeonSprawl.Server.Game.PositionState; + +/// Thread-safe in-memory positions; seeds the configured dev player on construction. +public sealed class InMemoryPositionStateStore : IPositionStateStore +{ + private readonly ConcurrentDictionary _positions = new(StringComparer.OrdinalIgnoreCase); + + public InMemoryPositionStateStore(IOptions options) + { + var o = options.Value; + var id = o.DevPlayerId.Trim(); + if (id.Length == 0) + throw new InvalidOperationException("Game:DevPlayerId must be non-empty."); + + var p = o.DefaultPosition ?? new DefaultPositionOptions(); + _positions[id] = new PositionSnapshot(p.X, p.Y, p.Z, 0); + } + + public bool TryGetPosition(string playerId, out PositionSnapshot snapshot) + { + var key = playerId?.Trim(); + if (string.IsNullOrEmpty(key)) + { + snapshot = default; + return false; + } + + return _positions.TryGetValue(key, out snapshot); + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionSnapshot.cs b/server/NeonSprawl.Server/Game/PositionState/PositionSnapshot.cs new file mode 100644 index 0000000..7a8c67d --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/PositionSnapshot.cs @@ -0,0 +1,4 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Authoritative position slice returned from the in-memory store. +public readonly record struct PositionSnapshot(double X, double Y, double Z, ulong Sequence); diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs new file mode 100644 index 0000000..a56c634 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs @@ -0,0 +1,27 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Maps HTTP routes for authoritative position read API. +public static class PositionStateApi +{ + public static WebApplication MapPositionStateApi(this WebApplication app) + { + app.MapGet( + "/game/players/{id}/position", + (string id, IPositionStateStore store) => + { + if (!store.TryGetPosition(id, out var snap)) + return Results.NotFound(); + + var body = new PositionStateResponse + { + SchemaVersion = PositionStateResponse.CurrentSchemaVersion, + PlayerId = id, + Position = new PositionVector { X = snap.X, Y = snap.Y, Z = snap.Z }, + Sequence = snap.Sequence, + }; + return Results.Json(body); + }); + + return app; + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs new file mode 100644 index 0000000..b9a0ad5 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs @@ -0,0 +1,29 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// +/// HTTP JSON body for GET /game/players/{{id}}/position. Consumers should read +/// before interpreting other fields. +/// +/// +/// Example v1 payload: +/// +/// {"schemaVersion":1,"playerId":"dev-local-1","position":{"x":0,"y":0,"z":0},"sequence":0} +/// +/// +public sealed class PositionStateResponse +{ + /// Schema version the server currently emits for this contract; increment when the shape or semantics change. + public const int CurrentSchemaVersion = 1; + + /// Contract version; should match when produced by this API. + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + /// Player id (echo of the route for sanity). + public required string PlayerId { get; init; } + + /// Authoritative world position. + public required PositionVector Position { get; init; } + + /// Monotonic-ish ordering hint for future sync; v1 is always 0. + public ulong Sequence { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs new file mode 100644 index 0000000..92fe0df --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs @@ -0,0 +1,12 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Registers in-memory position authority and options for NS-15. +public static class PositionStateServiceCollectionExtensions +{ + public static IServiceCollection AddPositionStateStore(this IServiceCollection services, IConfiguration configuration) + { + services.Configure(configuration.GetSection(GamePositionOptions.SectionName)); + services.AddSingleton(); + return services; + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionVector.cs b/server/NeonSprawl.Server/Game/PositionState/PositionVector.cs new file mode 100644 index 0000000..4392971 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/PositionVector.cs @@ -0,0 +1,11 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// +/// Serializable position in world space (JSON property names x, y, z). +/// +public sealed class PositionVector +{ + public double X { get; init; } + public double Y { get; init; } + public double Z { get; init; } +} diff --git a/server/NeonSprawl.Server/Program.Waf.cs b/server/NeonSprawl.Server/Program.Waf.cs new file mode 100644 index 0000000..61d8273 --- /dev/null +++ b/server/NeonSprawl.Server/Program.Waf.cs @@ -0,0 +1,2 @@ +/// Enables with top-level statements. +public partial class Program; diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index ddbd094..e48842a 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -1,4 +1,8 @@ +using NeonSprawl.Server.Game.PositionState; + var builder = WebApplication.CreateBuilder(args); +builder.Services.AddPositionStateStore(builder.Configuration); + var app = builder.Build(); app.MapGet("/", () => Results.Text( @@ -11,4 +15,6 @@ app.MapGet("/health", () => Results.Json(new service = "NeonSprawl.Server", })); +app.MapPositionStateApi(); + app.Run(); diff --git a/server/NeonSprawl.Server/appsettings.json b/server/NeonSprawl.Server/appsettings.json index 10f68b8..2bf62cf 100644 --- a/server/NeonSprawl.Server/appsettings.json +++ b/server/NeonSprawl.Server/appsettings.json @@ -5,5 +5,13 @@ "Microsoft.AspNetCore": "Warning" } }, - "AllowedHosts": "*" + "AllowedHosts": "*", + "Game": { + "DevPlayerId": "dev-local-1", + "DefaultPosition": { + "X": 0, + "Y": 0, + "Z": 0 + } + } } diff --git a/server/README.md b/server/README.md index 5df8ff9..91bcc4f 100644 --- a/server/README.md +++ b/server/README.md @@ -1,6 +1,6 @@ # Game server (`NeonSprawl.Server`) -ASP.NET Core **.NET 10** host for authoritative simulation (prototype: HTTP + health only). +ASP.NET Core **.NET 10** host for authoritative simulation (prototype: HTTP + health + in-memory position read API). ## Run @@ -12,6 +12,26 @@ dotnet run - Default URL is printed by Kestrel (see `Properties/launchSettings.json`, typically `http://localhost:5253`). - Check `GET /health` for a JSON heartbeat. +## Position state (NS-15) + +Authoritative player position is held **in memory** (no database yet). 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. + +Example (dev player id defaults to `dev-local-1` in `appsettings.json`): + +```bash +curl -s http://localhost:5253/game/players/dev-local-1/position +``` + +Sample response: + +```json +{"schemaVersion":1,"playerId":"dev-local-1","position":{"x":0,"y":0,"z":0},"sequence":0} +``` + +Unknown player ids return **404**. This endpoint is a **spike** until durable persistence and full sync exist. + +For a **PR/Jira-ready** contract blurb (formatted JSON + field table), see the [NS-15 implementation plan — Pull request description](../../docs/plans/NS-15-implementation-plan.md#pull-request-description-jira-ac). + ## Solution -From repo root: `dotnet build NeonSprawl.sln` +From repo root: `dotnet build NeonSprawl.sln` and `dotnet test NeonSprawl.sln`.