NS-15 implement position state api
parent
a48da30f2e
commit
f7bfd7cb32
|
|
@ -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.
|
||||
|
|
@ -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<OrderService> log)
|
||||
{
|
||||
public Task<Order?> 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.
|
||||
|
|
|
|||
|
|
@ -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`).
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<string, …>` 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<Program>`** 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<string, …>` 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<Program>`** 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<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)
|
||||
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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<Program>`, 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.
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<TargetFramework>net10.0</TargetFramework>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<Nullable>enable</Nullable>
|
||||
<RootNamespace>NeonSprawl.Server.Tests</RootNamespace>
|
||||
<IsPackable>false</IsPackable>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="10.0.0" />
|
||||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
|
||||
<PackageReference Include="xunit" Version="2.9.3" />
|
||||
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
|
||||
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
|
||||
<PrivateAssets>all</PrivateAssets>
|
||||
</PackageReference>
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\NeonSprawl.Server\NeonSprawl.Server.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
|
|
@ -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<Program> factory)
|
||||
: IClassFixture<WebApplicationFactory<Program>>
|
||||
{
|
||||
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<PositionStateResponse>();
|
||||
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<PositionStateResponse>();
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>
|
||||
/// Binds the <c>Game</c> configuration section (dev player seed for local authority).
|
||||
/// </summary>
|
||||
public sealed class GamePositionOptions
|
||||
{
|
||||
public const string SectionName = "Game";
|
||||
|
||||
/// <summary>Logical player id seeded in the in-memory store (URL-safe string).</summary>
|
||||
public string DevPlayerId { get; set; } = "dev-local-1";
|
||||
|
||||
/// <summary>World position for the dev player at process start.</summary>
|
||||
public DefaultPositionOptions DefaultPosition { get; set; } = new();
|
||||
}
|
||||
|
||||
/// <summary>Default spawn coordinates for the configured dev player.</summary>
|
||||
public sealed class DefaultPositionOptions
|
||||
{
|
||||
public double X { get; set; }
|
||||
public double Y { get; set; }
|
||||
public double Z { get; set; }
|
||||
}
|
||||
|
|
@ -0,0 +1,8 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>In-memory source of truth for player positions (NS-15 spike).</summary>
|
||||
public interface IPositionStateStore
|
||||
{
|
||||
/// <summary>Returns false if the player is unknown (HTTP 404). <paramref name="playerId"/> is trimmed; matching is ordinal case-insensitive.</summary>
|
||||
bool TryGetPosition(string playerId, out PositionSnapshot snapshot);
|
||||
}
|
||||
|
|
@ -0,0 +1,33 @@
|
|||
using System.Collections.Concurrent;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Thread-safe in-memory positions; seeds the configured dev player on construction.</summary>
|
||||
public sealed class InMemoryPositionStateStore : IPositionStateStore
|
||||
{
|
||||
private readonly ConcurrentDictionary<string, PositionSnapshot> _positions = new(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
public InMemoryPositionStateStore(IOptions<GamePositionOptions> 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);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Authoritative position slice returned from the in-memory store.</summary>
|
||||
public readonly record struct PositionSnapshot(double X, double Y, double Z, ulong Sequence);
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Maps HTTP routes for authoritative position read API.</summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>
|
||||
/// HTTP JSON body for <c>GET /game/players/{{id}}/position</c>. Consumers should read
|
||||
/// <see cref="SchemaVersion"/> before interpreting other fields.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Example v1 payload:
|
||||
/// <code>
|
||||
/// {"schemaVersion":1,"playerId":"dev-local-1","position":{"x":0,"y":0,"z":0},"sequence":0}
|
||||
/// </code>
|
||||
/// </remarks>
|
||||
public sealed class PositionStateResponse
|
||||
{
|
||||
/// <summary>Schema version the server currently emits for this contract; increment when the shape or semantics change.</summary>
|
||||
public const int CurrentSchemaVersion = 1;
|
||||
|
||||
/// <summary>Contract version; should match <see cref="CurrentSchemaVersion"/> when produced by this API.</summary>
|
||||
public int SchemaVersion { get; init; } = CurrentSchemaVersion;
|
||||
|
||||
/// <summary>Player id (echo of the route for sanity).</summary>
|
||||
public required string PlayerId { get; init; }
|
||||
|
||||
/// <summary>Authoritative world position.</summary>
|
||||
public required PositionVector Position { get; init; }
|
||||
|
||||
/// <summary>Monotonic-ish ordering hint for future sync; v1 is always 0.</summary>
|
||||
public ulong Sequence { get; init; }
|
||||
}
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Registers in-memory position authority and options for NS-15.</summary>
|
||||
public static class PositionStateServiceCollectionExtensions
|
||||
{
|
||||
public static IServiceCollection AddPositionStateStore(this IServiceCollection services, IConfiguration configuration)
|
||||
{
|
||||
services.Configure<GamePositionOptions>(configuration.GetSection(GamePositionOptions.SectionName));
|
||||
services.AddSingleton<IPositionStateStore, InMemoryPositionStateStore>();
|
||||
return services;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>
|
||||
/// Serializable position in world space (JSON property names <c>x</c>, <c>y</c>, <c>z</c>).
|
||||
/// </summary>
|
||||
public sealed class PositionVector
|
||||
{
|
||||
public double X { get; init; }
|
||||
public double Y { get; init; }
|
||||
public double Z { get; init; }
|
||||
}
|
||||
|
|
@ -0,0 +1,2 @@
|
|||
/// <summary>Enables <see cref="Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory{TEntryPoint}"/> with top-level statements.</summary>
|
||||
public partial class Program;
|
||||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -5,5 +5,13 @@
|
|||
"Microsoft.AspNetCore": "Warning"
|
||||
}
|
||||
},
|
||||
"AllowedHosts": "*"
|
||||
"AllowedHosts": "*",
|
||||
"Game": {
|
||||
"DevPlayerId": "dev-local-1",
|
||||
"DefaultPosition": {
|
||||
"X": 0,
|
||||
"Y": 0,
|
||||
"Z": 0
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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`.
|
||||
|
|
|
|||
Loading…
Reference in New Issue