Merge remote-tracking branch 'origin/ci'

2026-02-18 10:24:13 +01:00
parent ffe3fdaf3e f8db996eb9
commit bb94a6581c
25 changed files with 332 additions and 4 deletions
@@ -37,6 +37,10 @@ Format based on [Keep a Changelog](https://keepachangelog.com/).

 ### Fixed
 - MessagePack int_64 encoder dead code branch (#516) — `-(1 << 63)` overflowed making int_64 branch unreachable; negative values beyond int_32 now correctly encode as 0xd3 instead of 0xcf
+- Cross-encoder fixture pipeline hardened — encode failures now exit non-zero instead of writing empty .msgpack files (#475 review)
+- GDScript fixture test no longer silently skips on missing/empty fixture dir — asserts instead (#475 review)
+- GDScript fixture generator covers all PlayerAction variants (added MoveSouth, MoveEast, MoveWest, Unpause, ToggleStanceDown, WalkAway)
+- `make pre-pr` now checks GDScript fixture staleness alongside Rust fixtures
 - Protocol version bumped from 7 to 8 to match server — fixes 5 test failures from version mismatch
 - Interact action encoding changed from unit variant to struct variant to match server's PlayerAction::Interact { target_entity_id, verb }
 - Monologue duplication test (test_monologue_not_duplicated_after_consumption) fixed — was using poll_snapshot() which doesn't consume _last_snapshot in test mode
@@ -4,7 +4,8 @@ GODOT := $(shell command -v godot4 2>/dev/null || command -v godot 2>/dev/null)
       decisions-sync decisions-coverage decisions-active decisions-orphan \
       db-backup db-install validate-content content-ron check-fact-ids setup-hooks \
       pre-pr pre-pr-lint pre-pr-build pre-pr-test pre-pr-validate pre-pr-fixtures \
-       pre-pr-server pre-pr-client pre-pr-content
+       pre-pr-server pre-pr-client pre-pr-content \
+       fixtures-client

 # --- Configuration ---

@@ -37,6 +38,7 @@ help:
 	@echo "  make validate-content   Validate content YAML against schemas"
 	@echo "  make check-fact-ids    Check fact_id references against knowledge catalogs"
 	@echo "  make content-ron       Convert content YAML to RON (build-time)"
+	@echo "  make fixtures-client    Generate GDScript->Rust cross-encoder fixtures (#475)"
 	@echo ""
 	@echo "  make pre-pr             Run all pre-PR checks (lint, build, test, validate, fixtures)"
 	@echo "  make pre-pr-server      Server-scoped pre-PR (lint, build, test, fixtures)"
@@ -111,6 +113,14 @@ test-server:
 fixtures:
 	cd server && cargo test --test gen_fixtures -- --ignored

+fixtures-client:
+	@test -n "$(GODOT)" || { echo "Godot not found. Run 'make setup' first."; exit 1; }
+	@echo "Generating GDScript fixtures for Rust decoder..."
+	$(GODOT) --headless --path client -s res://tests/gen_client_fixtures.gd
+	@echo "Verifying Rust can decode GDScript fixtures..."
+	cd server && cargo test --test serialization gdscript_generated_fixtures
+	@echo "--- Fixtures-client: PASS ---"
+
 test-client:
 	@test -n "$(GODOT)" || { echo "Godot not found. Run 'make setup' first."; exit 1; }
 	$(GODOT) --headless --path client -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://tests/
@@ -148,13 +158,13 @@ pre-pr-validate: validate-content check-fact-ids
 	@echo "--- Content validation: PASS ---"

 pre-pr-fixtures:
-	@echo "Checking fixture staleness..."
+	@echo "Checking Rust->GDScript fixture staleness..."
 	@cd server && cargo test --test gen_fixtures -- --ignored
 	@if git diff --quiet client/tests/fixtures/; then \
-		echo "--- Fixtures: UP TO DATE ---"; \
+		echo "--- Rust fixtures: UP TO DATE ---"; \
 	else \
 		echo ""; \
-		echo "--- FIXTURES STALE ---"; \
+		echo "--- RUST FIXTURES STALE ---"; \
 		echo "  Protocol changed but fixtures not regenerated."; \
 		echo "  Stale fixtures make all client tests FALSE POSITIVES."; \
 		echo ""; \
@@ -164,6 +174,22 @@ pre-pr-fixtures:
 		echo "  Fix: commit the updated fixtures with your protocol change."; \
 		exit 1; \
 	fi
+	@echo "Checking GDScript->Rust fixture staleness..."
+	@$(MAKE) fixtures-client
+	@if git diff --quiet server/tests/fixtures/gdscript/; then \
+		echo "--- GDScript fixtures: UP TO DATE ---"; \
+	else \
+		echo ""; \
+		echo "--- GDSCRIPT FIXTURES STALE ---"; \
+		echo "  Protocol changed but GDScript fixtures not regenerated."; \
+		echo "  Stale fixtures make cross-encoder tests FALSE POSITIVES."; \
+		echo ""; \
+		echo "  Changed files:"; \
+		git diff --stat server/tests/fixtures/gdscript/; \
+		echo ""; \
+		echo "  Fix: commit the updated fixtures with your protocol change."; \
+		exit 1; \
+	fi

 # Branch-specific variants (faster, scope-appropriate)

@@ -0,0 +1,173 @@
+## Generate MessagePack fixtures from GDScript encoder for Rust decoder testing.
+## Run with: make fixtures-client
+## (or: godot --headless --path client -s res://tests/gen_client_fixtures.gd)
+##
+## Reverse direction of server/tests/gen_fixtures.rs: GDScript encodes -> Rust decodes.
+## Together with the Rust-generated fixtures, this closes the cross-encoder
+## compatibility loop (D-030 Layer 1, #475).
+##
+## Loads Messagepack encoder directly (not via class_name) because -s scripts
+## run before the project's class_name registry is fully populated.
+extends SceneTree
+
+var _Msgpack: GDScript
+var _count := 0
+var _errors := 0
+var _output_dir: String
+
+
+func _init():
+	_run.call_deferred()
+
+
+func _run():
+	_Msgpack = load("res://addons/messagepack/messagepack.gd")
+
+	# Resolve repo root from Godot project root (client/).
+	# Assumes client/ is one level below repo root — validated below.
+	var project_root := ProjectSettings.globalize_path("res://")
+	var repo_root := project_root.rstrip("/").get_base_dir()
+	_output_dir = repo_root.path_join("server/tests/fixtures/gdscript")
+
+	if not DirAccess.dir_exists_absolute(repo_root.path_join("server")):
+		push_error("Repo root detection failed: %s/server/ does not exist" % repo_root)
+		quit(1)
+		return
+
+	DirAccess.make_dir_recursive_absolute(_output_dir)
+
+	_generate_inputs()
+	_generate_boundary_inputs()
+	_generate_batch()
+
+	if _errors > 0:
+		push_error("FAILED: %d encode errors encountered" % _errors)
+		quit(1)
+		return
+	if _count == 0:
+		push_error("FAILED: no fixtures generated")
+		quit(1)
+		return
+	print("Generated %d GDScript fixtures at %s" % [_count, _output_dir])
+	quit()
+
+
+func _write_fixture(name: String, bytes: PackedByteArray) -> void:
+	if bytes.is_empty():
+		push_error("Encode produced empty bytes for fixture: %s" % name)
+		_errors += 1
+		return
+	var path := _output_dir.path_join(name + ".msgpack")
+	var file := FileAccess.open(path, FileAccess.WRITE)
+	if file == null:
+		push_error("Failed to write fixture: %s (error: %d)" % [path, FileAccess.get_open_error()])
+		_errors += 1
+		return
+	file.store_buffer(bytes)
+	file.flush()
+	file.close()
+	print("  Wrote %s (%d bytes)" % [name, bytes.size()])
+	_count += 1
+
+
+## Encode a single PlayerInput to MessagePack bytes.
+## Mirrors Protocol.encode_player_input() from protocol.gd.
+func _encode_input(tick: int, action_name: String, action_data: Variant = null) -> PackedByteArray:
+	var action: Variant
+	if action_data != null:
+		action = {action_name: action_data}
+	elif action_name == "Interact":
+		action = {"Interact": {"target_entity_id": null, "verb": null}}
+	else:
+		action = action_name
+
+	var result = _Msgpack.encode({"tick": tick, "action": action})
+	if result.status != null:
+		push_error("Encode failed: %s" % result.status)
+		return PackedByteArray()
+	return result.value
+
+
+## Encode an array of PlayerInputs to MessagePack bytes.
+## Mirrors Protocol.encode_player_inputs() from protocol.gd.
+func _encode_inputs(inputs: Array) -> PackedByteArray:
+	var wire_inputs: Array = []
+	for input in inputs:
+		var action_name: String = input["action_name"]
+		var action_data: Variant = input.get("action_data")
+		var action: Variant
+		if action_data != null:
+			action = {action_name: action_data}
+		elif action_name == "Interact":
+			action = {"Interact": {"target_entity_id": null, "verb": null}}
+		else:
+			action = action_name
+		wire_inputs.append({"tick": input["tick"], "action": action})
+
+	var result = _Msgpack.encode(wire_inputs)
+	if result.status != null:
+		push_error("Batch encode failed: %s" % result.status)
+		return PackedByteArray()
+	return result.value
+
+
+func _generate_inputs() -> void:
+	# Unit variants: all 8 movement directions (tick=100)
+	for dir_name in [
+		"MoveNorth", "MoveSouth", "MoveEast", "MoveWest",
+		"MoveNortheast", "MoveSoutheast", "MoveSouthwest", "MoveNorthwest",
+	]:
+		_write_fixture("input_%s" % dir_name.to_snake_case(),
+			_encode_input(100, dir_name))
+
+	# Data variant: UsePerceptionMode (tick=200)
+	_write_fixture("input_perception_mode",
+		_encode_input(200, "UsePerceptionMode", "thermal"))
+
+	# Struct variant: Interact with null fields (tick=100)
+	_write_fixture("input_interact",
+		_encode_input(100, "Interact"))
+
+	# Other unit variants
+	_write_fixture("input_pause",
+		_encode_input(100, "Pause"))
+	_write_fixture("input_unpause",
+		_encode_input(100, "Unpause"))
+	_write_fixture("input_toggle_stance_up",
+		_encode_input(100, "ToggleStanceUp"))
+	_write_fixture("input_toggle_stance_down",
+		_encode_input(100, "ToggleStanceDown"))
+	_write_fixture("input_walk_away",
+		_encode_input(100, "WalkAway"))
+
+
+func _generate_boundary_inputs() -> void:
+	# Tick values at MessagePack encoding format boundaries.
+	# GDScript encodes 256-32767 as int_16 (0xd1); Rust encodes as uint_16 (0xcd).
+	# GDScript encodes 65536-2147483647 as int_32 (0xd2); Rust as uint_32 (0xce).
+	# Both are valid MessagePack. Rust's rmp_serde must accept both.
+	var boundary_ticks: Array = [
+		[0, "boundary_tick_0"],
+		[127, "boundary_tick_127"],
+		[128, "boundary_tick_128"],
+		[255, "boundary_tick_255"],
+		[256, "boundary_tick_256"],                    # int_16 asymmetry start
+		[32767, "boundary_tick_32767"],                # int_16 asymmetry end
+		[32768, "boundary_tick_32768"],
+		[65535, "boundary_tick_65535"],
+		[65536, "boundary_tick_65536"],                # int_32 asymmetry start
+		[2147483647, "boundary_tick_2147483647"],      # int_32 asymmetry end
+	]
+
+	for pair in boundary_ticks:
+		_write_fixture(pair[1],
+			_encode_input(pair[0], "Pause"))
+
+
+func _generate_batch() -> void:
+	# Vec<PlayerInput> with two actions (mirrors Rust input_batch_two fixture)
+	_write_fixture("input_batch_two",
+		_encode_inputs([
+			{"tick": 0, "action_name": "MoveNorth"},
+			{"tick": 0, "action_name": "Interact"},
+		]))
@@ -69,6 +69,25 @@ make test-client        # gdUnit4 tests (headless runner pending)

 Server tests use Rust's built-in test framework with `#[cfg(test)]` inline tests and `tests/` integration tests (D-030). Client tests use gdUnit4 (D-030).

+### Cross-Encoder Fixtures
+
+```bash
+make fixtures           # Regenerate Rust->GDScript fixtures (server/tests/gen_fixtures.rs)
+make fixtures-client    # Generate GDScript->Rust fixtures + verify Rust decoder (#475)
+```
+
+The bidirectional protocol is validated by two sets of committed fixtures:
+- **Rust encodes, GDScript decodes:** `client/tests/fixtures/msgpack/` (generated by `make fixtures`)
+- **GDScript encodes, Rust decodes:** `server/tests/fixtures/gdscript/` (generated by `make fixtures-client`)
+
+Regenerate both after any protocol change. Commit the updated fixtures alongside the code change.
+
+**Troubleshooting fixture failures:**
+
+- **`make fixtures-client` fails with encode errors:** Check that `client/addons/messagepack/messagepack.gd` is up to date. The script exits non-zero on any encode failure.
+- **`gdscript_generated_fixtures_deserialize` fails:** Fixtures in `server/tests/fixtures/gdscript/` are stale or corrupted. Re-run `make fixtures-client` and commit the updated files.
+- **Fixture staleness in `make pre-pr`:** Protocol changed but fixtures were not regenerated. Run `make fixtures && make fixtures-client`, then commit both `client/tests/fixtures/` and `server/tests/fixtures/gdscript/`.
+
 ### Lint

 ```bash
@@ -0,0 +1 @@
+うtickｦaction･Pause
@@ -0,0 +1 @@
+うtickﾌｦaction･Pause
@@ -0,0 +1 @@
+‚¤tickÒÿÿÿ¦action¥Pause
@@ -0,0 +1 @@
+うtickﾌ�ｦaction･Pause
@@ -0,0 +1 @@
+うtickﾑ�ｦaction･Pause
@@ -0,0 +1 @@
+‚¤tickÍÿÿ¦action¥Pause
@@ -0,0 +1 @@
+うtickdｦaction→Interactげtarget_entity_idﾀ､verbﾀ
@@ -0,0 +1 @@
+うtickdｦactionｩMoveNorth
@@ -0,0 +1 @@
+うtickdｦactionｭMoveNortheast
@@ -0,0 +1 @@
+うtickdｦactionｭMoveNorthwest
@@ -0,0 +1 @@
+うtickdｦactionｭMoveSoutheast
@@ -0,0 +1 @@
+うtickdｦactionｭMoveSouthwest
@@ -0,0 +1 @@
+うtickdｦaction･Pause
@@ -0,0 +1 @@
+うtickﾌﾈｦaction�UsePerceptionModeｧthermal
@@ -0,0 +1 @@
+うtickdｦactionｮToggleStanceUp
@@ -1004,6 +1004,97 @@ fn malformed_input_in_batch_rejects_entire_batch() {
    );
 }

+/// GDScript-generated fixtures must deserialize correctly (D-030 Layer 1, #475).
+/// Validates the reverse direction: GDScript encoder -> Rust decoder.
+/// Together with all_fixtures_deserialize (Rust -> GDScript), this closes the
+/// cross-encoder compatibility loop.
+///
+/// Fixtures generated by: make fixtures-client
+/// (runs client/tests/gen_client_fixtures.gd via Godot headless)
+#[test]
+fn gdscript_generated_fixtures_deserialize() {
+    // CWD is server/ when cargo test runs (Cargo sets it to the package root)
+    let fixture_dir = std::path::Path::new("tests/fixtures/gdscript");
+
+    assert!(
+        fixture_dir.exists(),
+        "GDScript fixture directory not found at {}. Run `make fixtures-client` to generate.",
+        fixture_dir.display()
+    );
+
+    let mut count = 0;
+    for entry in fs::read_dir(&fixture_dir).expect("read gdscript fixture dir") {
+        let entry = entry.expect("read dir entry");
+        let path = entry.path();
+        if path.extension().and_then(|e| e.to_str()) != Some("msgpack") {
+            continue;
+        }
+        let name = path.file_stem().unwrap().to_str().unwrap().to_string();
+        let bytes = fs::read(&path).unwrap_or_else(|_| panic!("read fixture {}", name));
+
+        if name.starts_with("input_batch") {
+            let inputs: Vec<PlayerInput> = rmp_serde::from_slice(&bytes).unwrap_or_else(|e| {
+                panic!("deserialize GDScript batch fixture {}: {}", name, e)
+            });
+            assert!(
+                !inputs.is_empty(),
+                "batch fixture {} should not be empty",
+                name
+            );
+        } else if name.starts_with("input_") || name.starts_with("boundary_tick_") {
+            let input: PlayerInput = rmp_serde::from_slice(&bytes).unwrap_or_else(|e| {
+                panic!("deserialize GDScript input fixture {}: {}", name, e)
+            });
+            // Verify specific fixtures for extra confidence
+            match name.as_str() {
+                "input_move_north" => {
+                    assert_eq!(input.tick, 100);
+                    assert!(matches!(input.action, PlayerAction::MoveNorth));
+                }
+                "input_perception_mode" => {
+                    assert_eq!(input.tick, 200);
+                    assert!(
+                        matches!(input.action, PlayerAction::UsePerceptionMode(ref s) if s == "thermal")
+                    );
+                }
+                "input_interact" => {
+                    assert_eq!(input.tick, 100);
+                    assert!(matches!(input.action, PlayerAction::Interact { .. }));
+                }
+                "boundary_tick_256" => {
+                    assert_eq!(input.tick, 256, "int_16 asymmetry: tick=256");
+                }
+                "boundary_tick_32767" => {
+                    assert_eq!(input.tick, 32767, "int_16 asymmetry: tick=32767");
+                }
+                "boundary_tick_65536" => {
+                    assert_eq!(input.tick, 65536, "int_32 asymmetry: tick=65536");
+                }
+                "boundary_tick_2147483647" => {
+                    assert_eq!(
+                        input.tick, 2147483647,
+                        "int_32 asymmetry: tick=2^31-1"
+                    );
+                }
+                _ => {} // Other fixtures: deserialization success is sufficient
+            }
+        } else {
+            panic!(
+                "unknown GDScript fixture naming convention: {} — add a deserialization branch",
+                name
+            );
+        }
+        count += 1;
+    }
+
+    assert!(
+        count > 0,
+        "No .msgpack files found in {}. Run `make fixtures-client` to generate.",
+        fixture_dir.display()
+    );
+    eprintln!("Verified {} GDScript-generated fixtures", count);
+}
+
 /// NearbyInteraction.object_type round-trips through MessagePack (#422).
 /// Verifies object_type=Some(Container) survives the wire.
 #[test]
				`@@ -0,0 +1 @@`
				`うtickdｦaction→Interactげtarget_entity_idﾀ､verbﾀ`
				`@@ -0,0 +1 @@`
				`うtickﾌﾈｦaction�UsePerceptionModeｧthermal`