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

feat(client): E2E connection test and wire format fixes (#81) #7

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-02-11 21:45:13 +01:00
Owner
Copy Link
Copy Source

Summary

  • Fix input encoding to batch Vec per frame matching server wire protocol
  • Fix SimBridge server args (positional address) and port default (9876)
  • Add EntityKind::Player fixture and update multi-entity fixture with all 4 entity kinds
  • Add D-030 Layer 3 E2E test: spawns Rust server, connects via LocalBridge, sends MoveNorth, verifies player movement in snapshot
  • 43/43 client tests pass, 53/53 server tests pass

Test plan

  • make test-client (43/43 pass including new E2E test)
  • make test-server (53/53 pass)
  • E2E test verifies full round-trip: client to server to snapshot with correct player position (16.5, 15.5)
  • Batch encoding tested with single, multiple, data-variant, and empty input arrays
  • EntityKind::Player fixture decoded correctly from Rust-generated msgpack

Generated with Claude Code

## Summary - Fix input encoding to batch Vec<PlayerInput> per frame matching server wire protocol - Fix SimBridge server args (positional address) and port default (9876) - Add EntityKind::Player fixture and update multi-entity fixture with all 4 entity kinds - Add D-030 Layer 3 E2E test: spawns Rust server, connects via LocalBridge, sends MoveNorth, verifies player movement in snapshot - 43/43 client tests pass, 53/53 server tests pass ## Test plan - [x] make test-client (43/43 pass including new E2E test) - [x] make test-server (53/53 pass) - [x] E2E test verifies full round-trip: client to server to snapshot with correct player position (16.5, 15.5) - [x] Batch encoding tested with single, multiple, data-variant, and empty input arrays - [x] EntityKind::Player fixture decoded correctly from Rust-generated msgpack Generated with Claude Code
jpmschweitzer added 4 commits 2026-02-11 21:45:14 +01:00
fix(client): batch-encode inputs as Vec<PlayerInput> per server wire format 2338941721 
Server expects a MessagePack array of PlayerInput objects in one framed
message per tick, not individual inputs per frame. Added
Protocol.encode_player_inputs() for batch encoding. Changed SimBridge to
buffer raw input dicts and batch-encode in _process(). Also fixed server
port default (9876) and positional arg format to match server CLI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): add EntityKind::Player fixture and update multi-entity fixture e1e4e346c7 
Server team added EntityKind::Player variant. Added snapshot_player
fixture and updated snapshot_multi_entity to include all 4 entity kinds
(Player, Npc, Object, Terrain) for complete D-030 Layer 1 coverage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): add E2E connection test and batch encoding tests 5e4b8dbcf3 
D-030 Layer 3: E2E test spawns the Rust server binary, connects via
LocalBridge, sends a batched MoveNorth input, and verifies the player
moved to (16.5, 15.5). Also adds batch encoding roundtrip tests,
framed batch test, and Player entity fixture decode test. 43/43 pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog c737d687aa 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-11 21:53:02 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review: client -> main (PR #7)

Hoshe (Code Quality): APPROVE

Solid implementation with excellent test coverage (43 client tests, 53 server tests). Wire format fix correctly matches server's Vec<PlayerInput> expectations. Three-layer D-030 architecture is well-exercised.

# File Severity Suggestion
1 test_e2e_connection.gd:37 suggestion Fixed TEST_PORT (19876) could conflict in parallel CI. Consider ephemeral port 0 (like server-side game_loop.rs does with TcpListener::bind("127.0.0.1:0")).
2 sim_bridge.gd:~144 suggestion Failed batch encoding silently drops inputs after drain_outbound() clears the buffer. Either re-queue failed entries or add a comment explaining that encode failure = corrupt state (drop is intentional).
3 test_protocol.gd suggestion Add a test with malformed entities in a snapshot to verify the decode_errors counter increments correctly (exercises the D-010 boundary violation log path).

Tyre (Architecture): APPROVE

Clean D-030 three-layer IPC testing. Batch encoding fix aligns client with server wire format. No D-010/D-012/D-020 violations. Separation of concerns between layers 1/2/3 is correct.

# File Severity Suggestion
1 test_protocol.gd suggestion Add a Rust-generated Vec<PlayerInput> fixture (input_batch_two.msgpack) for bidirectional Layer 1 symmetry — currently batch encoding is GDScript-to-server one-way only.
2 gen_fixtures.rs suggestion Add a Rust-side deserialization smoke test that reads all .msgpack fixtures back and asserts they round-trip. Guards against fixture corruption during git operations.

Verdict: APPROVED

All suggestions are hardening improvements — no blockers found.

## Dual-Agent Review: `client` -> `main` (PR #7) ### Hoshe (Code Quality): APPROVE Solid implementation with excellent test coverage (43 client tests, 53 server tests). Wire format fix correctly matches server's `Vec<PlayerInput>` expectations. Three-layer D-030 architecture is well-exercised. | # | File | Severity | Suggestion | |---|------|----------|------------| | 1 | `test_e2e_connection.gd:37` | suggestion | Fixed `TEST_PORT` (19876) could conflict in parallel CI. Consider ephemeral port 0 (like server-side `game_loop.rs` does with `TcpListener::bind("127.0.0.1:0")`). | | 2 | `sim_bridge.gd:~144` | suggestion | Failed batch encoding silently drops inputs after `drain_outbound()` clears the buffer. Either re-queue failed entries or add a comment explaining that encode failure = corrupt state (drop is intentional). | | 3 | `test_protocol.gd` | suggestion | Add a test with malformed entities in a snapshot to verify the `decode_errors` counter increments correctly (exercises the D-010 boundary violation log path). | ### Tyre (Architecture): APPROVE Clean D-030 three-layer IPC testing. Batch encoding fix aligns client with server wire format. No D-010/D-012/D-020 violations. Separation of concerns between layers 1/2/3 is correct. | # | File | Severity | Suggestion | |---|------|----------|------------| | 1 | `test_protocol.gd` | suggestion | Add a Rust-generated `Vec<PlayerInput>` fixture (`input_batch_two.msgpack`) for bidirectional Layer 1 symmetry — currently batch encoding is GDScript-to-server one-way only. | | 2 | `gen_fixtures.rs` | suggestion | Add a Rust-side deserialization smoke test that reads all `.msgpack` fixtures back and asserts they round-trip. Guards against fixture corruption during git operations. | ### Verdict: APPROVED All suggestions are hardening improvements — no blockers found.
jpmschweitzer added 2 commits 2026-02-11 21:57:24 +01:00
fix(client): address PR #7 review — ephemeral port, input drop docs 8d12776b5d 
Hoshe #1: E2E test now uses random ephemeral port (49152-65535) with
port rotation on bind failure, avoiding conflicts in parallel CI.
Hoshe #2: Documented intentional input drop on encode failure in
SimBridge — re-queuing would retry bad data and server tick has
already advanced.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): add batch fixture, fixture smoke test, decode_errors test beee44b3c8 
Tyre #1: Added Rust-generated input_batch_two.msgpack fixture for
bidirectional D-030 Layer 1 symmetry (Vec<PlayerInput>).
Tyre #2: Added all_fixtures_deserialize Rust test that reads every
.msgpack fixture and verifies it deserializes (corruption guard).
Hoshe #3: Added test_decode_snapshot_malformed_entities_counted test
verifying the decode_errors counter on D-010 boundary violations.
45 client tests, 54 server tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-11 22:03:32 +01:00
Author
Owner
Copy Link
Copy Source

