fix(game_eval): cap eval readiness wait below the server command timeout (#500)#503
Merged
Conversation
…500) The game_eval not-ready path reused the 20s GAME_READY_WAIT_SEC tuned for screenshots. That wait exceeds the 15s server-side game_eval command timeout, so when a game isn't running/ready the editor polled for up to 20s while the server gave up first with an opaque ~15s TimeoutError — the editor's actionable "Is the game actually running?" error never reached the client. This is #500's residual ~15s TimeoutError bucket. Give eval its own EVAL_READY_WAIT_SEC (3s). Editor-side worst case is now 3s wait + 10s backstop = 13s, comfortably under the 15s server timeout, so the fast actionable error always wins the race. The 20s wait stays for screenshots (a freshly launched game needs longer to render a first frame). Refs #500. https://claude.ai/code/session_01Jq5X4ivngAf1N6r5UX2BVw
The actionable hint pointed at a nonexistent 'play_scene' op; the MCP tool that runs the game is project_run. https://claude.ai/code/session_01Jq5X4ivngAf1N6r5UX2BVw
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the editor-side game_eval “wait for game readiness” window so it can’t outlast the 15s server/dispatcher game_eval timeout, preventing opaque ~15s TimeoutError failures and allowing the editor to return a faster, actionable “game not running/ready” error instead.
Changes:
- Introduces a dedicated
EVAL_READY_WAIT_SEC = 3.0(separate from the 20s screenshot readiness wait). - Updates
game_eval’s readiness polling deadline to useEVAL_READY_WAIT_SEC. - Expands the inline timeout-ordering documentation around
game_evalto capture the new invariant (ready_wait + backstop < 15s).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+37
to
+38
| ## MUST stay below the server-side game_eval command timeout (15s, see | ||
| ## handlers/editor.py::game_eval / dispatcher.gd's 15000ms game_eval budget). |
Comment on lines
+434
to
+435
| _send_error(connection, request_id, ErrorCodes.INTERNAL_ERROR, | ||
| "Game-side autoload never registered its debugger capture within %ds. Is the game actually running?" % int(GAME_READY_WAIT_SEC)) | ||
| "Game-side autoload never registered its debugger capture within %ds. Is the game actually running? Start it with project_run / the editor's Play button, then retry." % int(EVAL_READY_WAIT_SEC)) |
…ady error (#500) Addresses Copilot review on #503: - The EVAL_READY_WAIT_SEC doc referenced 'handlers/editor.py' and lumped dispatcher.gd's 15000ms budget as 'server-side'. Correct to the full path (src/godot_ai/handlers/editor.py) and note dispatcher.gd's budget is editor/plugin-side; the 15s ceiling is enforced at both layers. - The not-ready error was optimized for 'game not running' but the same symptom occurs when the _mcp_game_helper autoload is missing/disabled. Point users to Project Settings → Autoload too (mirrors the screenshot path). https://claude.ai/code/session_01Jq5X4ivngAf1N6r5UX2BVw
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses the residual
TimeoutError~15s bucket in #500 (the larger of the two long-timeout slices the telemetry flagged post-2.5.10).Root cause
The
game_evaleditor-side path (mcp_debugger_plugin.gd::request_game_eval→_wait_then_eval) reusedGAME_READY_WAIT_SEC = 20.0— a wait tuned for the screenshot path, where a freshly launched game subprocess can take ~15s to boot and render a first frame.But the eval path's total editor-side budget is
ready_wait + eval_backstop (10s), and the server-sidegame_evalcommand timeout is 15s (handlers/editor.py::game_eval, mirrored by the dispatcher's 15000 msgame_evalbudget). With a 20s readiness wait, a game that isn't running/ready makes the editor poll past the 15s server deadline, so the server gives up first with an opaqueTimeoutError— the editor's actionable "Is the game actually running?" error never reaches the client.This matches #500's hypothesis exactly: a hang path where "the eval request never reaches
game_helper.gd, or no game is running," so only the outer server-side timeout catches it.Fix
Give eval its own
EVAL_READY_WAIT_SEC = 3.0instead of inheriting the 20s screenshot wait:TimeoutError.mcp:hello; if it needs longer, the user gets a fast "is it running? start it and retry" message rather than a 15s hang.GAME_READY_WAIT_SECis unchanged for the screenshot path (which genuinely needs the longer boot window and whose 8s send timeout sits differently).The TIMEOUT ORDERING comments are updated to document the new invariant:
EVAL_READY_WAIT_SEC + eval backstop < server command timeout.Scope / notes
INTERNAL_ERROR~10s bucket in game_eval: residual ~10–15s timeout failures not caught by the 8s eval bound (#488 follow-up) #500 is the editor-side eval backstop firing on a genuinely stuck/frozen eval (e.g. a backgrounded play-in-editor game with a frozen idle loop, or an infinite loop). That path is already actionable (it returns the "compiled and started running but never returned… likely an infinite loop or never-firing await" message), so this PR leaves it as-is — the opaqueTimeoutErrorbucket was the higher-value target.game_command(_wait_then_game_command) shares the same 20s-wait-vs-15s-command-timeout relationship and could get the same treatment, but it isn't in game_eval: residual ~10–15s timeout failures not caught by the 8s eval bound (#488 follow-up) #500's telemetry, so I've kept this change scoped togame_eval. Happy to extend if desired.Testing
script/ci-check-gdscript→ All GDScript files OK (parse-clean on the import pass).https://claude.ai/code/session_01Jq5X4ivngAf1N6r5UX2BVw
Generated by Claude Code