fix(gdscript): resolve class_name, scene and autoload references - #731
Open
XenuIsWatching wants to merge 4 commits into
Open
fix(gdscript): resolve class_name, scene and autoload references#731XenuIsWatching wants to merge 4 commits into
XenuIsWatching wants to merge 4 commits into
Conversation
The GDScript dependency graph only followed `preload("res://x.gd")` and
`extends "res://x.gd"`. Godot code is rarely linked that way: scripts refer to
each other by their global `class_name`, and are attached to nodes through a
scene's `[ext_resource type="Script"]` block or a `project.godot` autoload
entry. None of those were resolved, so on a real project essentially every
`.gd` file reported zero importers.
`_find_project_root` also searched upward only. A Godot project is commonly one
directory inside a larger repository (engine plugins, build tooling and CI
beside it), and the scan root is that repository — so no `project.godot` was
found, every `res://` path failed to resolve, and even the preload edges that
were implemented produced nothing.
Measured on a 384-file Godot project: orphaned findings drop from 402 (one per
file, plus a matching false test_coverage finding each) to 7 genuine ones.
- follow `class_name` declarations and their use sites as graph edges, with
comments and string literals stripped first so a name mentioned in prose does
not fabricate a dependency
- add `find_gdscript_dynamic_imports`, reporting scripts a scene or autoload
attaches, via the existing `dynamic_import_finder` orphan hook
- search downward for `project.godot` when no ancestor holds one
- thread `dynamic_import_finder` through `run_coupling_phase`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Godot's project templates and documentation use PascalCase directories, so a project's folders are `Scripts/`, `Scenes/` and `Tests/`. The zone rules matched only the lowercase spellings, so on a real project every suite in `Tests/` was classified production and scored as if it were shipping code, while the one file classified as a test was `test_scene_builder.gd` — production code that builds an in-game scene and merely starts with the Python/JS `test_` prefix. That prefix is not a Godot convention and is dropped; a Godot suite is `<name>_tests.gd`, `<name>_test.gd` or `<name>_suite.gd` inside a tests directory. Test-to-source mapping had the same import blindness as the dependency graph: it read only `preload`/`extends`, but a suite names what it exercises by global `class_name`, so nothing mapped and every file read as uncovered. Measured on a 384-file Godot project: test health 0.0% -> 42.1%, with the 23 suites now recognised as tests and credited against what they cover. - match Godot-cased test directories, and `_tests.gd`/`_suite.gd` suffixes - resolve a bare `class_name` in `resolve_import_spec` via a cached registry built from the production file set - report class names from `parse_test_import_specs`, with comments and strings stripped so a class merely described in prose is not counted as covered Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolving class_name references made the graph correct but reported the new edges as import cycles, including one spanning 86 files. A class_name reference cannot cycle at load time: Godot resolves the global registry lazily, so two scripts naming each other — a plug and its port, a cable and its socket — is ordinary and correct. Only preload and extends can cycle while parsing. Record class_name edges in `deferred_imports`, which `detect_cycles` already skips, leaving preload/extends cycles reported as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A registry or catalog commonly holds a script path in a data table and load()s
it later:
{"id": "atari_2600", "script": "res://Scripts/Objects/system_models/atari_2600_model.gd"}
No preload or extends pattern can see that, so every model registered this way
reported as orphaned even though the registry that names it is the only thing
that ever instantiates it. Comments are stripped first, so a path discussed in
prose is not counted as a reference.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a real Godot project, the GDScript detectors report essentially every
.gdfile as orphaned.That was 402 of 402 scanned scripts, each with a matching false
test_coveragefinding — 73% of the whole backlog, andTest healthpinned at 0.0%. Files that are obviously live (an autoloaded singleton, a script attached to the main scene) were all flagged.Two causes:
1. The dependency graph models the wrong linkage.
build_dep_graphfollowed onlypreload("res://x.gd")andextends "res://x.gd". Godot code is rarely linked that way. Scripts refer to each other by their globalclass_name:and they reach the running game through a scene's
[ext_resource type="Script" path="res://…"]block or aproject.godot[autoload]entry. None of those were resolved.2.
_find_project_rootsearched upward only. A Godot project is commonly one directory inside a larger repository — engine plugins, build tooling and CI sitting beside it — and the scan root is that repository:With no
project.godotin any ancestor of the scan root, the function fell back to the scan root itself, so everyres://path failed to resolve and even thepreloadedges that were implemented produced nothing.Fix
class_namedeclarations and their use sites as graph edges. Comments and string literals are stripped first, so a class name mentioned in prose does not fabricate a dependency.find_gdscript_dynamic_imports, reporting scripts that a scene (.tscn/.tres) or an autoload entry attaches, wired through the orphan detector's existingdynamic_import_finderhook.project.godotwhen no ancestor holds one, taking the shallowest match.dynamic_import_finderthroughrun_coupling_phase(previously it was reachable only from the TypeScript and Python phase runners).Result
Measured on a 384-file Godot 4.7 project: orphaned findings drop from 402 to 7, and those 7 are genuine — unreferenced model scripts and a leftover controller — so the detector now produces signal instead of noise.
Tests
desloppify/languages/gdscript/tests/test_deps.py— 8 new cases over a fixture repo whose Godot project sits one directory down: project-root discovery in both directions,class_nameandpreloadedges, a class name in a comment/string not becoming an edge, an unreferenced file still reporting zero importers, and scene/autoload scripts being reported as dynamic targets.Full suite: 6871 passed. The 32 failures are pre-existing on
main(review-runner tests that shell out tocodex) — identical counts before and after this change.