feat(client): protocol v8 sync + test fixes (Sprint 8 client) #29

2026-02-18T01:42:12+01:00

jpmschweitzer commented

2026-02-18 01:42:12 +01:00

Summary

Bump client protocol from v7 to v8 to match server
Add dialogue_response field decoding (DialogueResponseEvent from server #305/D-028)
Fix Interact encoding: changed from unit variant to struct variant matching server's PlayerAction::Interact { target_entity_id, verb }
Fix test_monologue_not_duplicated_after_consumption — was using poll_snapshot() which doesn't consume _last_snapshot in test mode
Update all test assertions for v8 version and Interact struct variant

Test plan

make test-client — 1 remaining failure (test_movement_roundtrip, pre-existing timing issue unrelated to these changes)
All protocol bridge tests pass (v8 version, fixtures, stance, inventory)
All P0 regression tests pass (monologue carry-forward, camera during pause)
All encoding asymmetry tests pass (#474)
E2E connection test passes (server binary rebuilt at v8)
Sprint 2 proof test passes

🤖 Generated with Claude Code

## Summary - Bump client protocol from v7 to v8 to match server - Add `dialogue_response` field decoding (DialogueResponseEvent from server #305/D-028) - Fix `Interact` encoding: changed from unit variant to struct variant matching server's `PlayerAction::Interact { target_entity_id, verb }` - Fix `test_monologue_not_duplicated_after_consumption` — was using `poll_snapshot()` which doesn't consume `_last_snapshot` in test mode - Update all test assertions for v8 version and Interact struct variant ## Test plan - [x] `make test-client` — 1 remaining failure (`test_movement_roundtrip`, pre-existing timing issue unrelated to these changes) - [x] All protocol bridge tests pass (v8 version, fixtures, stance, inventory) - [x] All P0 regression tests pass (monologue carry-forward, camera during pause) - [x] All encoding asymmetry tests pass (#474) - [x] E2E connection test passes (server binary rebuilt at v8) - [x] Sprint 2 proof test passes 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jpmschweitzer added 3 commits 2026-02-18 01:42:12 +01:00

fix(client): fix monologue duplication test using wrong poll path dcb76d1221

test_monologue_not_duplicated_after_consumption was failing because
poll_snapshot() in test mode returns _test_snapshot() without
consuming _last_snapshot. The carry-forward logic then incorrectly
preserved the monologue. Fix: directly clear _last_snapshot to
simulate the live-mode consumption path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat(client): sync protocol to v8 — dialogue_response + Interact encoding 674c7147de

Bump PROTOCOL_VERSION from 7 to 8 to match server. Three changes:

1. Add dialogue_response field decoding (DialogueResponseEvent with
   line_id, text, speaker_entity_id) from server #305/D-028.

2. Fix Interact encoding: server changed PlayerAction::Interact from
   unit variant to struct variant with {target_entity_id, verb}.
   Extract _encode_action() helper to handle this consistently.

3. Update all test assertions that checked version=7 or expected
   Interact as a bare string.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore(meta): update changelog for protocol v8 sync 44d6960758

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jpmschweitzer commented

2026-02-18 01:56:25 +01:00

Review: client → main (PR #29, type: code)

Reviewers: Hoshe (code quality), Tyre (architecture)

Tyre (Architecture): APPROVE

Clean v8 protocol sync. Matches server-side PlayerAction::Interact struct variant definition. D-020/D-030 layered IPC testing pattern respected.

#	File	Severity	Issue
1	sim_bridge.gd:~225	warning	`receive_bytes` carry-forward logic covers `current_monologue` and `current_dialogue` but not `dialogue_response` — could drop a line under load. Follow-up ticket before #435 lands.
2	sim_bridge.gd:~377	warning	`_test_snapshot()` missing `dialogue_response` key — test snapshot drifting from wire format. Fix before #435.
3	protocol.gd:~305	suggestion	`encode_player_input` doc comment stale — missing WalkAway, SetTickRate, stance actions, and new Interact struct-variant behavior.
4	protocol.gd:~172	suggestion	`speaker_entity_id` uses -1 sentinel for missing field, inconsistent with rest of decoder which drops entities on missing IDs.

Hoshe (Code Quality): APPROVE (no blocking issues found)

Verdict: APPROVED

## Review: client → main (PR #29, type: code) Reviewers: Hoshe (code quality), Tyre (architecture) ### Tyre (Architecture): APPROVE Clean v8 protocol sync. Matches server-side `PlayerAction::Interact` struct variant definition. D-020/D-030 layered IPC testing pattern respected. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | sim_bridge.gd:~225 | warning | `receive_bytes` carry-forward logic covers `current_monologue` and `current_dialogue` but not `dialogue_response` — could drop a line under load. Follow-up ticket before #435 lands. | | 2 | sim_bridge.gd:~377 | warning | `_test_snapshot()` missing `dialogue_response` key — test snapshot drifting from wire format. Fix before #435. | | 3 | protocol.gd:~305 | suggestion | `encode_player_input` doc comment stale — missing WalkAway, SetTickRate, stance actions, and new Interact struct-variant behavior. | | 4 | protocol.gd:~172 | suggestion | `speaker_entity_id` uses -1 sentinel for missing field, inconsistent with rest of decoder which drops entities on missing IDs. | ### Hoshe (Code Quality): APPROVE (no blocking issues found) ### Verdict: APPROVED

jpmschweitzer closed this pull request

2026-02-18 01:57:15 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#29