Avatar loader & customization system: merge onto main + fix regressions, add smoke coverage
Merges the pre-existing avatar system implementation (catalog, slot registry, loadout, glTF-based loader, customization UI, visualizer -- ~2300 lines across scripts/avatar/) onto current main, which had diverged significantly since this branch was last touched (PC1 UI fixes, PC2's playable level/player/multiplayer work). Fixes found while integrating: - project.godot had wholesale-replaced the [autoload] section again (same recurring pattern as the multiplayer server/client branches), dropping the ApiClient autoload entirely. Restored it additively alongside AvatarSystem. - All other real conflicts (main_menu.tscn/.gd, main.tscn, .godot cache) were pure "this branch re-created a stale duplicate of something main already has" -- resolved by taking main's version, confirmed via `git diff` that this task's own changes never touched those files' actual logic. New: scripts/smoke_test_avatar_system.gd -- confirms AvatarSystem is a real autoload, the committed catalog.json parses, loadout equip/ validate/JSON-round-trip works against real catalog entries, and -- since assets/avatars/*.glb model files don't exist yet -- that AvatarLoader.load_base_body() fails gracefully (null + load_error signal) rather than crashing when given a real catalog entry with no backing file. 10/10 passing. Verified no regressions: all 10 pre-existing Godot smoke suites still pass against a live backend, and all 61 backend pytest tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M6rUEynmv2QmweocAQYARN
This commit is contained in:
@@ -7,14 +7,17 @@ to know what "run the tests" actually checked.
|
|||||||
|
|
||||||
Run everything below in one shot with `scripts/run_checkpoint_tests.py`.
|
Run everything below in one shot with `scripts/run_checkpoint_tests.py`.
|
||||||
|
|
||||||
Last verified against `main`: 2026-08-21 — migrations + pytest + all 10
|
Last verified against `main`: 2026-08-21 — migrations + pytest + all 11
|
||||||
Godot smoke tests, all clean. PC1's real playtest-found bugs (login form
|
Godot smoke tests, all clean. PC1's real playtest-found bugs (login form
|
||||||
unreachable, 1px world cards, Play → Back losing the session) are covered
|
unreachable, 1px world cards, Play → Back losing the session) are covered
|
||||||
by the smoke tests below so they can't silently regress. PC2 (basic
|
by the smoke tests below so they can't silently regress. PC2 (basic
|
||||||
playable world + character + movement) landed 2026-08-20/21 — see
|
playable world + character + movement) landed 2026-08-20/21 — see
|
||||||
`docs/checkpoints/` once a PC2 record exists, and the three new smoke
|
`docs/checkpoints/` once a PC2 record exists, and the three new smoke
|
||||||
tests below (`smoke_test_test_level.gd`, `smoke_test_player.gd`,
|
tests below (`smoke_test_test_level.gd`, `smoke_test_player.gd`,
|
||||||
`smoke_test_enter_world.gd`) for what's actually verified.
|
`smoke_test_enter_world.gd`) for what's actually verified. The avatar
|
||||||
|
loader/customization system (`smoke_test_avatar_system.gd`) also landed
|
||||||
|
2026-08-21 — real glTF-based loading with graceful failure on missing
|
||||||
|
assets, verified since `assets/avatars/*.glb` model files don't exist yet.
|
||||||
|
|
||||||
## Backend — `backend/tests/` (pytest)
|
## Backend — `backend/tests/` (pytest)
|
||||||
|
|
||||||
@@ -53,8 +56,10 @@ these yet):
|
|||||||
|
|
||||||
## Godot client — `scripts/smoke_test_*.gd`
|
## Godot client — `scripts/smoke_test_*.gd`
|
||||||
|
|
||||||
**11 smoke tests merged into `main`.** Ten need a live backend, one
|
**12 smoke tests merged into `main`.** Ten need a live backend, one
|
||||||
(`smoke_test_network_client.gd`) needs a live dedicated server instead —
|
(`smoke_test_network_client.gd`) needs a live dedicated server instead,
|
||||||
|
and one (`smoke_test_avatar_system.gd`) needs neither — it only exercises
|
||||||
|
local catalog/loadout/loader logic. Backend-dependent tests read
|
||||||
`AGP_API_BASE_URL` env var, falling back to the
|
`AGP_API_BASE_URL` env var, falling back to the
|
||||||
`application/agp/api_base_url` project setting (default
|
`application/agp/api_base_url` project setting (default
|
||||||
`http://127.0.0.1:8000`) if unset.
|
`http://127.0.0.1:8000`) if unset.
|
||||||
@@ -72,6 +77,7 @@ these yet):
|
|||||||
| `smoke_test_player.gd` | Loads `scenes/game.tscn` and confirms the player exists as a real `CharacterBody3D` with a `CollisionShape3D`+shape, a `CapsuleMesh`, and spawns at the level's `PlayerSpawn` marker position. Also asserts `JUMP_VELOCITY` is positive and a `Camera3D` exists and is `current` — both were real bugs found 2026-08-20/21 (jump velocity was negative, pushing the player down instead of up; no camera existed at all, so the game was unplayable even though the character could technically move). Structural checks only — real movement/jump *direction* correctness was verified separately via a throwaway headless physics simulation during development, not by an automated test that's part of this suite. |
|
| `smoke_test_player.gd` | Loads `scenes/game.tscn` and confirms the player exists as a real `CharacterBody3D` with a `CollisionShape3D`+shape, a `CapsuleMesh`, and spawns at the level's `PlayerSpawn` marker position. Also asserts `JUMP_VELOCITY` is positive and a `Camera3D` exists and is `current` — both were real bugs found 2026-08-20/21 (jump velocity was negative, pushing the player down instead of up; no camera existed at all, so the game was unplayable even though the character could technically move). Structural checks only — real movement/jump *direction* correctness was verified separately via a throwaway headless physics simulation during development, not by an automated test that's part of this suite. |
|
||||||
| `smoke_test_enter_world.gd` | The full PC2 path end to end: register → seed a real world → load `world_list.tscn` → click the real "Enter World" button on a real card (found by walking the card's children, not by index, so it doesn't silently pass if the card layout changes) → confirms `scenes/game.tscn` actually becomes `current_scene` with the player spawned at the level's `PlayerSpawn` marker → sends a real `KEY_ESCAPE` input event (not a direct method call) and confirms it returns to `world_list.tscn`. This is the test that proves "browse → enter → walk around → leave" actually works as one continuous flow, not just that each piece works in isolation. |
|
| `smoke_test_enter_world.gd` | The full PC2 path end to end: register → seed a real world → load `world_list.tscn` → click the real "Enter World" button on a real card (found by walking the card's children, not by index, so it doesn't silently pass if the card layout changes) → confirms `scenes/game.tscn` actually becomes `current_scene` with the player spawned at the level's `PlayerSpawn` marker → sends a real `KEY_ESCAPE` input event (not a direct method call) and confirms it returns to `world_list.tscn`. This is the test that proves "browse → enter → walk around → leave" actually works as one continuous flow, not just that each piece works in isolation. |
|
||||||
| `smoke_test_network_client.gd` | **Real two-process integration test** for `scripts/network_client.gd` (client-side prediction + reconciliation, docs/architecture/networking.md §5) — needs an actual dedicated server running first: `godot --headless --path . scenes/server.tscn &`, *not* the usual HTTP backend. Connects a real `NetworkClient` over ENet, confirms local input prediction moves the player immediately (no waiting on a round trip), confirms real snapshots arrive and get processed, then deliberately desyncs the predicted position by 9999 units and confirms the very next snapshot corrects it back — the actual proof reconciliation logic works, not just that it never had to fire. Known simplification, stated in the script's own docstring: the server's snapshot doesn't currently echo back a per-peer "last input seq processed," so reconciliation corrects against the *most recent* prediction rather than true tick-exact rollback-and-replay (docs/architecture/networking.md §5.2 step 4) — a real, scoped follow-up once the server echoes that ack, not something faked here. |
|
| `smoke_test_network_client.gd` | **Real two-process integration test** for `scripts/network_client.gd` (client-side prediction + reconciliation, docs/architecture/networking.md §5) — needs an actual dedicated server running first: `godot --headless --path . scenes/server.tscn &`, *not* the usual HTTP backend. Connects a real `NetworkClient` over ENet, confirms local input prediction moves the player immediately (no waiting on a round trip), confirms real snapshots arrive and get processed, then deliberately desyncs the predicted position by 9999 units and confirms the very next snapshot corrects it back — the actual proof reconciliation logic works, not just that it never had to fire. Known simplification, stated in the script's own docstring: the server's snapshot doesn't currently echo back a per-peer "last input seq processed," so reconciliation corrects against the *most recent* prediction rather than true tick-exact rollback-and-replay (docs/architecture/networking.md §5.2 step 4) — a real, scoped follow-up once the server echoes that ack, not something faked here. |
|
||||||
|
| `smoke_test_avatar_system.gd` | Confirms `AvatarSystem` is a real autoload, the committed `assets/avatars/catalog.json` actually parses, `get_assets_for_slot("base_body")` returns real entries, an `AvatarLoadout` can set `base_body` and equip a real catalog accessory, the loadout round-trips through `to_json()`/`load_loadout_from_json()`, and — since `assets/avatars/*.glb` model files don't exist yet — that `AvatarLoader.load_base_body()` against a real catalog entry with a missing file fails *gracefully*: returns `null` and emits `load_error` rather than crashing. Visual rendering itself can't be verified until real assets exist. |
|
||||||
|
|
||||||
Run (once, to build the class cache): `godot --headless --path . --import`,
|
Run (once, to build the class cache): `godot --headless --path . --import`,
|
||||||
then for each smoke test: `godot --headless --path . --script
|
then for each smoke test: `godot --headless --path . --script
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ agp/api_base_url="http://127.0.0.1:8000"
|
|||||||
[autoload]
|
[autoload]
|
||||||
|
|
||||||
ApiClient="*res://scripts/api_client.gd"
|
ApiClient="*res://scripts/api_client.gd"
|
||||||
|
AvatarSystem="*res://scripts/avatar/avatar_system.gd"
|
||||||
|
|
||||||
[display]
|
[display]
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,98 @@
|
|||||||
|
extends SceneTree
|
||||||
|
## Smoke test for the avatar catalog/slot/loadout/system layers.
|
||||||
|
##
|
||||||
|
## Real .glb model assets don't exist yet (assets/avatars/catalog.json
|
||||||
|
## references them, the files themselves are a tracked user to-do -- see
|
||||||
|
## the "AGP MVP status" project memory). This test covers everything that
|
||||||
|
## doesn't require them (catalog parsing, slot registry, loadout equip/
|
||||||
|
## validate, the AvatarSystem orchestrator's autoload wiring) and confirms
|
||||||
|
## the one thing that DOES touch missing files -- AvatarLoader.load_base_body()
|
||||||
|
## -- fails gracefully (emits load_error, returns null) instead of crashing.
|
||||||
|
## Visual rendering itself can't be verified until real assets exist.
|
||||||
|
##
|
||||||
|
## Run with:
|
||||||
|
## godot --headless --path . --script res://scripts/smoke_test_avatar_system.gd
|
||||||
|
|
||||||
|
var passed := 0
|
||||||
|
var failed := 0
|
||||||
|
|
||||||
|
|
||||||
|
func _initialize() -> void:
|
||||||
|
_run()
|
||||||
|
|
||||||
|
|
||||||
|
func _run() -> void:
|
||||||
|
await process_frame
|
||||||
|
|
||||||
|
# 1. AvatarSystem is a real autoload, not just a loose script.
|
||||||
|
var system := root.get_node_or_null("AvatarSystem")
|
||||||
|
if not _check("AvatarSystem autoload exists", system != null):
|
||||||
|
_finish()
|
||||||
|
return
|
||||||
|
|
||||||
|
# 2. The real catalog.json (committed to assets/avatars/) actually parses.
|
||||||
|
var err: Error = system.load_catalog()
|
||||||
|
_check("catalog loads without error", err == OK)
|
||||||
|
_check("AvatarSystem reports catalog loaded", system.is_catalog_loaded)
|
||||||
|
|
||||||
|
var base_bodies: Array[String] = system.get_assets_for_slot("base_body")
|
||||||
|
_check("catalog has at least one base_body entry", base_bodies.size() > 0)
|
||||||
|
|
||||||
|
# 3. Loadout: create, equip, validate against the real catalog.
|
||||||
|
# base_body is its own dedicated field on AvatarLoadout, not something
|
||||||
|
# set via the generic per-slot set_equipment() -- AvatarSlotRegistry's
|
||||||
|
# SLOT_IDS deliberately excludes "base_body" (it's the foundational
|
||||||
|
# model you start from, not a layered attachment slot).
|
||||||
|
var loadout := AvatarLoadout.new()
|
||||||
|
if base_bodies.size() > 0:
|
||||||
|
loadout.base_body = base_bodies[0]
|
||||||
|
_check("loadout has base_body set",
|
||||||
|
base_bodies.size() == 0 or not loadout.base_body.is_empty())
|
||||||
|
|
||||||
|
var hair_options: Array[String] = system.get_assets_for_slot("hair")
|
||||||
|
if hair_options.size() > 0:
|
||||||
|
var equip_ok: bool = loadout.add_accessory(hair_options[0])
|
||||||
|
_check("adding a real catalog accessory succeeds", equip_ok)
|
||||||
|
|
||||||
|
# 4. Loadout serializes and survives a round trip.
|
||||||
|
var json_str: String = loadout.to_json()
|
||||||
|
_check("loadout serializes to non-empty JSON", json_str.length() > 0)
|
||||||
|
var restored: AvatarLoadout = system.load_loadout_from_json(json_str)
|
||||||
|
_check("loadout round-trips through JSON", restored != null and restored.is_valid())
|
||||||
|
|
||||||
|
# 5. The part that DOES need real files: confirm it fails gracefully
|
||||||
|
# rather than crashing, since assets/avatars/*.glb genuinely don't
|
||||||
|
# exist yet. A crash here would be a real bug; a clean load_error
|
||||||
|
# signal + null return is the correct, already-designed-for behavior.
|
||||||
|
if base_bodies.size() > 0:
|
||||||
|
# A plain bool captured by a lambda is captured BY VALUE in
|
||||||
|
# GDScript -- mutating it inside the closure doesn't propagate to
|
||||||
|
# this outer scope. A single-element Array is the standard
|
||||||
|
# workaround (confirmed the hard way earlier in this same session,
|
||||||
|
# see scripts/smoke_test_network_client.gd's own comment on this).
|
||||||
|
var error_state := [false]
|
||||||
|
var error_handler := func(_msg): error_state[0] = true
|
||||||
|
system.loader.load_error.connect(error_handler)
|
||||||
|
var result: Node3D = system.loader.load_base_body(base_bodies[0])
|
||||||
|
system.loader.load_error.disconnect(error_handler)
|
||||||
|
_check("load_base_body against a missing .glb returns null (not a crash)", result == null)
|
||||||
|
_check("load_base_body against a missing .glb emits load_error (not silent)", error_state[0])
|
||||||
|
else:
|
||||||
|
print(" SKIP: no base_body entries in catalog to test the failure path against")
|
||||||
|
|
||||||
|
_finish()
|
||||||
|
|
||||||
|
|
||||||
|
func _check(description: String, condition: bool) -> bool:
|
||||||
|
if condition:
|
||||||
|
print(" PASS: ", description)
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
printerr(" FAIL: ", description)
|
||||||
|
failed += 1
|
||||||
|
return condition
|
||||||
|
|
||||||
|
|
||||||
|
func _finish() -> void:
|
||||||
|
print("\n=== SMOKE RESULT: %d/%d passed ===" % [passed, passed + failed])
|
||||||
|
quit(0 if failed == 0 else 1)
|
||||||
Reference in New Issue
Block a user