don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/reviews/2026-05-24-NEO-74.md

4.9 KiB
Raw Permalink Blame History

Code review — NEO-74 client craft UI + recipe list

Date: 2026-05-24
Scope: Branch NEO-74-e3m2-client-craft-ui-recipe-list · commits dff58ad–c16b610 vs origin/main
Base: origin/main
Follow-up: Review feedback addressed (recipe-sync failure HUD, test .uid companions, unused _scroll binding).

Verdict

Approve — implementation matches the adopted plan and E3S5-03 acceptance criteria; GdUnit suites pass. Follow-up wired recipes_sync_failed HUD and added test .uid companions.

Summary

The branch adds recipe_definitions_client.gd, craft_client.gd, and craft_recipe_panel.gd, wires them from main.gd with CraftFeedbackLabel, a scrollable eight-recipe panel, refine row on SkillProgressionLabel, and post-success inventory + skill refresh (deny leaves bag unchanged). Three GdUnit suites cover POST payload/parse, recipe GET parse, and refresh policy. Manual QA, plan reconciliation, E3.M2 module note, alignment register, and E3S5 backlog checkboxes are updated. Design follows NEO-72/73/31 patterns (thin HTTP clients, _busy guard, injectable transport, authority config copy, dedicated feedback label). HTTP handlers correctly use PackedStringArray headers (NEO-72 lesson applied).

Documentation checked

Document Result
docs/plans/NEO-74-implementation-plan.md Matches — kickoff decisions (POST outputsGranted copy, dedicated CraftFeedbackLabel, left scroll panel, qty 1, deny refresh policy) reflected; acceptance checklist and reconciliation complete.
docs/plans/E3S5-client-prototype-backlog.md · E3S5-03 Matches — acceptance criteria checked; scope/out-of-scope honored; does not claim capstone (NEO-75) complete.
docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md Matches — NEO-74 client slice landed note added.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E3.M2 row notes NEO-74 landed; NEO-75 cited as remaining Slice 5 client work.
docs/decomposition/modules/client_server_authority.md Matches — client sends craft intent via POST; inventory/skill display refreshed from server GET after success.
docs/manual-qa/NEO-74.md Matches — deny/success/refine XP walkthrough; optional inventory_full spot-check.
client/README.md craft UI section Matches — endpoints, HUD labels, refresh policy; automated test Scope lists NEO-74 suites.
Full-stack epic decomposition Matches — E3S5-03 client counterpart for server NEO-68/NEO-70; Godot manual QA present.

Blocking issues

(none)

Suggestions

  1. Wire recipes_sync_failed in main.gd — RecipeDefinitionsClient emits recipes_sync_failed (tested in GdUnit) but main.gd never connects it. On boot failure the recipe panel stays empty with no HUD line (inventory/skill clients show errors). Connect to CraftFeedbackLabel or push_warning + a one-line error for parity with inventory_sync_failed / progression_sync_failed. Done. _on_recipes_sync_failed paints Recipes: error — … on CraftFeedbackLabel.

  2. Add .gd.uid companions for new GdUnit test scripts — craft_client_test.gd, recipe_definitions_client_test.gd, and craft_feedback_refresh_test.gd have no tracked *.gd.uid files (NEO-73 established this for new tests under client/test/). Generate in Godot and commit alongside the tests per godot-client-script-organization. Done.

Nits

  • Nit: craft_recipe_panel.gd declares @onready var _scroll but only uses _vbox — remove unused binding or use _scroll for max-height policy. Done.
  • Nit: craft_feedback_refresh_test.gd harness duplicates refresh logic from main.gd (intentional isolation) — acceptable; no main.gd integration test remains (same gap as NEO-73 gather refresh test).

Verification

# Server (separate terminal)
cd server/NeonSprawl.Server && dotnet run

# NEO-74 GdUnit suites (from client/)
godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode \
  -a res://test/craft_client_test.gd \
  -a res://test/recipe_definitions_client_test.gd \
  -a res://test/craft_feedback_refresh_test.gd

Manual: follow docs/manual-qa/NEO-74.md — deny insufficient_materials, refine_scrap_standard success, make_field_stim_mk0, refine XP delta without pressing I on success.