diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf6f9b56..18bc6856 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -689,6 +689,27 @@ jobs: $env:PYTHONPATH = Join-Path $PWD 'tests' python -m unittest tests.test_managed_recovery_live tests.test_managed_recovery_adversarial -v + # An editor opens its startup scenes once its first scan is applied, and + # a scene opened before then was replaced (#1069). The integration + # harness cannot see it: its fixture plugin opens a scene of its own once + # startup is over, so this launches a fresh editor and opens one at once. + - name: Verify a scene opened at editor startup stays current + shell: pwsh + env: + GODOT_VERSION: ${{ matrix.godot-version }} + run: | + $godot = Get-ChildItem -Path godot-bin -Filter "Godot_v${env:GODOT_VERSION}-stable_win64_console.exe" | + Select-Object -First 1 + if (-not $godot) { + $godot = Get-ChildItem -Path godot-bin -Filter "Godot_v${env:GODOT_VERSION}-stable_win64*.exe" | + Where-Object { $_.Name -notlike "*console*" } | Select-Object -First 1 + } + if (-not $godot) { throw "Godot executable not found under godot-bin" } + $env:DIDI_TEST_BINARY = (Resolve-Path build/didi.exe).Path + $env:DIDI_STARTUP_GODOT = $godot.FullName + $env:PYTHONPATH = Join-Path $PWD 'tests' + python -m unittest tests.test_editor_startup_live -v + # tests/contract_snapshots/live-.json: the listings once an editor # on this line is attached, and the answers to the read-only calls in # calls.json against tests/contract_fixture (Q3). It runs whenever the diff --git a/CHANGELOG.md b/CHANGELOG.md index 178f99c9..a0e1d3d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -367,6 +367,38 @@ The three Phase 7 blockers are unchanged; the newest name is `asset_configure_im ### Fixed +- **A writer that rebuilds an open tab says whether the tab had unsaved + changes (#1082).** `scene_create`, `scene_pack_branch` and + `viewport_create_test_lab` with `overwrite: true` rebuild a tab whatever it + holds, and the answer said only `editor_scene_reloaded: true`. It now carries + `editor_scene_discarded_unsaved`: `true` or `false` on Godot 4.7, `null` + before it. + +- **`runtime_launch` with a Godot that cannot be started says so (#1076).** It + answered `success: false` with `exit_code: 0`, which is what a game that ran + and failed looks like, and said to put godot on PATH whatever `GODOT_BIN` + named. It now answers `503 engine_unavailable`, like the other tools that + start their own Godot. + +- **`csharp_check_build` no longer calls a slow .NET SDK missing (#1078).** + Its `dotnet --version` probe stopped at a fixed 30 seconds, whatever + `timeout_seconds` said, and then told the caller to install the SDK. The + probe now runs under the call's `timeout_seconds`, and running out of it is + `504 timeout`, retryable. + +- **`scene_create` over a scene open in another tab opens it on Godot 4.5 and + 4.6 (#1079, #1073).** It rebuilt the tab and then opened it in one request, + and the editor ignores a scene change for the rest of that frame, so it + answered `opened: false`. The tab is now brought to the front first and + rebuilt on a later request, through the reload the other writers use, and + the answer carries `editor_scene_reloaded`. `previous_scene_file_path` now + names the scene that was current before the call. + +- **A scene opened straight after an editor starts stays the edited scene + (#1069).** `scene_open` and `scene_create` now wait until the editor has + opened its startup scenes, which it does once its first scan is applied. + Before, the startup made the main scene current a moment after the answer. + - **A pack over a scene open in another tab no longer comes undone on the next save (#1072).** `scene_pack_branch` with `overwrite: true` left that tab holding the old tree, and `editor_save_scene` wrote it back over the pack. diff --git a/README.md b/README.md index fbc78fa9..38782bfd 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ [![CI](https://github.com/saworbit/didi/actions/workflows/ci.yml/badge.svg)](https://github.com/saworbit/didi/actions/workflows/ci.yml) [![CodeQL](https://github.com/saworbit/didi/actions/workflows/codeql.yml/badge.svg)](https://github.com/saworbit/didi/actions/workflows/codeql.yml) [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/saworbit/didi/badge)](https://scorecard.dev/viewer/?uri=github.com/saworbit/didi) -[![Tests](https://img.shields.io/badge/tests-1493-2ea043?logo=pytest&logoColor=white)](docs/TEST_INVENTORY.md) +[![Tests](https://img.shields.io/badge/tests-1499-2ea043?logo=pytest&logoColor=white)](docs/TEST_INVENTORY.md) [![Release](https://img.shields.io/github/v/release/saworbit/didi?logo=github&color=blue)](https://github.com/saworbit/didi/releases/latest) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) [![Godot Engine](https://img.shields.io/badge/Godot-4.5%2B-478cbf?logo=godotengine&logoColor=white)](https://godotengine.org/) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5a2c59bf..4ed87b9f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -126,7 +126,7 @@ Godot's `SceneTree`, `EditorInterface`, and `RenderingServer` are **not thread-s ``` ### Key Safety Guarantees: -1. **Main-Thread Godot Calls**: Supported live scene and viewport operations run only after the native main-loop callback drains the synchronized queue. The engine can run that callback from inside one of its own calls: `EditorFileSystem.reimport_files` and `RenderingServer.force_draw` do, and so does every editor import pass, because a windowed editor's progress dialog pumps the main loop. Such a frame only advances the multi-frame work already in flight (runtime steps, reimports, profiler windows, invariant watches, scene exploration, captures, script calls) and takes nothing off the queue, so no command runs against a tree that is mid-reimport or mid-capture. The pass is open from `resources_reimporting` to `resources_reimported`, which the addon's `didi_import_watch.gd` counts, and on Godot 4.7 also while `EditorFileSystem.is_importing()` is true (#914). Applying a scan is not an import pass. A scan clears the editor's scanning flag on its own thread, and a later frame swaps in the new file index and registers new scripts' classes and documentation under progress tasks that pump the main loop, before it emits `sources_changed`. `asset_reimport` neither reimports nor answers after a scan it started until that signal, which the same addon script counts, has fired for a new index (#994). The queue itself is still open in those frames, so a command from another caller, or one sent during a scan the editor started, can run inside them (#995). +1. **Main-Thread Godot Calls**: Supported live scene and viewport operations run only after the native main-loop callback drains the synchronized queue. The engine can run that callback from inside one of its own calls: `EditorFileSystem.reimport_files` and `RenderingServer.force_draw` do, and so does every editor import pass, because a windowed editor's progress dialog pumps the main loop. Such a frame only advances the multi-frame work already in flight (runtime steps, reimports, profiler windows, invariant watches, scene exploration, captures, script calls) and takes nothing off the queue, so no command runs against a tree that is mid-reimport or mid-capture. The pass is open from `resources_reimporting` to `resources_reimported`, which the addon's `didi_import_watch.gd` counts, and on Godot 4.7 also while `EditorFileSystem.is_importing()` is true (#914). Applying a scan is not an import pass. A scan clears the editor's scanning flag on its own thread, and a later frame swaps in the new file index and registers new scripts' classes and documentation under progress tasks that pump the main loop, before it emits `sources_changed`. `asset_reimport` neither reimports nor answers after a scan it started until that signal, which the same addon script counts, has fired for a new index (#994). Those tasks pump the main loop through the editor's progress dialog, and a frame with a progress task open is held the same way, whoever started the work (#995). One more hold is not about a nested frame. The editor opens the scenes it restores, or the project's main scene, once its first scan is applied, and makes one of them current; `scene.open` and `scene.create` wait at the front of the queue until then, with everything behind them, because a scene they opened earlier was replaced a moment later (#1069). The editor's file index is empty until that scan is applied, which is how the hook tells. 2. **Editor Undo/Redo Integration**: All modifications register transactions with Godot's `EditorUndoRedoManager`, allowing human developers to press `Ctrl+Z` in the editor to undo any AI-generated modification. 3. **Timeout & Deadlock Protection**: - IPC client operations use recursive mutexes and platform-specific readiness checks with millisecond deadlines: `PeekNamedPipe` on Windows and `poll` on POSIX. diff --git a/docs/LLM_INSTRUCTIONS.md b/docs/LLM_INSTRUCTIONS.md index 78dc3302..556efb1a 100644 --- a/docs/LLM_INSTRUCTIONS.md +++ b/docs/LLM_INSTRUCTIONS.md @@ -81,7 +81,7 @@ When live, use focused tools: - Send `{"x": .., "y": ..}` or `{"x": .., "y": .., "z": ..}` for a Vector2/Vector3, whole numbers for the integer versions, `{"r": .., "g": .., "b": ..}` with an optional `a` or a `"#rrggbb"` string for a Color, and a `res://` path for a Resource slot. `null` clears a resource slot. An extra or missing member is refused rather than dropped. The same object shape works for a coordinate in `tilemap_set_cells` and `gridmap_set_cells`, which also still take `[x, y]` and `[x, y, z]`; each of those two takes the other's field name, `coords` or `position`, for the same thing. - `editor_undo` and `editor_redo` to verify reversibility. - `editor_save_scene` only when persistence is intended. -- `editor_reload_project` to request a resource-filesystem source rescan. It does not reload a resource the editor already holds, so it is not the fix for an editor answering from an old copy of a file. The tools that write files reload the attached editor's copy themselves: read `editor_copy_reloaded`, or `editor_copies_reloaded` from `project_apply_changes` and `project_rename_references`. `editor_copy_error` or `editor_copy_errors` names a copy that could not be reloaded, such as a script that no longer compiles. A scene those two rewrite that is open in a tab is rebuilt from the file and named in `editor_scenes_reloaded`; one with unsaved changes, or any open one before Godot 4.7, refuses the call until you save it and pass `discard_unsaved: true`. `scene_pack_branch` and `viewport_create_test_lab` with `overwrite: true` rebuild a tab that holds the scene they replace whatever it holds, unsaved changes included, and answer `editor_scene_reloaded: true`. +- `editor_reload_project` to request a resource-filesystem source rescan. It does not reload a resource the editor already holds, so it is not the fix for an editor answering from an old copy of a file. The tools that write files reload the attached editor's copy themselves: read `editor_copy_reloaded`, or `editor_copies_reloaded` from `project_apply_changes` and `project_rename_references`. `editor_copy_error` or `editor_copy_errors` names a copy that could not be reloaded, such as a script that no longer compiles. A scene those two rewrite that is open in a tab is rebuilt from the file and named in `editor_scenes_reloaded`; one with unsaved changes, or any open one before Godot 4.7, refuses the call until you save it and pass `discard_unsaved: true`. `scene_create`, `scene_pack_branch` and `viewport_create_test_lab` with `overwrite: true` rebuild a tab that holds the scene they replace whatever it holds, unsaved changes included, and answer `editor_scene_reloaded: true`, with `editor_scene_discarded_unsaved` saying whether changes were lost (`null` before Godot 4.7, where the engine cannot say). - `runtime_launch` with `detach: true` to start the game and leave it running. It answers with `game_session`; pass its `session_id` to `runtime_attach_session` and the runtime tools answer on the running game. End it with `runtime_stop`. Without `detach` the call blocks and kills the game at the timeout, which is what you want for a test and not for playing one. Before executing any implemented mutation, call the exact tool and arguments with `dry_run: true`, inspect `mutation_preview`, and verify the intended project and route. Dry-runs do not enter handlers. If the preview includes `confirmation_token`, repeat the exact original arguments without `dry_run` and add that token only after the destructive intent is authorized. Never combine `dry_run: true` and `confirmation_token`, alter arguments between preview and execution, persist a token, or retry it: tokens are 64 lowercase hex characters, expire after 120 seconds, and are consumed by the mutation they authorise. An attempt refused for not matching its own binding leaves the token usable, so retry it with the arguments you previewed rather than taking a fresh preview. diff --git a/docs/TEST_INVENTORY.md b/docs/TEST_INVENTORY.md index e0d781e6..46057936 100644 --- a/docs/TEST_INVENTORY.md +++ b/docs/TEST_INVENTORY.md @@ -10,14 +10,14 @@ Every number here is derived from the suites themselves rather than written down | Measure | Count | | --- | ---: | -| Automated tests | **1493** | -| Live-harness assertions | 1406 | +| Automated tests | **1499** | +| Live-harness assertions | 1414 | The badge in the [README](../README.md) shows the automated test total. Harness assertions are counted separately because they are assertions inside one live scenario, not independently runnable cases; adding them together would flatter the number. ## Native C++ suite (`didi_tests`) -**953 tests.** Derived from `didi_tests --list`, which prints the registry the runner iterates. +**958 tests.** Derived from `didi_tests --list`, which prints the registry the runner iterates. | Suite | Tests | | --- | ---: | @@ -32,7 +32,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `Checkpoints` | 14 | | `ControlRoom` | 24 | | `CrashCapture` | 3 | -| `EditorHook` | 9 | +| `EditorHook` | 10 | | `ElasticIngress` | 4 | | `EngineDiagnostics` | 7 | | `ErrorData` | 3 | @@ -48,7 +48,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `JsonRpc` | 5 | | `ManagedProcess` | 6 | | `McpServer` | 42 | -| `Phase5` | 30 | +| `Phase5` | 31 | | `Phase6` | 25 | | `Phase7Contract` | 9 | | `Phase7Diagnostics` | 1 | @@ -65,7 +65,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `ResourceIndexer` | 12 | | `Resources` | 2 | | `ResponseEconomy` | 18 | -| `RuntimeLaunch` | 11 | +| `RuntimeLaunch` | 12 | | `RuntimeLogs` | 12 | | `RuntimeOutput` | 6 | | `RuntimeRouting` | 65 | @@ -79,7 +79,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `TestRunner` | 2 | | `ToolManifest` | 1 | | `ToolProfile` | 6 | -| `Tools` | 169 | +| `Tools` | 171 | | `UiListControls` | 6 | | `ViewportIsolation` | 2 | | `autoload_diagnostics` | 9 | @@ -101,7 +101,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness ## Python contract suites (`tests/test_*.py`) -**540 tests.** Derived from `test_*` methods on every `unittest.TestCase` subclass, read with `ast`. +**541 tests.** Derived from `test_*` methods on every `unittest.TestCase` subclass, read with `ast`. | File | Tests | | --- | ---: | @@ -116,6 +116,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `test_didi_binary.py` | 10 | | `test_documentation_validator.py` | 104 | | `test_editor_console.py` | 9 | +| `test_editor_startup_live.py` | 1 | | `test_elastic_ingress.py` | 13 | | `test_elicitation_confirmation.py` | 6 | | `test_engine_identity.py` | 5 | @@ -142,7 +143,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness ## Live Godot harness (`tests/*.ps1`) -**1406 assertions.** Derived from `Assert-True` call sites. The harness is one long live scenario rather than a set of named cases, so this counts assertions and says so. +**1414 assertions.** Derived from `Assert-True` call sites. The harness is one long live scenario rather than a set of named cases, so this counts assertions and says so. | File | Assertions | | --- | ---: | @@ -151,7 +152,7 @@ The badge in the [README](../README.md) shows the automated test total. Harness | `observed_post_state.ps1` | 12 | | `refusal_remedies.ps1` | 3 | | `run_godot_integration.ps1` | 1367 | -| `scene_tab_reload.ps1` | 18 | +| `scene_tab_reload.ps1` | 26 | ## What is not counted here diff --git a/docs/TOOL_REFERENCE.md b/docs/TOOL_REFERENCE.md index c320e6e7..4e8199dd 100644 --- a/docs/TOOL_REFERENCE.md +++ b/docs/TOOL_REFERENCE.md @@ -679,7 +679,7 @@ The lab lives at the project root, not under `addons/didi`, so `project_audit_as The confirmation preview is about `res://didi_test_lab.tscn`, the one file this call replaces, and not about `target_resource_path`, which it reads and leaves alone. It reports `preview_kind: "target_state"` with the file's size and content digest, and `target_checked_on_confirm: true`, so a token approved against one lab scene is refused if that file changes inside the window. -With an editor attached, its copy of the lab scene is reloaded after the write, and `editor_copy_reloaded` says whether it held one. A tab that has the lab open is rebuilt from the new file whatever it holds, since `overwrite: true` already accepted replacing it, and `editor_scene_reloaded: true` says so. +With an editor attached, its copy of the lab scene is reloaded after the write, and `editor_copy_reloaded` says whether it held one. A tab that has the lab open is rebuilt from the new file whatever it holds, since `overwrite: true` already accepted replacing it, and `editor_scene_reloaded: true` says so. `editor_scene_discarded_unsaved` says whether that tab had unsaved changes the rebuild threw away: `true` or `false` on Godot 4.7, and `null` before it, where the engine cannot say (#1082). ### `viewport_set_camera_transform` — Live (editor only) @@ -1306,6 +1306,8 @@ Launches a separate Godot process, optionally headless, captures stdout/stderr, - `detach` (`boolean`, default `false`): start the game and leave it running. - Legacy alias: `execute_test_session`. +A Godot that cannot be started, because none is installed or the file at `GODOT_BIN` is not an executable, is refused `503` with `data.code: "engine_unavailable"` and `engine_executable`, the way the Phase 5 tools refuse it. It used to answer `success: false` with `exit_code: 0`, the shape of a game that ran and failed (#1076). A detached launch on POSIX cannot see this, because the game execs after the call has its pid; it answers that no session was published. + #### Detached: a game you can still drive Blocking is the default and is right for a test: run the project, see what it printed, get the exit code. It is the wrong shape for playing one. A game that runs is reported `success: false`, `exit_code: 124`, "timed out", and is gone by the time the answer arrives, which left the interactive half of the runtime surface -- `runtime_inject_input`, `runtime_step`, `runtime_set_paused`, `runtime_read_output`, `runtime_get_tree`, `runtime_explore_scene`, `runtime_watch_invariants`, `runtime_checkpoint` -- reachable only for a game somebody else had started. @@ -1519,10 +1521,12 @@ Group mutations use UndoRedo. ### Scene files -- `scene_create`: requires normalized `scene_path` ending in `.tscn`; accepts `root_type`, `root_name`, and `overwrite`. `root_type` is any Godot class that inherits `Node`, the same set `scene_instantiate_node` takes, and defaults to `Node2D`: a player scene is a `CharacterBody2D`, a pickup an `Area2D`, terrain a `StaticBody2D`, a HUD a `CanvasLayer`. A class the engine does not know and a class that is not a `Node` are refused separately, each naming the class, and neither writes a file. It creates the project-contained parent directory when that directory does not exist, the way `script_create` and `resource_create` do, then saves and verifies the active scene. +- `scene_create`: requires normalized `scene_path` ending in `.tscn`; accepts `root_type`, `root_name`, and `overwrite`. `root_type` is any Godot class that inherits `Node`, the same set `scene_instantiate_node` takes, and defaults to `Node2D`: a player scene is a `CharacterBody2D`, a pickup an `Area2D`, terrain a `StaticBody2D`, a HUD a `CanvasLayer`. A class the engine does not know and a class that is not a `Node` are refused separately, each naming the class, and neither writes a file. It creates the project-contained parent directory when that directory does not exist, the way `script_create` and `resource_create` do, then saves and verifies the active scene. A tab that already holds the scene is brought to the front and then rebuilt from the new file, whatever it holds, since `overwrite: true` accepted replacing it, and `editor_scene_reloaded: true` says so. Rebuilding it first left the editor changing scenes for the rest of the frame on Godot 4.5 and 4.6, so a tab right of the current one never came to the front and the call answered `opened: false` (#1079). `editor_scene_discarded_unsaved` says whether that tab had unsaved changes the rebuild threw away: `true` or `false` on Godot 4.7, and `null` before it, where the engine cannot say (#1082). - `scene_open`: validates and opens an existing `PackedScene`, then verifies its active resource path. - `scene_close`: closes the active scene. It probes for the `EditorInterface.get_unsaved_scenes` bind, which exists from Godot 4.7. Where it exists and the engine omits the active scene from the unsaved list, a call with no arguments closes and returns `dirty_state: "clean"`. Where the bind is missing (Godot 4.5 and 4.6), where the active scene has never been saved and so has no path for the engine to name, or where the engine reports the scene as unsaved, the call is refused with `409` unless `discard_unsaved: true` is passed. Results carry `dirty_state_readable` (whether this engine can answer), `dirty_state` (`clean` or `unchecked`), and `discarded_unsaved` (the flag as passed). -- `scene_pack_branch`: requires `target_node` and `scene_path`; duplicates the branch, normalizes descendant ownership, packs it, and protects existing targets unless `overwrite: true`. The editor's copy of the written scene is then reloaded, and `editor_copy_reloaded` says whether it held one. A tab that has the scene open is rebuilt from the new file whatever it holds, since `overwrite: true` already accepted replacing it, and `editor_scene_reloaded: true` says so; left alone, the next `editor_save_scene` wrote the old scene back over the pack (#1072). +- `scene_pack_branch`: requires `target_node` and `scene_path`; duplicates the branch, normalizes descendant ownership, packs it, and protects existing targets unless `overwrite: true`. The editor's copy of the written scene is then reloaded, and `editor_copy_reloaded` says whether it held one. A tab that has the scene open is rebuilt from the new file whatever it holds, since `overwrite: true` already accepted replacing it, and `editor_scene_reloaded: true` says so; left alone, the next `editor_save_scene` wrote the old scene back over the pack (#1072). `editor_scene_discarded_unsaved` says whether that tab had unsaved changes the rebuild threw away: `true` or `false` on Godot 4.7, and `null` before it, where the engine cannot say (#1082). + +Sent while an editor is still starting, `scene_open` and `scene_create` wait until it has opened the scenes it restores, or the project's main scene, which it does once its first scan of the project is applied. Before that wait, the startup made its own scene current a moment after the answer, and every later scene call acted on it (#1069). A wait past the route deadline answers `504 route_deadline_exceeded` with `outcome: not_started`, and nothing was opened or written. Both writers report the uid the scene file carries and whether the engine has been taught it: `uid`, and `uid_registered`. `ResourceSaver.save` writes the uid into the file, but only Godot's own save callback puts it in `ResourceUID`, and that callback does nothing while `EditorFileSystem` is scanning. A scene written inside that window used to end up with a uid in the file that the engine had never heard of, so every load of a scene referencing it printed `ext_resource, invalid UID ... using text path instead`, in the editor, in `runtime_launch` and in an exported game. @@ -1708,7 +1712,7 @@ A Godot that cannot be started, because nothing is at the path or the file there ### `csharp_check_build` — Offline -Runs `dotnet build` with `configuration` (`Debug` or `Release`, default `Debug`) and `timeout_seconds` (`1..300`, default `60`). Optional `project_file` must be a normalized project-contained `.sln` or `.csproj`; when omitted, exactly one of either must sit at the project root, and a `.sln` there is built in preference to a `.csproj` beside it. The result includes exit/timeout/output metadata and bounded structured MSBuild diagnostics, whose `path` is `res://` for any file inside the project. `dotnet_executable` and `dotnet_version` name the toolchain that answered, and `DOTNET_BIN` redirects it; a value that cannot be used is passed over and reported in `dotnet_executable_configured_rejected`, and an executable that is not a .NET SDK is a `503` rather than a build verdict. `projects_built` counts the projects MSBuild produced an assembly for, and `success` is false when `dotnet` exits `0` having built none, which is what a solution with unresolvable project paths or no configuration mapping does. This is a real build and may update normal `bin`/`obj` outputs. +Runs `dotnet build` with `configuration` (`Debug` or `Release`, default `Debug`) and `timeout_seconds` (`1..300`, default `60`). Optional `project_file` must be a normalized project-contained `.sln` or `.csproj`; when omitted, exactly one of either must sit at the project root, and a `.sln` there is built in preference to a `.csproj` beside it. The result includes exit/timeout/output metadata and bounded structured MSBuild diagnostics, whose `path` is `res://` for any file inside the project. `dotnet_executable` and `dotnet_version` name the toolchain that answered, and `DOTNET_BIN` redirects it; a value that cannot be used is passed over and reported in `dotnet_executable_configured_rejected`, and an executable that is not a .NET SDK is a `503` rather than a build verdict. `dotnet --version` runs first under the same `timeout_seconds`, and one that does not answer in time is `504 timeout`, retryable, since a slow SDK is still there (#1078). `projects_built` counts the projects MSBuild produced an assembly for, and `success` is false when `dotnet` exits `0` having built none, which is what a solution with unresolvable project paths or no configuration mapping does. This is a real build and may update normal `bin`/`obj` outputs. ### `shader_get_visual_graph` — Live diff --git a/include/didi/gdextension/editor_hook.hpp b/include/didi/gdextension/editor_hook.hpp index d3b26372..60224d7c 100644 --- a/include/didi/gdextension/editor_hook.hpp +++ b/include/didi/gdextension/editor_hook.hpp @@ -210,6 +210,10 @@ class EditorHook { // pumps the main loop on every step, so this frame is then inside that // work, whoever started it (#995). bool editorProgressTaskOpen(); + // Whether the editor has yet to open its startup scenes. It opens them + // once its first scan is applied and makes one current, so a scene opened + // before then is not the edited scene for long (#1069). + bool editorStarting(); // Runs a scene_call_method, and parks it when the method is a coroutine. // Returns false when the request is not one of these, so the caller runs // the ordinary synchronous path. @@ -368,6 +372,8 @@ class EditorHook { std::optional m_importPassOverride; // Test seam for editorProgressTaskOpen, which otherwise asks the engine. std::optional m_progressTaskOverride; + // Test seam for editorStarting, which otherwise asks the engine. + std::optional m_editorStartingOverride; std::optional m_pendingQuitExitCode; int m_pendingQuitFrames{0}; @@ -393,6 +399,7 @@ class EditorHookTestAccess { static void setPumping(EditorHook& hook, bool pumping); static void setImportPassOpen(EditorHook& hook, std::optional open); static void setProgressTaskOpen(EditorHook& hook, std::optional open); + static void setEditorStarting(EditorHook& hook, std::optional starting); static bool hasPendingQuit(const EditorHook& hook); }; diff --git a/include/didi/gdextension/godot_bridge.hpp b/include/didi/gdextension/godot_bridge.hpp index cba4be9b..c16ddc4a 100644 --- a/include/didi/gdextension/godot_bridge.hpp +++ b/include/didi/gdextension/godot_bridge.hpp @@ -327,6 +327,10 @@ class GodotBridge { // reimport started in one collides with it. False when the editor's // ProgressDialog cannot be found, with one warning. bool editorProgressOpen(); + // Whether the editor has applied its first scan of the project, which is + // when it opens the scenes it restores, or the project's main scene (#1069). + // True once seen and from then on, and true when it cannot be read. + bool editorFirstScanApplied(); // A wait for scan work already under way, begun now. Empty when the // addon's watch is unavailable, which leaves nothing to wait on. std::optional beginScanSettle(); diff --git a/include/didi/offline/test_runner.hpp b/include/didi/offline/test_runner.hpp index f57d2b5d..e4d86120 100644 --- a/include/didi/offline/test_runner.hpp +++ b/include/didi/offline/test_runner.hpp @@ -155,6 +155,11 @@ struct TestSessionResult { // nothing under it. False on POSIX, where the process group does that job // and always works. bool contained{false}; + // The engine was never started: Windows refused to create the process. + // POSIX finds out in the child, which exits 127. Kept apart from a run + // that failed, because the fix is GODOT_BIN, not the game. + bool launch_failed{false}; + std::string launch_error; // How the wait for the killed process tree ended. // // The timeout path terminates the job and then waits for it to empty, so diff --git a/include/didi/tools/editor_copy_refresh.hpp b/include/didi/tools/editor_copy_refresh.hpp index 988e261e..a67596b6 100644 --- a/include/didi/tools/editor_copy_refresh.hpp +++ b/include/didi/tools/editor_copy_refresh.hpp @@ -33,6 +33,10 @@ struct EditorCopyRefresh { std::vector reloaded; // The scenes open in a tab whose tab was rebuilt from the written file. std::vector scenes_reloaded; + // For each rebuilt tab the editor said this about, whether the rebuild + // threw away unsaved changes: true or false, or null where the engine + // cannot say, which is Godot before 4.7. + json scenes_discarded_unsaved = json::object(); // The files the editor held and could not reload, each with the reason. json failed = json::array(); }; diff --git a/src/gdextension/editor_hook.cpp b/src/gdextension/editor_hook.cpp index 82a9545f..0fa27609 100644 --- a/src/gdextension/editor_hook.cpp +++ b/src/gdextension/editor_hook.cpp @@ -194,10 +194,24 @@ void EditorHook::processQueue() { std::optional session_rejection; }; std::vector commands; + // Asked before the queue is locked, because it asks the engine. + const bool starting = editorStarting(); { std::lock_guard lock(m_queueMutex); constexpr size_t kMaxCommandsPerFrame = 64; while (!m_commandQueue.empty() && commands.size() < kMaxCommandsPerFrame) { + // The editor opens its startup scenes once its first scan is + // applied, and makes one of them current. A scene opened or + // created before then answered opened: true and was replaced a + // moment later, so every scene call after it acted on another + // scene (#1069). Those two wait at the front of the queue, and + // what is behind them waits with them, so nothing overtakes + // them. The route deadline still applies, and a command that + // never started says so. + if (starting && (m_commandQueue.front().method == "scene.open" || + m_commandQueue.front().method == "scene.create")) { + break; + } auto session_rejection = validateSessionKindForMethod( m_commandQueue.front().method, m_sessionKind); commands.push_back( @@ -1222,6 +1236,13 @@ bool EditorHook::editorProgressTaskOpen() { return GodotBridge::instance().editorProgressOpen(); } +bool EditorHook::editorStarting() { + if (m_editorStartingOverride.has_value()) return *m_editorStartingOverride; + // A game opens no editor scenes. + if (m_sessionKind != runtime::SessionKind::editor) return false; + return !GodotBridge::instance().editorFirstScanApplied(); +} + void EditorHook::processAssetReimportFrame() { std::optional completed; json response; @@ -1971,6 +1992,10 @@ void EditorHookTestAccess::setProgressTaskOpen(EditorHook& hook, std::optional starting) { + hook.m_editorStartingOverride = starting; +} + bool EditorHookTestAccess::hasPendingQuit(const EditorHook& hook) { return hook.m_pendingQuitExitCode.has_value(); } diff --git a/src/gdextension/godot_bridge.cpp b/src/gdextension/godot_bridge.cpp index 0782f081..86f24c22 100644 --- a/src/gdextension/godot_bridge.cpp +++ b/src/gdextension/godot_bridge.cpp @@ -3805,6 +3805,16 @@ void reloadOpenSceneTab(GDExtensionObjectPtr editor, const std::string& path, result["scene_reload_pending"] = true; return; } + // What the rebuild throws away, read before it: true or false where the + // engine can say, null before 4.7 where it cannot (#1082). + json held_unsaved = nullptr; + if (unsaved.value("unsaved_scenes_readable", false) && unsaved.contains("unsaved_scenes") && + unsaved["unsaved_scenes"].is_array()) { + held_unsaved = false; + for (const auto& entry : unsaved["unsaved_scenes"]) { + if (entry.is_string() && entry.get() == path) held_unsaved = true; + } + } auto before = openSceneRootId(editor, path); if (before.isErr()) { result["scene_reload_error"] = "The editor's tab could not be read: " + before.error().message; @@ -3843,6 +3853,7 @@ void reloadOpenSceneTab(GDExtensionObjectPtr editor, const std::string& path, } reloaded_one = true; result["scene_reloaded"] = true; + result["scene_discarded_unsaved"] = held_unsaved; auto current_after = editedSceneRoot(editor); if (current_before.isOk() && edited_before != path && current_after.isOk() && editedScenePath(current_after.value()) == path) { @@ -4054,6 +4065,54 @@ bool GodotBridge::editorProgressOpen() { return flag.isOk() && flag.value() != 0; } +// The editor opens the scenes it restores from its layout, and the project's +// main scene on a first open, in its handler for the first scan's +// sources_changed (EditorNode::_sources_changed while waiting_for_first_scan, +// 4.5.1, 4.6.2 and 4.7.2). Nothing binds that flag. The file index says the +// same thing: EditorFileSystem starts with an empty root and swaps the scanned +// one in just before it emits sources_changed, and a project that loads this +// extension holds at least the file that declares it. The frames the editor +// runs between the swap and the scenes opening are inside progress tasks. +// get_filesystem is 842323275 and both counts 3905245786 on all three lines. +bool GodotBridge::editorFirstScanApplied() { + static bool applied = false; + static bool unreadable_reported = false; + if (applied) return true; + // Not known is not a reason to hold anything, so it answers true, and it + // is asked again on the next frame. + const auto unreadable = [] { + if (!unreadable_reported) { + unreadable_reported = true; + DIDI_LOG_WARN("GODOT_BRIDGE", "The editor's file index cannot be read, so scene_open " + "and scene_create do not wait for the editor to open its startup scenes."); + } + return true; + }; + auto editor = editorInterface(); + if (editor.isErr()) return unreadable(); + auto filesystem = callObject(editor.value(), "EditorInterface", "get_resource_filesystem", 780151678LL); + auto filesystem_object = filesystem.isOk() ? objectFromVariant(filesystem.value()) + : Result(filesystem.error()); + if (filesystem_object.isErr() || !filesystem_object.value()) return unreadable(); + auto root = callObject(filesystem_object.value(), "EditorFileSystem", "get_filesystem", 842323275LL); + auto root_object = root.isOk() ? objectFromVariant(root.value()) + : Result(root.error()); + if (root_object.isErr()) return unreadable(); + // No root at all is an editor taking its filesystem down, not one starting. + if (!root_object.value()) return true; + for (const char* count : {"get_subdir_count", "get_file_count"}) { + auto value = callObject(root_object.value(), "EditorFileSystemDirectory", count, 3905245786LL); + auto number = value.isOk() ? scalarFromVariant(value.value(), GDEXTENSION_VARIANT_TYPE_INT) + : Result(value.error()); + if (number.isErr()) return unreadable(); + if (number.value() > 0) { + applied = true; + return true; + } + } + return false; +} + bool GodotBridge::assetImportSettled(const std::string& resource_path) { const auto slash = resource_path.find_last_of('/'); if (slash == std::string::npos || slash + 1 >= resource_path.size()) return true; @@ -12937,31 +12996,31 @@ json GodotBridge::execute(const std::string& method, const json& params, {"scene_path", scene_path}}; return failure; }; + // What was open before this call replaces it. `opened: true` said + // the new scene was open and nothing said the old one no longer + // was, so every later scene_* call answered about a different file + // with no field naming either one. Read before anything moves it. + auto previous_root = editedSceneRoot(editor); + const std::string previous_scene = + previous_root.isOk() ? editedScenePath(previous_root.value()) : std::string(); + bool tab_stale = false; if (target_exists.value()) { auto replace_cache = makeScalar(GDEXTENSION_VARIANT_TYPE_INT, static_cast(4)); if (replace_cache.isErr()) return openFailure(replace_cache.error()); auto refreshed = callObject(loader.value(), "ResourceLoader", "load", 3358495409LL, {&path.value(), &packed_hint.value(), &replace_cache.value()}); if (refreshed.isErr()) return openFailure(refreshed.error()); - // Only a tab that holds the scene is rebuilt. A reload of a path - // no tab holds printed "Can't reload scene" on 4.7, and on 4.5 - // and 4.6 clears the current scene's undo history. auto open_root = openSceneRootId(editor, scene_path); if (open_root.isErr()) return openFailure(open_root.error()); - if (open_root.value() != 0) { - auto reloaded = callObject(editor, "EditorInterface", "reload_scene_from_path", - 83702148LL, {&path.value()}); - if (reloaded.isErr()) return openFailure(reloaded.error()); - } - } - // What was open before this call replaces it. `opened: true` said - // the new scene was open and nothing said the old one no longer - // was, so every later scene_* call answered about a different file - // with no field naming either one. - auto previous_root = editedSceneRoot(editor); - const std::string previous_scene = - previous_root.isOk() ? editedScenePath(previous_root.value()) : std::string(); - + tab_stale = open_root.value() != 0; + } + // A tab that holds the scene still has the tree from before the + // write. The open below makes it current, and the server rebuilds + // it from the file on a later request, through the reload every + // writer uses. Rebuilding it here first left the editor switching + // scenes for the rest of the frame on 4.5 and 4.6, so the open did + // nothing and a tab right of the current one never came to the + // front (#1079). A current tab stays current when it is rebuilt. auto opened = open_and_verify(); if (opened.isErr()) return openFailure(opened.error()); json created = uidFields({{"status", "success"}, {"saved", true}, {"opened", true}, @@ -12970,6 +13029,7 @@ json GodotBridge::execute(const std::string& method, const json& params, created["previous_scene_file_path"] = previous_scene.empty() ? json(nullptr) : json(previous_scene); created["scene_file_path"] = scene_path; + if (tab_stale) created["scene_tab_stale"] = true; return liveResult(created); } return liveResult(uidFields({{"status", "success"}, {"saved", true}, diff --git a/src/offline/test_runner.cpp b/src/offline/test_runner.cpp index 02420aea..7b0efc1d 100644 --- a/src/offline/test_runner.cpp +++ b/src/offline/test_runner.cpp @@ -581,11 +581,15 @@ TestSessionResult TestRunner::runSession(const std::string& scene_path, // job, and a child that spawns during that window escapes it permanently. const BOOL spawned = CreateProcessW(application_name, cmd_writable.data(), NULL, NULL, TRUE, creation_flags, NULL, NULL, &six.StartupInfo, &pi); + const DWORD launch_error = spawned ? 0 : GetLastError(); if (attribute_list) DeleteProcThreadAttributeList(attribute_list); if (!spawned) { CloseHandle(hWritePipe); if (hReadPipe != INVALID_HANDLE_VALUE) CloseHandle(hReadPipe); result.success = false; + result.launch_failed = true; + result.launch_error = + "the process could not be launched (Windows error " + std::to_string(launch_error) + ")"; result.summary = "Failed to spawn Godot process. Ensure 'godot' is in system PATH."; return result; } diff --git a/src/tools/deep_domain_tools.cpp b/src/tools/deep_domain_tools.cpp index 2611c987..dfe3bce8 100644 --- a/src/tools/deep_domain_tools.cpp +++ b/src/tools/deep_domain_tools.cpp @@ -201,18 +201,24 @@ DotnetResolution resolveDotnet() { // depends on it. struct DotnetProbe { bool available{false}; + // Ran out of time, which says the SDK is slow, not that it is missing. + bool timed_out{false}; std::string version; std::string unavailable_reason; }; +// Under the call's own timeout_seconds. A fixed 30 seconds was shorter than a +// dotnet on a loaded CI runner took to answer, and the call then told a +// caller who had asked for 300 to install an SDK that was there (#1078). DotnetProbe probeDotnet(const std::string& executable, - const std::filesystem::path& working_directory) { + const std::filesystem::path& working_directory, + std::chrono::seconds budget) { DotnetProbe probe; offline::ProcessRequest request; request.executable = executable; request.arguments = {"--version"}; request.working_directory = working_directory; - request.timeout = std::chrono::seconds(30); + request.timeout = budget; request.max_output_bytes = 64 * 1024; auto run = offline::runProcess(request); if (run.isErr()) { @@ -220,7 +226,9 @@ DotnetProbe probeDotnet(const std::string& executable, return probe; } if (run.value().timed_out) { - probe.unavailable_reason = "'--version' did not answer within 30 seconds"; + probe.timed_out = true; + probe.unavailable_reason = "'--version' did not answer within " + + std::to_string(budget.count()) + " seconds"; return probe; } const auto first_line = strings::trim(strings::split(run.value().output, '\n').empty() @@ -477,7 +485,19 @@ CallToolResult handleCSharpCheckBuild(const json& args, std::shared_ptris_boolean() || discarded->is_null())) { + refresh.scenes_discarded_unsaved[path] = *discarded; + } const auto moved = result.find("edited_scene_moved_from"); if (!restore.needed && moved != result.end() && moved->is_string()) { restore = {true, moved->get(), path}; @@ -263,7 +267,16 @@ std::optional refuseUnsavedOpenScenes(const std::shared_ptr ipc) { - return forwardLiveSceneWiring(args, ipc, "scene.create", "create a scene"); + auto created = forwardLiveSceneWiring(args, ipc, "scene.create", "create a scene"); + if (created.isError || !created.structuredContent.has_value() || + !created.structuredContent->is_object() || + !created.structuredContent->value("scene_tab_stale", false)) { + return created; + } + // The overwritten scene was open in a tab, which the bridge made current + // and left holding the tree from before the write. It is rebuilt here, on + // a later request than the one that switched to it, because on 4.5 and 4.6 + // the editor ignores a scene change for the rest of the frame (#1079). + // overwrite: true is the consent to lose that tab's changes, as it is for + // scene_pack_branch. + // An editor that never answers the rebuild leaves scene_tab_stale saying so. + auto payload = *created.structuredContent; + const auto scene_path = payload.value("scene_path", args.value("scene_path", std::string())); + const auto refresh = refreshEditorCopies(ipc, {scene_path}, true); + if (refresh.answered) payload.erase("scene_tab_stale"); + reportEditorCopy(payload, refresh); + return CallToolResult::successJson(payload); } CallToolResult handleSceneOpen(const json& args, std::shared_ptr ipc) { return forwardLiveSceneWiring(args, ipc, "scene.open", "open a scene"); diff --git a/tests/run_godot_integration.ps1 b/tests/run_godot_integration.ps1 index e7fb1e86..c8e02581 100644 --- a/tests/run_godot_integration.ps1 +++ b/tests/run_godot_integration.ps1 @@ -3362,7 +3362,8 @@ try { Assert-True (-not $copyById[2735].result.isError) "main.tscn did not reopen after the editor copy block: $($copyById[2735].result.content[0].text)" Invoke-SceneTabReloadBlock -EditorSession $editorSession -FixtureRoot $fixtureRoot -DirtyStateReadable $dirtyStateReadable - Invoke-PackBranchTabBlock -EditorSession $editorSession -FixtureRoot $fixtureRoot + Invoke-PackBranchTabBlock -EditorSession $editorSession -FixtureRoot $fixtureRoot -DirtyStateReadable $dirtyStateReadable + Invoke-CreateOverOpenTabBlock -EditorSession $editorSession -FixtureRoot $fixtureRoot -DirtyStateReadable $dirtyStateReadable $previousGodotBin = $env:GODOT_BIN try { diff --git a/tests/scene_tab_reload.ps1 b/tests/scene_tab_reload.ps1 index 88a5d4d6..7cadd764 100644 --- a/tests/scene_tab_reload.ps1 +++ b/tests/scene_tab_reload.ps1 @@ -96,7 +96,8 @@ function Invoke-SceneTabReloadBlock { function Invoke-PackBranchTabBlock { param( [Parameter(Mandatory = $true)] [object]$EditorSession, - [Parameter(Mandatory = $true)] [string]$FixtureRoot + [Parameter(Mandatory = $true)] [string]$FixtureRoot, + [Parameter(Mandatory = $true)] [bool]$DirtyStateReadable ) $target = "res://pack_target.tscn" $source = "res://pack_source.tscn" @@ -114,6 +115,9 @@ function Invoke-PackBranchTabBlock { $packRequests = @(& $attach 2758) + @( (Tool-Request 2760 "scene_create" @{ scene_path = $target; root_type = "Node2D"; root_name = "PackTarget"; overwrite = $true }), (Tool-Request 2761 "editor_save_scene" @{}), + # An edit the target's tab has not saved, which the pack throws away. + # The answer says so on 4.7 and cannot on 4.5 and 4.6 (#1082). + (Tool-Request 2793 "scene_set_property" @{ target_node = "/root/PackTarget"; property_name = "position"; value = @{ x = 3; y = 3 } }), (Tool-Request 2762 "scene_create" @{ scene_path = $source; root_type = "Node2D"; root_name = "PackSource"; overwrite = $true }), (Tool-Request 2763 "scene_instantiate_node" @{ node_type = "Sprite2D"; parent_path = "/root/PackSource"; name = "Child" }), (Tool-Request 2764 "editor_save_scene" @{}), @@ -125,12 +129,14 @@ function Invoke-PackBranchTabBlock { (Tool-Request 2769 "editor_save_scene" @{}) ) $packById = & $responses (Invoke-Didi -Requests $packRequests -Arguments @("--project", $FixtureRoot, "--yolo")) - foreach ($id in 2760, 2762, 2763) { [void](Tool-Payload $packById[$id]) } + foreach ($id in 2760, 2762, 2763, 2793) { [void](Tool-Payload $packById[$id]) } foreach ($id in 2761, 2764, 2769) { Assert-True ((Tool-Payload $packById[$id]).status -eq "saved") "The pack scenes were not saved by request ${id}: $($packById[$id].result.content[0].text)" } $packed = Tool-Payload $packById[2765] Assert-True ($packed.saved -eq $true -and $packed.editor_scene_reloaded -eq $true -and $null -eq $packed.editor_copy_error) "A pack over a scene open in another tab did not rebuild that tab: $($packed | ConvertTo-Json -Depth 6 -Compress)" + $expectedDiscard = if ($DirtyStateReadable) { $true } else { $null } + Assert-True ($packed.PSObject.Properties.Name -contains "editor_scene_discarded_unsaved" -and $packed.editor_scene_discarded_unsaved -eq $expectedDiscard) "A pack over a tab with an unsaved edit did not say it threw the edit away ($expectedDiscard expected): $($packed | ConvertTo-Json -Depth 6 -Compress)" Assert-True ((Tool-Payload $packById[2766]).scene_file_path -eq $source) "Rebuilding the target's tab moved the edited scene off the pack's source." $targetTab = Tool-Payload $packById[2768] Assert-True ($targetTab.scene_tree.name -eq "Child") "The target's tab still held the scene from before the pack: $($targetTab | ConvertTo-Json -Depth 4 -Compress)" @@ -150,3 +156,62 @@ function Invoke-PackBranchTabBlock { Assert-True ((Tool-Payload $recreateById[2774]).scene_tree.name -eq "Recreated") "scene_create did not replace a scene no tab held." Assert-True (-not $recreateById[2777].result.isError) "main.tscn did not reopen after the pack block: $($recreateById[2777].result.content[0].text)" } + +# scene_create with overwrite: true over a scene open in a tab that is not the +# current one (#1079). It rebuilt the tab and then opened it in the same +# request, and on 4.5 and 4.6 the editor ignores a scene change for the rest of +# the frame after a rebuild, so a tab right of the current one never came to +# the front and the call answered opened: false. The tab is now made current +# first and rebuilt on a later request. The second overwrite is of a tab left +# of a current last tab, which the rebuild used to leave current by accident on +# those lines, with previous_scene_file_path naming the scene just written. +function Invoke-CreateOverOpenTabBlock { + param( + [Parameter(Mandatory = $true)] [object]$EditorSession, + [Parameter(Mandatory = $true)] [string]$FixtureRoot, + [Parameter(Mandatory = $true)] [bool]$DirtyStateReadable + ) + $left = "res://create_tab_left.tscn" + $right = "res://create_tab_right.tscn" + $requests = @( + (@{ jsonrpc = "2.0"; id = 2778; method = "initialize"; params = @{ protocolVersion = "2024-11-05" } } | ConvertTo-Json -Compress), + (Tool-Request 2779 "runtime_attach_session" @{ session_id = $EditorSession.session_id }), + (Tool-Request 2780 "scene_create" @{ scene_path = $left; root_type = "Node2D"; root_name = "LeftOld"; overwrite = $true }), + (Tool-Request 2781 "editor_save_scene" @{}), + (Tool-Request 2782 "scene_create" @{ scene_path = $right; root_type = "Node2D"; root_name = "RightOld"; overwrite = $true }), + (Tool-Request 2783 "editor_save_scene" @{}), + # The right tab is now to the right of the current one. + (Tool-Request 2784 "scene_open" @{ scene_path = $left }), + (Tool-Request 2785 "scene_create" @{ scene_path = $right; root_type = "Node2D"; root_name = "RightNew"; overwrite = $true }), + (Tool-Request 2786 "scene_get_hierarchy" @{ max_depth = 1 }), + # And the left tab is left of the current last one. + (Tool-Request 2787 "scene_create" @{ scene_path = $left; root_type = "Node2D"; root_name = "LeftNew"; overwrite = $true }), + (Tool-Request 2788 "scene_get_hierarchy" @{ max_depth = 1 }), + (Tool-Request 2789 "scene_close" @{ discard_unsaved = $true }), + (Tool-Request 2790 "scene_open" @{ scene_path = $right }), + (Tool-Request 2791 "scene_close" @{ discard_unsaved = $true }), + (Tool-Request 2792 "scene_open" @{ scene_path = "res://main.tscn" }) + ) + $raw = Invoke-Didi -Requests $requests -Arguments @("--project", $FixtureRoot, "--yolo") + $byId = @{} + foreach ($response in @($raw | Where-Object { $_ -like "{*" } | ForEach-Object { $_ | ConvertFrom-Json } | Where-Object { $_.PSObject.Properties.Name -contains "id" })) { $byId[[int]$response.id] = $response } + foreach ($id in 2780, 2782, 2784, 2789, 2790, 2791) { [void](Tool-Payload $byId[$id]) } + foreach ($id in 2781, 2783) { + Assert-True ((Tool-Payload $byId[$id]).status -eq "saved") "The create-over-tab scenes were not saved by request ${id}: $($byId[$id].result.content[0].text)" + } + foreach ($case in @( + @{ Id = 2785; Hierarchy = 2786; Scene = $right; Root = "RightNew"; Previous = $left; Where = "right of the current one" }, + @{ Id = 2787; Hierarchy = 2788; Scene = $left; Root = "LeftNew"; Previous = $right; Where = "left of a current last tab" })) { + $created = $byId[$case.Id] + Assert-True (-not $created.result.isError) "scene_create over a scene open in a tab $($case.Where) did not open it: $($created.result.content[0].text)" + $payload = Tool-Payload $created + Assert-True ($payload.opened -eq $true -and $payload.editor_scene_reloaded -eq $true -and $null -eq $payload.editor_copy_error -and $null -eq $payload.scene_tab_stale) "scene_create over a tab $($case.Where) did not open and rebuild it: $($payload | ConvertTo-Json -Depth 6 -Compress)" + Assert-True ($payload.previous_scene_file_path -eq $case.Previous -and $payload.edited_scene_changed -eq $true) "scene_create over a tab $($case.Where) named the wrong previous scene: $($payload | ConvertTo-Json -Depth 6 -Compress)" + # Both tabs were saved, so nothing was thrown away, where the engine can say. + $expectedDiscard = if ($DirtyStateReadable) { $false } else { $null } + Assert-True ($payload.PSObject.Properties.Name -contains "editor_scene_discarded_unsaved" -and $payload.editor_scene_discarded_unsaved -eq $expectedDiscard) "scene_create over a saved tab $($case.Where) did not say whether it lost changes ($expectedDiscard expected): $($payload | ConvertTo-Json -Depth 6 -Compress)" + $hierarchy = Tool-Payload $byId[$case.Hierarchy] + Assert-True ($hierarchy.scene_file_path -eq $case.Scene -and $hierarchy.scene_tree.name -eq $case.Root) "After scene_create over a tab $($case.Where), the edited scene was not the new one: $($hierarchy | ConvertTo-Json -Depth 4 -Compress)" + } + Assert-True (-not $byId[2792].result.isError) "main.tscn did not reopen after the create-over-tab block: $($byId[2792].result.content[0].text)" +} diff --git a/tests/test_editor_startup_live.py b/tests/test_editor_startup_live.py new file mode 100644 index 00000000..32229a85 --- /dev/null +++ b/tests/test_editor_startup_live.py @@ -0,0 +1,196 @@ +"""A scene opened straight after an editor starts stays the edited scene. + +Opt in with DIDI_TEST_BINARY (the server) and DIDI_STARTUP_GODOT (a Godot +editor binary). + +An editor publishes its session before it has finished starting. It opens the +scenes it restores, or the project's main scene, once its first scan of the +project is applied, and makes one of them current. A scene_open answered before +then said opened: true and was replaced a moment later, so every later scene +call acted on the main scene (#1069). +""" +import json +import os +from pathlib import Path +import queue +import shutil +import signal +import subprocess +import tempfile +import threading +import time +import unittest + +try: + import didi_binary + import stdio_process +except ImportError: + from tests import didi_binary, stdio_process + +READS = 10 + + +@unittest.skipUnless(os.environ.get('DIDI_TEST_BINARY') and os.environ.get('DIDI_STARTUP_GODOT'), + 'real Godot startup test is opt-in') +class EditorStartupSceneLive(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix='didi-startup-live-') + self.addCleanup(self._cleanup) + self.root = Path(self.temp.name) + self.project = self.root / 'project' + self.project.mkdir() + self.binary = didi_binary.resolve().resolve() + addon = self.binary.parent / 'addons' / 'didi' + if not addon.is_dir(): + addon = self.binary.parent.parent / 'addons' / 'didi' + shutil.copytree(addon, self.project / 'addons' / 'didi') + (self.project / 'project.godot').write_text( + 'config_version=5\n[application]\nconfig/name="Startup scene"\n' + 'run/main_scene="res://main.tscn"\n[rendering]\n' + 'renderer/rendering_method="gl_compatibility"\n[editor_plugins]\n' + 'enabled=PackedStringArray("res://addons/didi/plugin.cfg")\n') + (self.project / 'main.tscn').write_text('[gd_scene format=3]\n[node name="Main" type="Node2D"]\n') + (self.project / 'tab.tscn').write_text('[gd_scene format=3]\n[node name="Tab" type="Node2D"]\n') + self.godot = os.environ['DIDI_STARTUP_GODOT'] + self.env = os.environ.copy() + self.env['DIDI_SESSION_DIR'] = str(self.root / 'sessions') + Path(self.env['DIDI_SESSION_DIR']).mkdir() + self.editor = None + self.session_pid = None + self.host = None + # Imported once headless, so the editor below is the first to open the + # project and its startup opens the main scene. + subprocess.run([self.godot, '--headless', '--path', str(self.project), '--import'], + capture_output=True, timeout=600, env=self.env) + + def _cleanup(self): + if self.host is not None: + try: + self.host.stdin.close() + except OSError: + pass + try: + self.host.wait(timeout=20) + except subprocess.TimeoutExpired: + pass + stdio_process.stop(self.host) + if self.editor is not None: + # A console build is a launcher and the editor is its child, so the + # tree goes, and the pid the session published after it. + if os.name == 'nt': + subprocess.run(['taskkill', '/T', '/F', '/PID', str(self.editor.pid)], capture_output=True) + elif self.editor.poll() is None: + os.killpg(self.editor.pid, signal.SIGKILL) + if self.session_pid: + try: + os.kill(self.session_pid, signal.SIGTERM) + except OSError: + pass + try: + self.editor.wait(timeout=30) + except subprocess.TimeoutExpired: + self.editor.kill() + for _ in range(20): + try: + self.temp.cleanup() + return + except OSError: + time.sleep(0.5) + + def _start_host(self): + self.host = subprocess.Popen([str(self.binary), '--project', str(self.project)], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, text=True, encoding='utf-8', env=self.env) + self.lines = queue.Queue() + + def reader(): + for line in self.host.stdout: + self.lines.put(line) + self.lines.put(None) + + threading.Thread(target=reader, daemon=True).start() + self.seq = 0 + self.request('initialize', {'protocolVersion': '2025-03-26', 'capabilities': {}, + 'clientInfo': {'name': 'startup-test', 'version': '1'}}) + self.host.stdin.write(json.dumps({'jsonrpc': '2.0', 'method': 'notifications/initialized'}) + '\n') + self.host.stdin.flush() + + def request(self, method, params): + self.seq += 1 + self.host.stdin.write(json.dumps({'jsonrpc': '2.0', 'id': self.seq, 'method': method, + 'params': params}) + '\n') + self.host.stdin.flush() + deadline = time.monotonic() + 60 + while True: + line = self.lines.get(timeout=max(0, deadline - time.monotonic())) + self.assertIsNotNone(line, f'the server closed its output during {method}') + message = json.loads(line) + # Notifications carry no id and are interleaved with the replies. + if message.get('id') == self.seq: + self.assertNotIn('error', message, message) + return message['result'] + + def tool(self, name, **arguments): + result = self.request('tools/call', {'name': name, 'arguments': arguments}) + payload = result.get('structuredContent') + if payload is None: + payload = json.loads(result['content'][0]['text']) + return result.get('isError', False), payload + + def test_a_scene_opened_at_startup_stays_current(self): + self._start_host() + launcher_options = {'start_new_session': True} if os.name != 'nt' else {} + # The editor's output, with the extension's own INFO lines in it, is + # what a failure on a runner has to go on. + editor_env = dict(self.env, DIDI_LOG_LEVEL='INFO') + self.editor_output = open(self.root / 'editor.out', 'w') + self.addCleanup(self.editor_output.close) + self.editor = subprocess.Popen([self.godot, '--editor', '--path', str(self.project)], + stdout=self.editor_output, stderr=subprocess.STDOUT, + env=editor_env, **launcher_options) + session = None + deadline = time.monotonic() + 180 + while session is None and time.monotonic() < deadline: + _, listed = self.tool('runtime_list_sessions') + session = next((entry for entry in listed.get('sessions', []) + if entry.get('kind') == 'editor' and entry.get('alive') is not False), None) + if session is None: + time.sleep(0.25) + self.assertIsNotNone(session, 'the editor published no session in 180 s') + self.session_pid = int(session['pid']) + errored, attached = self.tool('runtime_attach_session', session_id=session['session_id']) + self.assertFalse(errored, attached) + + # A caller may be told to retry while the editor is still starting; + # what it may not be told is that the scene is open when it will not be. + deadline = time.monotonic() + 120 + while True: + errored, opened = self.tool('scene_open', scene_path='res://tab.tscn') + data = opened.get('data', {}) if errored else {} + if not errored or data.get('outcome') != 'not_started' or time.monotonic() > deadline: + break + time.sleep(1) + self.assertFalse(errored, f'{opened}\n{self._diagnostics()}') + self.assertTrue(opened['opened'], opened) + + reads = [] + for _ in range(READS): + time.sleep(1) + errored, hierarchy = self.tool('scene_get_hierarchy', max_depth=1) + reads.append(hierarchy if errored else hierarchy.get('scene_file_path')) + self.assertEqual(reads, ['res://tab.tscn'] * READS, self._diagnostics()) + + def _diagnostics(self): + try: + _, listed = self.tool('runtime_list_sessions') + except Exception as error: # the server may be what failed + listed = repr(error) + try: + output = (self.root / 'editor.out').read_text(encoding='utf-8', errors='replace')[-6000:] + except OSError as error: + output = repr(error) + return f'sessions: {json.dumps(listed)[:2000]}\n--- editor output ---\n{output}' + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_engine_identity.py b/tests/test_engine_identity.py index 9b4564ad..162dc614 100644 --- a/tests/test_engine_identity.py +++ b/tests/test_engine_identity.py @@ -130,6 +130,12 @@ def test_runtime_launch_names_the_engine_that_ran_the_project(self): environment.pop("GODOT_BIN", None) result = _call("runtime_launch", {"timeout_seconds": 5}, environment) body = json.loads(result["content"][0]["text"]) + if result.get("isError"): + # A machine with no Godot: nothing ran, and the refusal names the + # engine it tried, as the other tools that start Godot do (#1076). + self.assertEqual(body["error"]["data"]["code"], "engine_unavailable", body) + self.assertTrue(body["error"]["data"]["engine_executable"], body) + return for field in ("engine_executable", "engine_version", "attached_engine_version", "matches_attached_engine"): self.assertIn(field, body, body) diff --git a/tests/test_phase5.cpp b/tests/test_phase5.cpp index 0d155df6..46b3b75d 100644 --- a/tests/test_phase5.cpp +++ b/tests/test_phase5.cpp @@ -39,6 +39,7 @@ namespace didi::mcp { // launch, which the gate never reaches. CallToolResult handleGridmapExportMeshLibrary(const json& args, std::shared_ptr ipc); CallToolResult handleProjectExport(const json& args, std::shared_ptr ipc); +CallToolResult handleCSharpCheckBuild(const json& args, std::shared_ptr ipc); } // namespace didi::mcp namespace { @@ -92,6 +93,49 @@ class ScopedUnstartableGodot { std::string previous; }; +// DOTNET_BIN naming a script that answers --version with an SDK version after +// about two seconds, and anything else at once, for as long as the test runs. +class ScopedSlowDotnet { +public: + ScopedSlowDotnet() { +#if defined(_WIN32) + path = std::filesystem::current_path() / "slow_dotnet.cmd"; + // ping, because timeout.exe refuses a standard input that is not a + // console, and the runner gives it NUL. + std::ofstream(path) << "@echo off\r\n" + "if \"%~1\"==\"--version\" (\r\n" + " ping -n 3 127.0.0.1 >nul\r\n" + " echo 8.0.100\r\n" + " exit /b 0\r\n" + ")\r\n" + "echo Build succeeded.\r\n"; +#else + path = std::filesystem::current_path() / "slow_dotnet.sh"; + std::ofstream(path) << "#!/bin/sh\n" + "if [ \"$1\" = \"--version\" ]; then sleep 2; echo 8.0.100; exit 0; fi\n" + "echo Build succeeded.\n"; + std::filesystem::permissions(path, std::filesystem::perms::owner_all, + std::filesystem::perm_options::add); +#endif + if (const char* value = std::getenv("DOTNET_BIN")) previous = value; + set(path.string()); + } + ~ScopedSlowDotnet() { set(previous); } + + std::filesystem::path path; + +private: + static void set(const std::string& value) { +#if defined(_WIN32) + _putenv_s("DOTNET_BIN", value.c_str()); +#else + if (value.empty()) unsetenv("DOTNET_BIN"); + else setenv("DOTNET_BIN", value.c_str(), 1); +#endif + } + std::string previous; +}; + class RecordingUiClient final : public didi::ipc::IIpcClient { public: bool connect(const std::string&, int) override { connected = true; return true; } @@ -776,6 +820,30 @@ TEST(Phase5, DiagnosticsRejectWrongResourceTypesBeforeProcessLaunch) { ASSERT_TRUE(registry.callTool("csharp_check_build", {{"project_file", "res://not_csharp.txt"}}).isError); } +TEST(Phase5, ASlowDotnetIsGivenTheCallsBudgetAndNotCalledMissing) { + // Break caught: the SDK probe had a fixed 30 seconds whatever + // timeout_seconds said, and a probe that ran out answered 503 + // toolchain_unavailable, retryable: false, telling a caller who had asked + // for 300 seconds to install the SDK (#1078). The probe now runs under the + // call's budget, and running out of it is a retryable timeout. + ScopedPhase5Project project("slow-dotnet"); + std::ofstream("Game.csproj") << "\n"; + ScopedSlowDotnet dotnet; + const auto slow = didi::mcp::handleCSharpCheckBuild({{"timeout_seconds", 1}}, nullptr); + ASSERT_TRUE(slow.isError); + const auto envelope = toolPayload(slow); + ASSERT_EQ(envelope["error"]["code"], 504); + ASSERT_EQ(envelope["error"]["data"]["code"], "timeout"); + ASSERT_EQ(envelope["error"]["data"]["retryable"], true); + ASSERT_EQ(envelope["error"]["data"]["timeout_seconds"], 1); + ASSERT_TRUE(envelope["error"]["message"].get().find("Install") == std::string::npos); + + // Given the time, the same dotnet answers and the build runs. + const auto patient = didi::mcp::handleCSharpCheckBuild({{"timeout_seconds", 30}}, nullptr); + ASSERT_TRUE(!patient.isError); + ASSERT_EQ(toolPayload(patient)["dotnet_version"], "8.0.100"); +} + TEST(Phase5, AnEngineThatWillNotStartIsEngineUnavailable) { // Break caught: the three tools that start their own Godot answered a // launch failure as 500 internal_error, a fault in the server with no diff --git a/tests/test_test_runner.cpp b/tests/test_test_runner.cpp index 7cee4f21..0bffcd72 100644 --- a/tests/test_test_runner.cpp +++ b/tests/test_test_runner.cpp @@ -424,6 +424,43 @@ void test_detached_launch_selects_the_game_it_started() { ASSERT_EQ(fake->attached, std::string("aaaabbbbccccddddeeeeffff00001111")); } +void test_an_engine_that_will_not_start_is_engine_unavailable() { + // Break caught: with a GODOT_BIN that cannot be started, runtime_launch + // answered isError: false, success: false and exit_code 0, the shape of a + // game that ran and failed, and said to put godot on PATH. The other tools + // that start their own Godot answer 503 engine_unavailable (#1045, #1076). + // A file that is not an executable: Windows refuses it (error 193), and a + // POSIX exec of it fails in the child, which exits 127. + const auto directory = std::filesystem::temp_directory_path() / "didi-launch-unstartable"; + std::filesystem::create_directories(directory); + const auto not_godot = directory / "not_godot.txt"; + std::ofstream(not_godot) << "not an engine\n"; + ScopedEnvironmentVariable godot_bin("GODOT_BIN"); + godot_bin.set(not_godot.string()); + auto& registry = didi::mcp::ToolRegistry::instance(); + registry.registerAllDefaultTools(); + for (const bool detach : {false, true}) { + const auto result = registry.callTool( + "runtime_launch", + {{"scene_path", "res://none.tscn"}, {"timeout_seconds", 2}, {"detach", detach}}); +#if !defined(_WIN32) + // A detached POSIX game execs after the call has its pid, so its + // failure is not the launch's to see. + if (detach) continue; +#endif + ASSERT_TRUE(result.isError); + const auto envelope = didi::json::parse(result.content[0].text); + ASSERT_EQ(envelope["error"]["code"], 503); + ASSERT_EQ(envelope["error"]["data"]["code"], "engine_unavailable"); + ASSERT_TRUE(envelope["error"]["data"]["engine_executable"].get().find( + "not_godot.txt") != std::string::npos); + ASSERT_TRUE(envelope["error"]["message"].get().find("GODOT_BIN") != + std::string::npos); + } + std::error_code ignored; + std::filesystem::remove_all(directory, ignored); +} + struct RegisterTestRunnerTests { RegisterTestRunnerTests() { registerTest("RuntimeLaunch.CrashKeepsItsLocation", @@ -431,6 +468,8 @@ struct RegisterTestRunnerTests { registerTest("RuntimeLaunch.TimeoutSchema", test_runtime_launch_schema_bounds_timeout); registerTest("RuntimeLaunch.TimeoutValidation", test_runtime_launch_rejects_timeout_outside_public_range); registerTest("RuntimeLaunch.Godot451Discovery", test_resolver_finds_documented_godot_451_layout); + registerTest("RuntimeLaunch.AnEngineThatWillNotStartIsEngineUnavailable", + test_an_engine_that_will_not_start_is_engine_unavailable); #if defined(_WIN32) registerTest("RuntimeLaunch.WindowsExit259", test_windows_exit_code_259_is_completed_not_timed_out); registerTest("RuntimeLaunch.WindowsBoundedOutputDrain", test_windows_completed_parent_does_not_wait_for_inherited_stdout); diff --git a/tests/test_tools.cpp b/tests/test_tools.cpp index ecf4e97b..f7d68688 100644 --- a/tests/test_tools.cpp +++ b/tests/test_tools.cpp @@ -78,6 +78,7 @@ CallToolResult handleProjectApplyChanges(const json& args, std::shared_ptr ipc); +CallToolResult handleSceneCreate(const json& args, std::shared_ptr ipc); } // namespace mcp } // namespace didi @@ -4511,6 +4512,16 @@ class EditorCopyClient final : public didi::ipc::IIpcClient { return didi::json{{"status", "success"}, {"saved", true}, {"scene_path", params.value("scene_path", std::string())}}; } + if (method == "scene.create") { + // Like the bridge: an overwritten scene a tab holds is made current + // and named stale, and the rebuild is left to a later request. + const auto path = params.value("scene_path", std::string()); + didi::json created = {{"status", "success"}, {"saved", true}, {"opened", true}, + {"scene_path", path}}; + if (open.count(path)) created["scene_tab_stale"] = true; + current = path; + return created; + } if (method == "scene.open") { if (refuse_open) return didi::Error(500, "Godot did not activate the requested scene"); current = params.value("scene_path", std::string()); @@ -4552,6 +4563,8 @@ class EditorCopyClient final : public didi::ipc::IIpcClient { } else { rebuilt_one = true; result["scene_reloaded"] = true; + result["scene_discarded_unsaved"] = + unsaved_readable ? didi::json(unsaved.count(path) > 0) : didi::json(nullptr); rebuilt.push_back(path); if (stays_current.count(path) && path != current) { result["edited_scene_moved_from"] = current; @@ -4823,6 +4836,81 @@ static void test_a_pack_rebuilds_the_tab_of_the_scene_it_overwrote() { ASSERT_EQ(refusing->methods, std::vector{"scene.packBranch"}); } +static void test_a_create_over_an_open_tab_rebuilds_it_after_the_switch() { + // scene_create over a scene open in another tab rebuilt the tab and then + // opened it, in one request. On 4.5 and 4.6 the rebuild leaves the editor + // changing scenes for the rest of the frame, so the open did nothing and + // the call answered opened: false (#1079). The bridge now switches to the + // tab and says it is stale, and the tab is rebuilt from the file on a + // later request, whatever it holds, as a pack's is. + auto editor = std::make_shared(); + editor->held = {"res://q_b.tscn"}; + editor->open = {"res://q_a.tscn", "res://q_b.tscn"}; + editor->current = "res://q_a.tscn"; + editor->unsaved_readable = false; + const didi::json args = {{"scene_path", "res://q_b.tscn"}, {"overwrite", true}}; + const auto created = didi::mcp::handleSceneCreate(args, editor); + ASSERT_TRUE(!created.isError); + const auto report = didi::json::parse(created.content[0].text); + ASSERT_EQ(report["opened"], true); + ASSERT_EQ(report["editor_scene_reloaded"], true); + ASSERT_TRUE(!report.contains("scene_tab_stale")); + ASSERT_TRUE(!report.contains("editor_copy_error")); + ASSERT_EQ(*created.structuredContent, report); + ASSERT_EQ(editor->rebuilt, std::vector{"res://q_b.tscn"}); + ASSERT_EQ(editor->refresh_params[0]["discard_unsaved"], true); + ASSERT_EQ(editor->methods, + (std::vector{"scene.create", "resource.refreshCached"})); + ASSERT_EQ(editor->current, "res://q_b.tscn"); + + // A scene no tab held is opened from the new file, and nothing is rebuilt. + auto fresh = std::make_shared(); + const auto plain = didi::mcp::handleSceneCreate({{"scene_path", "res://new.tscn"}}, fresh); + ASSERT_TRUE(!plain.isError); + ASSERT_TRUE(!didi::json::parse(plain.content[0].text).contains("editor_scene_reloaded")); + ASSERT_EQ(fresh->methods, std::vector{"scene.create"}); + + // An editor that never answers the rebuild leaves the answer saying so. + auto quiet = std::make_shared(); + quiet->open = {"res://q_b.tscn"}; + quiet->answer_requests = 0; + const auto stale = didi::mcp::handleSceneCreate(args, quiet); + ASSERT_TRUE(!stale.isError); + ASSERT_EQ(didi::json::parse(stale.content[0].text)["scene_tab_stale"], true); +} + +static void test_a_rebuild_says_whether_it_discarded_unsaved_changes() { + // Break caught: a writer that rebuilds a tab whatever it holds answered + // editor_scene_reloaded: true and nothing else, so a tab's unsaved edits + // were thrown away with nothing naming it (#1082). The answer now says + // whether they were: true or false on 4.7, null where the engine cannot say. + const didi::json args = {{"target_node", "/root/Main/Child"}, + {"scene_path", "res://pack_target.tscn"}, + {"overwrite", true}}; + const auto discarded = [&](bool readable, bool unsaved) { + auto editor = std::make_shared(); + editor->held = {"res://pack_target.tscn"}; + editor->open = {"res://pack_target.tscn"}; + editor->unsaved_readable = readable; + if (unsaved) editor->unsaved = {"res://pack_target.tscn"}; + const auto packed = didi::mcp::handleScenePackBranch(args, editor); + ASSERT_TRUE(!packed.isError); + const auto report = didi::json::parse(packed.content[0].text); + ASSERT_EQ(report["editor_scene_reloaded"], true); + ASSERT_TRUE(report.contains("editor_scene_discarded_unsaved")); + return report["editor_scene_discarded_unsaved"]; + }; + ASSERT_EQ(discarded(true, true), true); + ASSERT_EQ(discarded(true, false), false); + ASSERT_TRUE(discarded(false, false).is_null()); + + // A writer that rebuilt nothing says nothing about it. + auto nothing_open = std::make_shared(); + nothing_open->held = {"res://pack_target.tscn"}; + const auto packed = didi::mcp::handleScenePackBranch(args, nothing_open); + ASSERT_TRUE(!didi::json::parse(packed.content[0].text).contains("editor_scene_discarded_unsaved")); +} + static void test_a_rebuild_that_moves_the_edited_scene_is_switched_back() { // On 4.5 and 4.6 a rebuilt tab left of a current last tab stays the edited // scene, so every later scene_* call acted on it (#1072). The scene the @@ -6460,6 +6548,62 @@ static void test_nothing_is_dequeued_inside_an_editor_progress_task() { ASSERT_EQ(answer["error"]["data"]["code"], "session_kind_rejected"); } +static void test_a_scene_open_waits_for_the_editor_to_open_its_startup_scenes() { + // Break caught: the editor opens the scenes it restores, or the project's + // main scene, once its first scan is applied and makes one of them + // current, so a scene_open answered before then was replaced a moment + // later (#1069). Opening or creating a scene waits at the front of the + // queue until then, nothing queued behind it overtakes it, and any other + // command is not held. + using didi::godot::EditorHookTestAccess; + auto& hook = didi::godot::EditorHook::instance(); + const auto ready = [](auto& ticket) { + return ticket.response.wait_for(std::chrono::seconds(0)) == std::future_status::ready; + }; + hook.cancelPendingCommands("test reset"); + EditorHookTestAccess::setSessionKind(hook, didi::runtime::SessionKind::editor); + EditorHookTestAccess::setImportPassOpen(hook, false); + EditorHookTestAccess::setProgressTaskOpen(hook, false); + EditorHookTestAccess::setEditorStarting(hook, true); + // Game-only, so on an editor session it answers from the session policy + // and touches no engine. + auto other = EditorHookTestAccess::enqueue(hook, "runtime.injectInput"); + hook.processQueue(); + const bool other_answered = ready(other); + bool scene_calls_held = true; + bool scene_calls_released = true; + for (const char* method : {"scene.open", "scene.create"}) { + EditorHookTestAccess::setEditorStarting(hook, true); + auto scene_call = EditorHookTestAccess::enqueue(hook, method); + auto behind = EditorHookTestAccess::enqueue(hook, "runtime.injectInput"); + hook.processQueue(); + hook.processQueue(); + scene_calls_held = scene_calls_held && EditorHookTestAccess::queueDepth(hook) == 2u && + !scene_call.control->hasEverStarted() && !ready(scene_call) && + !ready(behind); + // The route deadline passing while it waited, so that releasing it + // runs nothing in the engine. + scene_call.control->tryCancelPending(); + EditorHookTestAccess::setEditorStarting(hook, false); + hook.processQueue(); + const bool both = ready(scene_call) && ready(behind); + auto cancelled = both ? scene_call.response.get() : didi::json::object(); + auto rejected = both ? behind.response.get() : didi::json::object(); + scene_calls_released = scene_calls_released && both && + EditorHookTestAccess::queueDepth(hook) == 0u && + cancelled["error"]["data"]["code"] == "command_cancelled" && + rejected["error"]["data"]["code"] == "session_kind_rejected"; + } + EditorHookTestAccess::setEditorStarting(hook, std::nullopt); + EditorHookTestAccess::setProgressTaskOpen(hook, std::nullopt); + EditorHookTestAccess::setImportPassOpen(hook, std::nullopt); + EditorHookTestAccess::setSessionKind(hook, std::nullopt); + hook.cancelPendingCommands("test reset"); + ASSERT_TRUE(other_answered); + ASSERT_TRUE(scene_calls_held); + ASSERT_TRUE(scene_calls_released); +} + static void test_a_write_is_applied_when_every_member_landed() { // Break caught: a colour or vector write that landed correctly reports // applied: false, because the comparison was exact for composites (#618). @@ -9651,6 +9795,10 @@ struct RegisterToolTests { test_a_rebuild_that_moves_the_edited_scene_is_switched_back); registerTest("Tools.PackRebuildsTheTabOfTheSceneItOverwrote", test_a_pack_rebuilds_the_tab_of_the_scene_it_overwrote); + registerTest("Tools.CreateOverAnOpenTabRebuildsItAfterTheSwitch", + test_a_create_over_an_open_tab_rebuilds_it_after_the_switch); + registerTest("Tools.RebuildSaysWhetherItDiscardedUnsavedChanges", + test_a_rebuild_says_whether_it_discarded_unsaved_changes); registerTest("Tools.OpenTabCheckAsksOnlyWhenItCanMatter", test_the_open_tab_check_asks_only_when_it_can_matter); registerTest("Tools.ApplyRefusesASceneOpenWithUnsavedChanges", @@ -9738,6 +9886,8 @@ struct RegisterToolTests { test_nothing_is_dequeued_inside_an_import_pass); registerTest("EditorHook.NothingIsDequeuedInsideAProgressTask", test_nothing_is_dequeued_inside_an_editor_progress_task); + registerTest("EditorHook.SceneOpenWaitsForTheStartupScenes", + test_a_scene_open_waits_for_the_editor_to_open_its_startup_scenes); registerTest("Tools.ShaderWriteAppliedComparesMembers", test_a_write_is_applied_when_every_member_landed); registerTest("Tools.WriteThatDidNotLandSaysWhichOfTheTwo", diff --git a/tools/vibe/README.md b/tools/vibe/README.md index 7737f9cf..f4345736 100644 --- a/tools/vibe/README.md +++ b/tools/vibe/README.md @@ -111,6 +111,7 @@ twice. | `probes/engine_string_marks.py` | What each engine keeps of a leading U+FEFF in a String, a StringName, a NodePath, a node name and a Label's text, and what decoding the same text from UTF-8 gives back, which is why #948's fix hands such text over as UTF-32. Then, with no editor, a NUL in a live tool's string argument, which must be refused by where it is. The live round trip is in the harness at request 2530. | | `probes/editor_undo_engine.py` | #913's engine fact, asked of each line: a headless editor's probe plugin commits two actions around a save, undoes past the save and redoes, once through the Scene menu's `Undo` and `Redo` items (the route #966 takes, found by their shortcut's resource name) and once by stepping the scene's `UndoRedo` (the route #913 was about). The menu route must print no `Inconsistent` line and, on 4.7, read the scene as unsaved. No Didi, no addon. | | `probes/claude_host_argument_types.py` | What a real Claude Code host puts on the wire for each top-level argument whose shape is not a string, and the three settings it writes into project.godot offline. An argument with no JSON `type` arrived as a string whatever was asked (#1000); `oneOf`, `$ref` and `const` arguments already arrived typed. Spends one short session's tokens and needs `claude` on PATH, so run it when a schema changes shape. Against a server from before #1000 it prints DIFF for the three untyped arguments and the three settings. | +| `probes/startup_scene_open.py` | A scene opened the moment an editor's session is listed, then the edited scene read once a second. The editor opens its startup scenes once its first scan is applied, and before #1069 that made the main scene current a moment after `scene_open` answered `opened: true` for another. Builds its own sandbox with a main scene and imports it headless first, so the editor opens it for the first time. | | `editor_log.py` | The editor's own console, read by every probe. `Session` finds the `editor.log` that `sandbox.py --launch` writes beside the project, reports what the editor printed before the session began, prints every new ERROR or WARNING under the call that caused it, and keeps them on `session.engine_lines`; `session.engine_summary()` groups them by call. Pass `editor_log=False` to turn it off. | | `fixtures/write_csharp_fixture.py` | A `.csproj` with one error and one warning in it. Kept out of `sandbox.py` because a C# project changes what Godot does with the directory. | | `editor_exit_status.py` | Not a probe against the server: the same editor invocation with and without the built addon installed, exit statuses side by side. The control for a crash on shutdown. | diff --git a/tools/vibe/probes/startup_scene_open.py b/tools/vibe/probes/startup_scene_open.py new file mode 100644 index 00000000..656f8e51 --- /dev/null +++ b/tools/vibe/probes/startup_scene_open.py @@ -0,0 +1,144 @@ +"""Whether a scene opened straight after an editor starts stays the edited scene. + +An editor publishes its session long before it has finished starting. It opens +the scenes it restores from its saved layout, or the project's main scene on a +project it has never opened, only once its first scan of the project is +applied: `EditorNode::_sources_changed` for that scan, on 4.5.1, 4.6.2 and +4.7.2. A `scene_open` answered before then was followed by the startup opening +the main scene and making it current, so every later `scene_*` call acted on +the main scene while the caller had been told it was on the one it asked for +(#1069). + +This asks the surface. A sandbox with `run/main_scene` and a second scene, +`tab.tscn`, is imported once headless, so the editor that follows opens it for +the first time. The probe attaches as soon as `runtime_list_sessions` lists the +editor, sends `scene_open` for `tab.tscn`, and reads `scene_get_hierarchy` once +a second for `--watch` seconds. It prints each read and whether `tab.tscn` was +current at every one. Before the fix every read said `main.tscn`. + + python tools/vibe/probes/startup_scene_open.py --godot C:/Godot/Godot_v4.5.1-stable_win64_console.exe + +`--project DIR` builds the sandbox there instead of in a temporary directory, +and keeps it. +""" + +from __future__ import annotations + +import argparse +import json +import os +import signal +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +import sandbox # noqa: E402 +from mcp_client import Session # noqa: E402 +from wait_for_session import same_project # noqa: E402 + +TAB_TSCN = """[gd_scene format=3] + +[node name="Tab" type="Node2D"] +""" + + +def editor_sessions(project: Path) -> list[dict]: + with Session(project, editor_log=False) as look: + payload, _ = look.call("runtime_list_sessions", {}) + return [entry for entry in (payload or {}).get("sessions", []) + if entry.get("kind") == "editor" and entry.get("alive") is not False + and same_project(entry, project)] + + +def edited_scene(s: Session) -> str: + payload, errored = s.call("scene_get_hierarchy", {"max_depth": 1}) + if errored: + error = (payload or {}).get("error", payload) if isinstance(payload, dict) else payload + return f"refused: {json.dumps(error)[:200]}" + return str((payload or {}).get("scene_file_path")) + + +def main() -> int: + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--godot", required=True, help="a Godot console binary") + parser.add_argument("--project", help="build the sandbox here and keep it") + parser.add_argument("--build-tree", help="take the addon from this build directory") + parser.add_argument("--watch", type=int, default=15, help="seconds of reads after the open") + args = parser.parse_args() + + holder = None + if args.project: + project = Path(args.project).resolve() + else: + holder = tempfile.TemporaryDirectory(prefix="didi-startup-") + project = Path(holder.name) / "sandbox" + sandbox.create(project, name="StartupSceneOpen", overwrite=True, + build_tree=Path(args.build_tree) if args.build_tree else None) + (project / "tab.tscn").write_text(TAB_TSCN, encoding="utf-8", newline="\n") + subprocess.run([args.godot, "--headless", "--path", str(project), "--import"], + capture_output=True, timeout=600) + log = project.parent / "editor.log" + log.unlink(missing_ok=True) + launcher = subprocess.Popen([args.godot, "--editor", "--path", str(project), "--log-file", str(log)], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + started = time.monotonic() + editor = None + try: + deadline = started + 120 + while editor is None and time.monotonic() < deadline: + found = editor_sessions(project) + editor = found[0] if found else None + if editor is None: + time.sleep(0.25) + if editor is None: + print("the editor published no session in 120 s") + return 1 + print(f"session listed {time.monotonic() - started:5.1f} s after launch " + f"(Godot {editor.get('engine_version')}, pid {editor.get('pid')})") + with Session(project) as s: + s.call("runtime_attach_session", {"session_id": editor["session_id"]}) + print(f"before the open: {edited_scene(s)}") + asked = time.monotonic() + payload, errored = s.call("scene_open", {"scene_path": "res://tab.tscn"}) + answered = time.monotonic() - asked + outcome = "REFUSED " + json.dumps(payload)[:300] if errored else json.dumps( + {key: payload.get(key) for key in ("opened", "scene_path")}) + print(f"scene_open: {outcome} after {answered:.1f} s") + reads = [] + for second in range(1, args.watch + 1): + time.sleep(1) + reads.append(edited_scene(s)) + print(f" read {second:2d}: {reads[-1]}") + kept = bool(reads) and all(read == "res://tab.tscn" for read in reads) + print() + print(f"tab.tscn current at every read: {'yes' if kept else 'NO'}") + print(s.engine_summary()) + return 0 if kept else 1 + finally: + # The console build is a launcher: the editor is its child, and the + # session names the child's pid. + if editor is not None and editor.get("pid"): + try: + os.kill(int(editor["pid"]), signal.SIGTERM) + except OSError: + pass + launcher.terminate() + try: + launcher.wait(timeout=30) + except subprocess.TimeoutExpired: + launcher.kill() + if holder is not None: + time.sleep(1) + try: + holder.cleanup() + except OSError: + pass + + +if __name__ == "__main__": + raise SystemExit(main())