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-06-17-NEO-142.md

5.5 KiB
Raw Blame History

Code review — NEO-142 (E7M3-10)

Date: 2026-06-17
Scope: Branch NEO-142-client-faction-standing-gate-hud — commits fb9a8ab … e396228 vs main
Base: main

Verdict

Approve with nits

Summary

This branch lands E7M3-10: Godot client faction standing HUD and readable faction_gate_blocked accept feedback, wired through existing quest HUD patterns (NEO-122 / NEO-131). New faction_standing_client.gd mirrors skill_progression_client.gd (GET parse, pending-sync coalesce, 404 handling). quest_hud_controller.gd renders FactionStandingLabel, refreshes standing on boot + in-session completion transitions, formats reputationGrants reward lines, and builds readable gate deny copy from cached factionGateRules + last standing snapshot. quest_definitions_client.gd adds faction_gate_rules_for. GdUnit coverage spans client parse/sync, HUD render/deny/refresh, and operator-chain rep reward line. Docs: implementation plan, client/README.md faction subsection, docs/manual-qa/NEO-142.md. The branch also bundles NEO-141 story-end cleanup (remove docs/manual-qa/NEO-141.md, plan/README wording) and a small unrelated RewardRouterOperations collection-expression refactor. 24 targeted GdUnit tests pass locally. Low merge risk — read-only client projections; no server contract changes beyond doc/chore.

Documentation checked

Path Result
docs/plans/NEO-142-implementation-plan.md Matches — kickoff decisions, file list, technical approach, AC checklist, and Implementation reconciliation (shipped) align with the diff; PR bundled-commit note added.
docs/plans/E7M3-pre-production-backlog.md Matches — E7M3-10 scope bullets match; E7M3-10 AC checked + landed note.
docs/decomposition/modules/E7_M3_FactionReputationLedger.md Matches — module intent and contracts satisfied; Status line notes E7M3-10 landed (NEO-142); E7M3-11 capstone pending.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E7.M3 row includes NEO-142 client HUD landed note + README/manual QA links.
docs/decomposition/modules/module_dependency_register.md N/A change — no register row edit required beyond module doc / alignment table.
docs/manual-qa/NEO-142.md Matches — component checklist, honest gate-deny testing note, standing-label refresh lag note on step 4.
client/README.md Matches — Faction standing + gate feedback HUD (NEO-142) subsection documents GET endpoints, label placement, refresh triggers, and error pattern. Does not yet include End-to-end faction reputation loop capstone (E7M3-11 / NEO-143 scope per backlog).
docs/plans/NEO-141-implementation-plan.md Matches — bundled story-end manual QA removal consistent with NEO-140 precedent.

Blocking issues

None.

Suggestions

  1. Update decomposition tracking when NEO-142 closes. After manual QA, extend E7_M3_FactionReputationLedger.md Status and documentation_and_implementation_alignment.md E7.M3 row with E7M3-10 landed (NEO-142) — client faction standing GET + HUD + gate deny + rep reward lines; link client/README.md faction subsection and docs/manual-qa/NEO-142.md. Done.

  2. Reconcile plan AC at story end. Check off docs/plans/NEO-142-implementation-plan.md acceptance criteria and E7M3-10 backlog AC after Godot manual verification; mark Implementation reconciliation complete (same pattern as NEO-140/141 reviews). Done.

  3. PR description should call bundled commits. If opening one PR from this branch, mention NEO-142 HUD work plus NEO-141 manual-QA removal and the RewardRouterOperations collection-expression chore so reviewers are not surprised by server diff noise. Done. Noted in plan PR notes (bundled branch) section.

Nits

  • Nit: quest_hud_controller.gd _faction_display_name probes an instance display_name_for on FactionStandingClient, but the script exposes display_name_for as static only — the instance branch is likely dead; fallback static call is correct either way. Done. _faction_display_name calls FactionStandingClient.display_name_for directly.
  • Nit: _title_case_token is duplicated in faction_standing_client.gd and quest_hud_controller.gd — acceptable for prototype; consolidate only if a shared helper already exists elsewhere.
  • Nit: No dedicated GdUnit for quest_definitions_client.faction_gate_rules_for — gate rules are exercised indirectly via faction_standing_hud_test.gd; optional direct parse test if the helper grows. Done. quest_definitions_client_test.gd adds faction_gate_rules_for coverage.
  • Nit: Brief standing-label staleness on the same frame as completion (reward label shows +15 rep before faction GET returns) is adopted plan behavior; manual QA step 4 should confirm label catches up within one round-trip. Done. Step 4 notes one GET round-trip lag.

Verification

cd /Users/don/neon-sprawl/client && godot --headless --path . -s addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode \
  -a test/faction_standing_client_test.gd -a test/faction_standing_hud_test.gd -a test/quest_reward_hud_test.gd

Manual verification: docs/manual-qa/NEO-142.md — fresh server + Godot F5; operator chain → Grid Operators: 15 + reward rep line; optional dev fixture for live faction_gate_blocked deny copy.