From 140a758f87a5041191b11117b42053a18edb3dfd Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Fri, 28 Aug 2026 16:36:26 +0900 Subject: [PATCH 1/5] fix: seed extension_list.cfg so the first headless import cannot crash The crash is Godot's own, and now that it is understood it can be avoided outright rather than retried around. EditorHelp::_gen_extensions_docs() dereferences the static DocTools without a null check. Discovering an extension mid-scan emits extensions_reloaded, which sends EditorNode off to regenerate the class reference on a worker thread; that thread's last act is to queue _gen_extensions_docs as a deferred call. --import quits before the message queue is flushed, so the call lands on the flush at the end of Main::cleanup() -- by which time EditorHelp::cleanup_doc() has freed the DocTools and set it to null. Naming the extension in .godot/extension_list.cfg before Godot starts means it is loaded at startup instead, extensions_reloaded never fires, and nothing is queued. Godot rewrites that file itself, so seeding it decides only the run in which it did not exist yet. The retry is gone with it. It was a second run standing in for an explanation; with the file seeded the import either works or has failed for some other reason, and a crash is a crash. Earlier commits blamed the pairing of godot-cpp master with Godot 4.7. That was wrong -- godot-cpp is not in the stack at all. --- scripts/run-tests.ps1 | 58 +++++++++++++++++++++++----------------- scripts/run-tests.sh | 62 ++++++++++++++++++++++++------------------- 2 files changed, 68 insertions(+), 52 deletions(-) diff --git a/scripts/run-tests.ps1 b/scripts/run-tests.ps1 index 72689f6..9b43104 100644 --- a/scripts/run-tests.ps1 +++ b/scripts/run-tests.ps1 @@ -91,33 +91,38 @@ Write-Host "run-tests.ps1: $(& $GodotBin --headless --version | Select-Object -L # --- import pass ---------------------------------------------------------- # A project Godot has never opened has no .godot\, and the textures beside each # .ssab are not importable until it does. Cheap after the first run. -# It is run twice on purpose, and the SECOND run is the one that has to pass. # -# The run in which Godot first DISCOVERS the extension aborts on the way out -# (null dereference, caught by Godot's own crash handler). Not the import: a -# project with zero importable files does it too. What triggers it is the -# extension being loaded mid-scan rather than at startup from -# .godot/extension_list.cfg -- delete just that file and it happens again. +# The extension is named in .godot\extension_list.cfg BEFORE Godot is started, +# because the run in which Godot discovers an extension mid-scan crashes on the +# way out. That crash is Godot's own, and it is not the import -- a project with +# zero importable files does it too: # -# It is NOT this repository's code. godot-cpp's own test extension, with none of -# our sources and a different descriptor, reproduces it exactly; a project with -# no extension does not; and registering nothing at all still does. godot-cpp -# has no 4.6/4.7 release branch (it went from godot-4.5-stable straight to the -# 10.0 line), so an extension for Godot 4.7 is built from master against -# api_version=4.7, and that is the combination that does this. +# EditorHelp::_gen_extensions_docs() dereferences the static DocTools without +# a null check. Loading an extension mid-scan emits extensions_reloaded, which +# sends EditorNode off to regenerate the class reference on a worker thread; +# that thread's last act is to queue _gen_extensions_docs as a deferred call. +# --import then quits before the message queue is flushed, so the call lands +# on the flush at the end of Main::cleanup() -- by which time +# EditorHelp::cleanup_doc() has freed the DocTools and set it to null. # -# The scan's work completes -- the second run exits 0 with nothing left to do. -# So this is a retry, not a tolerance. A crash that repeats is still a failure -# here, and the clean second run is the evidence that the import finished -- -# nothing is being waved through on the strength of the first one. +# Naming the extension up front means it is loaded at startup instead, so +# extensions_reloaded never fires and nothing is ever queued. Godot rewrites +# this file itself, so seeding it decides only the run in which it did not exist +# yet. There is no retry here: with the file seeded the import either works or +# has failed for some other reason, and a crash is a crash. if ($DoImport -eq "yes") { + $extList = Join-Path $Project ".godot\extension_list.cfg" + if (-not (Test-Path $extList)) { + $descriptors = Get-ChildItem -Path (Join-Path $Project "addons\spritestudio") -Filter "*.gdextension" -ErrorAction SilentlyContinue + foreach ($desc in $descriptors) { + New-Item -ItemType Directory -Force -Path (Join-Path $Project ".godot") | Out-Null + $rel = $desc.FullName.Substring($Project.Length).TrimStart('\', '/').Replace('\', '/') + Add-Content -Path $extList -Value "res://$rel" + } + } + $importLog = & $GodotBin --headless --path $Project --import 2>&1 $importStatus = $LASTEXITCODE - if ($importStatus -ne 0) { - Write-Host "run-tests.ps1: the first import exited $importStatus (godot-cpp's known first-scan crash); retrying." - $importLog = & $GodotBin --headless --path $Project --import 2>&1 - $importStatus = $LASTEXITCODE - } $importErrors = $importLog | Select-String -Pattern '^ERROR:' if ($importStatus -ne 0 -or $importErrors) { Write-Host "run-tests.ps1: the import pass failed." @@ -127,10 +132,13 @@ if ($DoImport -eq "yes") { } # --- run ------------------------------------------------------------------ -# --quit-after is a hang guard, not a schedule: a script error inside a case -# aborts run_tests.gd's _init and leaves the tree idling forever. It costs the -# exit code its meaning on that path -- Godot leaves 0 on the way out -- which -# is why the marker below, and not the status, is what says a run completed. +# --quit-after is a hang guard, not a schedule: an error run_tests.gd's _init +# cannot walk away from -- a suite that will not parse -- leaves the tree idling +# forever. It costs the exit code its meaning on that path -- Godot leaves 0 on +# the way out -- which is why the marker below, and not the status, is what says +# a run completed. A script error inside one case is not that: GDScript +# abandons the case and carries on, and run_tests.gd fails it for recording +# nothing. $runArgs = @("--headless", "--path", $Project, "--quit-after", "100000", "--script", "res://run_tests.gd") if ($Only) { $runArgs += @("--", "--only=$Only") } diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index b5a0a7c..30cdc68 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -83,35 +83,39 @@ echo "$APP: $("$GODOT_BIN" --headless --version 2>/dev/null | tail -n 1) at ${GO # --- import pass ---------------------------------------------------------- # A project Godot has never opened has no .godot/, and the textures beside each # .ssab are not importable until it does. Cheap after the first run. -# It is run twice on purpose, and the SECOND run is the one that has to pass. # -# The run in which Godot first DISCOVERS the extension aborts on the way out -# (null dereference, caught by Godot's own crash handler). Not the import: a -# project with zero importable files does it too. What triggers it is the -# extension being loaded mid-scan rather than at startup from -# .godot/extension_list.cfg -- delete just that file and it happens again. +# The extension is named in .godot/extension_list.cfg BEFORE Godot is started, +# because the run in which Godot discovers an extension mid-scan crashes on the +# way out. That crash is Godot's own, and it is not the import -- a project with +# zero importable files does it too: # -# It is NOT this repository's code. godot-cpp's own test extension, with none of -# our sources and a different descriptor, reproduces it exactly; a project with -# no extension does not; and registering nothing at all still does. godot-cpp -# has no 4.6/4.7 release branch (it went from godot-4.5-stable straight to the -# 10.0 line), so an extension for Godot 4.7 is built from master against -# api_version=4.7, and that is the combination that does this. +# EditorHelp::_gen_extensions_docs() dereferences the static DocTools without +# a null check. Loading an extension mid-scan emits extensions_reloaded, which +# sends EditorNode off to regenerate the class reference on a worker thread; +# that thread's last act is to queue _gen_extensions_docs as a deferred call. +# --import then quits before the message queue is flushed, so the call lands +# on the flush at the end of Main::cleanup() -- by which time +# EditorHelp::cleanup_doc() has freed the DocTools and set it to null. # -# The scan's work completes -- the second run exits 0 with nothing left to do. -# So this is a retry, not a tolerance. A crash that repeats is still a failure -# here, and the clean second run is the evidence that the import finished -- -# nothing is being waved through on the strength of the first one. +# Naming the extension up front means it is loaded at startup instead, so +# extensions_reloaded never fires and nothing is ever queued. Godot rewrites +# this file itself, so seeding it decides only the run in which it did not exist +# yet. There is no retry here: with the file seeded the import either works or +# has failed for some other reason, and a crash is a crash. if [ "$DO_IMPORT" = "yes" ]; then + EXT_LIST="${PROJECT}/.godot/extension_list.cfg" + if [ ! -f "$EXT_LIST" ]; then + for desc in "${PROJECT}"/addons/spritestudio/*.gdextension; do + [ -f "$desc" ] || continue + mkdir -p "${PROJECT}/.godot" + echo "res://${desc#"${PROJECT}/"}" >>"$EXT_LIST" + done + fi + IMPORT_LOG=$(mktemp) set +e "$GODOT_BIN" --headless --path "$PROJECT" --import >"$IMPORT_LOG" 2>&1 IMPORT_STATUS=$? - if [ "$IMPORT_STATUS" -ne 0 ]; then - echo "$APP: the first import exited $IMPORT_STATUS (godot-cpp's known first-scan crash); retrying." - "$GODOT_BIN" --headless --path "$PROJECT" --import >"$IMPORT_LOG" 2>&1 - IMPORT_STATUS=$? - fi set -e if [ "$IMPORT_STATUS" -ne 0 ] || grep -q '^ERROR:' "$IMPORT_LOG"; then echo "$APP: the import pass failed." >&2 @@ -123,10 +127,13 @@ if [ "$DO_IMPORT" = "yes" ]; then fi # --- run ------------------------------------------------------------------ -# --quit-after is a hang guard, not a schedule: a script error inside a case -# aborts run_tests.gd's _init and leaves the tree idling forever. It costs the -# exit code its meaning on that path -- Godot leaves 0 on the way out -- which -# is why the marker below, and not the status, is what says a run completed. +# --quit-after is a hang guard, not a schedule: an error run_tests.gd's _init +# cannot walk away from -- a suite that will not parse -- leaves the tree idling +# forever. It costs the exit code its meaning on that path -- Godot leaves 0 on +# the way out -- which is why the marker below, and not the status, is what says +# a run completed. A script error inside one case is not that: GDScript +# abandons the case and carries on, and run_tests.gd fails it for recording +# nothing. ARGS=(--headless --path "$PROJECT" --quit-after 100000 --script res://run_tests.gd) [ -n "$ONLY" ] && ARGS+=(-- "--only=${ONLY}") @@ -151,8 +158,9 @@ EOF fi # run_tests.gd prints this last. Without it the run stopped in the middle -- a -# parse error in a suite, or a script error inside a case, either of which -# leaves Godot to be shut down by --quit-after with a status of 0. +# suite that will not parse, say -- and Godot was shut down by --quit-after with +# a status of 0. A script error inside one case does not stop the run; the +# runner catches that itself, by failing a case that recorded nothing. if ! echo "$OUTPUT" | grep -q "==== SUITE FINISHED ===="; then echo "" >&2 echo "$APP: the run stopped before the suite finished — see above." >&2 From a49f107e7786390b47d3a4f305650cb407a15a4c Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Fri, 28 Aug 2026 16:36:36 +0900 Subject: [PATCH 2/5] fix: the headless runner counted an aborted case as a pass GDScript answers a bad call -- the wrong argument count, a method that is not there -- by printing SCRIPT ERROR and abandoning that one function, not by stopping the run. The case then left no assertion, no skip and no failure behind, and a suite whose failure count had not grown printed PASS. The suite has been reporting a case it never ran as green. A case that records nothing at all is now a failure, named as such, with a pointer to the SCRIPT ERROR above it. The check is three counters read before and after the call, so it costs nothing and cannot be forgotten by a suite author. only= took the runner down rather than filtering. _only_filter() returned an untyped Array from a function declared Array[String], which is a runtime error that aborts _init -- the tree then idles until --quit-after fires, about a minute, and the run ends with no marker and a status of 0. It reads as a hang. Array.assign() is the conversion that was wanted. The comments explaining --quit-after said a script error inside a case aborts _init. It does not, as above; corrected in all three files. --- test_gdextension/run_tests.gd | 27 +++++++++++++++++++++------ test_gdextension/test_base.gd | 12 ++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/test_gdextension/run_tests.gd b/test_gdextension/run_tests.gd index a686603..9dc4893 100644 --- a/test_gdextension/run_tests.gd +++ b/test_gdextension/run_tests.gd @@ -75,11 +75,17 @@ func _init() -> void: var before_failures: int = suite.failures.size() for case in cases: suite.begin_case(case) + var mark := [suite.assertions, suite.skips.size(), suite.failures.size()] suite.setup() suite.call(case) suite.teardown() suite.release_owned() total_cases += 1 + # A case that recorded nothing at all did not run: GDScript abandons a + # function on a bad call without stopping the run, and the case would + # otherwise be indistinguishable from one that passed. + if mark == [suite.assertions, suite.skips.size(), suite.failures.size()]: + suite.record_empty_case() total_assertions += suite.assertions failures.append_array(suite.failures) @@ -110,11 +116,15 @@ func _init() -> void: ## Prints the marker `run-tests.*` looks for, then exits. ## -## The exit code alone is not enough to trust. A script error inside a case -## aborts `_init` before anything below it runs, and the tree then idles forever -## — so the wrapper passes `--quit-after`, which makes Godot exit 0 on the way -## out and turns a crashed run into a green one. The marker is what tells a run -## that finished apart from one that stopped in the middle. +## The exit code alone is not enough to trust. An error `_init` cannot walk away +## from — a suite that will not parse, above all — leaves the tree idling +## forever, so the wrapper passes `--quit-after`, which makes Godot exit 0 on the +## way out and turns a stopped run into a green one. The marker is what tells a +## run that finished apart from one that stopped in the middle. +## +## It says nothing about a single case. GDScript abandons a case's function on a +## bad call and carries straight on to the next one, which is what the +## `record_empty_case` check above is for. func _finish(code: int) -> void: print("==== SUITE FINISHED ====") quit(code) @@ -174,5 +184,10 @@ func _cases_of(suite) -> Array[String]: func _only_filter() -> Array[String]: for arg in OS.get_cmdline_user_args(): if arg.begins_with("--only="): - return Array(arg.trim_prefix("--only=").split(",", false)) + # assign(), not a cast: split() hands back a PackedStringArray, and + # returning it -- or an untyped Array around it -- from an Array[String] + # is a runtime error that takes _init down with it. + var names: Array[String] = [] + names.assign(arg.trim_prefix("--only=").split(",", false)) + return names return [] diff --git a/test_gdextension/test_base.gd b/test_gdextension/test_base.gd index 42e689d..2253f7f 100644 --- a/test_gdextension/test_base.gd +++ b/test_gdextension/test_base.gd @@ -134,3 +134,15 @@ func has(collection, value, what: String) -> bool: ## Declares this case unrunnable here, with the reason. Not a pass. func skip(reason: String) -> void: skips.append("%s: %s" % [_case, reason]) + + +## Marks the case just run as a defect because it asserted nothing. +## +## Called by `run_tests.gd` between cases, never by a case itself. GDScript +## answers a bad call -- the wrong argument count, a method that is not there -- +## by printing SCRIPT ERROR and abandoning that one function, not by stopping +## the run. The case then leaves no assertion, no skip and no failure behind, +## and a suite whose failure count did not grow reads as a pass. A case that +## asserted nothing was not run, so it is counted here as a failure. +func record_empty_case() -> void: + _record_failure("asserted nothing -- a SCRIPT ERROR above will say why") From 61bb169cb9e43557759e1b3bb26acf124dbafd41 Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Fri, 28 Aug 2026 16:36:43 +0900 Subject: [PATCH 3/5] fix: set_part_color_override takes four arguments, not five test_colour_and_cell_overrides_are_accepted_for_a_real_part passed a stray 1.0 between the blend operation and the priority, so every call in it failed at the first line and the case asserted nothing. The bound signature is (part_name, color, blend_op, priority) -- the rate the extra argument looks like is the colour's own alpha. Found by the check in the previous commit, which is what it is for. --- test_gdextension/suites/test_overrides.gd | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test_gdextension/suites/test_overrides.gd b/test_gdextension/suites/test_overrides.gd index da9daf5..c0b4eff 100644 --- a/test_gdextension/suites/test_overrides.gd +++ b/test_gdextension/suites/test_overrides.gd @@ -93,7 +93,8 @@ func test_an_unknown_part_is_refused_rather_than_ignored() -> void: ## is a rendering question and out of scope here; that the call reports success ## for a real part and failure for an imaginary one is not. func test_colour_and_cell_overrides_are_accepted_for_a_real_part() -> void: - ok(player.set_part_color_override(part, Color(1, 0, 0, 1), 0, 1.0, 0), + # The two zeroes are COLOR_BLEND_MIX and OVERRIDE_PRIORITY_OVERWRITE_ON_NEXT_KEYFRAME. + ok(player.set_part_color_override(part, Color(1, 0, 0, 1), 0, 0), "a colour override on '%s'" % part) player.advance(dt) ok(player.clear_part_color_override(part), "clearing it") From 37af09b2b84ffdef410e40e337014e8fbf449585 Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Fri, 28 Aug 2026 16:36:51 +0900 Subject: [PATCH 4/5] docs: the first-scan crash is Godot's, and it is now designed out The headless suite section blamed godot-cpp master paired with Godot 4.7, because that was the last axis left after six other suspects were ruled out. It was the wrong one: godot-cpp does not appear in the stack. The paragraph now names the function, the deferred call and the flush that runs it too late, and describes the seeding that avoids it instead of the retry that used to absorb it. Also states the counting rule the runner now enforces: a case that recorded nothing at all is a failure, not a pass. --- AGENTS.md | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7ec460d..8b8c0f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,22 +110,30 @@ builds. Cases step with `advance()` under `ANIMATION_PROCESS_MANUAL`, never the frame clock, so a result does not depend on how long a frame took. A case that cannot run on this host declares a **skip**, which is reported apart from the passes -and never counted as one. - -**The first headless import crashes, and `run-tests.*` retries it once. It is a -godot-cpp problem, not ours.** The run in which Godot first *discovers* the -extension aborts on the way out (null dereference, caught by Godot's own crash -handler). Not the import — a project with **zero importable files** does it too; -what triggers it is the extension being loaded mid-scan rather than at startup -from `.godot/extension_list.cfg`, and deleting just that file brings it back. -**godot-cpp's own `test/` extension reproduces it exactly**, a project with no -extension does not, and registering nothing at all still does — so it is the -pairing, not this code. godot-cpp has no 4.6/4.7 release branch: it went from -`godot-4.5-stable` straight to the 10.0 line, so an extension for Godot 4.7 is -built from master against `api_version=4.7`. The scan's work completes, so the -second run is clean; the retry requires that second run to pass, because a crash -that repeats is still a failure. Not test-only — anything running -`godot --headless --import` on a fresh checkout meets it. +and never counted as one. A case that recorded nothing at all — no assertion, no +skip, no failure — is counted as a **failure**: GDScript answers a bad call by +abandoning that one function and carrying straight on, so without that check a +case that never ran would be indistinguishable from one that passed. + +**A first headless import that discovers the extension mid-scan crashes on the +way out, so `run-tests.*` names the extension in `.godot/extension_list.cfg` +before it starts Godot. The bug is Godot's — not this code, and not +godot-cpp's.** `EditorHelp::_gen_extensions_docs()` dereferences the static +`DocTools` without a null check. Loading an extension mid-scan emits +`GDExtensionManager::extensions_reloaded`, which sends `EditorNode` off to +regenerate the class reference on a worker thread; that thread's last act is to +queue `_gen_extensions_docs` as a deferred call. `--import` quits before the +message queue is flushed, so the call lands on the flush at the end of +`Main::cleanup()` — by which time `EditorHelp::cleanup_doc()` has freed the +`DocTools` and set it to null. It is not the import: a project with **zero +importable files** does it too, and godot-cpp's own `test/` extension reproduces +it. Naming the extension up front means it is loaded at startup instead, so +`extensions_reloaded` never fires and nothing is ever queued; Godot rewrites the +file itself, so seeding it decides only the run in which it did not exist yet. +The engine-side fix is a null check in that one function, so a future Godot may +make the seeding redundant — it stays either way, being what makes the first run +deterministic. Not test-only: anything running `godot --headless --import` over +a project whose `.godot/` has never seen the extension meets it. ## Releases From 22da6155bef9e357158635fd94ee71163089aef6 Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Fri, 28 Aug 2026 16:40:04 +0900 Subject: [PATCH 5/5] docs: the ERROR lines in a green headless run are the dummy rasteriser's "Condition !actions.custom_samplers.has(...) is true. Continuing." shows up once per player and reads like a defect. It is not one. RendererDummy::MaterialStorage builds its shader compiler with a default-constructed DefaultIdentifierActions, so custom_samplers is empty, while the real canvas renderer fills it with TEXTURE and its siblings. ss_blur.fs passes the builtin TEXTURE into ss_input_texture(), so headless cannot resolve a sampler for it and says so through ERR_CONTINUE -- compilation still returns OK. The same suite under a real renderer prints none of them, with the same 35 cases and 101 assertions. --- AGENTS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 8b8c0f4..8d84629 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,6 +107,18 @@ not share is a layer of `#ifdef SPRITESTUDIO_GODOT_EXTENSION` adapters, which are includes and type conversions — so the module build's guard is that it still builds. +**A green run still prints `ERROR:` lines, and they belong to that dummy +rasteriser.** `Condition "!actions.custom_samplers.has(...)" is true. Continuing.` +comes from Godot's shader compiler, once per player. +`RendererDummy::MaterialStorage` validates every shader through a compiler built +with a default-constructed `DefaultIdentifierActions`, so its `custom_samplers` +table is empty; the real canvas renderer fills that table with `TEXTURE` and its +siblings. `ss_blur.fs` passes the builtin `TEXTURE` into `ss_input_texture()`, +which headless therefore cannot resolve a sampler for. `ERR_CONTINUE` is not a +failure — compilation still returns OK — and the same suite under a real +renderer prints none of them. The run's verdict is the RESULT line and the +marker after it, not the absence of `ERROR:` in the log. + Cases step with `advance()` under `ANIMATION_PROCESS_MANUAL`, never the frame clock, so a result does not depend on how long a frame took. A case that cannot run on this host declares a **skip**, which is reported apart from the passes