feat(plugins): Controller discovery + ship_from_plugin + two-block scripts

[apotema]

Summary

Step 1 of the Plugin Controllers RFC (flying-platform-labelle#208). Three additive, backward-compatible changes to labelle-assembler:

• Controller discovery & auto-wiring. Generated main now includes a PluginControllers scaffold that discovers pub const Controller = struct { setup, deinit, ... } in each plugin root via comptime @hasDecl. setup fires after scene load; deinit fires on unload. Plugins without a Controller export contribute nothing (no runtime cost).
• ship_from_plugin ConventionDirMode. New variant of ConventionDirMode. Copies <plugin>/<name>/** into the generated build tree (reverse of today's copy_and_scan, which flows <game>/<name>/ → target). Enables plugins to ship their own scanned directories.
• Two-block script ordering. ScriptScanner now emits the game's scripts first (unchanged), then plugin-shipped scripts per-plugin in project.labelle .plugins declaration order. Each plugin lives in its own numeric-prefix namespace — 05_foo.zig in two different plugins is fine; 05_foo.zig twice in the same plugin is a build error. Game-vs-plugin and plugin-vs-plugin prefix collisions are impossible by construction.

The three changes land in two commits:

1. feat(controllers): discover plugin Controller exports and auto-wire setup/deinit
2. feat(plugins): ship_from_plugin mode + two-block script ordering

Every change is opt-in. Existing plugins (labelle-fsm, labelle-pathfinding today) continue to work unchanged — no Controller export, no plugin scripts/ directory, no ship_from_plugin declarations.

Refs Flying-Platform/flying-platform-labelle#210
Refs Flying-Platform/flying-platform-labelle#208

Test plan

• zig build test passes locally.
• Snapshot tests cover Controller-exporting plugin wiring (PLUGIN_CONTROLLERS in test/tests.zig).
• Non-Controller plugins keep working (backward-compat snapshot test).
• ship_from_plugin mode parses and validates end-to-end (plugin_manifest.zig tests).
• copyAndScanAbs copies + scans files and preserves subdirectory structure (script_scanner_tests.zig).
• Two-block script ordering puts game scripts before plugin scripts; .plugins declaration order drives inter-plugin order.
• Per-plugin namespace duplicate-prefix validator: same prefix across plugins is OK, within one plugin errors.
• scanDir ignores .plugin_<name>/ subdirs so plugin entries register through scanPluginDir only.
• Downstream games (flying-platform-labelle, bakery-game) re-generate cleanly against this assembler — to be validated in step 2 PR.

[cursor]

PR Summary

Medium Risk
Touches code generation and script discovery/ordering for plugin-enabled projects, so regressions could break runtime behavior or generated code imports. Changes are additive/guarded but affect core generation paths and CI now runs a new raylib binary check.

Overview
Adds plugin Controller lifecycle wiring to generated main.zig via a new PluginControllers dispatcher that @hasDecl-discovers Controller.setup/deinit per plugin and invokes them during scene load/unload (with loop vs callback backend differences).

Extends plugin manifests with ConventionDirMode.ship_from_plugin, allowing convention directories to be copied/scanned from a plugin’s cached package into the generated target (with validation that scan modes must specify an extension).

Introduces two-block script ordering: plugin-shipped scripts/ trees are copied into scripts/.plugin_<name>/…, scanned as per-plugin namespaces, and sorted after game scripts (ordered by .plugins declaration order); updates identifier sanitization to handle leading . paths. Adds extensive tests, a new examples/plugin-controllers fixture, and CI steps that build and headlessly run it under xvfb, asserting canonical log order.

^{Reviewed by Cursor Bugbot for commit 6a3b4e2. Bugbot is set up for automated code reviews on this repo. Configure here.}

[gemini-code-assist]

Code Review

This pull request introduces a plugin controller system that allows plugins to hook into the game's lifecycle via setup and deinit methods. It also adds a new convention mode, ship_from_plugin, which enables plugins to provide their own scripts and assets that are automatically integrated into the generated build. Key changes include the generation of a PluginControllers dispatcher in main.zig, updates to the script scanner to support plugin-specific namespaces and ordering, and the addition of absolute path scanning utilities. Review feedback suggests refactoring repetitive code generation logic into a helper function, ensuring consistent error handling for plugin resolution, and simplifying complex conditional logic within the script scanner's validation process.

[gemini-code-assist]

The code generation for the setup and deinit functions is very repetitive. You could extract the common logic into a helper function to make writePluginControllersBlock more concise and easier to maintain, especially if more lifecycle methods are added in the future.

For example, you could introduce a helper like writeLifecycleFn:

fn writeLifecycleFn(bw: anytype, comptime name: []const u8, comptime ret: []const u8, comptime try_kw: []const u8) !void {
    try bw.print("    /// Call Controller.{s}(game) on every plugin that declares one.\n", .{name});
    // ... other comments ...
    try bw.print("    pub fn {s}(game: anytype) {s} {{\n", .{name, ret});
    try bw.writeAll("        inline for (_plugin_mods) |mod| {\n");
    try bw.writeAll("            if (@hasDecl(mod, \"Controller\")) {\n");
    try bw.writeAll("                const C = @field(mod, \"Controller\");\n");
    try bw.print("                if (@hasDecl(C, \"{s}\")) {s} C.{s}(game);\n", .{name, try_kw, name});
    try bw.writeAll("            }\n");
    try bw.writeAll("        }\n");
    try bw.writeAll("    }\n\n");
}

// Then in writePluginControllersBlock:
try writeLifecycleFn(bw, "setup", "!void", "try");
try writeLifecycleFn(bw, "deinit", "void", "");

[gemini-code-assist]

The error handling for cache.resolvePlugin is inconsistent. Here, you use catch continue, which silently skips if a plugin can't be resolved. However, in the plugin manifest loading loop (lines 202-203), plugin_manifest.loadOptional calls resolvePlugin with try, which would fail the entire generation process.

For consistency and robustness, it would be better to use try here as well. If a plugin is declared in project.labelle, it should probably be an error if it cannot be found.

        const plugin_src_dir = try cache.resolvePlugin(allocator, plugin, game_dir);

[gemini-code-assist]

The logic to determine same_plugin and same_subdir using labeled blocks is a bit complex and can be hard to read. You can simplify this logic using if and else with optional unwrapping, which would improve readability.

                const same_plugin = if (a.plugin_name) |a_p| (b.plugin_name) |b_p| and std.mem.eql(u8, a_p, b_p) else b.plugin_name == null;
                if (!same_plugin) continue;

                const same_subdir = if (a.subdir) |a_sub| (b.subdir) |b_sub| and std.mem.eql(u8, a_sub, b_sub) else b.subdir == null;

[Copilot]

Pull request overview

Implements step 1 of the Plugin Controllers RFC for labelle-assembler: adds generated PluginControllers discovery/wiring, introduces a ship_from_plugin convention-dir mode, and extends script scanning to support a second “plugin scripts” block ordered by .plugins declaration order.

Changes:

• Generate a PluginControllers scaffold in main.zig and wire setup/deinit into both loop and callback lifecycles.
• Add plugin-shipped scripts support: copy plugin scripts/ into scripts/.plugin_<name>/ and scan them as a separate ordered block.
• Add ship_from_plugin convention-dir mode and supporting copyAndScanAbs helper; expand tests for the new behaviors.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/main_zig.zig	Emits PluginControllers scaffold, wires lifecycle calls, and updates identifier sanitization for .plugin_* script paths.
src/root.zig	Adds ship_from_plugin handling and copies/scans plugin scripts/ into .plugin_<name> namespaces.
src/script_scanner.zig	Tracks plugin provenance per script entry and enforces two-block ordering + per-plugin duplicate-prefix validation.
src/scanner.zig	Adds copyAndScanAbs to support copying/scanning between arbitrary absolute paths.
src/plugin_manifest.zig	Adds ConventionDirMode.ship_from_plugin plus parsing/validation tests (extension required).
test/tests.zig	Adds snapshot-style assertions for generated PluginControllers output and plugin-script import identifiers.
test/script_scanner_tests.zig	Adds tests for two-block script ordering, per-plugin duplicate-prefix scoping, and .plugin_* subtree ignore behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The existence probe opens a directory handle and immediately discards it (_ = cwd.openDir(...)) without closing. This can leak file descriptors when multiple plugins are processed. Store the opened Dir in a variable and defer close() (or use openDir just-in-time inside the copy step and handle error.FileNotFound).

Suggested change

	_ = cwd.openDir(plugin_scripts_src, .{}) catch continue;
	var plugin_scripts_dir = cwd.openDir(plugin_scripts_src, .{}) catch continue;
	defer plugin_scripts_dir.close();

[Copilot]

This doc comment references a finalize() API, but ScriptScanner doesn’t define finalize() (only sortEntries() + validateNoDuplicateOrders()). Either add the method or update the comment to describe the actual required call sequence (e.g. caller must explicitly sort/validate after all scans, or scanPluginDir always sorts/validates internally).

Suggested change

	/// then loop over plugins calling `scanPluginDir`. After all calls,
	/// either call `finalize()` or rely on `scanDir`'s end-of-call
	/// sort+validate — but note that the final sort must see every entry,
	/// so plugins added *after* `scanDir` returned will need `finalize()`
	/// (or a re-sort and re-validate).
	/// then loop over plugins calling `scanPluginDir`. `scanDir()` sorts
	/// and validates at the end of its own call, so if any plugins are
	/// scanned afterward, the caller must explicitly call `sortEntries()`
	/// and then `validateNoDuplicateOrders()` after all scans complete so
	/// the final ordering and duplicate-prefix checks see every entry.

[Copilot]

script_entries is captured via script_scan.getEntries() before this plugin-scripts loop runs (see earlier const script_entries = script_scan.getEntries();). Since getEntries() returns a slice with a fixed length, any later scanPluginDir calls will not be reflected in script_entries, so plugin-shipped scripts won’t be emitted into generated main.zig. Fix by deferring the getEntries() call until after all scanPluginDir calls (or passing script_scan.getEntries() directly into generateMainZigFromTemplate).

[apotema]

Implementation notes for reviewers

A couple of deliberate trade-offs in the `examples/plugin-controllers/` addition that are worth surfacing:

1. Runtime assertion covers `setup` + `tick`, NOT `deinit`

The CI test (`xvfb-run timeout 3 ./game` + `diff -u` against a canonical 11-line log) asserts:

```
[demo-plugin] setup
[game] game-tick frame=1
[demo-plugin] plugin-tick frame=1
[game] game-tick frame=2
[demo-plugin] plugin-tick frame=2
...
[game] game-tick frame=5
[demo-plugin] plugin-tick frame=5
```

`[demo-plugin] deinit` is not part of the runtime assertion because:

• raylib's main-loop guard `windowShouldClose()` can't flip on a hidden window, so the loop doesn't exit naturally.
• Calling `rl.closeWindow()` from inside a tick crashes the same iteration's draw calls.
• The binary is terminated via `timeout 3`, which SIGTERMs before the `defer PluginControllers.deinit(&g)` can run.

The `deinit` wiring itself is covered by the snapshot tests in `test/tests.zig::PLUGIN_CONTROLLERS` — we assert the generated `main.zig` contains the correct `defer` call in both loop (raylib/sdl) and callback (sokol) lifecycle paths. So the codegen is verified; just not the runtime exit path.

Follow-up opportunity: a sokol-backend variant of the example would let CI exercise the deinit runtime path too (sokol's main-loop ownership makes graceful shutdown tractable). Out of scope for this PR; a single-backend example demonstrates the pattern. Tracking at Flying-Platform/flying-platform-labelle#210's follow-up list.

2. New CI dependency: `xvfb` on Linux runner

The `examples-integration` job now needs `xvfb` to run the raylib binary headlessly on ubuntu-latest. The dependency is declared inline in the workflow step but worth calling out here so it's not a stealth infrastructure cost if this project ends up running on non-default runners (self-hosted, ARM, etc.). macOS doesn't need it — the agent verified the whole flow on macOS without xvfb.

If CI flakiness emerges around the xvfb step, a fallback is to run the binary under `LABELLE_HEADLESS=1` (not wired yet) and have the generated main skip raylib's window init entirely — that's a bigger change than this PR warrants.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit e92c48e. Configure here.}

[cursor]

CI references wrong binary name for runtime check

High Severity

The CI runtime log-order check runs ./zig-out/bin/game but the project is named plugin_controllers_demo in project.labelle. The README's local-run instructions correctly reference ./zig-out/bin/plugin_controllers_demo. The generated build.zig names the executable after the project's .name field, so the binary will be plugin_controllers_demo, not game. This will cause the CI step to fail with a "file not found" error every time.

Additional Locations (2)

• examples/plugin-controllers/project.labelle#L14-L15
• examples/plugin-controllers/README.md#L85-L86

 

^{Reviewed by Cursor Bugbot for commit e92c48e. Configure here.}

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The run command uses ./zig-out/bin/plugin_controllers_demo, but the generated build template hardcodes the executable name as game (build_zig.txt uses .name = "game"). Update the example's local run instructions (and any mentions of the binary name) to match the actual output path so the docs work as written.

Suggested change

	timeout 3 ./zig-out/bin/plugin_controllers_demo 2>&1 | head -40
	timeout 3 ./zig-out/bin/game 2>&1 | head -40

[Copilot]

This comment references addPluginBlock below, but there is no addPluginBlock in this file (or nearby). Consider updating the wording to point at the actual implementation (ScriptScanner.scanPluginDir / the plugin-script loop here) to avoid misleading future readers.

Suggested change

	// Scanning is driven by the `ScriptScanner` (see addPluginBlock below),
	// so the duplicate-prefix validator treats each plugin block as
	// independent. Cross-plugin prefix collisions are impossible by
	// construction. Game-vs-plugin collisions are also impossible — the
	// game scripts live under `scripts/` while plugin scripts live under
	// Scanning is driven by the `ScriptScanner` via the plugin-script loop
	// here and the `script_scan.scanPluginDir(...)` call below, so the
	// duplicate-prefix validator treats each plugin block as independent.
	// Cross-plugin prefix collisions are impossible by construction.
	// Game-vs-plugin collisions are also impossible — the game scripts
	// live under `scripts/` while plugin scripts live under

[apotema]

@gemini review

[gemini-code-assist]

Code Review

This pull request introduces plugin controller machinery, allowing plugins to export lifecycle hooks and ship their own scripts via a new ship_from_plugin manifest mode. The implementation includes a comptime dispatcher and updates to the script scanner for plugin-namespaced scripts. Review feedback identifies several improvement opportunities, including the use of errdefer to prevent memory leaks during allocation failures in ScriptScanner, the propagation of directory iteration errors, and the removal of a redundant directory probe.

[gemini-code-assist]

This directory probe is redundant. Both scanner.copyAndScanAbs (called on line 341) and script_scan.scanPluginDir (called on line 352) already handle the case where the source directory does not exist by silently returning or skipping. Removing this avoids unnecessary system calls.

[gemini-code-assist]

If shared_plugin_names.append fails with OutOfMemory, the name_dup allocation is leaked. Using errdefer ensures the memory is freed if the append operation fails.

        const name_dup = try self.allocator.dupe(u8, plugin_name);
        errdefer self.allocator.free(name_dup);
        try self.shared_plugin_names.append(self.allocator, name_dup);

[gemini-code-assist]

Iteration errors from iter.next() are silently swallowed, which can lead to incomplete script scanning being reported as a success. It is better to propagate the error. This would require adding std.fs.Dir.IterateError to the ScanError error set.

[gemini-code-assist]

These allocations are leaked if subsequent operations fail. For example, if allocPrint fails, name_copy is leaked. If addEntryWithPath fails, both name_copy and rel_path are leaked. errdefer should be used to manage these temporary ownerships until they are successfully handed off to the scanner's internal state.

                const name_copy = try self.allocator.dupe(u8, entry.name);
                errdefer self.allocator.free(name_copy);
                const rel_path = try std.fmt.allocPrint(self.allocator, "{s}/{s}", .{ rel_prefix, entry.name });
                errdefer self.allocator.free(rel_path);
                try self.addEntryWithPath(name_copy, null, &.{}, rel_path);

[gemini-code-assist]

Multiple allocations in this block are leaked if subsequent operations fail before ownership is transferred to shared_subdirs or shared_states. Specifically, dir_states is leaked if subdir_path, subdir_name, or the appends fail. subdir_name is leaked if its append fails. errdefer blocks should be used to ensure proper cleanup on failure.