diff --git a/docs/plans/NEO-121-implementation-plan.md b/docs/plans/NEO-121-implementation-plan.md index 4d0cddf..33fc418 100644 --- a/docs/plans/NEO-121-implementation-plan.md +++ b/docs/plans/NEO-121-implementation-plan.md @@ -56,7 +56,7 @@ - **`quest_start`:** after successful **`TryActivate`** in **`TryAccept`**. - **`quest_step_complete`:** after successful **`TryAdvanceStep`** store commit. - **`quest_complete`:** after first-time **`TryMarkComplete`** commit (not idempotent replay). -- **`quest_accept_denied`:** **`DenyAccept`** helper on all **`TryAccept`** deny paths. +- **`quest_accept_denied`:** **`DenyAccept(playerId, questId, …)`** on all **`TryAccept`** deny paths; optional accept context on **`DenyFromProgressSnapshot`** for activate-failure rereads. - **Docs:** `server/README.md` quest telemetry subsection; E7.M1 module + alignment register + E7M1 backlog updated. - **Manual QA:** `docs/manual-qa/NEO-121.md`. diff --git a/docs/reviews/2026-06-07-NEO-121.md b/docs/reviews/2026-06-07-NEO-121.md index 0c31cf8..708d1f6 100644 --- a/docs/reviews/2026-06-07-NEO-121.md +++ b/docs/reviews/2026-06-07-NEO-121.md @@ -37,9 +37,9 @@ NEO-121 adds **comment-only** E9.M1 telemetry hook sites to **`QuestStateOperati ## Nits -1. **Nit:** Plan technical approach lists optional **`schemaVersion`** in **`quest_start`** planned payload; the inline comment omits it. Harmless for comment-only work; consider adding for parity when touching the file again. +1. ~~**Nit:** Plan technical approach lists optional **`schemaVersion`** in **`quest_start`** planned payload; the inline comment omits it. Harmless for comment-only work; consider adding for parity when touching the file again.~~ **Done.** Added optional **`schemaVersion`** to **`quest_start`** inline comment. -2. **Nit:** **`DenyFromProgressSnapshotAccept`** duplicates **`DenyFromProgressSnapshot`** except for **`DenyAccept`** vs **`Deny`** — intentional funneling, but E9.M1 wiring may want **`playerId`** / **`questId`** threaded into **`DenyAccept`** (comment already documents those fields; implementer will need caller context). +2. ~~**Nit:** **`DenyFromProgressSnapshotAccept`** duplicates **`DenyFromProgressSnapshot`** except for **`DenyAccept`** vs **`Deny`** — intentional funneling, but E9.M1 wiring may want **`playerId`** / **`questId`** threaded into **`DenyAccept`** (comment already documents those fields; implementer will need caller context).~~ **Done.** Merged into **`DenyFromProgressSnapshot`** with optional accept-deny context; **`DenyAccept`** now takes **`playerId`** and **`questId`** at all call sites. ## Verification diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs index a8ab09d..7dac722 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs @@ -25,22 +25,22 @@ public static class QuestStateOperations if (!questRegistry.TryNormalizeKnown(questId, out var normalizedQuestId) || !questRegistry.TryGetDefinition(normalizedQuestId, out var definition)) { - return DenyAccept(QuestStateReasonCodes.UnknownQuest); + return DenyAccept(playerId, questId, QuestStateReasonCodes.UnknownQuest); } if (!progressStore.CanWritePlayer(playerId)) { - return DenyAccept(QuestStateReasonCodes.UnknownPlayer); + return DenyAccept(playerId, normalizedQuestId, QuestStateReasonCodes.UnknownPlayer); } if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing)) { if (existing.Status == QuestProgressStatus.Completed) { - return DenyAccept(QuestStateReasonCodes.AlreadyCompleted, existing); + return DenyAccept(playerId, normalizedQuestId, QuestStateReasonCodes.AlreadyCompleted, existing); } - return DenyAccept(QuestStateReasonCodes.AlreadyActive, existing); + return DenyAccept(playerId, normalizedQuestId, QuestStateReasonCodes.AlreadyActive, existing); } foreach (var prerequisiteId in definition.PrerequisiteQuestIds) @@ -48,7 +48,7 @@ public static class QuestStateOperations if (!progressStore.TryGetProgress(playerId, prerequisiteId, out var prerequisite) || prerequisite.Status != QuestProgressStatus.Completed) { - return DenyAccept(QuestStateReasonCodes.PrerequisiteIncomplete); + return DenyAccept(playerId, normalizedQuestId, QuestStateReasonCodes.PrerequisiteIncomplete); } } @@ -56,7 +56,8 @@ public static class QuestStateOperations { // --- Telemetry hook site (NEO-121): future E9.M1 catalog event `quest_start` --- // TODO(E9.M1): catalog emit — first activation only (deny paths excluded). - // Planned payload fields: playerId, questId, currentStepIndex (0). No ingest or ILogger here (comments-only). + // Planned payload fields: playerId, questId, currentStepIndex (0), optional schemaVersion. + // No ingest or ILogger here (comments-only). QuestObjectiveWiring.TryProcessInventoryHasItemForQuest( playerId, @@ -224,11 +225,11 @@ public static class QuestStateOperations string playerId, string questId, QuestStepState attemptSnapshot) => - DenyFromProgressSnapshotAccept(attemptSnapshot, QuestStateReasonCodes.AlreadyActive) + DenyFromProgressSnapshot(attemptSnapshot, QuestStateReasonCodes.AlreadyActive, playerId, questId) ?? (progressStore.TryGetProgress(playerId, questId, out var reread) - ? DenyFromProgressSnapshotAccept(reread, QuestStateReasonCodes.AlreadyActive) + ? DenyFromProgressSnapshot(reread, QuestStateReasonCodes.AlreadyActive, playerId, questId) : null) - ?? DenyAccept(QuestStateReasonCodes.UnknownPlayer); + ?? DenyAccept(playerId, questId, QuestStateReasonCodes.UnknownPlayer); private static QuestStateOperationResult DenyAdvanceFailure( IPlayerQuestStateStore progressStore, @@ -244,7 +245,9 @@ public static class QuestStateOperations private static QuestStateOperationResult? DenyFromProgressSnapshot( QuestStepState? snapshot, - string activeOrIntermediateReasonCode) + string activeOrIntermediateReasonCode, + string? acceptDenyPlayerId = null, + string? acceptDenyQuestId = null) { if (snapshot is null) { @@ -253,34 +256,16 @@ public static class QuestStateOperations if (snapshot.Status == QuestProgressStatus.Completed) { - return Deny(QuestStateReasonCodes.AlreadyCompleted, snapshot); + return acceptDenyPlayerId is not null + ? DenyAccept(acceptDenyPlayerId, acceptDenyQuestId ?? string.Empty, QuestStateReasonCodes.AlreadyCompleted, snapshot) + : Deny(QuestStateReasonCodes.AlreadyCompleted, snapshot); } if (snapshot.Status == QuestProgressStatus.Active) { - return Deny(activeOrIntermediateReasonCode, snapshot); - } - - return null; - } - - private static QuestStateOperationResult? DenyFromProgressSnapshotAccept( - QuestStepState? snapshot, - string activeOrIntermediateReasonCode) - { - if (snapshot is null) - { - return null; - } - - if (snapshot.Status == QuestProgressStatus.Completed) - { - return DenyAccept(QuestStateReasonCodes.AlreadyCompleted, snapshot); - } - - if (snapshot.Status == QuestProgressStatus.Active) - { - return DenyAccept(activeOrIntermediateReasonCode, snapshot); + return acceptDenyPlayerId is not null + ? DenyAccept(acceptDenyPlayerId, acceptDenyQuestId ?? string.Empty, activeOrIntermediateReasonCode, snapshot) + : Deny(activeOrIntermediateReasonCode, snapshot); } return null; @@ -289,11 +274,16 @@ public static class QuestStateOperations private static QuestStateOperationResult Success(QuestStepState snapshot) => new(Success: true, ReasonCode: null, Snapshot: snapshot); - private static QuestStateOperationResult DenyAccept(string reasonCode, QuestStepState? snapshot = null) + private static QuestStateOperationResult DenyAccept( + string playerId, + string questId, + string reasonCode, + QuestStepState? snapshot = null) { // --- Telemetry hook site (NEO-121): future E9.M1 catalog event `quest_accept_denied` --- // TODO(E9.M1): catalog emit — reasonCode from QuestStateReasonCodes (unknown_quest, prerequisite_incomplete, etc.). - // Planned payload fields: playerId, questId, reasonCode. No ingest or ILogger here (comments-only). + // Planned payload fields: playerId, questId, reasonCode (caller context threaded for E9.M1 wiring). + // No ingest or ILogger here (comments-only). return Deny(reasonCode, snapshot); }