Fix: rewrite relative .path deps when copying local libs (#129) - #1
Merged
Conversation
PR SummaryMedium Risk Overview Adds Reviewed by Cursor Bugbot for commit 983dc14. Bugbot is set up for automated code reviews on this repo. Configure here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds logic to src/deps_linker.zig to rewrite relative .path dependencies in build.zig.zon files when local plugins are hardlinked to a new directory. The review feedback identifies several critical improvements: replacing manual path relativization and normalization with std.fs.path.relative to ensure cross-platform compatibility (specifically for Windows), addressing silent error handling in file operations, and making the ZON parsing logic more robust against varying whitespace.
Comment on lines
+298
to
+340
| fn computeRelativePath(allocator: std.mem.Allocator, from_dir: []const u8, to_path: []const u8) ![]u8 { | ||
| const from_norm = try normalizePath(allocator, from_dir); | ||
| defer allocator.free(from_norm); | ||
| const to_norm = try normalizePath(allocator, to_path); | ||
| defer allocator.free(to_norm); | ||
|
|
||
| var from_parts = std.ArrayList([]const u8){}; | ||
| defer from_parts.deinit(allocator); | ||
| var to_parts = std.ArrayList([]const u8){}; | ||
| defer to_parts.deinit(allocator); | ||
|
|
||
| var from_iter = std.mem.splitScalar(u8, from_norm, '/'); | ||
| while (from_iter.next()) |part| { | ||
| if (part.len > 0) try from_parts.append(allocator, part); | ||
| } | ||
| var to_iter = std.mem.splitScalar(u8, to_norm, '/'); | ||
| while (to_iter.next()) |part| { | ||
| if (part.len > 0) try to_parts.append(allocator, part); | ||
| } | ||
|
|
||
| var common: usize = 0; | ||
| while (common < from_parts.items.len and common < to_parts.items.len) { | ||
| if (!std.mem.eql(u8, from_parts.items[common], to_parts.items[common])) break; | ||
| common += 1; | ||
| } | ||
|
|
||
| var rel = std.ArrayList(u8){}; | ||
| const ups = from_parts.items.len - common; | ||
| for (0..ups) |j| { | ||
| if (j > 0) try rel.append(allocator, '/'); | ||
| try rel.appendSlice(allocator, ".."); | ||
| } | ||
| for (common..to_parts.items.len) |j| { | ||
| if (rel.items.len > 0) try rel.append(allocator, '/'); | ||
| try rel.appendSlice(allocator, to_parts.items[j]); | ||
| } | ||
|
|
||
| if (rel.items.len == 0) { | ||
| try rel.append(allocator, '.'); | ||
| } | ||
|
|
||
| return rel.toOwnedSlice(allocator); | ||
| } |
There was a problem hiding this comment.
This function manually implements path relativization using a hardcoded / separator, which will fail on Windows where \ is the standard path separator. Additionally, it reinvents functionality already provided by the Zig standard library. It is recommended to use std.fs.path.relative, which is more robust and handles cross-platform path normalization automatically. Since build.zig.zon files should use / for portability, you can replace any backslashes in the result when running on Windows. Note that adopting this change makes normalizePath redundant.
fn computeRelativePath(allocator: std.mem.Allocator, from_dir: []const u8, to_path: []const u8) ![]u8 {
const rel = try std.fs.path.relative(allocator, from_dir, to_path);
if (comptime @import("builtin").os.tag == .windows) {
for (rel) |*c| if (c.* == '\\') {
c.* = '/';
};
}
return rel;
}
| const zon_path = try std.fs.path.join(allocator, &.{ dest_dir, "build.zig.zon" }); | ||
| defer allocator.free(zon_path); | ||
|
|
||
| const content = std.fs.cwd().readFileAlloc(allocator, zon_path, 256 * 1024) catch return; |
There was a problem hiding this comment.
Errors from readFileAlloc are silently swallowed by the catch return statement. If the file cannot be read (e.g., due to permissions or if it's missing when expected), the function will return successfully without performing any rewrites. It would be better to propagate the error or at least log a warning.
Comment on lines
+245
to
+250
| if (i + 10 <= content.len and std.mem.eql(u8, content[i..][0..8], ".path = ")) { | ||
| const prefix_start = i; | ||
| i += 8; // skip `.path = ` | ||
| // skip whitespace | ||
| while (i < content.len and (content[i] == ' ' or content[i] == '\t')) i += 1; | ||
| if (i < content.len and content[i] == '"') { |
There was a problem hiding this comment.
The parsing logic for .path entries is brittle as it expects an exact string match for .path = . Zig's ZON format allows varying whitespace around the assignment operator (e.g., .path=, .path = ). A more robust approach would be to search for .path, then skip any whitespace and the = character before looking for the opening quote.
Comment on lines
+343
to
+369
| fn normalizePath(allocator: std.mem.Allocator, path: []const u8) ![]u8 { | ||
| var parts = std.ArrayList([]const u8){}; | ||
| defer parts.deinit(allocator); | ||
|
|
||
| var iter = std.mem.splitScalar(u8, path, '/'); | ||
| while (iter.next()) |part| { | ||
| if (part.len == 0 or std.mem.eql(u8, part, ".")) continue; | ||
| if (std.mem.eql(u8, part, "..")) { | ||
| if (parts.items.len > 0) { | ||
| _ = parts.pop(); | ||
| } | ||
| } else { | ||
| try parts.append(allocator, part); | ||
| } | ||
| } | ||
|
|
||
| var result = std.ArrayList(u8){}; | ||
| if (path.len > 0 and path[0] == '/') { | ||
| try result.append(allocator, '/'); | ||
| } | ||
| for (parts.items, 0..) |part, j| { | ||
| if (j > 0) try result.append(allocator, '/'); | ||
| try result.appendSlice(allocator, part); | ||
| } | ||
|
|
||
| return result.toOwnedSlice(allocator); | ||
| } |
There was a problem hiding this comment.
This function is redundant as its functionality is covered by std.fs.path.resolve (which is called internally by std.fs.path.relative). If the suggestion to use std.fs.path.relative in computeRelativePath is adopted, this function can be removed entirely.
There was a problem hiding this comment.
Pull request overview
Updates the deps hardlinking flow so that when local plugins are copied into .labelle/deps/, any relative .path dependencies inside their build.zig.zon are rewritten to remain valid from the new location.
Changes:
- Adds a post-hardlink pass that scans local plugins’
build.zig.zonfiles and rewrites relative.path = "..."entries based on the new deps location. - Introduces helper functions to compute/normalize relative paths for rewriting.
- Adds unit tests covering relative path computation, normalization, and rewrite/skip behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+106
to
+112
| // Rewrite relative .path deps in local plugins' build.zig.zon files. | ||
| // After hardlinking, the paths still point relative to the original location | ||
| // which is wrong from .labelle/deps/. Resolve each path against the original | ||
| // abs location and recompute the relative path from the new dest location. | ||
| for (cfg.plugins) |plugin| { | ||
| if (!plugin.isLocal()) continue; | ||
|
|
There was a problem hiding this comment.
The rewrite pass only iterates cfg.plugins, so local GUI packages hardlinked via cfg.resolved_gui ("labelle-gui" / "gui-bridge") can still end up with broken relative .path deps after being copied into .labelle/deps/. Consider applying rewriteZonPaths to gui.plugin_dir (and bridge_dir when present) when those paths point to local directories as well.
| const abs = cwd.realpathAlloc(allocator, plugin_path) catch continue; | ||
| defer allocator.free(abs); | ||
|
|
||
| try rewriteZonPaths(allocator, abs, dest); |
There was a problem hiding this comment.
rewriteZonPaths computes a relative path from dest_dir to an absolute abs_target, but dest_dir here is built from target_dir which can be relative (depends on CLI --project-root). That violates computeRelativePath’s assumption (“both paths should be absolute”) and can yield incorrect rewritten .path values. Suggest realpath’ing dest (or ensuring both inputs share the same base) before calling rewriteZonPaths.
Suggested change
| try rewriteZonPaths(allocator, abs, dest); | |
| const abs_dest = cwd.realpathAlloc(allocator, dest) catch continue; | |
| defer allocator.free(abs_dest); | |
| try rewriteZonPaths(allocator, abs, abs_dest); |
Comment on lines
+342
to
+368
| /// Normalize a path by resolving `.` and `..` components. | ||
| fn normalizePath(allocator: std.mem.Allocator, path: []const u8) ![]u8 { | ||
| var parts = std.ArrayList([]const u8){}; | ||
| defer parts.deinit(allocator); | ||
|
|
||
| var iter = std.mem.splitScalar(u8, path, '/'); | ||
| while (iter.next()) |part| { | ||
| if (part.len == 0 or std.mem.eql(u8, part, ".")) continue; | ||
| if (std.mem.eql(u8, part, "..")) { | ||
| if (parts.items.len > 0) { | ||
| _ = parts.pop(); | ||
| } | ||
| } else { | ||
| try parts.append(allocator, part); | ||
| } | ||
| } | ||
|
|
||
| var result = std.ArrayList(u8){}; | ||
| if (path.len > 0 and path[0] == '/') { | ||
| try result.append(allocator, '/'); | ||
| } | ||
| for (parts.items, 0..) |part, j| { | ||
| if (j > 0) try result.append(allocator, '/'); | ||
| try result.appendSlice(allocator, part); | ||
| } | ||
|
|
||
| return result.toOwnedSlice(allocator); |
There was a problem hiding this comment.
computeRelativePath/normalizePath hardcode '/' as the separator and don’t handle Windows drive/UNC paths. Since src_dir comes from realpathAlloc and abs_target from std.fs.path.join, on Windows these will contain \, causing the split/normalize logic to treat the whole path as a single component and rewrite .path incorrectly. Prefer std.fs.path.relative/std.fs.path.resolve (OS-aware), and if ZON should always use /, normalize the final string to forward slashes before writing.
Suggested change
| /// Normalize a path by resolving `.` and `..` components. | |
| fn normalizePath(allocator: std.mem.Allocator, path: []const u8) ![]u8 { | |
| var parts = std.ArrayList([]const u8){}; | |
| defer parts.deinit(allocator); | |
| var iter = std.mem.splitScalar(u8, path, '/'); | |
| while (iter.next()) |part| { | |
| if (part.len == 0 or std.mem.eql(u8, part, ".")) continue; | |
| if (std.mem.eql(u8, part, "..")) { | |
| if (parts.items.len > 0) { | |
| _ = parts.pop(); | |
| } | |
| } else { | |
| try parts.append(allocator, part); | |
| } | |
| } | |
| var result = std.ArrayList(u8){}; | |
| if (path.len > 0 and path[0] == '/') { | |
| try result.append(allocator, '/'); | |
| } | |
| for (parts.items, 0..) |part, j| { | |
| if (j > 0) try result.append(allocator, '/'); | |
| try result.appendSlice(allocator, part); | |
| } | |
| return result.toOwnedSlice(allocator); | |
| /// Normalize a path for ZON output by converting native separators to `/`. | |
| /// Native path resolution/relativization must be done with `std.fs.path`. | |
| fn normalizePath(allocator: std.mem.Allocator, path: []const u8) ![]u8 { | |
| var result = try allocator.dupe(u8, path); | |
| for (result) |*ch| { | |
| if (ch.* == '\\') ch.* = '/'; | |
| } | |
| return result; |
| var i: usize = 0; | ||
| while (i < content.len) { | ||
| // Look for: .path = " | ||
| if (i + 10 <= content.len and std.mem.eql(u8, content[i..][0..8], ".path = ")) { |
There was a problem hiding this comment.
The bounds check i + 10 <= content.len doesn’t match the slice actually read (content[i..][0..8]). This can skip a valid match near EOF and is confusing to maintain. It should be i + 8 <= content.len (or use the literal length).
Suggested change
| if (i + 10 <= content.len and std.mem.eql(u8, content[i..][0..8], ".path = ")) { | |
| if (i + 8 <= content.len and std.mem.eql(u8, content[i..][0..8], ".path = ")) { |
Comment on lines
+287
to
+292
| // Delete the hardlink and write a new file (don't modify the original) | ||
| const cwd = std.fs.cwd(); | ||
| cwd.deleteFile(zon_path) catch {}; | ||
| const file = try cwd.createFile(zon_path, .{}); | ||
| defer file.close(); | ||
| try file.writeAll(result.items); |
There was a problem hiding this comment.
Ignoring deleteFile errors here is risky: if unlinking the hardlink fails (permissions, file locked, etc.), createFile may truncate/rewrite the existing hardlinked file, unintentionally mutating the original package’s build.zig.zon. Recommend handling deleteFile failures (at least propagate non-FileNotFound) and using an exclusive create/write/rename approach to guarantee the hardlink is broken before writing.
Suggested change
| // Delete the hardlink and write a new file (don't modify the original) | |
| const cwd = std.fs.cwd(); | |
| cwd.deleteFile(zon_path) catch {}; | |
| const file = try cwd.createFile(zon_path, .{}); | |
| defer file.close(); | |
| try file.writeAll(result.items); | |
| // Delete the hardlink entry first so we never rewrite the original package file. | |
| const cwd = std.fs.cwd(); | |
| cwd.deleteFile(zon_path) catch |err| switch (err) { | |
| error.FileNotFound => {}, | |
| else => return err, | |
| }; | |
| const tmp_path = try std.fs.path.join(allocator, &.{ dest_dir, "build.zig.zon.tmp" }); | |
| defer allocator.free(tmp_path); | |
| // Best-effort cleanup of a stale temp file from an earlier failed run. | |
| cwd.deleteFile(tmp_path) catch |err| switch (err) { | |
| error.FileNotFound => {}, | |
| else => return err, | |
| }; | |
| var wrote_tmp = false; | |
| defer if (wrote_tmp) { | |
| cwd.deleteFile(tmp_path) catch {}; | |
| }; | |
| const file = try cwd.createFile(tmp_path, .{ .exclusive = true }); | |
| defer file.close(); | |
| try file.writeAll(result.items); | |
| wrote_tmp = true; | |
| try cwd.rename(tmp_path, zon_path); | |
| wrote_tmp = false; |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…deps/ (#129) Local plugins with relative .path dependencies in build.zig.zon would break after being hardlinked into .labelle/deps/, because the paths still resolved relative to the original location. Now the assembler rewrites those paths to resolve correctly from the new location. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
apotema
force-pushed
the
fix/129-rewrite-local-dep-paths
branch
from
April 13, 2026 13:46
64c8cb2 to
983dc14
Compare
There was a problem hiding this comment.
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 983dc14. Configure here.
| }; | ||
| } | ||
| return rel; | ||
| } |
There was a problem hiding this comment.
Asymmetric path resolution may produce wrong relative paths
Low Severity
computeRelativePath resolves .. components in to_path via std.fs.path.resolve but does not resolve from_dir. Since std.fs.path.relative compares path components literally, an unnormalized from_dir with .. segments would produce an incorrect relative path. Current callers pass realpathAlloc results so the bug isn't triggered today, but the asymmetry makes the function silently incorrect for any future caller that passes an unnormalized path.
Reviewed by Cursor Bugbot for commit 983dc14. Configure here.
This was referenced Apr 14, 2026
apotema
added a commit
that referenced
this pull request
May 4, 2026
Follow-up #1 from PR #84. The first commit on this branch (40f646a) got the tests target generating but reused the full exe build flow, so `.labelle/tests/build.zig` carried a vestigial null-backend exe plus a `main.zig` that nothing invoked. This commit makes the trimming explicit: - `generate` and `generateBuildZig` both gain options structs with an `is_tests_target` flag. Adding it via a struct avoids future parameter churn (more knobs are likely once the labelle-cli side starts treating `.labelle/tests/` as the canonical test surface). - `generateTestsTarget` sets the flag when calling `generate`. - When the flag is set: - `generate` skips writing `main.zig` entirely (no exe → no backend lifecycle wiring needed at the build root). - `generateBuildZig` skips the `exe_start..exe_end` sections, the backend-artifact `linkLibrary`, and the GUI bridge wiring. - The `.footer` (`b.installArtifact(exe)` + `run` step) is replaced by a new `.tests_only_footer` that just closes the build function. The `overrideImport` helper is duplicated into the new section because the plugin/gfx/engine wiring above the test step still calls it. The deps section, plugin dep declarations, backend-module wiring, and ECS/GUI module imports all still run — the test compile unit needs the same module graph as the exe target so test files can `@import("components/foo.zig")` and resolve every transitive game import the same way `main.zig` would. Verified end-to-end on flying-platform-labelle: $ labelle generate labelle-assembler: generated .labelle/sokol_desktop/ labelle-assembler: generated .labelle/tests/ $ ls .labelle/tests/main.zig (no such file) $ cd .labelle/tests && zig build test Build Summary: 6/6 steps succeeded; 25/25 tests passed Assembler self-tests: 220/220 (was 219, +1 new case verifying the trimmed build.zig has no `addExecutable`, no `installArtifact`, and no `b.step("run", ...)`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
apotema
added a commit
that referenced
this pull request
May 4, 2026
* feat: emit .labelle/tests/ backend-agnostic test target (#83) Adds a second emission step to `labelle generate` that produces `.labelle/tests/` alongside the user's chosen `<backend>_<platform>` target. The tests target hard-codes `cfg.backend = .null`, so the test compile unit links pure-Zig stub backend modules and no native artifact — `zig build test` runs without X11/GL/Cocoa/etc., on any host, regardless of which backend the project ships. Mechanics: - `generate` gains an optional `target_name_override` parameter. When null (the default), it computes the existing `<backend>_<platform>` name. When set, it uses the override directly. This is how the tests target lands at `.labelle/tests/` instead of the otherwise- awkward `.labelle/null_<platform>/`. - `generateTestsTarget` is a thin wrapper: copies the project config, swaps `backend` to `.null`, calls `generate` with override "tests". - `main.zig` (assembler binary) calls both after parsing project.labelle. Verified locally on flying-platform-labelle: 25/25 tests pass via `zig build test` in `.labelle/tests/` (same as the sokol_desktop target, but without any system-lib install). Known follow-ups (deferred to keep this commit small): - `.labelle/tests/build.zig` still emits a vestigial null-backend exe + main.zig because `generate` always does. Nothing invokes it; trim the template to test-only when override is set. - labelle-cli's `runner.fixFingerprint` only patches the exe target's build.zig.zon. The tests target hits the same Zig fingerprint dance on first build and needs the same patch. CLI should iterate every emitted target dir. - labelle-cli's `labelle test` should prefer `.labelle/tests/` over the active backend dir (cleaner, no project-config lookup needed). Refs #83. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(#83): trim .labelle/tests/build.zig to test-only Follow-up #1 from PR #84. The first commit on this branch (40f646a) got the tests target generating but reused the full exe build flow, so `.labelle/tests/build.zig` carried a vestigial null-backend exe plus a `main.zig` that nothing invoked. This commit makes the trimming explicit: - `generate` and `generateBuildZig` both gain options structs with an `is_tests_target` flag. Adding it via a struct avoids future parameter churn (more knobs are likely once the labelle-cli side starts treating `.labelle/tests/` as the canonical test surface). - `generateTestsTarget` sets the flag when calling `generate`. - When the flag is set: - `generate` skips writing `main.zig` entirely (no exe → no backend lifecycle wiring needed at the build root). - `generateBuildZig` skips the `exe_start..exe_end` sections, the backend-artifact `linkLibrary`, and the GUI bridge wiring. - The `.footer` (`b.installArtifact(exe)` + `run` step) is replaced by a new `.tests_only_footer` that just closes the build function. The `overrideImport` helper is duplicated into the new section because the plugin/gfx/engine wiring above the test step still calls it. The deps section, plugin dep declarations, backend-module wiring, and ECS/GUI module imports all still run — the test compile unit needs the same module graph as the exe target so test files can `@import("components/foo.zig")` and resolve every transitive game import the same way `main.zig` would. Verified end-to-end on flying-platform-labelle: $ labelle generate labelle-assembler: generated .labelle/sokol_desktop/ labelle-assembler: generated .labelle/tests/ $ ls .labelle/tests/main.zig (no such file) $ cd .labelle/tests && zig build test Build Summary: 6/6 steps succeeded; 25/25 tests passed Assembler self-tests: 220/220 (was 219, +1 new case verifying the trimmed build.zig has no `addExecutable`, no `installArtifact`, and no `b.step("run", ...)`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#83): additive deps merge for tests target The previous commits emitted both targets but the second `generate` call (tests, null backend) wiped `.labelle/deps/` because `createDepsLinks` always called `deleteTree(deps_dir)` first. Net effect: after `labelle generate` the exe target's chosen-backend dep (e.g. `labelle-sokol`) was gone, and the next `cd .labelle/<backend>_<platform> && zig build` failed with "unable to open .../deps/labelle-<backend>: FileNotFound". The Examples-integration CI job on PR #84 caught it. Fix: - `createDepsLinks` accepts a `DepsLinkOptions{ recreate: bool }`. Default is true (preserves the original wipe-and-rewrite for the exe target). When false, the dir is kept as-is and links that already exist are skipped (avoids the "destination already exists" hardlink error). - `generateBuildZigZon` exposes the same knob via `BuildZigZonOptions { recreate_deps: bool }` and forwards it. - `root.generate` sets `recreate_deps = !is_tests_target`. The exe target's first pass still wipes (clean slate per generate), and the tests target's second pass adds `labelle-null` next to the pre-existing chosen-backend link. Verified locally on flying-platform-labelle: $ rm -rf .labelle && labelle generate labelle-assembler: generated .labelle/sokol_desktop/ labelle-assembler: generated .labelle/tests/ $ ls .labelle/deps | grep -E 'sokol|null' labelle-null labelle-sokol Existing call sites (test/tests.zig: 13 `generateBuildZigZon` calls) updated to pass `.{}` for the new options struct. Assembler self-tests: 220/220. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#83): address cursor bot review findings Two bugs cursor[bot] flagged on this PR, both validated against flying-platform-labelle (which has a local plugin tree exercising these paths). Without these fixes, projects using local plugins or non-desktop platforms hit the issues. HIGH — deps_linker double-rewrite corrupts local plugin paths: When createDepsLinks runs with `recreate=false` (the tests-target additive pass), the hardlink loop correctly skips already-linked deps, but the rewriteZonPaths block ran unconditionally on every local plugin. The first pass had already rewritten each dest zon's `.path` entries to be relative to `.labelle/deps/<plugin>/`. The second pass re-read those rewritten paths and treated them as if they were original (relative to `abs_src` = the source-tree location), producing paths with one extra `../` segment. End-result: `zig build` against either target dir failed with `FileNotFound: /Users/.../labelle-fsm` (one directory above the correct location). Fix: skip the rewrite block entirely when `!opts.recreate`. The set of local plugins is identical between exe and tests passes — only the bundled backend/ECS deps differ — and bundled deps don't have local `.path` entries, so the additive pass has nothing legitimate to rewrite anyway. MEDIUM — generateTestsTarget omits cfg.platform override: For wasm/ios/android projects, leaving cfg.platform untouched routed the tests target through the cross-compile build template, which produces a build.zig with no test step. The generated dir built cleanly but `zig build test` was a silent no-op. Fix: also set `cfg.platform = .desktop` in generateTestsTarget. Tests are meant to run on the developer's host regardless of what the exe target ships as. Validation: `LABELLE_ASSEMBLER=<this-build> labelle test` against flying-platform-labelle's full local-plugin tree — 11/11 test files pass, including `.labelle/tests/build.zig.zon`'s `zig build test` step which previously failed. The third bot finding (gemini's std.log.err vs std.debug.print style nit) was deliberately not addressed — main.zig's existing convention (per the comment at line 70) is to reserve stdout for `--protocol-version` and route everything else through `std.debug.print` to stderr. Changing the two new lines would break the file's intentional pattern; a wider migration would be a separate refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#83): defer loadBackendTemplate to !is_tests_target branch cursor[bot] follow-up review on PR #84 (commit 9f55201): backend_tmpl is only consumed inside the `if (!is_tests_target)` block that emits main.zig, but loadBackendTemplate ran unconditionally before. For the tests target this loaded backends/null/templates/desktop.txt purely as a side-effect — and would have failed the whole tests-target generate if that template were ever missing from the cache, despite the result going unused. Move the load (and its `defer free`) inside the same guard as its sole consumer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#83): adopt std.log for tests-target status/error lines Address gemini-code-assist's review on PR #84 (re-requested): the tests-target generate's success and failure messages now go through `std.log.info` / `std.log.err` instead of `std.debug.print`. Both default to stderr, so the file's existing stdout-vs-stderr split (stdout reserved for `--protocol-version`, see comment at line 70) is preserved. This is a partial migration — the pre-existing exe-target lines (165, 171) still use `std.debug.print`. Output now shows the std.log `error:`/`info:` prefix only on the new lines; a wider migration of the file is a separate concern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 13, 2026
apotema
added a commit
that referenced
this pull request
May 22, 2026
Phase 1 of RFC-PLUGIN-EVENTS (#174 / flow-codegen#11). Extends the existing GameEvents / AllHookPayloads codegen blocks at `src/main_zig.zig:2689-2697, :2755-2775` to also walk plugin modules with `@hasDecl(plugin, "Events")` — the same convention `Components` / `Systems` / `GizmoCategories` already use. Codegen changes (no new payload, no new dispatcher): - Emits `pub const PluginEvents = blk: { … };` next to `GameEvents` when `cfg.plugins.len > 0`. The block walks each plugin module at comptime and folds every `pub const <name>` inside the plugin's `Events` struct into a tagged-union variant with a plugin-qualified tag: `<plugin>__<event>` (e.g. `box2d__collision_begin`). `.` is not a valid Zig identifier character, so the JSONC on-disk dot form `box2d.collision_begin` resolves to the qualified tag at codegen time. Plugins without a `pub const Events` contribute zero variants (the `@hasDecl` guard makes them no-ops). - Merges `PluginEvents` into the SAME `AllHookPayloads`: `MergeHookPayloads(.{HookPayload, GameEvents, PluginEvents})`. `MergeHookPayloads` already special-cases an empty union, so a plugin set with no `Events` produces a no-op merge — every shipped game keeps building unchanged (verified on `bouncing-ball`, `flying-platform-labelle`, `flows-smoke`). - `GameEvents` and `PluginEvents` are now `pub const` (were module-private) so flow-codegen-emitted hook handler structs (phase 3) can reference them by name. Resolver shape (option (a)): the resolver IS the generated `PluginEvents` decl. Phase 3 will emit Zig handler code that reflects on `@FieldType(PluginEvents, "<tag>")` and `@typeInfo(...).@"struct".fields` — no JSON sidecar to keep in sync, no separate registry file. The dotted JSONC form (`box2d.collision_begin`) → qualified tag (`box2d__collision_begin`) mapping is mechanical (replace `.` with `__`) and can be done by flow-codegen directly without a runtime lookup table. Sanitizes plugin names with non-identifier bytes (e.g. `labelle-imgui` → `labelle_imgui`) for the variant prefix. Duplicate sanitized names would collide on a shared event name; that produces a `MergeHookPayloads` duplicate-field compile error rather than silently overwriting. Flow sort (RFC-PLUGIN-EVENTS O3 — same phase): - `flow_scanner.zig:188` previously hard-coded `.sort_order = null` for every emitted ScriptEntry, parking flows in the alphabetical tail of the script scanner's sort. Worse, the tail was sorted by raw `rel_path` string, so `10_late.flow.jsonc` came before `2_early.flow.jsonc` (`'1' < '2'`). - Now reuses `script_scanner.extractSortOrder` (`:289`, `:306`) on the flow basename — flows pick up the same numeric-prefix convention scripts already use (`01_input.flow.jsonc` before `02_count.flow.jsonc`, unnumbered alphabetical tail). `extractSortOrder` is extension-agnostic so feeding `<stem>` works identically whether the trailing extension is `.zig` or `.flow.jsonc`. - Sorts flow entries before returning, so the merge into `script_entries` lands them in the right order. Tests: - 7 new tests in `test/flow_scanner_tests.zig` (`pub const PluginEvents` and `pub const FlowSortOrder` blocks): pins the codegen shape (PluginEvents decl, qualified-name builder, AllHookPayloads merge, plugin-name sanitization, GameEvents + PluginEvents co-existence) and the sort-order behaviour (numeric before unnumbered, `2_early` before `10_late`, mixed sort). - Generated Zig is parsed with `std.zig.Ast.parse` to catch syntax regressions in the `@Union` / `@Enum` / `comptime var` arrangement. - `bouncing-ball` end-to-end build verified (with and without a synthetic `pub const Events` on the local labelle-box2d checkout) to confirm the comptime walk works against a real plugin module. `zig build` + `zig build test` green (448/452 tests pass, 4 pre-existing skipped). `flows-smoke` example regenerates and compiles unchanged. Out of scope (later phases): - labelle-box2d declaring `pub const Events` — phase 2 (labelle-box2d#8). - flow-codegen new-form `OnEvent` resolution — phase 3 (flow-codegen#12). - Wiring flow handler structs into `GameHooks` — phase 4 (#175). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
apotema
added a commit
that referenced
this pull request
Jun 20, 2026
…888 (FP#549) Closes the #1 real-device gap: the prior ByteBuffer path only converted NV12 + I420, so the many handsets that emit COLOR_FormatYUV420Flexible / tiled vendor formats would black-screen (the emulator's NV12 hid this). - android.zig: the decoder now renders into a YUV_420_888 AImageReader and reads frames via the AImage plane API. The decoder normalizes whatever vendor/tiled format it produced into Y/U/V planes with row+pixel strides, so a single converter covers every device. Replaces the color-format switch + ByteBuffer read. PTS now comes from AImage_getTimestamp. - yuv.zig: yuv420ToRgba — generic stride-driven YUV 4:2:0 -> RGBA (pixel_stride 1 = planar, 2 = semi-planar; one loop handles both/Flexible). Host tests prove it matches the dedicated NV12/I420 converters (5/5). Re-verified on the Pixel-7 API-34 emulator via the apk harness: extract -> AMediaCodec -> AImage YUV_420_888 -> RGBA, identical pixel output, 10 frames PASS (13 tries, fewer than the old path's 37). Host compile + desktop example still build; android.zig 341 lines. Remaining: confirm on a real handset with a Flexible/tiled clip (emulator-only so far), crop-rect, and audio (#306). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apotema
added a commit
that referenced
this pull request
Jun 21, 2026
…/V sync (FP#549 Path A) (#375) * spike(bgfx): ffmpeg decode -> dynamic texture (FP#549 Path A Half 2) Desktop 'decode half': video.zig spawns ffmpeg via libc popen to decode an H.264 mp4 into a looping RGBA8 frame stream, fed into the Half-1 dynamic texture each tick (paced to the clip fps so the 60fps loop never stalls on the pipe). A self-contained test clip is generated at startup, so the demo needs no bundled asset; if ffmpeg is absent it falls back to the plasma. Proves real H.264 -> RGBA -> bgfx display end-to-end on desktop without linking libav or a YUV shader (ffmpeg does demux+decode+pix_fmt rgba). The production decoders (libavcodec desktop / AMediaCodec Android) feed updateTexture identically. Verified: clip generated (256x192 h264), clean 12s run, 0 errors/leaks. video.zig 88 lines, example main.zig 671 (under the 1000-line ceiling). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * spike(bgfx): Android AMediaCodec H.264 decoder + YUV->RGBA (FP#549 Half 2) Starts the production Android decode path for in-engine video: - video/yuv.zig: NV12 + I420 -> RGBA8 conversion (BT.601, stride-aware). Pure Zig, no NDK -> host-unit-tested (3/3 pass). This is the conversion AMediaCodec's ByteBuffer (YUV) output needs before updateTexture. - video/android.zig: AMediaExtractor + AMediaCodec decoder in ByteBuffer mode, hand-declared against the real NDK C ABI (no JNI). Demux -> select video track -> configure HW decoder -> pump input/drain output -> YUV->RGBA. comptime-gated to the Android ABI (Unsupported stub elsewhere), so host/ desktop builds never reference NDK symbols. Verification: type-checks + compiles to an aarch64-linux-android object (externs resolve via the NDK at on-device link); host-compiles clean via the stub; yuv host tests pass. Not run-testable locally (no device) — the ByteBuffer YUV->RGBA path is the portable first cut; Flexible/vendor color formats, crop rect, and the Surface/OES zero-copy path are the next slices (see #549). Files 148 + 278 lines (under the 1000-line ceiling). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * spike(bgfx): on-device decoder test harness + emulator findings (FP#549 Half 2) Native CLI harness that runs the Android VideoDecoder against a real clip on a device/emulator. Built for aarch64-linux-android against the real NDK libmediandk and run on a Pixel_7 API-34 emulator (arm64). On-device result: AMediaExtractor works fully — opens the fd, demuxes, detects video/avc 320x240, negotiates NV12 color. AMediaCodec creation fails from a bare adb-shell exec because the codec service needs a Binder threadpool + JVM/ART context (logcat: 'NdkJavaVMHelper: Failed to get JVM instance'), which only an app process (APK/NativeActivity) provides. So the extractor half is hardware-verified; full codec verification needs an APK — which converges with the Path B Android shell work. Build (off the normal build graph; needs NDK + emulator): zig build-exe test_decode.zig -target aarch64-linux-android.34 \ --libc <ndk-libc.txt> -L<sysroot>/usr/lib/aarch64-linux-android/34 \ -lc -lmediandk -llog Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(video): copy mime before format delete; on-device APK harness verifies decode (FP#549 Half 2) On-device verification of the Android AMediaCodec decoder via a minimal NativeActivity APK, run on a Pixel-7 API-34 emulator (arm64). Result: extract -> AMediaCodec decode -> YUV->RGBA, 10 frames, RESULT PASS. Two things the on-device run exposed and fixed: - **Use-after-free bug in the decoder**: the mime string from AMediaFormat_getString is owned by the format and freed by AMediaFormat_delete; openFd stored the pointer and used it after the format died, so createDecoderByType got a dangling pointer and returned null. Now copied into a local buffer before the format is deleted. (Would have failed in production too — caught only by running it.) - **AMediaCodec needs a real app process** (Binder threadpool + JVM/ART) — a bare adb-shell exec can't create the codec; a NativeActivity can. New apk/ harness (build-verified, off the normal build graph): - native.zig: NativeActivity entry; opens bundled dectest.mp4 asset via AAssetManager -> fd -> VideoDecoder -> logs frames decoded. - decode_shim.h: NDK header @cImport shim (strips nullability quals). - AndroidManifest.xml: hasCode=false NativeActivity, lib_name=decodetest. - build_apk.sh: aapt2 link + add lib + zipalign + apksigner (debug key). Build (needs NDK + emulator; see script): zig build-lib -dynamic -target aarch64-linux-android.34 ... -lmediandk -landroid -llog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): gfx.VideoPlayer wires decoder -> dynamic texture (FP#549 Path A) Promotes in-engine video into the backend proper. gfx.VideoPlayer owns a dynamic texture and a decoder and ties the whole path together: decode -> updateTexture -> drawTexturePro, paced to the clip fps. - video/player.zig: VideoPlayer(Decoder) — generic over the decoder so the same wiring drives ffmpeg (desktop) or AMediaCodec (Android). init() creates the dynamic texture sized to the video; update(dt) decodes+uploads a frame when due; draw(dest) blits it. - video/desktop.zig: the ffmpeg decoder promoted from the example into the backend, with the same width/height/decodeFrame/deinit interface as the Android decoder. - gfx.zig: export VideoPlayer + Desktop/Android decoders. - example: replaced the ad-hoc dynamic-texture/plasma wiring with gfx.VideoPlayer(gfx.DesktopVideoDecoder); deleted example/video.zig. Verified: desktop example builds + runs end-to-end through the backend player (clip decoded into the bgfx texture, clean init/shutdown, 0 leaks). The Android player uses the identical generic wiring over the hardware-verified AndroidVideoDecoder; its full compile rides the CI bgfx-Android build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): VideoPlayer audio via labelle's music API (FP#549 audio) Adds the audio half of in-engine video, reusing labelle's existing streaming audio (audio.zig: loadMusic/playMusic/updateMusic + pure-Zig mixer + miniaudio device on desktop). - player.zig: AudioHooks (start/update/stop) injected into the VideoPlayer, so it drives an audio track in lockstep with the video WITHOUT gfx depending on the audio module. Best-effort A/V sync (both start together). - desktop.zig: generateTestClip now muxes a 440Hz sine audio track; extractAudioWav() pulls the clip's audio to a 48kHz/stereo/s16 WAV (the format loadMusic decodes). - example: extracts the audio, audio.loadMusic, and wires the player's audio hooks to playMusic/updateMusic/stopMusic. Verified on desktop: clip carries an aac audio stream, extracted WAV is pcm_s16le 48000x2, loaded + played through the mixer; clean run, 0 leaks. Android: audio rides the NoopDevice today (#306) so it's silent until AAudio lands; the identical hooks will drive it then. The Android *decode* side would pull PCM from AMediaCodec's audio track (android.zig is video-only so far) — next slice. This is why Path B (platform player) stays the pick for shipping the intro with sound now. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): PTS-accurate A/V sync in VideoPlayer (FP#549) Replaces the fixed-fps video pacing with audio-mastered, PTS-driven frame selection so audio and video stay locked instead of drifting. - audio.zig: musicPositionSeconds(id) — the audio device's real playback position (frame position / sample_rate), the master clock. - player.zig: AudioHooks.clock supplies the master clock. update() accumulates the audio clock's positive deltas into play_time (loop-safe: a negative delta at a loop boundary contributes zero), then presents the frame whose PTS the master has reached — decoding past (dropping) late frames, holding the current one when the next is still future, capped at MAX_CATCHUP_FRAMES per tick. Falls back to dt pacing when no audio clock, still selecting by PTS (handles VFR). First frame decoded in init(). - decoders now return each frame's PTS (seconds) from decodeFrame: android from AMediaCodec BufferInfo.presentation_time_us (the real container PTS); desktop from frame_index/fps (nominal CFR — rawvideo carries no timestamps). - example: wires .clock to audio.musicPositionSeconds. Verified: desktop builds + runs clean (0 leaks, no decode spiral); Android decoder recompiles; yuv host tests pass. Visual lip-sync isn't automatable, but the master-clock + drop/hold logic is the standard player approach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(video): format-agnostic Android decode via AImageReader/YUV_420_888 (FP#549) Closes the #1 real-device gap: the prior ByteBuffer path only converted NV12 + I420, so the many handsets that emit COLOR_FormatYUV420Flexible / tiled vendor formats would black-screen (the emulator's NV12 hid this). - android.zig: the decoder now renders into a YUV_420_888 AImageReader and reads frames via the AImage plane API. The decoder normalizes whatever vendor/tiled format it produced into Y/U/V planes with row+pixel strides, so a single converter covers every device. Replaces the color-format switch + ByteBuffer read. PTS now comes from AImage_getTimestamp. - yuv.zig: yuv420ToRgba — generic stride-driven YUV 4:2:0 -> RGBA (pixel_stride 1 = planar, 2 = semi-planar; one loop handles both/Flexible). Host tests prove it matches the dedicated NV12/I420 converters (5/5). Re-verified on the Pixel-7 API-34 emulator via the apk harness: extract -> AMediaCodec -> AImage YUV_420_888 -> RGBA, identical pixel output, 10 frames PASS (13 tries, fewer than the old path's 37). Host compile + desktop example still build; android.zig 341 lines. Remaining: confirm on a real handset with a Flexible/tiled clip (emulator-only so far), crop-rect, and audio (#306). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(video): apply AImage crop rect (real-device 1080p padding) (FP#549) The decoder output buffer can be padded beyond the display frame (e.g. 1080 -> 1088-tall, or a non-zero crop offset), so sampling from (0,0) of the raw plane would pull alignment padding into the edges. Now reads AImage_getCropRect and offsets each plane to the crop's top-left before converting. Defaults to (0,0) when absent; bounds-guarded. Re-verified on the Pixel-7 API-34 emulator (crop 0,0 there → identical pixel output, 10 frames PASS); host compile + yuv tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(audio): AAudio output device for Android — closes #306 The bgfx audio backend was device-less on Android (NoopDevice): the mixer ran but nothing pulled from it, so all engine audio — including the video's — was silent. Adds a real AAudio output device. - audio_device_android.zig: AAudio (NDK, libaaudio) PCM_I16 stereo 48kHz output stream whose data callback drives the existing pure-Zig mixer — same ensureStarted/stop/framesMixed surface as the desktop miniaudio device, so audio.zig calls through unchanged. Graceful no-op if AAudio can't open. - audio.zig: select the AAudio device on Android (was NoopDevice); expose deviceFramesMixed() for proof-of-life. - apk harness: after the video test, start the device and confirm it pulls frames. Hardware-verified on the Pixel-7 API-34 emulator: device opens, callback fires, 30472 frames mixed in ~0.5s, AAUDIO PASS, clean close. Desktop unaffected (still miniaudio). This unblocks engine audio on Android generally, and video-with-sound specifically — the VideoPlayer's audio hooks now reach a real output device. Remaining for Android video *with its own track*: decode the audio track via AMediaCodec (the decoder is video-only) and feed the mixer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(video): Android audio-track decode → mixer → AAudio (FP#549) Completes Android video WITH its own sound. The decoder was video-only; this decodes the mp4's audio track and feeds labelle's mixer + the new AAudio device. - android_audio.zig: AMediaExtractor + AMediaCodec (ByteBuffer PCM_16) decode of the audio track, then linear-resample to the mixer's 48kHz stereo (the mixer doesn't resample, matching the desktop ffmpeg -ar 48000 -ac 2 path). Whole track decoded up front (fine for an intro). comptime-gated to Android. - audio.zig: loadMusicFromPcm() — register an in-memory PCM buffer as looping music (the in-memory counterpart of loadMusic), for the decoded audio. - apk harness: decode the audio track, play it, confirm AAudio pulled it. Hardware-verified on the Pixel-7 API-34 emulator: AAC 44100 mono → resampled to 48k stereo (96966 frames for the 2s clip) → played via AAudio. Full on-device chain now proven: extract → AMediaCodec (video→AImage, audio→PCM) → resample → mixer → AAudio. Desktop unaffected. Remaining: run the whole VideoPlayer + audio inside the real bgfx-Android game; PTS-sync the Android audio to video; merge/release/wire into FP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): bgfx-Android video app — VideoPlayer drawn on-device (FP#549) The capstone: the full VideoPlayer running inside a real bgfx-Android app, visually verified on the Pixel-7 API-34 emulator. Implements the "phase 4" (#303) full Android .so link the backend's compile-checks deferred. - example/android_video.zig: minimal NativeActivity app (gameInit/gameFrame/ android_main, mirroring templates/android.txt) that opens the bundled intro asset, decodes with the Android AMediaCodec decoder, and draws it via gfx.VideoPlayer -> bgfx dynamic texture. No-op stubs for the gamepad-detection callbacks the engine normally provides (engine-less demo). - build.zig: the android-app step links libgame.so for aarch64-linux-android, reusing the compile-verified module graph + bgfx artifact, adding the EGL/GLESv3 link and an NDK-libc config (the two NDK include roots split across include_dir/sys_include_dir so zig's bundled libc++ finds asm headers). - example/apk_video/AndroidManifest.xml: NativeActivity, lib_name=game. On-device result: bgfx inits on GLES, compiles the sprite shaders, creates the 320x240 RGBA8 dynamic texture, and draws the decoding clip - the testsrc burned-in timecode advances (live playback). End-to-end proven: AMediaCodec -> AImageReader -> YUV->RGBA -> gfx dynamic texture -> bgfx GLES draw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): wire decoded audio + PTS sync into bgfx-Android video app (FP#549) The android_video.zig demo was video-only. Wire its mp4 audio track in so the SAME PTS-accurate A/V sync path that works on desktop runs on Android: - gameInit re-opens the intro.mp4 asset for a fresh fd (the video decoder owns the first one), decodes the AAC track to 48k stereo PCM via android_audio.decodeTrack, loads it with audio.loadMusicFromPcm, and attaches the start/update/stop/clock hooks via player.setAudio. The .clock hook (audio.musicPositionSeconds) is the master clock that drives the player's drop-late/hold-early frame presentation. - build.zig: import the existing audio_mod and a new android_audio_mod (src/video/android_audio.zig, NDK sysroot + mediandk) into the android-app's app_mod, and link libaaudio so the AAudio output device resolves at the .so link. No change to player.zig's sync logic. Verified on the Pixel_7_API_34 emulator: libgame.so links for aarch64-linux-android, bgfx inits on GLES, AAudioStreamBuilder_openStream returns AAUDIO_OK and the stream starts (audio device pumps frames), and the testsrc clip plays with its advancing burned-in timecode (screenshot /tmp/android_pts.png at 00:00:03.875). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPKQDTbMdqjFx56DParQV4 * feat(bgfx): VideoBackend — name -> player handle pool (FP#549 VideoInterface impl) Satisfies labelle-core's VideoInterface so the engine's VideoImpl slot can be the bgfx backend: openVideo(name) resolves the asset by name and builds the right per-platform decoder + VideoPlayer (desktop: assets/<name>, native size via ffprobe; Android: the <name> APK asset via the shell's NativeActivity AAssetManager), returning a handle. update/draw/close/isPlaying/dimensions operate the pool (up to 8 concurrent). So a game plays a clip with just its asset name — the VideoComponent path. - desktop.zig: probe() reads native width/height/fps via ffprobe (so 'just the name' needs no caller-specified size). - gfx.zig: export VideoBackend. Video-only first cut: in-engine audio needs the audio MODULE's mixer, which gfx must not import (would fork a second mixer) — wired via the player's AudioHooks injection seam next (the decode/audio/AAudio path is already proven in the example + bgfx-Android app). Compiles for desktop + aarch64-linux-android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: register VideoComponent for prefabs + backend []const u8 open (FP#549) - main_template: emit `.VideoComponent = engine.core.VideoComponent` in every project's ComponentRegistry, so a VideoComponent is declarable in a prefab/ scene .jsonc and the engine's video system plays it at the entity's position. - bgfx VideoBackend.openVideo now takes []const u8 (matches core's interface), null-terminating internally for the Android asset API. Assembler tests 656/660 (4 skipped); bgfx backend compiles desktop + android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): VideoBackend.drawVideoFullscreen — edge-to-edge background fill (FP#549) Fills the whole framebuffer with the current frame (design dims, setApplyFit false → no aspect pillarbox, like a screen_fill sprite layer; fit toggle bracketed). Satisfies core's drawFullscreen path for background videos. Compiles desktop + aarch64-android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): cover/contain fit math in drawVideoFullscreen (FP#549) cover center-crops the source to the screen aspect (fill, no distortion); contain letterboxes the dest; stretch fills. player.drawRegion is the src→dest seam. Compiles desktop + aarch64-android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bgfx): play-once decode + EOF + replay for engine-driven loop (FP#549) Desktop decoder drops ffmpeg -stream_loop (it hid stream end): plays once, reports eof, and replay() re-spawns ffmpeg. Player tracks ended (via decoder eof) + exposes isEnded/replay; backend isVideoPlaying reflects it + adds replayVideo. So the engine drives loop (replay) and play-once finish uniformly. Android eof/replay are @hasDecl-gated off pending on-device EOS testing (videos hold the last frame, no crash). Compiles desktop + aarch64-android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: check out labelle-core/engine from feat/video-interface for video PR The bgfx-android-build and examples-integration jobs cloned labelle-core and labelle-engine at `ref: main`, which lack the FP#549 in-engine video symbols (VideoInterface/VideoComponent/VideoFit/StubVideo in core, the VideoImpl slot + main.zig.template wiring in engine). The generated main.zig referenced engine.VideoComponent, so both builds failed with: main.zig: error: root source file struct 'root' has no member named 'VideoComponent' Clone core and engine from `feat/video-interface` first, falling back to `main` so this keeps working once the video branches merge. labelle-gfx is unchanged (still ref: main; it needs no video work). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPKQDTbMdqjFx56DParQV4 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apotema
added a commit
that referenced
this pull request
Jun 21, 2026
…cycle hook contract POC'd open question #1 (#377): the run-loop splice isn't a text merge (which can't even span desktop-loop vs mobile-callback entry shapes) — the assembler emits a backend-blind Game (init/frame/deinit hooks); the backend owns its entry point and drives them, composing at the module level. The code splice is largely answered; the residuals are the build-splice (#5) and the full lifecycle ABI (input/resize/suspend-resume + context-loss, the new #1).
apotema
added a commit
that referenced
this pull request
Jun 27, 2026
…cycle hook contract POC'd open question #1 (#377): the run-loop splice isn't a text merge (which can't even span desktop-loop vs mobile-callback entry shapes) — the assembler emits a backend-blind Game (init/frame/deinit hooks); the backend owns its entry point and drives them, composing at the module level. The code splice is largely answered; the residuals are the build-splice (#5) and the full lifecycle ABI (input/resize/suspend-resume + context-loss, the new #1).
apotema
added a commit
that referenced
this pull request
Jun 29, 2026
* docs(rfc): pluggable backends — make the assembler backend-agnostic (#377) Draft RFC synthesizing the design discussion + the runnable POC: promote backends from a closed enum to the open plugin model, behind four versioned comptime contracts (render/input/audio/window) in a new labelle-platform-abi crate, with the GPU context kept package-private at comptime. Captures the per-layer changes, the gfx impact, the codegen-splice as the core remaining work, an incremental migration, and the open questions. * rfc(backends): revision 2 — reframe the codegen-splice as a Game-lifecycle hook contract POC'd open question #1 (#377): the run-loop splice isn't a text merge (which can't even span desktop-loop vs mobile-callback entry shapes) — the assembler emits a backend-blind Game (init/frame/deinit hooks); the backend owns its entry point and drives them, composing at the module level. The code splice is largely answered; the residuals are the build-splice (#5) and the full lifecycle ABI (input/resize/suspend-resume + context-loss, the new #1). * rfc(backends): revision 3 — independently-pluggable contracts; audio worked example Correct the 'one package = four contracts' framing: the four contracts are independently pluggable (audio is the proof — AudioInterface is already in labelle-core, yet bgfx+wgpu each reimplement a WAV mixer because their render lib has no audio). A 'backend' is a composition of per-contract providers, declared as full-stack packages with per-contract overrides. Audio decomposes into a shared labelle-audio mixer + pluggable device sinks (sokol_audio/ miniaudio/...) and is the ideal first extraction (already contracted, zero context-sharing). Packaging: one labelle-backends monorepo + Zig lazy deps (contract granularity ≠ repo count); resolves the monorepo open question. Adds the audio pilot to the migration plan. * rfc(backends): revision 4 — per-contract struct is canonical; render-anchored cascade Make the per-contract struct the canonical declaration (self-documenting) and demote bare `.backend = .sokol` to sugar for `.{ .render = .sokol }`. Document the cascade: render is the anchor (render needs a compatible window → window default), window→input (input ships with the window lib), audio independent (sokol→sokol_audio else miniaudio). You pin render + any slot to override; the defaulting is principled (render⇄window coupling), not a magic table. * rfc(backends): revision 5 — address the rev-4 review + add platform-packaging/manifest Folds in the review findings: - HIGH: render/audio contracts are thicker than draw/play — spell out the asset-loader surface (decodeImage/uploadTexture/unloadTexture/compressed/font, decodeAudio/uploadSound/unloadSound) so a backend can't conform-but-fail. - HIGH: the cascade is platform-qualified — resolver is (platform, render) -> window/input/audio (bgfx Android != GLFW); incompatible overrides are errors. - MED: lazy-deps reframed from a guarantee to a packaging requirement. - MED: new GUI-bridge open question (imgui bridges vs per-contract providers). - New 'Platform packaging & the manifest' section: the (backend x platform) matrix, the manifest schema sketch, window-entry vs shared platform-packager. - Suggestions: crate->package, AudioInterface home, gamepad-as-input-extension, lifecycle-ABI inventory note, stale 'monorepo or per-backend' wording fixed. * rfc(backends): revision 10 — answer all six open questions (#377) Answers all six open questions, grounded in inventories of the shipped codebase. Each answer cites the specific files + line counts it is verified against. Q#1 — the Game-lifecycle ABI (rev 6): the full hook surface is init/deinit/frame(dt)/running/event/suspend_/resume_/contextLost (last four @hasDecl/null-gated). Grounded in an inventory of all seven shipped templates. Per-frame work (screenshot, preview, GUI, setScreenSize) stays codegen, not lifecycle. Residual: contextLost semantics + the Event type shape — both gated on the audio-extraction pilot. Q#2 — where the contracts live + versioning (rev 8): the ABI package IS labelle-core (7 of 8 contracts already live there). Backend(Impl) + its value types relocate from gfx to core. Versioning: a contract_version integer on each contract + a targets_<contract>_version on each backend, asserted at comptime. Q#3 — monorepo (rev 3, resolved): one labelle-backends monorepo + Zig lazy deps per provider. Q#4 — gamepad as an input-extension (rev 9): gamepad is NOT a fifth contract. The three existing sources are packages composed alongside the input provider. The manifest input_extensions field replaces deps_linker.zig staging switches. Q#5 — the build-graph manifest (rev 7): the manifest build-side schema (.modules/.artifacts/.system_libs/.frameworks/.platforms/.build_hook), the core-diamond generalization (8 hand-coded sites to 1 generic walk), the build-hook escape hatch, and lazy native deps as a zon-level requirement. Q#6 — GUI-bridge compatibility (rev 10): bridges keyed by render provider name (not a closed enum). Two integration patterns: external C++ bridge (default) and in-backend adapter (via provider manifest build_options). The with_imgui/gui_enabled flags are replaced by manifest-declared options. render_interface GUIs are unaffected. All residuals are migration-gated, not design-blockers. The RFC is ready to move from Draft to Accepted pending the audio-extraction pilot validating the context-handoff story. * rfc(backends): revision 11 — address rev-10 review (#377) Four fixes from the rev-10 review: 1. Collapse labelle-platform-abi to labelle-core everywhere (CodeRabbit). The migration plan and per-layer-changes section used the old name even after Q#2 established that the ABI package IS labelle-core. Renamed all remaining references; the only historical mention is in Q#2 where the rename is explained. 2. Split the build hook into pre_wire/post_wire (CodeRabbit + apotema). The single wire()-after-generic-wiring contract was known-insufficient: sokol with_imgui is a shipped consumer that must set b.dependency options BEFORE the artifact is built. pre_wire returns DependencyOptions the assembler passes to b.dependency; post_wire supplements the graph after generic wiring (NDK sysroot, emcc shell-out, extra links). Removed the hook-ordering residual from Q#5 open-questions — it is now answered. 3. Reframe the Accept gate to the bgfx-Android pilot (apotema). The audio-extraction pilot (step 2) validates extraction mechanics (contract home, versioning, build-graph wiring, lazy deps) but has ZERO GPU context — it cannot exercise contextLost or the TERM_WINDOW+INIT_WINDOW surface-recreation cycle. The Accept gate is now on the bgfx-Android pilot (step 3), which already has the init_done one-shot guard for exactly this cycle. 4. Fix the contract_version check direction (apotema). The rev-8 check (targets > provided) only caught old-core+new-backend (rare). The dominant ecosystem failure is new-core+old-backend (targets < provided) — which fell through to the raw @CompileError("Backend must define 'foo'") the validator emits, defeating the purpose of versioning. Now gates on strict equality with direction-branched diagnostics: t > p = "upgrade core", t < p = "upgrade backend". Updated the third-party-pinning residual to match. * rfc(backends): revision 12 — address rev-11 review (#378) Five findings from the rev-11 review: 1. High — audio-contract conflation. The RFC attributed the loader surface (decodeAudio/uploadSound/unloadSound(Sound)) to core's AudioInterface. It isn't there: core only contracts runtime playback (playSound/stopSound, optional loadSound(id)/music). The loader is a separate Backend(Impl) in labelle-engine/audio_backend. Split both the contract inventory (audio bullet) and the gfx Backend(Impl) section so the "already contracted" claim is scoped to playback only. 2. Medium — versioning summary contradicted its code sample. Changed the prose "asserts N <= M" to "N == M"; the generated check @compileerrors on both N > M and N < M (N <= M would permit the dominant old-backend-vs-new-core failure the t < p branch rejects). 3. Medium — pilot/migration inconsistency. Q#1 called the GPU-context Accept gate "step 3 — sokol conversion", but sokol-desktop can't exercise TERM_WINDOW/INIT_WINDOW. Split into step 3 (sokol-desktop, extraction mechanics) and a distinct step 4 (bgfx-Android GPU-context gate); renumbered resolver/extract to 5/6 and fixed all cross-refs. 4. Low — refreshed the build_zig.txt line count to 1142. 5. Low — last "crate" → "package"; PR description refreshed separately (labelle-core, six answered questions, bgfx-Android gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * rfc(backends): revision 13 — address rev-12 design review (#378) Seven design points from the rev-12 review: 1. Separate runtime vs asset-loader contracts. Audio was split in rev 12; this names render's two sub-surfaces — draw API (drawTriangle/...) vs asset-streaming/loader (decodeImage/uploadTexture/font decls) — making the split symmetric with audio. 2. Conformance suites per contract. New "Opening the ecosystem" section: each contract ships a shared conformance suite in labelle-core (next to mock_backend), parameterized over the provider Impl, checking behavior (round-trips, event mapping, surface-loss state preservation) — not just @hasDecl shape. A provider is conformant iff it passes the suite for every capability it advertises. 3. Explicit lifecycle. Replaced the single under-specified contextLost with surfaceLost/surfaceRestored, with the engine responding via gpuResourcesInvalidated -> reuploadAssets instead of overloading deinit/init for mobile surface recreation. Only the re-upload granularity stays pilot-gated (bgfx-Android, step 4). 4. Constrain build hooks. Manifest now ~95%, hook ~5%; HookContext/ DependencyOptions are versioned types, hook may read only documented ctx fields and construct build-graph nodes only — no arbitrary FS/ network/shell-out. Extend the manifest, not the hook. 5. Provider identity & collisions. Canonical <namespace>.<name> IDs; labelle.* reserved for the official monorepo and is what the enum shorthands resolve to; collision (or a third party claiming labelle.*) is a hard resolve-time error. The ID is the stable key for GUI bridges and capabilities. 6. Capability negotiation. Providers declare a .capabilities set; the assembler checks project-required capabilities (explicit .requires + derived from platform/target/GUI) before emitting the build graph, erroring with a project-level message instead of a deep @CompileError. 7. Migration pilots — already addressed in rev 12 (audio = extraction mechanics, sokol-desktop = full-stack, bgfx-Android = lifecycle/context gate); status/residual wording updated to match the new hook names. Added open-questions 7-9 (identity, capabilities, conformance) and bumped the status header to revision 13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * rfc(backends): fix stale audio worked-example references (#378) The "Audio — the worked example" section (and migration step 2) still described labelle-audio as "the AudioInterface impl (decode + mix)" and called audio "already contracted (core.AudioInterface)", conflating the two surfaces rev 12 split. Corrected: playback/mix IS AudioInterface (core); decode/upload is the separate audio-loader contract (Backend(Impl) in labelle-engine/audio_backend). "Already contracted" is now scoped to the playback half, with the loader half noted as having a ready home. PR description updated separately (revision 11 → 13; contextLost → surfaceLost/surfaceRestored; rev 12-13 additions summarized). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * rfc(backends): revision 14 — fold Phase 1 & 2 deployment reality Phases 1 & 2 shipped (labelle-core #45 render contract; labelle-audio v0.3.0 shared Mixer(Sink)+DeviceSink i16+f32; bgfx/wgpu/sokol collapsed). Mark them DONE in the migration plan; correct the audio worked-example (raudio/sdl_audio are monolithic engines, NOT shared-mixer device sinks — raylib/sdl delegate decode+ mix and didn't collapse); add the composable-vs-monolithic provider distinction to the resolver (Phase 5); flag the WAV-shared/OGG-backend decode split + the writeAudioBackendWiring codegen as Phase-6 targets; record that audio proved the provider-composition mechanic + no-codegen-change for a module dep but NOT the run-loop splice (Phase 3 remains the gating crux). * rfc(backends): revision 15 — build-splice POC verdict (Q1) A throwaway manifest-driven generation path for sokol-desktop produced byte-identical main.zig+build.zig with no =>.sokol branch in the splice logic, builds + runs (headless screenshot, negative-control verified). Verdict: the build splice is VIABLE; externalizing the embedded build_zig.txt sections was the easy part. Refines the model to 'manifest declarations + fixed assembler-computed params + capability-flag-keyed lifecycle block library' (NOT pure-data) and names the three things that stay code + the name->package registry that is the Phase-5 pluggability seam. * rfc(backends): revision 16 — ACCEPTED (bgfx-Android Accept gate validated) The surfaceLost/surfaceRestored GPU-context-loss gate — the one validation CI structurally can't run — passed on real hardware (Tab A7 / Adreno 610 / Android 12): 9 surface destroy/recreate cycles, hooks fire in order, 0 crashes, assets re-upload, GPU memory plateaus. That was the last gating residual, so the RFC moves Draft→Accepted. Remaining work is Phase-6 implementation, not design. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
…hase 6c) (#429) wgpu is resolved out-of-tree now (`.backend = .wgpu` → the labelle-wgpu provider, flipped in #428), so the in-tree backends/wgpu/ slot is dead weight. Delete it + the assembler example + the coverage that built them in-tree: - Remove backends/wgpu/ (src/templates/build_fragments/manifest/build + example). - Remove examples/wgpu (the assembler-generated wgpu project) — its assembler-builds-a-wgpu-project coverage (incl. the Foundation/QuartzCore/Metal link-fragment regression guard) moved to labelle-wgpu's CI (PR #1). - CI: drop the `wgpu backend WAV parser tests` + `wgpu backend demo build (macOS)` steps and the `Generate + build the wgpu example` examples-integration step. - Tests: drop the desktop wgpu-artifacts unit test (no in-tree package to resolve) and the wgpu case from the in-tree-template preview regression list. Two backends now fully out of the assembler bundle (bgfx + wgpu). The agnostic external-fetch path stays covered by the `external-null` (nullfixture) step.
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
…step 2) (#430) * chore(slim): remove the bundled wgpu backend — it's extracted (#386 Phase 6c) wgpu is resolved out-of-tree now (`.backend = .wgpu` → the labelle-wgpu provider, flipped in #428), so the in-tree backends/wgpu/ slot is dead weight. Delete it + the assembler example + the coverage that built them in-tree: - Remove backends/wgpu/ (src/templates/build_fragments/manifest/build + example). - Remove examples/wgpu (the assembler-generated wgpu project) — its assembler-builds-a-wgpu-project coverage (incl. the Foundation/QuartzCore/Metal link-fragment regression guard) moved to labelle-wgpu's CI (PR #1). - CI: drop the `wgpu backend WAV parser tests` + `wgpu backend demo build (macOS)` steps and the `Generate + build the wgpu example` examples-integration step. - Tests: drop the desktop wgpu-artifacts unit test (no in-tree package to resolve) and the wgpu case from the in-tree-template preview regression list. Two backends now fully out of the assembler bundle (bgfx + wgpu). The agnostic external-fetch path stays covered by the `external-null` (nullfixture) step. * feat(null): add backend.manifest.zon — manifest-splice codegen (#386 step 2) null extraction step 2 (window conformance was done in #411). Presence of backends/null/backend.manifest.zon opts the null DESKTOP build into the manifest-splice path instead of the enum `switch (cfg.backend)` sections. Loop-style (the headless main drives a fixed-frame tick loop), pure-Zig, zero deps. The LINK fragment is EMPTY — null has no native artifact (the enum path's `.null => {}` emitted nothing). backend_dep.txt is the verbatim .backend_null section body. Output BYTE-IDENTICAL to the enum path (diffed a generated baseline → 0 diff), including the is_tests_target path (which forces .backend=.null on host). `zig build test` green.
apotema
added a commit
that referenced
this pull request
Jul 1, 2026
chatgpt-codex raised 8 findings on PR #459 not covered by the #456 round. Verified each against the real code and folded the valid ones in: - root_build_deps now carry required resolution (url+hash/path/builtin); emsdk is a pinned template section, not name-synthesizable (#2) - bgfx-android android_app extra module requires root_alias="backend_app" (#3) - hookless mobile uses an assembler-owned default resolve_target; the backend-agnostic resolver means .resolved does not force a hook (#4) - build_hook must be a dedicated backend.hook.zig, not the provider build.zig (top-level @import("sokol") re-exports don't resolve in the root package) (#5) - android_target_sdk is required for Android; post_wire panics instead of the silent orelse 34 fallback (#6) - golden gate strengthened for hook-bearing cells: snapshot hook source and/or run the hook against a fixture *std.Build (#7) - carried v1 .capabilities forward into the v2 schema so opting into v2 doesn't bypass capability negotiation (.id was already present) (#8) Finding #1 (dep-option removal) was already resolved by ea24373 (base = universal options, per-platform = appends, no subtractive form) — recorded, not re-edited. Added a "PR #459 corrections" section documenting each. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
apotema
added a commit
that referenced
this pull request
Jul 1, 2026
* docs(#453): fold PR #456 review findings into manifest-v2 design Revise the build-graph manifest v2 design doc to address the substantive coderabbitai/chatgpt-codex findings that PR #456 merged without incorporating, so the #453 item-3 implementation does not inherit the design flaws. Central corrections (all verified against the real code): - Dependency options are declarative (DepOption name + closed ValueSource predicate set), NOT a runtime pre_wire hook returning []Flag — a b.dependency options literal needs comptime-known field names. pre_wire/DependencyOptions deleted. - Target selection is a pre-dependency resolve_target phase (iOS device/sim + SDK, Android ABI) resolved from -Ddevice/-Demulator/-Dandroid_arch + host, not a static .triple; iOS SDK now computed before plugin b.dependency calls. - Core-diamond walk carries a gfx_mod singleton so it preserves engine->gfx. - Header-first bounded version parse (v1 stays readable, > SUPPORTED rejected). - Preserve backend_* import aliases; per-platform loop_style/artifacts/link_libc; root_build_deps for the emsdk wasm hook; android_target_sdk into HookContext. - Hook reframed as trusted build code (not mechanically sandboxable). - Byte-identical gate -> one desktop anchor + golden snapshots; packager PR moved before Android/wasm conversions. Docs-only. Adds a "Review corrections (PR #456)" summary section. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * docs(#453): specify dep_options merge semantics + fix code-span spacing Address CodeRabbit findings on PR #459. Major — dep_options merge is now defined precisely: a comptime, name-keyed fold of per-platform entries over the base (override on collision, append otherwise), with NO subtractive form (an empty per-platform list inherits the base unchanged). Grounded in the v1 paramValue/param_names mechanism (manifest_splice.zig): the merge is a codegen-time operation on the NAME set, and because each platform's b.dependency literal is emitted independently, a name absent from a platform's folded set is simply never written. Corrected the sokol worked example to match build_zig.txt ground truth (:94/:124/:535/:763): with_imgui is the only base option (common to all four platforms); gamepad_* is a desktop-only append; dont_link_system_libs is an ios/android append (android's was missing). This dissolves the empty-wasm-list "drop" conflict — wasm forwards only with_imgui because gamepad_* was never in the base, not by removing it. Updated the "Review corrections" section to note the clarification. Minor — fixed inline code spans that wrapped across lines (MD038): the version gate spans (`< 1 or > SUPPORTED`, `2 <= v <= SUPPORTED_MANIFEST_VERSION`, `v > SUPPORTED`) and the `switch (target.result.os.tag)` span now sit on single lines. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * docs(#453): fold PR #459 review findings into manifest-v2 design chatgpt-codex raised 8 findings on PR #459 not covered by the #456 round. Verified each against the real code and folded the valid ones in: - root_build_deps now carry required resolution (url+hash/path/builtin); emsdk is a pinned template section, not name-synthesizable (#2) - bgfx-android android_app extra module requires root_alias="backend_app" (#3) - hookless mobile uses an assembler-owned default resolve_target; the backend-agnostic resolver means .resolved does not force a hook (#4) - build_hook must be a dedicated backend.hook.zig, not the provider build.zig (top-level @import("sokol") re-exports don't resolve in the root package) (#5) - android_target_sdk is required for Android; post_wire panics instead of the silent orelse 34 fallback (#6) - golden gate strengthened for hook-bearing cells: snapshot hook source and/or run the hook against a fixture *std.Build (#7) - carried v1 .capabilities forward into the v2 schema so opting into v2 doesn't bypass capability negotiation (.id was already present) (#8) Finding #1 (dep-option removal) was already resolved by ea24373 (base = universal options, per-platform = appends, no subtractive form) — recorded, not re-edited. Added a "PR #459 corrections" section documenting each. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
apotema
added a commit
that referenced
this pull request
Jul 1, 2026
…atch (#487) (#496) A light pack scanned components/events/prefabs/hooks but NOT scripts/, so a pack's per-frame SYSTEM had to leak into the game root (the #1 gap the pack-colony-demo eval found). This copies a pack's scripts/<state>/*.zig into `packs/<name>/scripts/` and registers them into the SAME per-state script dispatch the game root + plugins use, so a pack script's `pub fn tick(game, dt)` runs in its declared state. Key decisions: - Placement UNDER the pack dir (packs/<name>/scripts/…), NOT scripts/.plugin_<name>/. A pack has no importable module, so its script reaches its own components by relative import (../../components/foo.zig) exactly as a game-root script does. Copying the subtree under the pack — beside the components/ scanPack already copies — preserves that relative offset so the import resolves unchanged. - ScriptEntry.import_base: "scripts/" for game+plugin scripts (unchanged), "" for pack scripts whose rel_path is already a full packs/<name>/scripts/… target-relative path. AllScripts emits @import(import_base ++ rel_path). - Pack scripts carry plugin_name = pack name, so they form their own numeric-prefix scope, sort into the plugin block (after game scripts) by declaration order, and are skipped by the game-script FlowNode walk. Pack scripts are therefore lifecycle-only (tick/setup/State/drawGui); FlowNode discovery inside pack scripts is a deliberate follow-up. - Packs are skipped in the plugin-scripts loop to avoid double-scanning. - Re-add scripts/ to the `add pack` scaffold's convention dirs (#485). Existing game-root-only projects are unaffected (regression test guards the "scripts/" prefix). Adds 4 tests. Unblocks assembler#491. Claude-Session: https://claude.ai/code/session_01P7B7UzgrWEbBYLT3YBrAog
apotema
added a commit
that referenced
this pull request
Jul 7, 2026
- #1 escape generated string literals: emit the registry key AND @embedfile path through std.zig.fmtString (`{f}`), not raw `{s}` — a backslash/quote in an image source or asset_name (e.g. Windows `tiles\terrain.png`) no longer produces invalid Zig or a mis-keyed literal. - #2 same image key → different paths: `img_seen` now maps key→resolved embed path; two maps in different dirs both referencing `tiles.png` (different files, same runtime key) hard-error instead of silently reusing the first's bytes. Same-key-same-path stays a benign dedup. - #3 external tilesets: detect `<tileset source="*.tsx">` (external, no inline <image>) in `collect` and fail loud with the offending .tsx named — gfx returns error.ExternalTilesetUnsupported at runtime otherwise. Filed assembler#563. - #4 attribute syntax: `attrValue` now tolerates whitespace around `=` and single OR double quotes (`source = "x"`, `source='x'`), still a tight scan. Order: locate attr → strip quotes → XML-unescape. Tests: escaping round-trips to valid Zig (ast-parsed), diff-path collision, benign same-path dedup, external-tileset error, and the new attr syntaxes. zig build + test green, goldens byte-identical, fmt clean, files < 1000. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
apotema
added a commit
that referenced
this pull request
Jul 7, 2026
- #1 free TMX buffers per iteration: extract the per-map read+scan into `processMap`, so each map's `.tmx` bytes (up to 8 MiB) + extracted image slice free at that frame's end instead of accumulating until `collect` returns. Registrations/keys are dup'd into `regs` first, so they outlive the per-map buffers. Fixes OOM risk for projects with many large maps. - #2 skip XML comments in the tag scan: `indexOfTagSkippingComments` skips `<!-- ... -->` spans, so a commented-out `<image>`/`<tileset source>` no longer emits a bogus @embedfile (which could fail the build) or a false external-tileset error. - #3 include pack prefabs in the prefab fail-loud: move the tilemap phase after `loadPackScans` and thread `pack_scans` into `collectRegistrations`; `failOnPrefabTilemaps` now also walks `<import_prefix>/prefabs/*.jsonc`, so a Tilemap in a light-pack prefab aborts with the same #561 message instead of shipping a silently-broken binary. Updated #561 body. Tests: commented-out <image>/<tileset> ignored (real one still found); existing collect tests exercise per-map frees under testing.allocator. Verified e2e: pack-prefab Tilemap fails loud; happy-path unchanged + ast-checks. zig build + test green, goldens byte-identical, fmt clean, files < 1000. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
apotema
added a commit
that referenced
this pull request
Jul 7, 2026
…Phase 4) (#560) * tilemap: scan scenes for Tilemap + embed .tmx and tileset images (T2 Phase 4) Scan scene JSONC for `Tilemap` components, resolve each `asset_name` to a `.tmx` under the project's `assets/` dir, and comptime-embed the `.tmx` plus every tileset `<image source>` it references so labelle-engine v1.75.0 can decode + render the tilemap. Engine contract (labelle-engine v1.75.0, game/tilemap_mixin.zig): a single `Game.addEmbeddedTilemapAsset(name, bytes)` registry is keyed by BOTH the scene `asset_name` (-> .tmx bytes) AND each tileset's verbatim `image_source` string (-> image bytes; the engine's `ImageProvider.get` looks up by that exact string). The generated `init()` now populates it. - scene_manifest: `SceneManifest.tilemap_assets` — deep-walk the entity tree for `Tilemap` components (flat/wrapped/children/bundle shapes). - tilemap_scan.zig: resolve `asset_name` -> `assets/<asset_name>.tmx`, read it, extract `<image source>` refs (tight XML scan), and build a deduped flat list of `addEmbeddedTilemapAsset` registrations. Image registry key = verbatim `image_source`; @embedfile path = resolved relative to the .tmx dir. - codegen: emit registrations before `setScene` in both lifecycle paths (loop `try`, callback `catch @panic`), via the module-level-var pattern used by pack_scans. Purely additive — empty for tilemap-free projects. - Bump default engine_version 1.60.0 -> 1.75.0. Tests: scan extractor + path convention + collect (tmpDir) + scene extraction + both emit spellings. Verified e2e: a null-backend fixture generates registrations before setScene and passes `zig ast-check`. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * tilemap: re-pin default engine to 1.75.1 (fixes null-backend tilemap compile) engine 1.75.0's tilemap_runtime failed to compile against any gfx-1.21.0 backend (generic getTextureInfo seam) — the Examples integration test's null-backend asset-streaming-smoke build caught it. 1.75.1 (engine#707) hardens supported() + derives the texture type from the concrete resolver fn. Brings the fix in transitively (engine 1.75.1 pins gfx 1.21.0). Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * tilemap: address codex review (P1 sibling pins + 5 P2 correctness fixes) P1 — bump core/gfx defaults to match engine 1.75.1's transitive pins so a fresh `labelle init && build` resolves a compatible set: core 1.24.0, gfx 1.21.0 (read from engine v1.75.1 → gfx v1.21.0 → core v1.24.0 build.zig.zon). P2 fixes: - Walker over-match: replace the generic deep scan with a STRUCTURED entity-tree walk (`tilemap_scene_scan.zig`) — a `Tilemap` key is only collected when it's a direct member of an entity's `components`/`overrides` map or a flat-form component key, never nested in another component's data. - tmx/image key collision: track the `.tmx` and image key spaces separately and hard-error (`TilemapKeyCollision`) when a scene `asset_name` collides with a tileset `<image source>` in the shared engine registry. - XML-unescape `<image source>` (& < > " ') before using it as the @embedfile path AND the registry key — gfx hands the engine the DECODED string. - Honor project-registered `Tilemap` (engine C2): skip built-in embedding when a project component pascal-matches `Tilemap`. Plugin/pack-registered Tilemap deferred (fails loud via missing-asset) — filed assembler#562. - Prefab-borne Tilemaps: not embedded in minimal-T2 — detect + fail loud with a clear message instead of a silently broken binary. Filed #561. Refactor to keep files < 1000 lines: extract the JSON walker into `tilemap_scene_scan.zig` and the generate-phase policy into `root/tilemap_phase.zig`. Tests: walker over-match (nested non-component Tilemap), key collision, XML-unescape, prefab-scan detection. zig build + test green, fmt clean. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * tilemap: address codex 2nd-pass review (4 P2 correctness fixes) - #1 escape generated string literals: emit the registry key AND @embedfile path through std.zig.fmtString (`{f}`), not raw `{s}` — a backslash/quote in an image source or asset_name (e.g. Windows `tiles\terrain.png`) no longer produces invalid Zig or a mis-keyed literal. - #2 same image key → different paths: `img_seen` now maps key→resolved embed path; two maps in different dirs both referencing `tiles.png` (different files, same runtime key) hard-error instead of silently reusing the first's bytes. Same-key-same-path stays a benign dedup. - #3 external tilesets: detect `<tileset source="*.tsx">` (external, no inline <image>) in `collect` and fail loud with the offending .tsx named — gfx returns error.ExternalTilesetUnsupported at runtime otherwise. Filed assembler#563. - #4 attribute syntax: `attrValue` now tolerates whitespace around `=` and single OR double quotes (`source = "x"`, `source='x'`), still a tight scan. Order: locate attr → strip quotes → XML-unescape. Tests: escaping round-trips to valid Zig (ast-parsed), diff-path collision, benign same-path dedup, external-tileset error, and the new attr syntaxes. zig build + test green, goldens byte-identical, fmt clean, files < 1000. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * tilemap: address codex 3rd-pass review (3 P2 robustness fixes) - #1 free TMX buffers per iteration: extract the per-map read+scan into `processMap`, so each map's `.tmx` bytes (up to 8 MiB) + extracted image slice free at that frame's end instead of accumulating until `collect` returns. Registrations/keys are dup'd into `regs` first, so they outlive the per-map buffers. Fixes OOM risk for projects with many large maps. - #2 skip XML comments in the tag scan: `indexOfTagSkippingComments` skips `<!-- ... -->` spans, so a commented-out `<image>`/`<tileset source>` no longer emits a bogus @embedfile (which could fail the build) or a false external-tileset error. - #3 include pack prefabs in the prefab fail-loud: move the tilemap phase after `loadPackScans` and thread `pack_scans` into `collectRegistrations`; `failOnPrefabTilemaps` now also walks `<import_prefix>/prefabs/*.jsonc`, so a Tilemap in a light-pack prefab aborts with the same #561 message instead of shipping a silently-broken binary. Updated #561 body. Tests: commented-out <image>/<tileset> ignored (real one still found); existing collect tests exercise per-map frees under testing.allocator. Verified e2e: pack-prefab Tilemap fails loud; happy-path unchanged + ast-checks. zig build + test green, goldens byte-identical, fmt clean, files < 1000. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * tilemap: address codex 4th-pass review (#A tileset-scoped scan, #B backslash key/path) + revert unescape per gfx Grounded in gfx v1.21.0 tilemap/src/root.zig: its TMX parser stores `image_source` as a RAW dupe of the attribute-value bytes (parseAttributes reads verbatim between double-quotes; `tileset.image_source = dupe(src)`) — NO XML-entity decoding, NO separator normalization. The engine's ImageProvider.get looks up by that raw string, so the registry key must match it byte-for-byte. - #A tileset-scoped image scan: `extractImageSources` now only collects `<image>` INSIDE a `<tileset>…</tileset>` span. An `<imagelayer><image/>` background is ignored — the engine fetches images only for decoded TILESETS, so embedding an imagelayer image would require an absent file / collide though the runtime never requests it. - #B Windows backslash: the registry KEY stays the RAW `image_source` (backslash intact, matching gfx's lookup); only the @embedfile PATH normalizes `\`→`/` (+ existing `.`/`..` collapse) so it resolves to the copied asset `assets/tiles/terrain.png`. - Revert round-2 xmlUnescape: gfx does NOT decode entities, so decoding the key was a silent-mismatch bug (engine keys by raw `&…`). Keys/paths are now raw; the ambiguous XML-entity-in-path edge is deferred + filed as assembler#564. Tests: imagelayer image ignored (tileset image still found); raw source preserved (no decode); backslash → raw key + normalized path (unit + collect e2e). zig build + test green, goldens byte-identical, fmt clean, files < 1000. Happy-path fixture regenerates identical + ast-checks. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
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.
Summary
@libs/needs_machine) into.labelle/deps/, it now rewrites relative.pathdependencies inbuild.zig.zonso they resolve correctly from the new location.pathdeps are skipped entirely (no-op for most plugins)Closes labelle-toolkit/labelle-cli#129
Test plan
computeRelativePath,normalizePath,rewriteZonPaths(rewrite + skip)flying-platform-labellegame runs correctly with the change (no impact on current plugins: pathfinder, debug, imgui, fsm)🤖 Generated with Claude Code