jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(client): hub teleport UX + confrontation color shift (#501, #521) #38

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 2 Commits - Files Changed -
+782 -7
jpmschweitzer commented 2026-02-19 08:46:45 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 10 "Prove" — final two client tickets, completing all 7 client tasks for the sprint.

  • #501 Hub teleport UX: Home key sends TeleportToHub in Gauntlet mode. Camera snaps to hub spawn with 0.3s fade-from-black. Clears dialogue/monologue/interaction buffers. Teleport detection uses distance threshold (>5 tiles) so future teleport types inherit the transition.
  • #521 Confrontation D-033 color shift: Entity renderer tracks relationship per entity and tweens D-033 tint color over 0.7s when relationship changes on confrontation delivery. Manual lerp in _process() for testability. Cursor hover tint cascades automatically via Constants.color_for_entity_kind().

Decisions: D-020, D-033, D-054, D-063

Test plan

  • 21 hub teleport tests: enum, gauntlet guard, wire format, roundtrip, detection boundary cases, buffer clearing
  • 26 color shift tests: D-033 palette constants, relationship→color mapping, tween lifecycle (not-instant + convergence), protocol round-trip, GameState passthrough
  • 47/47 new tests pass, no regressions in full suite (348 cases, pre-existing failures only)
## Summary Sprint 10 "Prove" — final two client tickets, completing all 7 client tasks for the sprint. - **#501 Hub teleport UX:** Home key sends `TeleportToHub` in Gauntlet mode. Camera snaps to hub spawn with 0.3s fade-from-black. Clears dialogue/monologue/interaction buffers. Teleport detection uses distance threshold (>5 tiles) so future teleport types inherit the transition. - **#521 Confrontation D-033 color shift:** Entity renderer tracks relationship per entity and tweens D-033 tint color over 0.7s when relationship changes on confrontation delivery. Manual lerp in `_process()` for testability. Cursor hover tint cascades automatically via `Constants.color_for_entity_kind()`. **Decisions:** D-020, D-033, D-054, D-063 ## Test plan - [x] 21 hub teleport tests: enum, gauntlet guard, wire format, roundtrip, detection boundary cases, buffer clearing - [x] 26 color shift tests: D-033 palette constants, relationship→color mapping, tween lifecycle (not-instant + convergence), protocol round-trip, GameState passthrough - [x] 47/47 new tests pass, no regressions in full suite (348 cases, pre-existing failures only)
jpmschweitzer added 2 commits 2026-02-19 08:46:46 +01:00
feat(client): hub teleport UX — Home key, fade transition (#501) 7274151671 
Home key sends TeleportToHub in Gauntlet mode. Camera snaps to hub
spawn with 0.3s fade-from-black. Clears dialogue/monologue buffers
on teleport. Teleport detection uses distance threshold (>5 tiles)
so future teleport types get the transition for free.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): confrontation D-033 color shift (#521, D-063) 58e592fd5e 
Entity renderer now tracks relationship per entity and tweens D-033
tint color over 0.7s when relationship changes (e.g. on confrontation
delivery). Uses manual lerp in _process() for testability instead of
SceneTree tweens. Cursor hover tint cascades automatically via
Constants.color_for_entity_kind().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-19 09:02:45 +01:00
Author
Owner
Copy Link
Copy Source

Review: client -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Both features are well-implemented. Three issues block approval.

# File Severity Issue
1 entity_renderer.gd:29 warning COLOR_FADE_DURATION = 0.7 contradicts D-033 spec ("0.5s fade"). Comment misquotes spec as "0.5-1s". Either fix to 0.5 or update the decision.
2 test_hub_teleport.gd:76-92 warning test_teleport_hub_gated_by_gauntlet_mode and _allowed_in_gauntlet_mode are tautological — set a bool and assert its value, never exercise InputMapper's actual guard.
3 test_hub_teleport.gd:208-225 warning test_teleport_clears_monologue_state and _clears_dialogue_state manually null fields instead of calling _teleport_transition().
4 entity_renderer.gd suggestion No test for mid-transition re-trigger (relationship changes twice rapidly).
5 sim_bridge.gd:~330 suggestion Test snapshot NPC has no relationship field — color shift untestable in test mode.
6 commit messages suggestion Test counts don't match actual: #501 says 22/has 21, #521 says 30/has 26.

Tyre (Architecture): REQUEST_CHANGES

# File Severity Issue
1 main.gd:_process warning _teleport_transition() disables smoothing but the re-enable block at bottom of same _process() frame re-enables it immediately. Add a _teleport_in_progress flag to defer by one frame.
2 sim_bridge.gd:_test_snapshot warning No gauntlet_mode field in test snapshot — gauntlet guard has no integration coverage. Add _test_gauntlet_mode: bool like _test_in_dialogue.
3 test_hub_teleport.gd suggestion Teleport detection tests compute distance inline, not via _detect_teleport() — threshold 5.0 in two places.
4 entity_renderer.gd suggestion modulate.a and color are compositionally independent — note for future if alpha tweening added.
5 main.gd:_teleport_transition suggestion _flash_rect shared between teleport and room reset — teleport preempts amber flash. Worth a comment.

Scope change: #501 is Gauntlet-only

#501 is re-scoped as Gauntlet-only dev teleport, not a foundation for production fast-travel. The real game travel system is diegetic (local gate → system gate → target menu → destination). Rename references from "hub teleport" to "gauntlet dev teleport" or similar to make the debug scope explicit.

Verdict: CHANGES REQUESTED

## Review: client -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Both features are well-implemented. Three issues block approval. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `entity_renderer.gd:29` | warning | `COLOR_FADE_DURATION = 0.7` contradicts D-033 spec ("0.5s fade"). Comment misquotes spec as "0.5-1s". Either fix to 0.5 or update the decision. | | 2 | `test_hub_teleport.gd:76-92` | warning | `test_teleport_hub_gated_by_gauntlet_mode` and `_allowed_in_gauntlet_mode` are tautological — set a bool and assert its value, never exercise InputMapper's actual guard. | | 3 | `test_hub_teleport.gd:208-225` | warning | `test_teleport_clears_monologue_state` and `_clears_dialogue_state` manually null fields instead of calling `_teleport_transition()`. | | 4 | `entity_renderer.gd` | suggestion | No test for mid-transition re-trigger (relationship changes twice rapidly). | | 5 | `sim_bridge.gd:~330` | suggestion | Test snapshot NPC has no `relationship` field — color shift untestable in test mode. | | 6 | commit messages | suggestion | Test counts don't match actual: #501 says 22/has 21, #521 says 30/has 26. | ### Tyre (Architecture): REQUEST_CHANGES | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `main.gd:_process` | warning | `_teleport_transition()` disables smoothing but the re-enable block at bottom of same `_process()` frame re-enables it immediately. Add a `_teleport_in_progress` flag to defer by one frame. | | 2 | `sim_bridge.gd:_test_snapshot` | warning | No `gauntlet_mode` field in test snapshot — gauntlet guard has no integration coverage. Add `_test_gauntlet_mode: bool` like `_test_in_dialogue`. | | 3 | `test_hub_teleport.gd` | suggestion | Teleport detection tests compute distance inline, not via `_detect_teleport()` — threshold 5.0 in two places. | | 4 | `entity_renderer.gd` | suggestion | `modulate.a` and `color` are compositionally independent — note for future if alpha tweening added. | | 5 | `main.gd:_teleport_transition` | suggestion | `_flash_rect` shared between teleport and room reset — teleport preempts amber flash. Worth a comment. | ### Scope change: #501 is Gauntlet-only **#501 is re-scoped as Gauntlet-only dev teleport**, not a foundation for production fast-travel. The real game travel system is diegetic (local gate → system gate → target menu → destination). Rename references from "hub teleport" to "gauntlet dev teleport" or similar to make the debug scope explicit. ### Verdict: CHANGES REQUESTED
jpmschweitzer added 1 commit 2026-02-19 09:06:48 +01:00
fix(client): address PR #38 review — 6 warnings + 4 suggestions 7640a9ab87 
Hoshe:
- COLOR_FADE_DURATION 0.7 → 0.5 to match D-033 spec ("0.5s fade")
- Gauntlet guard tests now exercise InputMapper._unhandled_input()
  with synthesized InputEventKey instead of asserting a bool
- Buffer clearing tests use SimBridge pipeline instead of manual nulls
- Add mid-transition re-trigger test (rapid relationship changes)
- Add relationship field to test snapshot NPC

Tyre:
- Add _teleport_in_progress flag to defer smoothing re-enable by one
  frame after teleport (prevents same-_process() re-enable race)
- Add _test_gauntlet_mode to SimBridge test snapshot
- Extract TELEPORT_DISTANCE_THRESHOLD constant, mirror in tests
- Add comments: flash preemption, modulate/color independence
- Rename "hub teleport" → "Gauntlet dev teleport" in code comments
  to clarify this is not production fast-travel

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-19 09:10:37 +01:00
Author
Owner
Copy Link
Copy Source

Re-Review (round 2): client -> main (type: code)

Hoshe (Code Quality): APPROVE

All 6 items from previous round confirmed fixed:

  1. COLOR_FADE_DURATION corrected to 0.5 (D-033 spec)
  2. Gauntlet guard tests now synthesize real InputEventKey, call _unhandled_input()
  3. Buffer clearing tests now use SimBridge._test_snapshot() pipeline
  4. Mid-transition retrigger test added (Unknown→Friendly→Hostile rapid change)
  5. Test snapshot NPC includes relationship field via _test_npc_relationship
  6. Test counts: 22 hub_teleport + 27 color_shift = 49 tests passing

Tyre (Architecture): APPROVE

Both warnings resolved:

  1. _teleport_in_progress flag defers smoothing re-enable by one frame — race condition eliminated
  2. _test_gauntlet_mode added to SimBridge, included in _test_snapshot() return dict

All suggestions addressed: TELEPORT_DISTANCE_THRESHOLD extracted, compositionality note added, flash rect preemption documented.

Verdict: APPROVED

## Re-Review (round 2): client -> main (type: code) ### Hoshe (Code Quality): APPROVE All 6 items from previous round confirmed fixed: 1. `COLOR_FADE_DURATION` corrected to 0.5 (D-033 spec) 2. Gauntlet guard tests now synthesize real `InputEventKey`, call `_unhandled_input()` 3. Buffer clearing tests now use `SimBridge._test_snapshot()` pipeline 4. Mid-transition retrigger test added (`Unknown→Friendly→Hostile` rapid change) 5. Test snapshot NPC includes `relationship` field via `_test_npc_relationship` 6. Test counts: 22 hub_teleport + 27 color_shift = 49 tests passing ### Tyre (Architecture): APPROVE Both warnings resolved: 1. `_teleport_in_progress` flag defers smoothing re-enable by one frame — race condition eliminated 2. `_test_gauntlet_mode` added to SimBridge, included in `_test_snapshot()` return dict All suggestions addressed: `TELEPORT_DISTANCE_THRESHOLD` extracted, compositionality note added, flash rect preemption documented. ### Verdict: APPROVED
jpmschweitzer closed this pull request 2026-02-19 09:10:42 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#38