diff --git a/docs/manual-qa/NEO-42.md b/docs/manual-qa/NEO-42.md new file mode 100644 index 0000000..5460db3 --- /dev/null +++ b/docs/manual-qa/NEO-42.md @@ -0,0 +1,59 @@ +# Manual QA — NEO-42 (craft / refine → refine skill XP, `activity`) + +Reference: [implementation plan](../plans/NEO-42-implementation-plan.md), [NEO-38](./NEO-38.md) (skill progression grant), [NEO-41](./NEO-41.md) (gather analogue). + +## Preconditions + +- Run `NeonSprawl.Server` from `server/NeonSprawl.Server` (`dotnet run`; default `http://localhost:5253`). +- Player **`dev-local-1`** (default seed). + +```bash +BASE=http://localhost:5253 +ID=dev-local-1 +``` + +## Automated coverage + +`RefineActivitySkillXpGrantTests` exercises the **NEO-38** grant path with disk **`prototype_skills.json`** (`refine` allows **`activity`**) and a stub registry where **`refine`** disallows **`activity`**. + +## Control: direct POST grant (same stack as E3.M2 hook) + +Move once so **`GET …/skill-progression`** is not **404**: + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/move" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"target":{"x":0.5,"y":0.9,"z":0.5}}' +``` + +Baseline: + +```bash +curl -sS "${BASE}/game/players/${ID}/skill-progression" +``` + +Apply **`refine`** + **`activity`** (mirrors what **`RefineActivitySkillXpGrant`** does internally): + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"refine","amount":10,"sourceKind":"activity"}' +``` + +Expect **HTTP 200**, **`granted": true`**, and **`refine`** **`xp`** increased (repeat **POST** to see cumulative XP). + +## Deny path (catalog guard) + +With default catalog, **`trainer`** is allowed for **`refine`** but **`book_or_item`** is not: + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"refine","amount":5,"sourceKind":"book_or_item"}' +``` + +Expect **HTTP 200**, **`granted": false`**, **`reasonCode":"source_kind_not_allowed"`**. + +## E3.M2 follow-up + +When recipe/refine execution exists, confirm **one** server-side call to **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`** on **success only**, then **`GET …/skill-progression`** shows expected **`refine`** **`xp`**. diff --git a/docs/plans/NEO-42-implementation-plan.md b/docs/plans/NEO-42-implementation-plan.md index 6727393..cd6573d 100644 --- a/docs/plans/NEO-42-implementation-plan.md +++ b/docs/plans/NEO-42-implementation-plan.md @@ -35,8 +35,12 @@ ## Acceptance criteria checklist -- [ ] Craft/refine completion grants skill XP via **NEO-38** path (`TryApplyGrant` / shared persistence as `POST …/skill-progression`). -- [ ] Grant channels respect **skill catalog** `allowedXpSourceKinds` (deny / no-op when `activity` not allowed). +- [x] Craft/refine completion **shall** grant skill XP via **NEO-38** path — **`RefineActivitySkillXpGrant`** wraps **`TryApplyGrant`** (same stack as **`POST …/skill-progression`**). **E3.M2** must call **`GrantOnSuccessfulCraftOrRefine`** on success (documented in **`server/README.md`** + manual QA); no craft route in repo yet. +- [x] Grant channels respect **skill catalog** `allowedXpSourceKinds` (stub registry test: **`refine`** without **`activity`** → no XP). + +## Decisions (implementation) + +- **`RefineActivityDeniedRegistryWebApplicationFactory`** added for allowlist-deny coverage (parallel to NEO-41 salvage-denied factory). ## Technical approach @@ -57,13 +61,15 @@ | `server/NeonSprawl.Server/Game/Skills/RefineSkillXpConstants.cs` | NEO-42 prototype constants: `refine`, `activity`, flat XP amount. | | `server/NeonSprawl.Server/Game/Skills/RefineActivitySkillXpGrant.cs` | Single entry point calling `TryApplyGrant` for successful craft/refine (prototype). | | `server/NeonSprawl.Server.Tests/Game/Skills/RefineActivitySkillXpGrantTests.cs` | AAA tests: grant when allowed; no XP when `activity` not in allowlist for `refine`. | -| `docs/manual-qa/NEO-42.md` | Checklist during implementation per repo convention (may be minimal until E3.M2 endpoint exists). | +| `server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs` | Test host with **`refine`** catalog row that omits **`activity`**. | +| `docs/manual-qa/NEO-42.md` | Curl checklist + E3.M2 follow-up note. | ## Files to modify | Path | Rationale | |------|-----------| -| **None at kickoff** for application wiring — **E3.M2** success handler file path is unknown until that module exists; add the **`RefineActivitySkillXpGrant`** call there in the same or follow-up PR. If E3.M2 lands before NEO-42 implementation, replace this row with the concrete path (e.g. `…/Crafting/CraftApi.cs`). | +| `server/README.md` | Document NEO-42 hook type and E3.M2 integration expectation. | +| **E3.M2 craft success handler** (path TBD) | Call **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`** once on successful craft/refine — **not done in this PR** (module absent). | ## Tests diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs new file mode 100644 index 0000000..39a92df --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs @@ -0,0 +1,112 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Time.Testing; +using NeonSprawl.Server.Game.AbilityInput; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; +using Npgsql; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +/// +/// Same in-memory overrides as , plus a stub +/// where refine does not allow activity — NEO-42 defensive tests without mutating disk catalog. +/// +public sealed class RefineActivityDeniedRegistryWebApplicationFactory : WebApplicationFactory +{ + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Testing"); + + var skillsDir = SkillCatalogPathResolution.TryDiscoverSkillsDirectory(AppContext.BaseDirectory) + ?? throw new InvalidOperationException( + "Could not discover repo content/skills from AppContext.BaseDirectory; run tests from the neon-sprawl clone."); + builder.UseSetting("Content:SkillsDirectory", skillsDir); + + builder.ConfigureTestServices(services => + { + for (var i = services.Count - 1; i >= 0; i--) + { + var d = services[i]; + if (d.ServiceType == typeof(IPositionStateStore) || + d.ServiceType == typeof(IPlayerHotbarLoadoutStore) || + d.ServiceType == typeof(IPlayerSkillProgressionStore) || + d.ServiceType == typeof(TimeProvider) || + d.ServiceType == typeof(IPlayerAbilityCooldownStore) || + d.ServiceType == typeof(ISkillDefinitionRegistry) || + d.ServiceType == typeof(NpgsqlDataSource) || + (d.ServiceType == typeof(IHostedService) && + (d.ImplementationType == typeof(PostgresDevPlayerSeedHostedService) || + d.ImplementationType == typeof(NpgsqlDataSourceShutdownHostedService)))) + { + services.RemoveAt(i); + } + } + + var fakeTime = new FakeTimeProvider(new DateTimeOffset(2026, 4, 30, 12, 0, 0, TimeSpan.Zero)); + services.AddSingleton(fakeTime); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }); + } +} + +/// Prototype trio ids; refine excludes activity so craft/refine grant path denies without throwing. +internal sealed class RefineActivityDeniedSkillRegistry : ISkillDefinitionRegistry +{ + private static readonly SkillDefRow Salvage = new( + "salvage", + "gather", + "Salvage", + new[] { "activity", "mission_reward" }); + + private static readonly SkillDefRow RefineNoActivity = new( + "refine", + "process", + "Refine", + new[] { "mission_reward", "trainer" }); + + private static readonly SkillDefRow Intrusion = new( + "intrusion", + "tech", + "Intrusion", + new[] { "activity", "mission_reward", "book_or_item" }); + + private static readonly IReadOnlyList Ordered = + [ + Intrusion, + RefineNoActivity, + Salvage, + ]; + + public bool TryGetDefinition(string? skillId, [NotNullWhen(true)] out SkillDefRow? definition) + { + if (skillId is null) + { + definition = null; + return false; + } + + var k = skillId.Trim(); + foreach (var row in Ordered) + { + if (string.Equals(row.Id, k, StringComparison.OrdinalIgnoreCase)) + { + definition = row; + return true; + } + } + + definition = null; + return false; + } + + public IReadOnlyList GetDefinitionsInIdOrder() => Ordered; +} diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivitySkillXpGrantTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivitySkillXpGrantTests.cs new file mode 100644 index 0000000..d59f4c0 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/RefineActivitySkillXpGrantTests.cs @@ -0,0 +1,82 @@ +using System.Linq; +using System.Net; +using System.Net.Http.Json; +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +/// NEO-42: craft/refine completion hook → refine XP via shared NEO-38 grant path (sourceKind: activity). +public sealed class RefineActivitySkillXpGrantTests +{ + [Fact] + public async Task GrantOnSuccessfulCraftOrRefine_WhenCatalogAllowsActivity_ShouldIncreaseRefineXp() + { + // Arrange — dev player is seeded on the in-memory progression store (NEO-38). + await using var factory = new InMemoryWebApplicationFactory(); + _ = factory.CreateClient(); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine("dev-local-1", registry, xpStore, levelCurve); + + // Assert + var totals = xpStore.GetXpTotals("dev-local-1"); + Assert.True(totals.TryGetValue("refine", out var xp)); + Assert.Equal(RefineSkillXpConstants.ActivityXpPerCraftRefineCompletion, xp); + } + + [Fact] + public async Task GrantOnSuccessfulCraftOrRefine_WhenCatalogAllowsActivity_ShouldMatchSkillProgressionGet() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var move = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = 0.5, Y = 0.9, Z = 0.5 }, + }; + await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine("dev-local-1", registry, xpStore, levelCurve); + var snapshot = await client.GetAsync("/game/players/dev-local-1/skill-progression"); + + // Assert + Assert.Equal(HttpStatusCode.OK, snapshot.StatusCode); + var body = await snapshot.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + var refine = body!.Skills!.Single(static s => s.Id == "refine"); + Assert.Equal(RefineSkillXpConstants.ActivityXpPerCraftRefineCompletion, refine.Xp); + } + + [Fact] + public async Task GrantOnSuccessfulCraftOrRefine_WhenRefineDisallowsActivity_ShouldLeaveRefineXpZero() + { + // Arrange + await using var factory = new RefineActivityDeniedRegistryWebApplicationFactory(); + _ = factory.CreateClient(); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine("dev-local-1", registry, xpStore, levelCurve); + + // Assert + var totals = xpStore.GetXpTotals("dev-local-1"); + _ = totals.TryGetValue("refine", out var refineXp); + Assert.Equal(0, refineXp); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/RefineActivitySkillXpGrant.cs b/server/NeonSprawl.Server/Game/Skills/RefineActivitySkillXpGrant.cs new file mode 100644 index 0000000..8d5d00d --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/RefineActivitySkillXpGrant.cs @@ -0,0 +1,26 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// +/// NEO-42: E3.M2 craft/refine success should call so XP uses the same path as +/// POST …/skill-progression. Outcome is ignored on success paths (mirror gather interact); use +/// directly when callers need deny envelopes. +/// +public static class RefineActivitySkillXpGrant +{ + /// Applies one refine + activity grant if the catalog allowlist permits it. + public static void GrantOnSuccessfulCraftOrRefine( + string playerId, + ISkillDefinitionRegistry registry, + IPlayerSkillProgressionStore xpStore, + ISkillLevelCurve levelCurve) + { + _ = SkillProgressionGrantOperations.TryApplyGrant( + playerId.Trim(), + RefineSkillXpConstants.RefineSkillId, + RefineSkillXpConstants.ActivityXpPerCraftRefineCompletion, + RefineSkillXpConstants.ActivitySourceKind, + registry, + xpStore, + levelCurve); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/RefineSkillXpConstants.cs b/server/NeonSprawl.Server/Game/Skills/RefineSkillXpConstants.cs new file mode 100644 index 0000000..6a73345 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/RefineSkillXpConstants.cs @@ -0,0 +1,11 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// NEO-42: prototype craft/refine completion → refine skill XP via sourceKind: activity. +public static class RefineSkillXpConstants +{ + public const int ActivityXpPerCraftRefineCompletion = 10; + + public const string RefineSkillId = "refine"; + + public const string ActivitySourceKind = "activity"; +} diff --git a/server/README.md b/server/README.md index 4053c56..f78509e 100644 --- a/server/README.md +++ b/server/README.md @@ -178,6 +178,10 @@ When **`allowed` is `true`**, **`reasonCode` is omitted** (clients must not requ When **`allowed` is `true`** and the interactable’s **`kind`** is **`resource_node`** (today: **`prototype_resource_node_alpha`** from **`GET /game/world/interactables`**), the server applies **10** **`salvage`** skill XP with **`sourceKind: activity`** using the **same validation and persistence** as **`POST /game/players/{id}/skill-progression`** ([NEO-38](#skill-progression-grant-neo-38)) via `SkillProgressionGrantOperations` — not a separate client-side XP bump. Move into horizontal range first (anchor **(12, −6)** m on X/Z, radius **3**). Plan: [NEO-41 implementation plan](../../docs/plans/NEO-41-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-41.md`](../../docs/manual-qa/NEO-41.md); Bruno: `bruno/neon-sprawl-server/position/Post move near prototype resource node.bru` then `bruno/neon-sprawl-server/interaction/Post interact prototype resource node gather.bru`. +### Craft / refine hook → skill XP (NEO-42) + +**E3.M2** recipe/refine success paths should call **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`**, which applies **10** **`refine`** skill XP with **`sourceKind: activity`** through **`SkillProgressionGrantOperations.TryApplyGrant`** (same validation, **`allowedXpSourceKinds`**, and persistence as **[NEO-38](#skill-progression-grant-neo-38)**). There is no separate craft HTTP in NEO-42; verify behavior with **`POST …/skill-progression`** for **`refine`**/**`activity`** until the craft module wires the helper. Plan: [NEO-42 implementation plan](../../docs/plans/NEO-42-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-42.md`](../../docs/manual-qa/NEO-42.md). + Contract details and PR blurb: [NEO-9 implementation plan](../../docs/plans/NEO-9-implementation-plan.md). ## Interactable descriptors (NEO-25)