Follow-up Review: client -> main (PR #7)

All 5 suggestions from the initial review have been addressed.

Hoshe (Code Quality): APPROVE

  • Ephemeral port: Random port from IANA ephemeral range (49152-65535) with 5-attempt rotation. Correct.
  • Input drop docs: Comment explains intentional drop — stale inputs invalid after tick advance. Clear.
  • decode_errors test: 1 valid + 2 malformed entities, asserts decode_errors == 2. Covers D-010 boundary path.
  • Batch fixture: Rust-generated input_batch_two.msgpack, GDScript decodes and verifies. Bidirectional Layer 1 complete.
  • Rust smoke tests: 6 tests in serialization.rs — round-trips for all 12 PlayerAction + 4 EntityKind variants, plus all_fixtures_deserialize corruption guard over all 11 .msgpack fixtures.

No remaining issues.

Tyre (Architecture): APPROVE

Test layering correct per D-030. No D-010/D-012/D-020 violations. The all_fixtures_deserialize test is an elegant corruption guard that runs on every cargo test.

Verdict: APPROVED — ready to merge.

## Follow-up Review: `client` -> `main` (PR #7) All 5 suggestions from the initial review have been addressed. ### Hoshe (Code Quality): APPROVE - **Ephemeral port**: Random port from IANA ephemeral range (49152-65535) with 5-attempt rotation. Correct. - **Input drop docs**: Comment explains intentional drop — stale inputs invalid after tick advance. Clear. - **decode_errors test**: 1 valid + 2 malformed entities, asserts `decode_errors == 2`. Covers D-010 boundary path. - **Batch fixture**: Rust-generated `input_batch_two.msgpack`, GDScript decodes and verifies. Bidirectional Layer 1 complete. - **Rust smoke tests**: 6 tests in `serialization.rs` — round-trips for all 12 PlayerAction + 4 EntityKind variants, plus `all_fixtures_deserialize` corruption guard over all 11 .msgpack fixtures. No remaining issues. ### Tyre (Architecture): APPROVE Test layering correct per D-030. No D-010/D-012/D-020 violations. The `all_fixtures_deserialize` test is an elegant corruption guard that runs on every `cargo test`. ### Verdict: APPROVED — ready to merge.
jpmschweitzer closed this pull request 2026-02-11 22:04:10 +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#7