don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

NEO-71: address code review — CraftResult tense and README bullets

pull/105/head
VinPropane 2026-05-24 18:32:08 -04:00
parent 2be35c1640
commit 5f0f703555
3 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
docs/reviews/2026-05-24-NEO-71.md
View File

@ -6,7 +6,7 @@
## Verdict
**Approve with nits** — comment-only hook sites and documentation match the adopted plan and E3M2-07 acceptance criteria; ready to merge after optional `CraftResult.cs` wording tweak.
**Approve with nits** — comment-only hook sites and documentation match the adopted plan and E3M2-07 acceptance criteria; ready to merge.
## Summary
@ -33,11 +33,11 @@ None.
## Suggestions
1. **`CraftResult.cs` summary tense** — Plan item 4 asked to confirm cross-reference wording after hooks land. File was not updated; summary still reads “NEO-71 telemetry hook sites **will live** in **`TryCraft`**”. Update to present tense (e.g. “live in **`CraftOperations.TryCraft`** (success path + **`Deny`**)”) so it matches landed hooks and the class summary on **`CraftOperations`**.
1. ~~**`CraftResult.cs` summary tense** — Plan item 4 asked to confirm cross-reference wording after hooks land. File was not updated; summary still reads “NEO-71 telemetry hook sites **will live** in **`TryCraft`**”. Update to present tense (e.g. “live in **`CraftOperations.TryCraft`** (success path + **`Deny`**)”) so it matches landed hooks and the class summary on **`CraftOperations`**.~~ **Done.**
## Nits
- Nit: README craft telemetry subsection is a single dense paragraph; a two-bullet list mirroring the code hook sites (`item_crafted` / `craft_failed`) would match NEO-64 gather README scanability — optional only.
- ~~Nit: README craft telemetry subsection is a single dense paragraph; a two-bullet list mirroring the code hook sites (`item_crafted` / `craft_failed`) would match NEO-64 gather README scanability — optional only.~~ **Done.**
## Verification

2
server/NeonSprawl.Server/Game/Crafting/CraftResult.cs
View File

@ -2,7 +2,7 @@ namespace NeonSprawl.Server.Game.Crafting;
/// <summary>
/// Server-internal craft resolution envelope (NEO-69); promoted to wire JSON via <see cref="CraftResponse"/> (NEO-70).
/// NEO-71 telemetry hook sites will live in <see cref="CraftOperations.TryCraft"/>.
/// NEO-71 telemetry hook sites live in <see cref="CraftOperations.TryCraft"/> (success path + centralized <c>Deny</c> helper).
/// </summary>
/// <param name="InputsConsumed">Scaled inputs removed on success; empty when denied.</param>
/// <param name="OutputsGranted">Scaled outputs added on success; empty when denied.</param>

7
server/README.md
View File

@ -119,7 +119,12 @@ Plan: [NEO-69 implementation plan](../../docs/plans/NEO-69-implementation-plan.m
### Craft telemetry hooks (NEO-71)
Comment-only hook sites in **`CraftOperations.TryCraft`** for future E9.M1 catalog events — **`item_crafted`** on every successful craft (inputs removed, outputs granted, refine XP applied); **`craft_failed`** with stable **`reasonCode`** on every structured deny via private **`Deny`** (all **`CraftReasonCodes`**, including store-missing and post-mutation rollback paths). **`TODO(E9.M1)`** — no production ingest. Successful craft output grants also pass through **`PlayerInventoryOperations`** (**`item_created`** hook, [NEO-56](#player-inventory-neo-54-store-neo-55-http)). HTTP **`POST …/craft`** calls the engine only (no duplicate API-layer hooks). Epic 3 Slice 3 metric **`time-to-first-craft`** is derived at E9.M1 from **`item_crafted`** + session context (no separate prototype hook). Plan: [NEO-71 implementation plan](../../docs/plans/NEO-71-implementation-plan.md).
Comment-only hook sites in **`CraftOperations.TryCraft`** for future E9.M1 catalog events. **`TODO(E9.M1)`** — no production ingest. HTTP **`POST …/craft`** calls the engine only (no duplicate API-layer hooks).
- **`item_crafted`** — on every successful craft (inputs removed, outputs granted, refine XP applied).
- **`craft_failed`** — with stable **`reasonCode`** on every structured deny via private **`Deny`** (all **`CraftReasonCodes`**, including store-missing and post-mutation rollback paths).
Successful craft output grants also pass through **`PlayerInventoryOperations`** (**`item_created`** hook, [NEO-56](#player-inventory-neo-54-store-neo-55-http)). Epic 3 Slice 3 metric **`time-to-first-craft`** is derived at E9.M1 from **`item_crafted`** + session context (no separate prototype hook). Plan: [NEO-71 implementation plan](../../docs/plans/NEO-71-implementation-plan.md).
## Resource node definitions (NEO-60)