Repository navigation
tilemap: scan prefab + pack-prefab JSONC for Tilemap; honor plugin/pack Tilemap (T3, closes #561 #562) - #568
Conversation
…ck Tilemap (T3, closes #561 #562) #561 — prefab tilemap embedding: extend the tilemap phase to walk game-root prefabs (<target>/prefabs/*.jsonc) AND pack prefabs (<target>/<import_prefix>/prefabs/*.jsonc), embedding each prefab-borne Tilemap's .tmx + tileset images exactly like scene tilemaps. Reuses the shared scanTilemapAssets extractor + tilemap_scan.collect (collision / comment-skip / imagelayer / external-.tsx hardening intact). Removes the failOnPrefabTilemaps fail-loud guard — prefab tilemaps are supported now. #562 — plugin/pack Tilemap precedence: registersTilemap now also inspects pack-registered component names (PackScan.component_names), so a pack that ships its own Tilemap suppresses the built-in embedding like a project component does. A pack's own Tilemap prefab ref is namespaced by scanPack, so scanning the staged pack prefab naturally embeds nothing for it. Default-pin bump to the current T3 trio: engine 1.76.0 -> 1.78.0, gfx 1.21.0 -> 1.23.0 (core stays 1.24.0). Package version 0.76.1 -> 0.77.0. Tests (tilemap_scan_test.zig): game-root prefab embed, pack prefab embed (pack-keyed), pack-registered Tilemap skips built-in, scene+prefab both embedded. Existing scene-tilemap goldens byte-identical. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBumps default engine/gfx dependency versions and package manifest version. Reworks tilemap_phase.zig's collectRegistrations to scan prefab JSONC (game-root and pack) for Tilemap components and embed their assets, replacing the prior fail-loud guard, with new override-detection logic for pack/plugin-registered Tilemap components, plus expanded tests. ChangesPrefab Tilemap Embedding
Dependency and Package Version Bump
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Generate
participant collectRegistrations
participant registersTilemap
participant appendPrefabAssets
participant tilemap_scan
Generate->>collectRegistrations: invoke after loadPackScans
collectRegistrations->>registersTilemap: check component_names for override
registersTilemap-->>collectRegistrations: override true/false
alt no override
collectRegistrations->>appendPrefabAssets: scan game-root and pack prefab JSONC
appendPrefabAssets-->>collectRegistrations: prefab asset_names
collectRegistrations->>tilemap_scan: collect(scene + prefab asset_names)
tilemap_scan-->>collectRegistrations: deduped Registrations
else override detected
collectRegistrations-->>Generate: empty registration set
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 updates the assembler to support embedding tilemaps from both game-root and pack prefabs (resolving assembler#561), and respects pack-registered Tilemap component overrides (resolving assembler#562). It also bumps the default engine and gfx versions in build.zig and increments the package version in build.zig.zon. Feedback on the changes suggests avoiding swallowing error.OutOfMemory when reading prefab files in appendPrefabAssets by explicitly propagating OOM errors while continuing on other non-critical file-reading errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (prefab_names) |name| { | ||
| const rel = try std.fmt.allocPrint(allocator, "{s}/{s}/{s}.jsonc", .{ target_dir, subdir, name }); | ||
| defer allocator.free(rel); | ||
| const src = cwd.readFileAlloc(io, rel, allocator, .limited(1024 * 1024)) catch continue; |
There was a problem hiding this comment.
Swallowing error.OutOfMemory with a generic catch continue can lead to silent failures or hard-to-debug issues under memory pressure. It is safer to explicitly check for error.OutOfMemory and propagate it, while continuing for other non-critical file-reading errors.
const src = cwd.readFileAlloc(io, rel, allocator, .limited(1024 * 1024)) catch |err| switch (err) {
error.OutOfMemory => return error.OutOfMemory,
else => continue,
};
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7fee894a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (pack.component_names) |name| { | ||
| if (std.mem.eql(u8, idents.pathToPascal(name, &pascal_buf), "Tilemap")) return true; |
There was a problem hiding this comment.
When a project uses the built-in Tilemap in a scene or game-root prefab and also imports any light pack that happens to ship components/Tilemap.zig, this branch makes collectRegistrations return no embedded .tmx/image registrations at all. Pack components are emitted under namespaced registry fields like .<pack>__Tilemap (see writeComponentRegistryBlock in src/codegen/blocks/registries.zig lines 88-93), so they do not replace the bare built-in Tilemap used by game-root JSON; the generated game will then load a bare Tilemap without the required embedded assets.
Useful? React with 👍 / 👎.
gemini: appendPrefabAssets' `readFileAlloc ... catch continue` swallowed error.OutOfMemory, so a real prefab tilemap could be silently dropped under memory pressure. Switch on the error — return OOM, continue only on other (absent-file) read errors. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
…P1, #562) codex P1: registersTilemap checked pack component names too, but a pack's `components/Tilemap.zig` registers as the NAMESPACED `<pack>__Tilemap` — a different component that does NOT satisfy the engine's exact-name `Components.has("Tilemap")` gate. So a project legitimately using the built-in `Tilemap` while importing any pack that happens to ship a Tilemap got NO `.tmx`/image registrations — its tilemaps silently vanished. Fix: registersTilemap now checks ONLY project component names (exact built-in name), matching `has("Tilemap")` semantics; the PackScan pack scan is removed. A pack's own tilemap usage stays a normal namespaced component handled by the prefab scan. Decl-module plugins with an un-namespaced Tilemap remain a documented, non-enumerable limitation. Tests: replace the (now-incorrect) "pack Tilemap skips built-in" with a project-Tilemap-skips test + a P1 regression proving a pack's namespaced Tilemap does NOT suppress the built-in scene embed. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa32c0dfb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const src = cwd.readFileAlloc(io, rel, allocator, .limited(1024 * 1024)) catch |err| switch (err) { | ||
| error.OutOfMemory => return error.OutOfMemory, | ||
| else => continue, | ||
| }; |
There was a problem hiding this comment.
When a discovered prefab can't be read for any non-OOM reason (for example it exceeds this 1 MiB readFileAlloc limit), this continue silently omits its Tilemap asset names. In the generate path these names come from the fresh prefab scans, and the generated init still embeds/registers the prefab, so that prefab can later instantiate without the required addEmbeddedTilemapAsset registrations; only a truly missing stale file should be skipped.
Useful? React with 👍 / 👎.
codex: the prefab scan's `else => continue` silently dropped a prefab whose file EXISTS but can't be read (e.g. exceeds the 1 MiB readFileAlloc limit) — yet the generated init still embeds+registers that prefab, so it would instantiate at runtime with no addEmbeddedTilemapAsset (missing-asset). Now only error.FileNotFound (a stale/absent name) skips; every other read error (OOM, too-big, I/O) propagates and fails the build loudly. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
Fixed in the latest commit — thanks @codex. The prefab-scan catch now propagates every non-absent read failure: |
|
To use Codex here, create an environment for this repo. |
1 participant
What
T3 follow-ups on the T2 Phase 4 tilemap epic. Replaces the fail-loud prefab guard with real prefab-tilemap support and extends the registered-
Tilemapoverride check to packs.#561 — prefab tilemap embedding
tilemap_phase.collectRegistrationsnow aggregatesTilemapasset_names from scenes AND prefabs:<target>/prefabs/*.jsonc<target>/<import_prefix>/prefabs/*.jsonc(the same staged, ref-rewritten tree the generatedinit()@embedFiles pack prefabs from)Each prefab-borne
Tilemap { asset_name }gets its.tmx+ tileset images embedded via the sharedscanTilemapAssetsextractor +tilemap_scan.collect— reusing the existing collision / comment-skip / imagelayer / external-.tsx(#563) hardening. ThefailOnPrefabTilemapsfail-loud guard is removed; genuinely-unsupported cases the scanner already rejects still fail loud incollect.#562 — plugin/pack Tilemap precedence
registersTilemapnow inspects project AND pack component names (PackScan.component_names), so a pack shipping its ownTilemapsuppresses the built-in embedding exactly like a projectcomponents/Tilemap.zig. (A pack's ownTilemapprefab reference is namespaced to<pack>__TilemapbyscanPack, so scanning the staged pack prefab also embeds nothing for it. Decl-module plugin component names aren't enumerable by the assembler; a pluginTilemapwithout anasset_namealready embeds nothing via the benign path.)Default-pin bumps (current T3 trio)
1.76.0->1.78.0, gfx1.21.0->1.23.0(core stays1.24.0) — verified against the tags'build.zig.zon.0.76.1->0.77.0.Tests
New end-to-end (hermetic tmpDir) tests in
tilemap_scan_test.zig:collectRegistrations embeds a GAME-ROOT prefab's Tilemap (.tmx + image)collectRegistrations embeds a PACK prefab's TilemapcollectRegistrations: a PACK-registered Tilemap skips the built-in embedcollectRegistrations embeds BOTH a scene and a prefab tilemapzig build+zig build testgreen (the 2 failing tests —emitFlowCatalogSidecar round-trips— are pre-existing on origin/main, unrelated).zig fmtclean on touched files. Purely additive to non-tilemap codegen; scene-tilemap goldens byte-identical.Closes #561, closes #562.
https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests