diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index d8c23b8..7a5d623 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -36,6 +36,12 @@ "source": "./jenkins", "description": "Observe, operate, and safely diagnose a Jenkins CI/CD server. Read-first; pairs with the Perforce agent's change-commit build loop.", "keywords": ["jenkins", "ci", "cd", "gamedev", "unreal", "unity"] + }, + { + "name": "godot", + "source": "./godot", + "description": "Observe, diagnose, and safely operate a Godot 4 project from the command line - read-first, gamedev-focused. Import-cache doctor, export-preset packing rules, GUT green-run verification, and the GDScript traps no linter catches. Verified against Godot 4.7.1, GDScript only.", + "keywords": ["godot", "godot4", "gdscript", "gamedev", "headless", "export-presets", "gut", "ci"] } ] } diff --git a/README.md b/README.md index a3723e7..376f3a6 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ against our own production depots and builds. | [**unreal**](./unreal) | Observe, diagnose, and safely operate Unreal Engine build/cook/package pipelines and editor automation - read-first, version-matching doctor, cook-log triage, gated `BuildCookRun`/UBT, first-party editor scripting. | Ready (v0.1.0) | | [**lore**](./lore) | Observe, analyze, and safely operate an Epic Games [Lore](https://github.com/EpicGames/lore) VCS (the `lore` CLI, formerly Unreal Revision Control) - read-first, staging/commit/push/sync/branch/merge, gated `obliterate` & history rewrites. | Ready (v0.1.0) | | [**unity**](./unity) | Observe, diagnose, and safely operate a Unity project's CLI build pipeline via the standalone Unity CLI - read-first, Editor-version routing (2022 LTS batchmode vs Unity 6), the batchmode exit-0 trap, gated `unity build`/`test`/`run`, license-vs-auth diagnosis, and the verified Unity 6 Pipeline live-Editor + MCP surface (140 tools, Unity's own confirm/dry_run gates). | Ready (v0.2.0) | -| [godot](./godot) | Godot headless export/build workflows. | Planned | +| [**godot**](./godot) | Observe, diagnose, and safely operate a Godot 4 project from the command line - read-first, `.godot` import-cache doctor, `.uid`/`.import` sidecar hygiene, export-preset packing rules, gated headless import/export, GUT runs with green-run verification, and the GDScript traps no linter catches. Verified against Godot 4.7.1, GDScript only. | Ready (v0.1.0) | | [reddit](./reddit) | Reddit Devvit app workflows - `devvit` CLI playtest/upload/publish and CI integration for games shipped as Reddit apps. | Planned | | [youtube](./youtube) | YouTube Playables packaging and pre-submission readiness checks (no public deploy API - Developer Portal uploads are manual). | Planned | | [**jenkins**](./jenkins) | Observe, operate, and safely diagnose a Jenkins CI/CD server; read-first, built around the Perforce `change-commit` → Jenkins build loop, with gated triggers/aborts and the Script Console refused. | Ready (v0.1.0) | @@ -63,11 +63,12 @@ MIT. See [LICENSE](./LICENSE). Perforce, Helix Core, and P4 are trademarks or registered trademarks of Perforce Software, Inc. Unreal Engine, Epic Games, and Lore are trademarks or registered trademarks of Epic Games, Inc. Unity is a trademark or registered trademark of -Unity Technologies. Jenkins is a trademark of the Jenkins project (a -Continuous Delivery Foundation project). +Unity Technologies. Godot is a trademark of the Godot Foundation. Jenkins is a +trademark of the Jenkins project (a Continuous Delivery Foundation project). ButterStack is not affiliated with, endorsed by, or sponsored by Perforce -Software, Epic Games, Unity Technologies, or the Jenkins project. These marks +Software, Epic Games, Unity Technologies, the Godot Foundation, or the Jenkins +project. These marks are used only to identify the third-party tools these agents observe and operate; naming a tool is not a claim of partnership or endorsement. diff --git a/godot/.claude-plugin/plugin.json b/godot/.claude-plugin/plugin.json new file mode 100644 index 0000000..c9e836f --- /dev/null +++ b/godot/.claude-plugin/plugin.json @@ -0,0 +1,24 @@ +{ + "name": "godot", + "displayName": "Godot Agent", + "description": "Observe, diagnose, and safely operate a Godot 4 project from the command line. A read-first, gamedev-focused Claude Code agent built around the headless CLI: an import-state DOCTOR (the stale .godot class cache that fails whole suites with phantom errors), export-preset packing rules, GUT's exit-green-on-parse-failure trap, and the GDScript language and lifetime traps that no linter catches. Verified against Godot 4.7.1, GDScript only.", + "version": "0.1.0", + "author": { + "name": "ButterStack" + }, + "homepage": "https://github.com/ButterStack/gamedev-agents/tree/main/godot", + "repository": "https://github.com/ButterStack/gamedev-agents", + "license": "MIT", + "keywords": [ + "godot", + "godot4", + "gdscript", + "gamedev", + "headless", + "godot-cli", + "export-presets", + "gut", + "class-cache", + "ci" + ] +} diff --git a/godot/LEARNINGS.md b/godot/LEARNINGS.md new file mode 100644 index 0000000..0543222 --- /dev/null +++ b/godot/LEARNINGS.md @@ -0,0 +1,55 @@ +# Godot Agent - Learnings (running log) + +Companion to [`NOTES.md`](./NOTES.md) (design rationale and validation status). +Two kinds of learning, kept separate: **(A)** findings that shaped what the +skills say, and **(B)** the operational reality of running Godot headlessly in +CI and in worktrees. Full detail lives in the skills; this log records why +they say what they say. No secrets. + +## 2026-08-25 - v0.1.0, distilled from Pilot Light + +Everything below comes from building and shipping Pilot Light, ButterStack's +Godot 4.7.1 shmup, to Android and iOS through CI. + +### A. Agent / skill learnings + +- **Import state outranks everything else in a Godot doctor.** A stale + `global_script_class_cache.cfg` after a branch switch produced 31 failing + tests on a commit that was a green tip of main; one `--headless --import` + took it to 271/271 with no other change. So the doctor leads with cache + state, and the failure-signature table's first row maps "burst of + could-not-find-type errors" straight to it. *(⤳ `godot-observe` §3, §7.)* +- **A green test run needed its own trust model.** Two independent mechanisms + produce a green exit while real failures happen, and they compose: GUT + cannot distinguish a parse failure from a non-GutTest file (exit 0), and + GitHub Actions' default `bash -e {0}` has no `pipefail`, so any `| tee` gate + reports `tee`'s status. Both were live simultaneously across multiple + merges - which is why `godot-test` is a skill. *(⤳ `godot-test` §1, §3.)* +- **Several real defects are invisible to every cheap gate.** A native-method + shadow that killed all collision on device passed `gdlint`, `--import`, a + boot check, and the pure-math suites; only a scene-tree integration test + caught it. Hence the explicit gate-coverage table in `godot-test` §4. +- **Honest non-reproduction is worth recording.** The skills carry only the + reproduced class-cache trigger, not a plausible-sounding explanation that + was quoted but never reproduced. + +### B. Operational / rig learnings + +1. **A first `--import` in a fresh worktree is a git hazard**: roughly 150 + untracked `.uid`/`.import` files that a `git add -A` would sweep into a + feature PR. Hence stage-by-filename and the guard hook. *(⤳ `godot-observe` §4.)* +2. **Import ordering around a merge is destructive.** `--import` before a + merge creates untracked files the incoming branch also carries; git refuses + ("untracked working tree files would be overwritten by merge") and the + merge aborts with **no conflict list**. Import after merging, never before. + *(⤳ `godot-build` §1.)* +3. **`--headless` cannot render, and fails by hanging rather than erroring.** + A scripted screenshot via `SubViewport.get_texture().get_image()` sits at + near-zero CPU forever; no flag fixes it. *(⤳ `godot-build` §4.)* +4. **Only an export proves packing.** Editor and headless runs read the + project directory, not the PCK - a CI-generated plain text file read fine + everywhere except on device, because `export_filter="all_resources"` does + not sweep non-resource files. *(⤳ `godot-observe` §5, `godot-build` §2.)* +5. **Pin the engine.** All of the above was validated against + `barichello/godot-ci:4.7.1`, matching CI exactly; a local result and a + CI result that disagree are usually a version difference. diff --git a/godot/NOTES.md b/godot/NOTES.md new file mode 100644 index 0000000..a5d27b1 --- /dev/null +++ b/godot/NOTES.md @@ -0,0 +1,57 @@ +# Godot Agent - design notes and validation status + +## Why these four skills + +The split follows the same observe / operate / verify shape as the other +plugins, with one addition: + +- **`godot-observe`** is the anchor, as `p4-observe` and `unreal-observe` are in + theirs. Nearly every confusing Godot failure traces back to import state, so + the doctor leads with the `.godot/` cache rather than with the engine binary. +- **`godot-build`** holds anything that writes. `--headless --import` lives here + rather than in observe precisely because it mutates the project. +- **`godot-test`** exists as its own skill because the strongest cluster of + findings is not "how to run tests" but "why a green run is lying", which is + too big to bury in a section of another skill. +- **`godot-gdscript`** is the one departure from the three-skill shape. The + language and lifetime traps are about reading and writing code rather than + operating a rig, and there are enough of them (nine, several of which shipped + as real defects) to stand alone. + +## Validation status + +**Verified against Godot 4.7.1, GDScript only**, on Pilot Light - a vertical +shmup built AI-end-to-end, shipping Android and iOS through CI, with a GUT +suite in the 270-400 test range and the engine pinned via +`barichello/godot-ci:4.7.1`, so the findings come from a reproducible engine. + +### Verified against a real engine and project + +Ten findings, each with a deterministic repro or an on-device confirmation: +the stale-class-cache suite failure (31 failures to 271/271 on one import), +both green-run lies (GUT's silent skip and `tee` under `bash -e {0}`), the +native-method shadow and the unpacked plain non-resource file caught only on +device, the merge aborting with no conflict list, the `--headless` +`get_image()` hang, the dropped `RefCounted` signal target, 4.7's constant +checker, and the timer-residual quantization. Exact strings and numbers live +in the skills and [`LEARNINGS.md`](./LEARNINGS.md). + +### Not verified - do not imply otherwise + +- **Any engine version other than 4.7.1.** Several findings are explicitly 4.7 + static-analyzer or constant-checker rules; none of this has been re-run on a + 4.2/4.3 LTS build, where they may simply not apply. +- **C# / Mono.** Every finding is GDScript. The `.godot` cache and + export-preset behaviors are plausibly engine-general, but untested. +- **Web export.** `godot-build` deliberately says nothing about it; the + validation project has no Web preset, so there is no basis for a claim. +- **Godot 3.x.** Out of scope entirely. +- **Whether the `.uid` sidecar flood generalizes.** Observed on a repo whose + `.gitignore` has no `*.uid`/`*.import` pattern; a project that ignores or + commits them will not see it. + +### Deliberately not vendored + +Pilot Light's boot-check script and allowlist are project-shaped, so the +boot-check *pattern* is described in `godot-test` §4 instead; the shipped +`godot-export-retry.sh` is generic (preset and output path as arguments). diff --git a/godot/README.md b/godot/README.md index 004d516..19cc7b4 100644 --- a/godot/README.md +++ b/godot/README.md @@ -1,10 +1,66 @@ -# Godot Agent (planned) +# Godot Agent -A ButterStack gamedev agent for **Godot** workflows - headless export/build -(`--export-release`), `.import/` and `.godot/` cache handling, and CI integration. +A ButterStack gamedev agent for **Godot 4** - observe, diagnose, and safely +operate a project from the command line: the `.godot/` import cache and the +phantom test failures a stale one produces, `.uid`/`.import` sidecar hygiene, +export-preset packing rules, gated headless import/export, GUT runs and the two +ways a green suite is lying, and the GDScript traps that survive every linter. -> 🚧 **Placeholder - not yet implemented.** Part of the ButterStack gamedev agents -> series. See [`perforce`](../perforce) for the first shipped agent. +> Part of the series of public gamedev agents from ButterStack, alongside +> [`perforce`](../perforce), [`unreal`](../unreal), [`unity`](../unity), +> [`lore`](../lore), and [`jenkins`](../jenkins). + +> **Verified against Godot 4.7.1, GDScript only.** No C#/Mono coverage, and no +> claim about 4.2/4.3 LTS - several findings are explicitly 4.7 rules. Findings +> were validated on Pilot Light, ButterStack's vertical shmup built +> AI-end-to-end in Godot and shipping Android and iOS builds through CI. + +## Skills + +| Skill | Use for | +|---|---| +| **`godot-observe`** | Read-only doctor: engine/project version match, `.godot/` cache state, sidecar hygiene, export presets, log diagnosis, and a failure-signature table. | +| **`godot-build`** | Gated `--headless --import` and `--export-release`/`--export-debug`, the merge-ordering rule, Android/iOS notes, and the headless rendering limits. | +| **`godot-test`** | GUT runs, and the two independent ways a suite exits green while real failures happen. | +| **`godot-gdscript`** | Language and object-lifetime traps that pass lint, import, and boot checks - and in one case shipped. | + +## Commands + +- `/godot-doctor [path]` - read-only project diagnosis +- `/godot-build [import|export] [preset]` - gated import or export +- `/godot-test [path]` - run GUT and verify the result is trustworthy + +## The three findings worth reading first + +1. **A stale `.godot/` class cache fails whole suites with phantom errors.** The + cache is gitignored, does not travel with a checkout, and nothing + invalidates it on a branch switch. One observed case: 31 failing tests on a + commit that was a green tip of main, fixed to 271/271 by a single + `--headless --import` with no other change. Suspect the cache before the + code. +2. **A green GUT run is not evidence the tests ran.** GUT cannot tell "this + script failed to parse" from "this script isn't a GutTest" - it logs one + `Ignoring script ...` line and exits 0. Separately, `tee` without `pipefail` + makes every piped CI gate unfailable. Both were live in a real repo for + multiple merges. +3. **Never shadow a native method.** GDScript has no overloading, so a + same-named script method collides with the native one even at a different + arity. A pool's `get_position(index)` colliding with `Node2D.get_position()` + silently killed all collision detection on device, and no gate caught it. + +## Safety + +A `PreToolUse` hook (`scripts/guard-godot.sh`) hard-blocks removing +`project.godot`/`export_presets.cfg`, deleting `.import` sidecars on their own +(Godot's `.meta` analog), and unscoped `git clean -f`. It deliberately allows +`rm -rf .godot`, which is regenerable and a legitimate fix. + +## Install + +``` +/plugin marketplace add ButterStack/gamedev-agents +/plugin install godot@gamedev-agents +``` --- diff --git a/godot/agents/godot.md b/godot/agents/godot.md new file mode 100644 index 0000000..6e896e9 --- /dev/null +++ b/godot/agents/godot.md @@ -0,0 +1,64 @@ +--- +name: godot +description: > + Use this agent to observe, diagnose, and safely operate a Godot 4 project + from the command line in game development: resolving the engine binary and + matching it against the project's `config/features`, diagnosing the `.godot/` + import cache (whose staleness after a branch switch fails whole test suites + with phantom "could not find type" errors), `.uid`/`.import` sidecar hygiene + and the `git add -A` hazard they create, export-preset packing rules + (`export_filter="all_resources"` does not pack plain non-resource files), + gated `--headless --import` and `--export-release`/`--export-debug`, the + merge-ordering rule that makes an early import abort a merge outright, GUT + test runs and the two independent ways a suite exits green while failing, + and the GDScript language and object-lifetime traps no linter catches. + Invoke when the user mentions Godot, GDScript, a project.godot, an + export_presets.cfg, a .tscn/.tres/.gd file, the .godot cache, .uid/.import + sidecars, GUT, gdformat/gdlint, `--headless`, or asks to import, export, + test, or diagnose a Godot project. Prefer this agent over ad-hoc shell + commands whenever the repo contains a `project.godot`. +tools: Bash, Read, Edit, Write, Grep, Glob +model: sonnet +--- + +# Godot Agent - ButterStack Gamedev Series + +Read-first. Diagnose before you touch anything, and prefer the cheapest command +that answers the question. + +**Verified against Godot 4.7.1, GDScript only.** No C#/Mono coverage, and no +claim about 4.2/4.3 LTS. Several findings are explicitly 4.7 rules. Say which +engine version produced any result you report - a build or test claim without +its engine version is not reproducible. + +## Skills + +| Skill | Use for | +|---|---| +| `godot-observe` | read-only doctor: engine/project version match, `.godot/` cache state, sidecar hygiene, export presets, log diagnosis, failure-signature table | +| `godot-build` | gated `--headless --import` and export, the merge-ordering rule, platform notes, headless rendering limits | +| `godot-test` | GUT runs, and the two ways a green exit is lying | +| `godot-gdscript` | language and lifetime traps that survive lint, import, and boot checks | + +## Operating rules + +1. **Doctor first.** `godot-observe` §1-§3. An engine/project version mismatch + or a stale `.godot/` cache makes every downstream error misleading. +2. **Suspect the cache before the code.** A burst of "could not find type" + errors, or many tests failing with only generic "Unexpected Errors", is a + stale class cache until proven otherwise. One `--headless --import` settles + it. +3. **Import after a merge, never before.** An early import generates the very + sidecars the incoming branch carries and aborts the merge with no conflict + list at all. +4. **Never `git add -A` in a Godot repo.** A first import in a fresh worktree + leaves ~150 untracked `.uid`/`.import` files. Stage by explicit filename. +5. **Do not trust a green test run on its own.** GUT exits 0 on a script it + could not parse, and `tee` without `pipefail` makes every CI gate unfailable. + Check that the suite actually collected what it should have. +6. **Only a real export proves packing.** Editor and headless runs read the + project directory, not the PCK. Label that evidence honestly. +7. **Gate the expensive things.** Show the exact command and its cost estimate + before an export, and wait for confirmation. +8. **No screenshots under `--headless`.** `get_image()` on a viewport texture + hangs forever; there is no flag that fixes it. diff --git a/godot/commands/godot-build.md b/godot/commands/godot-build.md new file mode 100644 index 0000000..2c462c5 --- /dev/null +++ b/godot/commands/godot-build.md @@ -0,0 +1,24 @@ +--- +description: Import or export a Godot 4 project (gated) - headless import, export-release/debug against a preset, with cost shown before running +argument-hint: "[import|export] [preset-name]" +--- + +Drive a Godot import or export for the user, per `$ARGUMENTS`. Use the +`godot-build` skill for exact command forms and `godot-observe` for the +preflight. + +Before running anything: + +1. Run the doctor preflight (`godot-build` §0). Do not proceed past an + engine/project version mismatch without saying so. +2. **If a merge is pending, stop** and apply the ordering rule (`godot-build` + §1): import *after* merging, never before, or the merge aborts with no + conflict list. +3. **Show the exact command and a cost estimate, then wait for confirmation** + before any export. An import is cheap enough to run without ceremony unless + the merge case applies. + +After running: report the exit status, the engine version, and the first real +error lines rather than just "it failed". For an export, state the artifact +path and note that only this export proves packing - an editor or headless run +against the project directory does not. diff --git a/godot/commands/godot-doctor.md b/godot/commands/godot-doctor.md new file mode 100644 index 0000000..0accabc --- /dev/null +++ b/godot/commands/godot-doctor.md @@ -0,0 +1,33 @@ +--- +description: Diagnose a Godot 4 project - engine/project version match, .godot import-cache staleness, .uid/.import sidecar hygiene, export-preset packing +argument-hint: "[path-to-project-dir]" +--- + +Run the read-only Godot doctor for the user, scoped to `$ARGUMENTS` if a +project path is given (else find `project.godot` in the current tree, excluding +`addons/`). Run no import, no export, and mutate nothing. Use the +`godot-observe` skill for exact command forms. + +Check and report, in order: + +1. **Project and engine version** (`godot-observe` §1) - `config_version` and + `config/features` from `project.godot`, against `godot --version`. A + mismatch makes every later error misleading; surface it before anything + else. Note the renderer from `config/features` too. +2. **Engine binary** (§2) - which binary resolved, and whether a pinned + container is available for reproducibility. +3. **Import-cache state** (§3) - **the highest-value check.** Does `.godot/` + exist at all? If the working tree was switched since it was written, say + plainly that any script or test error is untrustworthy until + `--headless --import` has been run, and recommend that as step one. +4. **Sidecar hygiene** (§4) - count untracked `.uid`/`.import` files and how + many are actually tracked. If the count is large, warn explicitly about + `git add -A` sweeping them into a PR. +5. **Export presets** (§5) - preset names, platforms, and `include_filter`. + Flag any runtime-read plain file under `res://` that no `include_filter` + names, since it will be absent on device. + +Close with a short briefing: what is healthy, what is suspect, and the single +cheapest next command. Say "no action" plainly when the project is clean rather +than manufacturing findings. Always state the engine version the report was +produced against. diff --git a/godot/commands/godot-test.md b/godot/commands/godot-test.md new file mode 100644 index 0000000..e838d58 --- /dev/null +++ b/godot/commands/godot-test.md @@ -0,0 +1,25 @@ +--- +description: Run a Godot GUT suite and verify the result is trustworthy - checks for silently skipped scripts and unfailable CI gates +argument-hint: "[test-path-or-empty-for-full-suite]" +--- + +Run the project's GUT suite for the user, scoped by `$ARGUMENTS` if given, and +then **verify the result is real**. Use the `godot-test` skill. + +1. Preflight: if the branch changed since the last import, run + `--headless --import` first (`godot-observe` §3) or the results are + phantom. +2. Prefer a **full-suite** invocation (`-gdir=`). A `-gtest=` run cannot + exercise the suite-integrity check. +3. After the run, do not report "all tests passed" on the exit code alone: + - scan for `Ignoring script ... because it does not extend GutTest` - each + one may be a script that **failed to parse**, and the run still exits 0 + - compare the collected test count against what the suite should contain + - if the run went through a pipe in CI, confirm `pipefail` is set, or the + exit status is `tee`'s and means nothing +4. Recognise `Invalid call. Nonexistent function 'new' in base 'GDScript'` as + "a script failed to compile" (often a native-method shadow escalated by + GUT), not "the class is missing". + +Report the collected-vs-expected count alongside pass/fail, and state the +engine version. diff --git a/godot/hooks/hooks.json b/godot/hooks/hooks.json new file mode 100644 index 0000000..e303828 --- /dev/null +++ b/godot/hooks/hooks.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/scripts/guard-godot.sh" + } + ] + } + ] + } +} diff --git a/godot/scripts/godot-export-retry.sh b/godot/scripts/godot-export-retry.sh new file mode 100755 index 0000000..6f00472 --- /dev/null +++ b/godot/scripts/godot-export-retry.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# +# Bounded-retry wrapper around a Godot export. Mobile exports fail on transient +# dependency resolution often enough to be worth retrying, and rarely enough +# that an unbounded loop just hides a real failure - so this retries a fixed +# number of times with backoff and then gives up loudly. +# +# godot-export-retry.sh [--debug] [--attempts N] +# +# GODOT may be set to the engine binary; defaults to `godot` on PATH. +set -uo pipefail + +GODOT="${GODOT:-godot}" +ATTEMPTS=3 +MODE="--export-release" +PRESET="" +OUT="" + +while [ $# -gt 0 ]; do + case "$1" in + --debug) MODE="--export-debug"; shift ;; + --attempts) ATTEMPTS="${2:?--attempts needs a number}"; shift 2 ;; + -h|--help) sed -n '2,12p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; + -*) echo "unknown flag: $1" >&2; exit 2 ;; + *) if [ -z "$PRESET" ]; then PRESET="$1"; else OUT="$1"; fi; shift ;; + esac +done + +[ -n "$PRESET" ] && [ -n "$OUT" ] || { echo "usage: godot-export-retry.sh [--debug] [--attempts N]" >&2; exit 2; } +[ -f project.godot ] || { echo "no project.godot in $(pwd) - run from the project root" >&2; exit 1; } + +command -v "$GODOT" >/dev/null 2>&1 || { echo "engine binary not found: $GODOT (set GODOT=)" >&2; exit 1; } +echo "engine : $("$GODOT" --version 2>/dev/null | head -1)" +echo "preset : $PRESET" +echo "mode : $MODE" +echo "output : $OUT" + +mkdir -p "$(dirname "$OUT")" + +attempt=1 +while [ "$attempt" -le "$ATTEMPTS" ]; do + echo "--- attempt $attempt/$ATTEMPTS ---" + # NB: no `| tee` here. A pipe would make the exit status tee's, not Godot's, + # which is exactly the trap that made a whole CI gate suite unfailable. + if "$GODOT" --headless "$MODE" "$PRESET" "$OUT"; then + echo "EXPORT OK -> $OUT" + echo "NOTE: this export is the only evidence that packing is correct; an editor or headless run against the project directory does not prove it." + exit 0 + fi + code=$? + echo "attempt $attempt failed (exit $code)" + attempt=$((attempt + 1)) + [ "$attempt" -le "$ATTEMPTS" ] && sleep $(( attempt * 5 )) +done + +echo "EXPORT FAILED after $ATTEMPTS attempts" >&2 +exit 1 diff --git a/godot/scripts/guard-godot.sh b/godot/scripts/guard-godot.sh new file mode 100755 index 0000000..0f1f63f --- /dev/null +++ b/godot/scripts/guard-godot.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# +# PreToolUse(Bash) guard - hard block for catastrophic filesystem operations against +# a Godot project, regardless of how permissions are configured. Backstop in the same +# shape as guard-unity.sh / guard-unreal.sh: even a broad Bash allowlist cannot let +# the agent +# - rm/mv project.godot or export_presets.cfg (the project's identity and its +# packing contract - source-controlled, not `rm` material), +# - delete a *.import sidecar (Godot's analog of Unity's .meta: it carries the +# asset's import settings and UID mapping, and a `git rm -f '*.wav'` glob does +# NOT match the matching '*.wav.import', which is exactly how orphaned sidecars +# get committed), +# - run `git clean -f` unscoped, which deletes tracked content alongside the +# regenerable .uid/.import sidecars it is usually aimed at, +# - write into or delete from a Godot engine install tree (a read-only reference). +# `rm -rf .godot` is explicitly ALLOWED: that cache is regenerable by +# `godot --headless --import` and clearing it is a legitimate fix for a stale +# class cache. Reads and `godot ...` invocations pass through untouched. Heredoc +# bodies are skipped so documentation that merely mentions rm/mv does not +# false-positive. +# +# stdin : PreToolUse JSON { "tool_name": "Bash", "tool_input": { "command": "..." } } +# exit : 0 = allow; 2 = BLOCK (stderr is shown to the model). + +input="$(cat)" + +if command -v jq >/dev/null 2>&1; then + cmd="$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)" +else + cmd="$(printf '%s' "$input" | sed -n 's/.*"command"[[:space:]]*:[[:space:]]*"\(.*\)"[[:space:]]*}.*/\1/p')" +fi + +[ -n "$cmd" ] || exit 0 + +# Strip heredoc bodies: prose that mentions rm/mv must not trip the guard. +scan="$(printf '%s\n' "$cmd" | awk ' + /<<-?[[:space:]]*'"'"'?[A-Za-z_][A-Za-z0-9_]*'"'"'?/ { inhd=1; print; next } + inhd { if ($0 ~ /^[[:space:]]*[A-Za-z_][A-Za-z0-9_]*[[:space:]]*$/) inhd=0; next } + { print } +')" + +block() { + printf 'BLOCKED by guard-godot: %s\n' "$1" >&2 + printf 'This is a hard backstop, not a permissions prompt. If you genuinely need it, ask the user to run it themselves.\n' >&2 + exit 2 +} + +# Only destructive verbs are inspected at all. +printf '%s' "$scan" | grep -qE '(^|[;&|[:space:]])(rm|mv|shred|truncate)([[:space:]]|$)|(^|[;&|[:space:]])git[[:space:]]+(clean|rm)([[:space:]]|$)|(^|[;&|[:space:]])find[[:space:]].*(-delete|-exec[[:space:]]+rm)' || exit 0 + +# 1. The project's identity and packing contract. +printf '%s' "$scan" | grep -qE '(rm|mv|shred)([[:space:]]+-[^[:space:]]+)*[[:space:]]+[^;&|]*\b(project\.godot|export_presets\.cfg)\b' \ + && block "refusing to remove or move project.godot / export_presets.cfg - these are source-controlled project identity, not disposable files." + +# 2. .import sidecars - Godot's .meta analog. +printf '%s' "$scan" | grep -qE '(rm|shred)([[:space:]]+-[^[:space:]]+)*[[:space:]]+[^;&|]*\.import\b' \ + && block "refusing to delete a .import sidecar - it carries the asset's import settings and UID. Deleting it orphans references. If you are removing an asset, remove BOTH the asset and its .import together (a '*.wav' glob does not match '*.wav.import')." +printf '%s' "$scan" | grep -qE 'git[[:space:]]+rm([[:space:]]+-[^[:space:]]+)*[[:space:]]+[^;&|]*\.import\b' \ + && block "refusing 'git rm' on a .import sidecar on its own - remove the asset and its sidecar together, or you will commit an orphan." + +# 3. Unscoped `git clean -f` - deletes tracked content, not just regenerable sidecars. +if printf '%s' "$scan" | grep -qE 'git[[:space:]]+clean[[:space:]]+(-[a-zA-Z]*f[a-zA-Z]*)' \ + && ! printf '%s' "$scan" | grep -qE 'git[[:space:]]+clean[^;&|]*--[[:space:]]+'; then + block "refusing an unscoped 'git clean -f'. Scope it to the regenerable sidecars and verify none are tracked first: git ls-files -- '*.uid' '*.import' && git clean -f -- '*.uid' '*.import'" +fi + +# 4. The engine install tree is a read-only reference. +printf '%s' "$scan" | grep -qE '(rm|mv|shred|truncate)([[:space:]]+-[^[:space:]]+)*[[:space:]]+[^;&|]*(/Godot\.app/|/godot[-_][0-9]|/usr/local/bin/godot\b)' \ + && block "refusing to modify or delete a Godot engine install. Treat the engine as a read-only reference." + +exit 0 diff --git a/godot/skills/godot-build/SKILL.md b/godot/skills/godot-build/SKILL.md new file mode 100644 index 0000000..73b8a2b --- /dev/null +++ b/godot/skills/godot-build/SKILL.md @@ -0,0 +1,141 @@ +--- +name: godot-build +description: > + Gated Godot 4 headless playbooks - `--headless --import` (when it is + required, and the merge-ordering rule that makes it destructive if run + early), `--export-release`/`--export-debug` against export_presets.cfg, + Android and iOS export including the signing failure that looks like a + certificate problem, and the headless rendering limits that make scripted + screenshots impossible. Use whenever actually importing, exporting, or + packaging a Godot project. For read-only diagnosis use `godot-observe`. +--- + +# Godot Build, Import & Export + +`$GODOT` is the engine binary resolved by `godot-observe` §2 - resolve it +first, never guess it. Placeholders use ``. + +**Gating.** An import is cheap (seconds to a couple of minutes); an export is +not, and an export can overwrite artifacts. Show the exact command and its +cost estimate before running anything in §2 onward, and wait for confirmation - +the same show-then-confirm pattern the other plugins use. `--import` (§1) is +routine enough to run without ceremony **except** in the merge case below, +which is genuinely destructive. + +**Scope of verification:** Godot **4.7.1**, GDScript only, validated on a +project shipping Android and iOS through CI. Web export is **not** covered +here - it has not been validated and no claim is made about it. + +--- + +## 0. Preflight - every time + +1. **Doctor** - `godot-observe` §1-§3: project found, engine version matches + `config/features`, and the `.godot/` cache state understood. +2. **Has the branch changed since the last import?** If yes, §1 is mandatory + before trusting any script error or test result. +3. **Is a merge pending?** If yes, read §1's ordering rule before importing. +4. **Capture stderr.** Godot's diagnostics go to stderr; pipe with `2>&1` and + `tee`, but see `godot-test` §3 - `tee` will swallow the exit status unless + `pipefail` is on. + +## 1. `--headless --import` + +```sh +$GODOT --headless --import # run from the project root +``` + +Populates `.godot/`, including `global_script_class_cache.cfg`, and writes a +`.uid`/`.import` sidecar for every script and asset. + +**Run it after:** + +- any branch or commit switch (a stale cache fails whole suites with phantom + errors - `godot-observe` §3) +- adding a `class_name` that other scripts need to see +- **a merge** - always after, never before + +**The merge-ordering rule.** `--import` generates exactly the `.uid`/`.import` +files an incoming branch may also carry. Running it *before* a merge leaves a +working tree full of untracked files that collide with what the merge wants to +create, and git refuses outright: + +``` +error: untracked working tree files would be overwritten by merge +``` + +The `ort` strategy fails and the merge aborts **before producing any conflict +list**, which reads as a confusing, cause-less failure. Recovery: + +```sh +git ls-files -- '*.uid' '*.import' # confirm none are tracked FIRST +git clean -f -- '*.uid' '*.import' # a blind clean -f on a tracked path deletes real content +git merge +$GODOT --headless --import # regenerate after +``` + +The sidecars are regenerable output, not merge input. + +## 2. Export - the preset is the contract + +```sh +$GODOT --headless --export-release "" +$GODOT --headless --export-debug "" +``` + +Preset names come from `export_presets.cfg` (`godot-observe` §5). Two rules +that bite: + +- **`export_filter="all_resources"` does not pack plain non-resource files.** + Anything read at runtime through `FileAccess` that is not an imported + resource needs an explicit `include_filter` entry, or it is simply absent on + device and the feature degrades to whatever its fallback is - silently. +- **Only a real export proves packing.** Editor and headless runs read from the + project directory, not the PCK, so they cannot distinguish "packed" from + "present in the repo". Say so when reporting evidence. + +## 3. Platform notes + +**Android.** Export can fail on transient dependency resolution. A bounded +retry around the export call (rather than an unbounded loop) is the pattern +that held up in CI; `scripts/godot-export-retry.sh` in this plugin implements +it. + +**iOS.** Godot generates an Xcode project which is then archived. A Release +archive failure here is frequently misread: + +> **Verified:** Xcode automatic signing can **create a certificate and then +> still fail on provisioning profiles.** The certificate now existing is not +> evidence that signing succeeded - read the profile error, not the +> certificate state. + +## 4. What headless cannot do + +`--headless` uses a dummy rendering driver with no display or GPU context. +Logic frames still fire, so `process_frame` proceeds and a scene tree runs +normally - but a viewport never actually rasterizes. + +**Consequence: you cannot take a screenshot under `--headless`.** A script that +builds a `SubViewport`, awaits a couple of `process_frame`s and calls +`viewport.get_texture().get_image()` **hangs forever** - no error, no timeout, +an idle process at near-zero CPU waiting on a frame that will never composite. + +There is no flag that fixes this. If in-engine visual ground truth is needed, +use a real editor/runtime instance, not `--headless`. If this pattern is +attempted by accident, `kill -9` it; it will not resolve on its own. + +This is *different* from a headless boot check, which only runs the tree for N +frames and greps stderr - that never asks a viewport for pixels and is +unaffected. + +## 5. Cost expectations + +| Operation | Rough cost | Notes | +|---|---|---| +| `--headless --import` (warm) | seconds | after a branch switch this is mandatory and cheap | +| `--headless --import` (cold/fresh worktree) | a minute or two | also generates ~150 sidecars - `godot-observe` §4 | +| headless boot check (N frames) | seconds | cheap gate; misses anything not constructed at boot | +| export (mobile) | minutes | the only thing that proves packing | + +Report the engine version alongside any result. A build claim without the +version it was produced on is not reproducible. diff --git a/godot/skills/godot-gdscript/SKILL.md b/godot/skills/godot-gdscript/SKILL.md new file mode 100644 index 0000000..77f9f2f --- /dev/null +++ b/godot/skills/godot-gdscript/SKILL.md @@ -0,0 +1,174 @@ +--- +name: godot-gdscript +description: > + GDScript language and object-lifetime traps that no linter catches - native + method shadowing that silently kills a code path on device, closures + capturing by value, signals not retaining a RefCounted target, 4.7's + stricter constant checker and type-inference gaps, float precision through + Vector2, and the timer-residual bug that quantizes any computed interval to + whole physics frames. Use when writing or reviewing GDScript, or when a + defect survived lint, import, and boot checks. +--- + +# GDScript traps + +Each entry here is a defect that **passed** `gdformat`, `gdlint`, and +`--headless --import`, and in several cases shipped. They are grouped by what +they break, not by language feature. + +**Scope of verification:** Godot **4.7.1**, GDScript only. Rules marked *4.7* +are version-specific and may not apply to 4.2/4.3 LTS. + +--- + +## 1. Never shadow a native method - it kills the call path silently + +**GDScript has no method overloading.** A script method with the same name as +*any* method on the base class (`Object`/`Node`/`CanvasItem`/`Node2D`/...) does +not add a signature - it collides with the native one, **even with a different +argument count**, and Godot 4 resolves the call against the *native* signature. + +> **Verified, shipped, and caught only on device:** a pool's +> `get_position(index: int)` collided with `Node2D.get_position()` (zero-arg, +> native). Every call site hit +> `Invalid call to function 'get_position' in base 'Node2D (BulletPool)'. +> Expected 0 argument(s)` on every physics tick, silently killing **all +> collision detection** - score and pickups stuck at zero deep into a stage. +> Fixed by renaming to `bullet_position`. + +**No gate catches this.** `gdlint` has no knowledge of native signatures (it +enforces name shape only), `--import` and a boot check never instantiate the +objects, and pure-math test suites never call the API. Only a scene-tree +integration test that builds the real nodes and exercises the real path catches +it. + +Its two faces (a runtime `Invalid call` normally, a hard compile error under +GUT) are described in `godot-test` §2. + +## 2. Closures capture locals by value + +```gdscript +var hits := 0 +var cb := func(): hits += 1 # mutates the lambda's private copy +# ... later: hits is still 0 +``` + +The lambda snapshots `hits` when it is created. **Reference types behave +differently**: mutating the *contents* of a captured `Array`/`Dictionary`/ +`Object` is visible outside, because both scopes hold the same reference - +reassigning the captured variable itself is not. + +**Practice:** when a callback must record that it ran (call count, last args), +default to an `Array`/`Dictionary` accumulator, never a bare +`int`/`float`/`bool`/`String`. + +## 3. A signal does not keep a `RefCounted` target alive + +`connect()` stores only the target's ObjectID for a **bound-method** `Callable`. +It takes **no reference**. + +> **Verified by headless repro:** a `RefCounted` built as a function-local, +> connected via `some_signal.connect(obj.on_thing)`, and stored nowhere else, +> was freed the moment the function returned. The connection count went +> **1 -> 0** the instant the reference dropped, before the signal ever fired. +> No error, no warning, nothing in stderr - the feature simply never happened, +> for the entire time it shipped. + +A **lambda** would have captured strongly and kept it alive. This is +specifically a method-Callable-on-a-`RefCounted` failure, not a general +"signals don't retain" rule. + +**Practice:** any non-`Node` object a handler needs to stay alive on must be +retained in a **member field**, not a function-local. And see `godot-test` §6 - +a test that calls the handler directly cannot catch this. + +## 4. *4.7* The constant checker rejects cross-class constants and constructors + +```gdscript +const MAP := { OtherClass.SOME_ID: PackedInt32Array([0, 1, 2]) } +# Assigned value for constant ... isn't a constant expression +``` + +**Both halves are rejected**: another class's constant as a key, and a +constructor call as a value. The failure compiles nothing in that script and +**cascades into every dependent script**. + +**Fix pattern:** string-literal keys and plain `Array` literals, plus a test +asserting every key maps to a real id - restoring at runtime the referential +safety the const expression can no longer give you at compile time. + +**Broader lesson: desk-checking GDScript is not parsing it.** This survived +author review and a second review, and was caught only by running against a +real engine binary. + +## 5. *4.7* `:=` cannot always infer, and `preload` types are not `Script` + +- `:=` can fail to infer from `floor()`/`ceil()`, though both return `float`. + Annotate explicitly when inference fails rather than restructuring the math. +- A `const C := preload("res://path.gd")` where the target has **no + `class_name`** is typed as that script's own class pseudo-type - the type that + supports `C.new()` sugar - **not** as `Script`/`Object`. Calling a `Script` + instance method on it is a parse error. Cast at the call site: + `(C as Script).get_script_constant_map()`, leaving the const itself intact. + +## 6. Float precision: `Vector2` components are 32-bit + +A scalar read back out of a `Vector2` field can **silently disagree** with the +same decimal literal written as a plain `float`, because `Vector2` components +round-trip through 32-bit while plain GDScript floats are 64-bit. A computed +plain-float `const` can also miss a decimal literal by 1 ULP with no `Vector2` +involved. Compare with a tolerance; never `==` a float that has been through a +`Vector2`. + +## 7. Repeating timers: accumulate the residual, never assign + +```gdscript +# WRONG - throws away the leftover, quantizing to whole physics frames +_fire_timer -= delta +if _fire_timer <= 0.0: + _fire_timer = INTERVAL / multiplier + +# RIGHT - the negative leftover carries into the next interval +_fire_timer += INTERVAL / multiplier +``` + +Assigning a fresh interval discards the (negative) leftover from the tick that +just fired, so the real period is `ceil(interval / physics_delta) * +physics_delta`, not the interval you computed. + +> **Verified:** at a 0.15s base interval on a 60Hz tick there were only **nine +> reachable cadences** no matter how many upgrade ranks sat between them, and +> the rounding error's sign flipped with rank - so realized ratios could +> *increase* with rank, the opposite of the intended curve. + +This applies to **any** interval derived from a continuous stat rather than +hand-picked to land on frame boundaries. + +**Test shape:** a magic-number frame-count assertion can pass by coincidence. +Drive `_physics_process(delta)` for many ticks and assert the *average* rate +converges to the mathematical ratio within a small tolerance. + +## 8. Layout: a `Control` under a `Node2D` anchors against nothing + +`Control.set_anchors_and_offsets_preset()` resolves against the nearest +ancestor's `CanvasItem.get_anchorable_rect()`, and **`Node2D` returns a +degenerate `Rect2(0,0,0,0)`**. `PRESET_FULL_RECT` under a `Node2D` parent +therefore anchors against nothing: the `Control` keeps its natural min-size at +the origin, and all centering computes against a zero-size rect. + +`CanvasLayer` is **not** a `CanvasItem` (it derives from `Node`), so under a +`CanvasLayer` ancestor Godot falls through to `Viewport.get_visible_rect()` - +usually the behavior actually wanted. + +**Debugging corollary:** centered and left-aligned text render **identically** +inside a zero-width rect. Check the rect's actual size *before* touching +alignment properties. + +## 9. Miscellaneous engine semantics + +- **`Engine.time_scale` does not affect `AudioStreamPlayer` playback speed.** + `get_tree().paused` does, and stops it outright. +- **A `SceneTree`-subclass probe script** must defer real tree-membership work + to the first `_process()` call, not `_initialize()`. +- **A freshly-added `class_name` is invisible** to other scripts in a + `--headless --script` run until an `--import` has registered it. diff --git a/godot/skills/godot-observe/SKILL.md b/godot/skills/godot-observe/SKILL.md new file mode 100644 index 0000000..db333c6 --- /dev/null +++ b/godot/skills/godot-observe/SKILL.md @@ -0,0 +1,159 @@ +--- +name: godot-observe +description: > + Read-only Godot 4 diagnostics - the project + engine DOCTOR (project.godot + and its config_version, the resolved engine binary, and above all the + .godot/ import cache, whose staleness fails whole test suites with phantom + "could not find type" errors), .uid/.import sidecar hygiene, export-preset + packing rules, and build/run LOG diagnosis with a failure-signature table. + Use for inspecting and reasoning about a Godot project WITHOUT importing, + building, or changing it. To run an import/export use `godot-build`; for the + test suite use `godot-test`; for GDScript language traps use + `godot-gdscript`. +--- + +# Godot Observe - read-only diagnosis + +Everything here is **read-only**: it inspects files and logs and runs no +mutating command. `godot --headless --import` *writes* to the project (see §3), +so it lives in `godot-build`, not here. + +**Scope of verification:** every finding below was verified against **Godot +4.7.1** on a real, shipping GDScript project (ButterStack's Pilot Light, a +vertical shmup shipping Android and iOS builds through CI). **GDScript only** - +no C#/Mono coverage. Where a behavior is a 4.7-specific rule, it says so. +Do not represent any of this as verified on 4.2/4.3 LTS; it has not been. + +--- + +## 1. Find the project and its engine version + +```sh +# the project root is the directory holding project.godot +find . -maxdepth 3 -name project.godot -not -path '*/addons/*' + +# engine version the project expects, and its feature tags +grep -E '^config_version|^\[application\]|config/features' project.godot +``` + +`config/features` carries the engine version the project was last saved with +(e.g. `PackedStringArray("4.7", "GL Compatibility")`). `config_version=5` is +Godot 4.x. Compare against the binary you actually have: + +```sh +godot --version # e.g. 4.7.1.stable.official +``` + +A mismatch here makes every later error misleading - resolve it before reading +any log. Note the renderer too: `GL Compatibility` matters for web/mobile +targets and is visible in the same `config/features` array. + +## 2. Resolve the engine binary + +```sh +command -v godot || ls /usr/local/bin/godot /Applications/Godot.app/Contents/MacOS/Godot 2>/dev/null +``` + +For reproducibility, prefer a pinned container over a local install when one is +available - `barichello/godot-ci:` is the image this agent's findings +were validated against, and matching CI's engine exactly removes a whole class +of "works locally" confusion. + +## 3. Import state - the single highest-value check + +**Check this first, before believing any script or test error.** Godot keeps a +generated import cache in `.godot/`, which is gitignored, does **not** travel +with a checkout, and **nothing invalidates it on a branch switch**. + +```sh +ls -la .godot/ 2>/dev/null || echo "NO .godot - project has never been imported here" +grep -c '=' .godot/global_script_class_cache.cfg 2>/dev/null +``` + +`.godot/global_script_class_cache.cfg` maps `class_name` declarations to files. +After switching branches it can still list classes from the branch you left. +Godot resolves those names against files that no longer exist and every script +touching the affected types errors at load. + +> **Verified on 4.7.1:** checking out an older commit in a worktree produced +> **31 failing tests** reporting only generic "Unexpected Errors", plus leaked +> `CanvasItem` RIDs at exit - on a commit that was a green tip of main. One +> `godot --headless --import` took the same commit to 271/271 passing with no +> other change. + +**Rule: treat "missing class_name types" or a burst of unexplained script +errors as a stale cache first, and a code problem second.** The re-import is in +`godot-build` §1. + +Related: a freshly-added `class_name` is **invisible** to other scripts in a +`godot --headless --script` run until an `--import` has registered it. + +## 4. Sidecar hygiene - `.uid` and `.import` + +A first `--import` in a fresh worktree generates a `.uid` sidecar for +essentially every `.gd` script and an `.import` for every asset. On a project +whose `.gitignore` has no `*.uid`/`*.import` pattern, that lands as **~150 +untracked files** that nobody created deliberately. + +```sh +git status --porcelain | grep -cE '\.(uid|import)$' +git ls-files | grep -c '\.gd\.uid$' # how many are actually tracked +``` + +Why an agent must care: this pile is easy to mistake for damage you caused, and +a `git add -A` will sweep ~150 unrelated files into a feature PR. **Stage by +explicit filename, never `-A` or `.`**, and confirm with `gh pr diff +--name-only` rather than trusting local `git status`. + +Two corollaries worth knowing: + +- `.gdignore` only stops **future** imports. It does not retroactively delete + `.import` sidecars a directory already has. +- Moving an already-imported asset with `git mv` needs a manual fix to the + `.import` file's `source_file` entry, or the reimport looks stale. + +## 5. Export presets - what actually gets packed + +```sh +grep -E '^name=|^platform=|include_filter|export_filter' export_presets.cfg +``` + +**`export_filter="all_resources"` does NOT sweep plain non-resource files into +the exported PCK/APK.** Any runtime-read plain file (a `.txt`/`.json`/`.csv` +not imported as a resource) needs an explicit `include_filter` entry. The +editor's own label says so: "Filters to export non-resource files/folders". + +> **Verified on 4.7.1:** a CI-generated `build_stamp.txt` under `res://` read +> fine from the editor and from headless runs against the project directory, +> but without `include_filter="build_stamp.txt"` the on-device +> `FileAccess.open("res://build_stamp.txt")` returned null and the feature +> silently degraded to its fallback. + +**Corollary for reviews: local editor or headless verification cannot prove +packing.** Only a real export (CI artifact or device install) can. Label that +evidence honestly rather than implying an export was tested. + +## 6. Reading logs + +Godot writes diagnostics to stderr. A headless boot that only *runs* the tree +for N frames and greps stderr is a cheap, safe gate - but note what it cannot +see: it never instantiates objects that only construct when a run starts, so it +misses whole classes of defect (see §7 and `godot-test`). + +## 7. Failure signatures - from error string to root cause + +Fix the FIRST one; later errors usually cascade. + +| Signature | Root cause | Fix / next step | +|---|---|---| +| A burst of `Could not find type "X" in the current scope`, or many tests failing with only GUT's generic "Unexpected Errors", often with leaked `CanvasItem` RIDs at exit | **stale `.godot/global_script_class_cache.cfg`** after a branch/commit switch - §3 | `godot --headless --import`, then re-run. Suspect this *before* reading the errors as a code defect | +| `Parse Error: Cannot call non-static function "X()" on the class "res://path.gd" directly. Make an instance instead.` | a `const C := preload("res://path.gd")` where the script has no `class_name` is typed by 4.7's static analyzer as that script's own class pseudo-type, not as `Script`/`Object` | cast at the call site: `(C as Script).get_script_constant_map()`. Leave the `preload` const alone for its legitimate `.new()` uses | +| `Assigned value for constant ... isn't a constant expression` | **4.7's stricter constant checker** rejects another class's constant as a key and a constructor call (e.g. `PackedInt32Array(...)`) as a value inside a top-level `const` | use string-literal keys and plain `Array` literals, and restore the lost referential safety with a test asserting each key maps to a real id | +| `Invalid call to function 'X' in base 'Node2D (Y)'. Expected 0 argument(s)` at runtime, every tick | a script method **shadows a native base-class method** - GDScript has no overloading, so a same-named method collides rather than adding a signature | rename to a non-colliding name. See `godot-gdscript` - no linter catches this | +| `untracked working tree files would be overwritten by merge`, strategy `ort` fails, merge aborts with **no conflict list** | `--import` was run **before** a merge, generating the very `.uid`/`.import` sidecars the incoming branch also carries | `git clean -f -- '*.uid' '*.import'` (verify none are tracked first), merge, then re-import. Import *after* merging, never before | +| A `--headless -s script.gd` run that sits idle forever at near-zero CPU after calling `get_image()` on a `SubViewport` texture | `--headless` uses a dummy rendering driver; the viewport never rasterizes, so the render target never signals ready and `get_image()` blocks forever | there is no flag that fixes this - do not scripted-screenshot under `--headless`. `kill -9`; it will not resolve on its own. See `godot-build` §4 | +| `load()` returns `null` at runtime on a path constant, after a clean merge with **zero conflicts** | a stale path constant from a branch that predated an asset-layout change won without a fight - zero conflicts means only one side touched the file, not that the result is correct | grep the merged tree for retired path strings explicitly; cover every path constant in a test | + +**Exit codes:** Godot's own exit status is only as trustworthy as the harness +around it. See `godot-test` for the two ways a green exit hides a real failure +(GUT's silent skip, and `tee` swallowing pipe status in CI). diff --git a/godot/skills/godot-test/SKILL.md b/godot/skills/godot-test/SKILL.md new file mode 100644 index 0000000..bc50404 --- /dev/null +++ b/godot/skills/godot-test/SKILL.md @@ -0,0 +1,153 @@ +--- +name: godot-test +description: > + Running and trusting a Godot 4 test suite (GUT) - the two independent ways a + run exits GREEN while real failures are happening (GUT silently skipping a + script that failed to parse, and `tee` swallowing the exit status in CI), the + suite-integrity canary that closes the first hole, what a headless boot gate + can and cannot catch, and how to write tests that survive being run outside a + scene tree. Use when running, reading, or trusting Godot test results. +--- + +# Godot Test - and why a green run may be lying + +The central lesson of this skill: **on a Godot project, "all tests passed" is +not by itself evidence that the tests ran.** Two independent mechanisms produce +a green exit while real failures are happening, and they compose. + +**Scope of verification:** Godot **4.7.1** with GUT, GDScript only, on a suite +of ~270-400 tests in CI and locally. + +--- + +## 1. GUT exits green on a script it could not parse + +GUT's `test_collector.gd` **cannot distinguish "this script failed to +load/parse" from "this script legitimately isn't a GutTest"**. `_parse_script()` +loads the file; if the load fails *or* the loaded script doesn't inherit +`GutTest`, `add_script()` logs one line and moves on: + +``` +Ignoring script because it does not extend GutTest +``` + +A real `SCRIPT ERROR: Parse Error` at load time and a stray helper file someone +dropped in `tests/` are **byte-for-byte identical** in the run summary, and the +run still **exits 0**. + +> **Verified:** an entire 11-test integration file went uncounted in every +> "all tests passed" report, local and CI, for multiple merges - because nobody +> reads WARNING lines in a green run. + +**Antidote - a suite-integrity canary.** Add a test that lists `test_*.gd` on +disk and diffs it against the scripts GUT actually collected for the current +run: + +```gdscript +# fails loudly, by name, on any mismatch +var on_disk := +var collected := gut.get_test_collector().scripts.map(func(s): return s.get_filename()) +assert_eq(on_disk_not_in(collected), [], "tests silently skipped") +``` + +It only has full power in a **full-suite** run (`-gdir=res://tests`). Invoked +standalone with `-gtest=`, it correctly reports every other file as missing, +because only one file was asked to run - so gate on the full-suite invocation. + +Demonstrated live: a deliberately unparseable file dropped into `tests/` made +the canary fail with `["test_zzz_broken.gd"] != []` and the suite exit 1; +removing it restored a clean green run. + +## 2. GUT escalates a native-override warning into a hard error + +A script method that shadows a native base-class method (see `godot-gdscript`) +behaves differently depending on how it is loaded: + +- **Normal boot/export:** a non-fatal parse *warning* ("The method ... overrides + a method from native class ... This won't be called by the engine"). The + script loads; the failure only appears at runtime as an `Invalid call`. +- **Under GUT** (`addons/gut/warnings_manager.gd`): the same warning is + escalated to a **hard compile error** ("Warning treated as error"). The whole + script fails to load and every dependent test fails with a cascading, + confusing `Invalid call. Nonexistent function 'new' in base 'GDScript'`. + +So the *same defect* reads as a runtime bug in one path and a nonsense +constructor error in the other. Recognise the `Nonexistent function 'new' in +base 'GDScript'` shape as "a script failed to compile", not "the class is +missing". + +## 3. `tee` makes every CI gate unfailable + +GitHub Actions' default `run:` shell is `bash -e {0}` - **`-e` only, no +`pipefail`**. A piped command's exit status is the last command's, which is +always `tee`'s `0`: + +```yaml +- run: gdlint src/ 2>&1 | tee -a ci_output.log # ALWAYS green +``` + +> **Verified:** unit tests, `gdformat --check`, `gdlint`, an app-id guard, +> `--headless --import`, a boot check, and GUT could all fail loudly in the log +> while the job reported green. Real lint debt merged green for multiple PRs +> before this was found. + +**Fix**, once, at the workflow level: + +```yaml +defaults: + run: + shell: bash # expands to: bash --noprofile --norc -eo pipefail {0} +``` + +**Before turning this on, check what reads the captured log.** A step that +consumes the piped file may now see it truncated, because the command can fail +before `tee` finishes writing. If nothing consumes it, this is a pure win. + +## 4. What each gate actually catches + +Gates are not interchangeable, and the cheap ones have large blind spots: + +| Gate | Catches | Blind to | +|---|---|---| +| `gdformat --check` / `gdlint` | formatting, name-shape rules | native method signatures; anything semantic | +| `--headless --import` | parse errors at import | broken string paths; anything not constructed | +| headless boot check (N frames) | boot-time script errors | anything that only constructs when a *run* starts | +| GUT full suite | behavior, including broken sprite/audio paths | only what a test actually exercises | + +> **Verified:** a broken sprite/audio path was caught **only** by the GUT +> suite - `gdformat`, `gdlint`, `--import` and the boot check all missed it, +> because those nodes only construct when a run starts. + +And the converse: a collision-killing native-method shadow was caught by +**none** of the existing gates, because the pure-math test suites never called +the pooled APIs at all. Only a scene-tree integration test that builds the real +nodes and calls the real path catches that class of bug. + +## 5. Writing tests that work outside a scene tree + +GUT tests often instantiate a node with `.new()` and never add it to a tree. +Three consequences: + +- **`create_tween()` errors outside the tree.** Guard it: create the tween only + when `is_inside_tree()`, store it, and `if tween != null and tween.is_valid(): + await tween.finished`. With no tree there is no tween, the coroutine never + suspends, and the caller's `await` returns inline - so a test can assert the + finished state on the very next line. **A GDScript function containing `await` + still runs to completion synchronously if it never actually suspends.** No + test-only branches in production code. +- **A node that never entered the tree reports `is_processing_input()` and + `is_physics_processing()` as false**, even with those callbacks defined - + Godot only enables them on tree entry. A test asserting "input was live + before, locked after" must first call `set_process_input(true)` / + `set_physics_process(true)` to restore the in-tree baseline, or the "before" + assertion fails and the "after" one proves nothing. +- **Closures capture by value.** A lambda recording that it ran must accumulate + into an `Array`/`Dictionary`, never a bare `int` - see `godot-gdscript` §2. + +## 6. Testing a signal bridge + +A test that calls a handler method **directly** cannot catch a lifetime bug in +the wiring, because the test's own local variable supplies the strong reference +the bug depends on to hide. Any test covering a signal bridge must build the +wiring exactly as production does, fire the signal from outside, and assert the +effect. See `godot-gdscript` §3 for the underlying failure.