diff --git a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md index 38cde7c..e7b0f31 100644 --- a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md +++ b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md @@ -44,3 +44,7 @@ Epic 7 **Slice 2** — `reward_delivery`, `unlock_granted`; no double-claim on r - Master plan: [`neon_sprawl_vision.plan.md`](../../../neon_sprawl_vision.plan.md) — Epic 7. - [Module dependency register](module_dependency_register.md) + +## Implementation anchor (server) + +**NEO-43:** When applying scripted **skill XP** from a mission/quest reward bundle, call **`MissionRewardSkillXpGrant.GrantFromMissionReward`** (`server/NeonSprawl.Server/Game/Skills/`) — fixed **`sourceKind: mission_reward`**, **`skillId`** and **`amount`** from reward data — [NEO-43 implementation plan](../../plans/NEO-43-implementation-plan.md). diff --git a/docs/manual-qa/NEO-43.md b/docs/manual-qa/NEO-43.md new file mode 100644 index 0000000..29aa6c0 --- /dev/null +++ b/docs/manual-qa/NEO-43.md @@ -0,0 +1,63 @@ +# Manual QA — NEO-43 (mission / quest → skill XP, `mission_reward`) + +Reference: [implementation plan](../plans/NEO-43-implementation-plan.md), [NEO-38](./NEO-38.md) (skill progression grant), [NEO-42](./NEO-42.md) (prep-hook 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 + +`MissionRewardSkillXpGrantTests` exercises the **NEO-38** grant path with disk **`prototype_skills.json`** (`salvage` allows **`mission_reward`**) and a stub registry where **`salvage`** omits **`mission_reward`** from **`allowedXpSourceKinds`**. + +## Control: direct POST grant (same stack as E7.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 **`salvage`** + **`mission_reward`** (mirrors what **`MissionRewardSkillXpGrant.GrantFromMissionReward`** does internally for that skill/amount): + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":25,"sourceKind":"mission_reward"}' +``` + +Expect **HTTP 200**, **`granted": true`**, and **`salvage`** **`xp`** increased (repeat **POST** to see cumulative XP). + +## Deny path (catalog guard) + +With default catalog, **`trainer`** is **not** allowed for **`salvage`**: + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":5,"sourceKind":"trainer"}' +``` + +Expect **HTTP 200**, **`granted": false`**, **`reasonCode":"source_kind_not_allowed"`**. + +## Combat vs mission XP + +**Skill XP** from missions uses **`mission_reward`** (this story). **Combat encounters** must not grant skill XP by default — they award **gig XP** separately; see [progression.md](../game-design/progression.md) (*Mission rewards vs combat XP*). There is no encounter reward implementation under `server/` yet; when **E5 / E7** adds encounter payouts, keep skill progression grants off the default encounter pipe unless a product exception is explicitly modeled. + +## E7.M2 follow-up + +When quest hand-in / **`QuestRewardBundle`** apply exists, confirm **one** server-side call to **`MissionRewardSkillXpGrant.GrantFromMissionReward`** per scripted skill XP line item (with content-driven **`skillId`** and **`amount`**), then **`GET …/skill-progression`** shows expected totals. diff --git a/docs/plans/NEO-43-implementation-plan.md b/docs/plans/NEO-43-implementation-plan.md index ed4ec95..7d4bd90 100644 --- a/docs/plans/NEO-43-implementation-plan.md +++ b/docs/plans/NEO-43-implementation-plan.md @@ -14,9 +14,9 @@ | Topic | Question / note | Resolution | |--------|-----------------|------------| -| **Delivery before E7.M2 router** | E7.M2 router does not exist in repo; how to ship NEO-43? | **AskQuestion** offered options; **user skipped** — **default (agent):** mirror **[NEO-42](NEO-42-implementation-plan.md)** — add a **thin static helper** + **xUnit coverage** + **README / manual QA** documenting the contract; **E7.M2** calls the helper from the real quest/mission reward apply path when that module lands. | -| **Reward shape** | Fixed prototype vs generic skill + amount? | **Default:** **generic** parameters **`skillId`** + **`amount`** (positive int) with **`sourceKind`** fixed to catalog value **`mission_reward`** (matches future explicit reward line items; content chooses skill and payout). | -| **Combat encounters ≠ skill XP** | No encounter/gig XP code under `server/` today. | **Default:** satisfy AC by **(1)** plan + README stating the invariant; **(2)** confirming current grant call sites are **interact (gather)**, **refine prep (NEO-42)**, and **`POST …/skill-progression`** only — **no** combat encounter pipe. Future **E5 / encounter reward** work must **not** call `SkillProgressionGrantOperations` for default encounter outcomes (gig XP remains separate per [progression.md](../game-design/progression.md)). Optional follow-up: architecture test or doc lint if the team wants a machine-enforced guard once encounter code exists. | +| **Delivery before E7.M2 router** | E7.M2 router does not exist in repo; how to ship NEO-43? | **User (implementation):** accepted agent recommendation — **NEO-42-style prep:** **`MissionRewardSkillXpGrant`** + tests + README + manual QA; **E7.M2** wires the real quest hand-in later. | +| **Reward shape** | Fixed prototype vs generic skill + amount? | **User (implementation):** accepted agent recommendation — **generic** **`skillId`** + **`amount`**; helper fixes **`sourceKind: mission_reward`**. | +| **Combat encounters ≠ skill XP** | No encounter/gig XP code under `server/` today. | **User (implementation):** accepted agent recommendation — **README + manual QA + plan** document the invariant and current call sites; no synthetic encounter guard test until encounter code exists. | ## Goal, scope, and out-of-scope @@ -36,8 +36,13 @@ ## Acceptance criteria checklist -- [ ] **Scripted mission rewards can include skill XP;** validated against **`allowedXpSourceKinds`** (helper + tests; manual QA documents **`POST …/skill-progression`** with **`mission_reward`** for skills that allow it). -- [ ] **Combat encounters do not grant skill XP by default** — documented invariant + **current codebase** has no encounter grant path; gig XP remains separate (design reference: [progression.md](../game-design/progression.md), [E2_M1](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md)). +- [x] **Scripted mission rewards can include skill XP;** validated against **`allowedXpSourceKinds`** (helper + tests; manual QA documents **`POST …/skill-progression`** with **`mission_reward`** for skills that allow it). +- [x] **Combat encounters do not grant skill XP by default** — documented invariant + **current codebase** has no encounter grant path; gig XP remains separate (design reference: [progression.md](../game-design/progression.md), [E2_M1](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md)). + +## Decisions (implementation) + +- **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** — single canonical string for the helper and docs. +- **`MissionRewardDeniedRegistryWebApplicationFactory`** — **`salvage`** stub omits **`mission_reward`** (deny coverage); happy paths use disk catalog via **`InMemoryWebApplicationFactory`**. ## Technical approach @@ -79,5 +84,4 @@ ## Open questions / risks -- **User override:** If you want a **fixed prototype** reward (single skill + constant XP) instead of **generic skillId + amount**, say so before implementation — the helper signature and tests adjust trivially. -- **Risk:** Low — additive surface; same persistence and validation as NEO-38; helper **unreferenced** until E7.M2 (same pattern as NEO-42). +None. **Risk:** Low — additive surface; helper **unreferenced** until E7.M2 (same pattern as NEO-42). diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs new file mode 100644 index 0000000..3d6d35d --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.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 salvage does not allow mission_reward — NEO-43 defensive tests without mutating disk catalog. +/// +public sealed class MissionRewardDeniedRegistryWebApplicationFactory : 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; salvage excludes mission_reward so mission grant path denies without throwing. +internal sealed class MissionRewardDeniedSkillRegistry : ISkillDefinitionRegistry +{ + private static readonly SkillDefRow SalvageNoMissionReward = new( + "salvage", + "gather", + "Salvage", + new[] { "activity" }); + + private static readonly SkillDefRow Refine = new( + "refine", + "process", + "Refine", + new[] { "activity", "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, + Refine, + SalvageNoMissionReward, + ]; + + 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/MissionRewardSkillXpGrantTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs new file mode 100644 index 0000000..06c6dc5 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs @@ -0,0 +1,101 @@ +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-43: mission/quest reward hook → skill XP via shared NEO-38 grant path (sourceKind: mission_reward). +public sealed class MissionRewardSkillXpGrantTests +{ + private const int TestMissionRewardXp = 25; + + [Fact] + public async Task GrantFromMissionReward_WhenCatalogAllowsMissionReward_ShouldIncreaseSalvageXp() + { + // 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 + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + registry, + xpStore, + levelCurve); + + // Assert + var totals = xpStore.GetXpTotals("dev-local-1"); + Assert.True(totals.TryGetValue("salvage", out var xp)); + Assert.Equal(TestMissionRewardXp, xp); + } + + [Fact] + public async Task GrantFromMissionReward_WhenCatalogAllowsMissionReward_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 + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + 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 salvage = body!.Skills!.Single(static s => s.Id == "salvage"); + Assert.Equal(TestMissionRewardXp, salvage.Xp); + } + + [Fact] + public async Task GrantFromMissionReward_WhenSalvageDisallowsMissionReward_ShouldLeaveSalvageXpZero() + { + // Arrange + await using var factory = new MissionRewardDeniedRegistryWebApplicationFactory(); + _ = factory.CreateClient(); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + registry, + xpStore, + levelCurve); + + // Assert — deny returns before TryApplyXpDelta; salvage row must not appear in stored XP map. + var totals = xpStore.GetXpTotals("dev-local-1"); + Assert.False(totals.ContainsKey("salvage")); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs new file mode 100644 index 0000000..5401a0e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs @@ -0,0 +1,7 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// NEO-43: quest/mission scripted payouts use sourceKind: mission_reward (catalog + NEO-38). +public static class MissionRewardSkillXpConstants +{ + public const string MissionRewardSourceKind = "mission_reward"; +} diff --git a/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs new file mode 100644 index 0000000..31f024c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs @@ -0,0 +1,29 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// +/// NEO-43: E7.M2 mission/quest reward apply should call so XP uses the same path as +/// POST …/skill-progression. Outcome is ignored on success paths (mirror NEO-42); use +/// directly when callers need deny envelopes. +/// +public static class MissionRewardSkillXpGrant +{ + /// Applies one grant with sourceKind: mission_reward if the catalog allowlist permits it. + /// Pass a normalized (e.g. route-trimmed), matching other grant hook callers. + public static void GrantFromMissionReward( + string playerId, + string skillId, + int amount, + ISkillDefinitionRegistry registry, + IPlayerSkillProgressionStore xpStore, + ISkillLevelCurve levelCurve) + { + _ = SkillProgressionGrantOperations.TryApplyGrant( + playerId, + skillId, + amount, + MissionRewardSkillXpConstants.MissionRewardSourceKind, + registry, + xpStore, + levelCurve); + } +} diff --git a/server/README.md b/server/README.md index f78509e..631e4b2 100644 --- a/server/README.md +++ b/server/README.md @@ -182,6 +182,10 @@ When **`allowed` is `true`** and the interactable’s **`kind`** is **`resource_ **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). +### Mission / quest reward → skill XP (NEO-43) + +**E7.M2** quest/mission payout paths should call **`MissionRewardSkillXpGrant.GrantFromMissionReward`** with content-driven **`skillId`** and **`amount`**; the helper always uses **`sourceKind: mission_reward`** through **`SkillProgressionGrantOperations.TryApplyGrant`** (same validation, **`allowedXpSourceKinds`**, and persistence as **[NEO-38](#skill-progression-grant-neo-38)**). **Combat encounter** rewards must **not** use this path for default clears — gig XP stays separate ([`docs/game-design/progression.md`](../docs/game-design/progression.md)). Until the router exists, verify **`mission_reward`** with **`POST …/skill-progression`**. Plan: [NEO-43 implementation plan](../../docs/plans/NEO-43-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-43.md`](../../docs/manual-qa/NEO-43.md). + Contract details and PR blurb: [NEO-9 implementation plan](../../docs/plans/NEO-9-implementation-plan.md). ## Interactable descriptors (NEO-25)