Repository navigation
feat: frame-boundary hook — the i18n resetFrameArena splice point - #812
Conversation
The assembler-generated i18n module's tf() formats into a module-owned
16KiB ring whose designed lifetime is "results live for the current
frame" — something must call its resetFrameArena() at each frame
boundary, and only the engine's main loop knows where a frame begins
(RFC-I18N §4 / Open Question 1).
Add `Game.frame_boundary_fn: ?*const fn () void` + `setFrameBoundaryFn`,
invoked as the FIRST thing in `tick()` — before the pause gate, so it
fires exactly once per frame on every frame, paused ones included (the
pause menu is the i18n-heaviest screen). The generated main wires it
with one line:
game.setFrameBoundaryFn(i18n.resetFrameArena);
Deliberately a single fn-pointer slot, not a registry: the registrant
is generated code with one author (the assembler), which can compose
multiple per-frame resets into one generated function if a second
concern ever appears. Same generated-code splice idiom as
`gizmo_reconcile_fn` / `spawn_prefab_fn`. `null` (no i18n) costs one
branch per frame.
Not the `frame_start` hook payload because that fires only on unpaused
frames and routes through the game's script-hook receivers — wrong
lifetime and needless dispatcher ceremony for a fixed, engine-known
concern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pbwN1ytAa8tctZMaTMjAk
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 28 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe game now supports one nullable frame-boundary callback. ChangesFrame-boundary callback
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EditorFrame
participant GameTick
participant FrameBoundaryFn
EditorFrame->>GameTick: invoke tick for active or stepped frame
GameTick->>FrameBoundaryFn: invoke at tick start
EditorFrame->>FrameBoundaryFn: invoke for paused frame that skips tick
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@codex review |
|
@coderabbitai review |
|
![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: 4c35fb673d
ℹ️ 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".
| if (self.frame_boundary_fn) |frame_boundary_fn| { | ||
| frame_boundary_fn(); |
There was a problem hiding this comment.
In Studio preview mode, pausing without a pending step makes editor_api.shouldTick() return false while editor_api.frame(&g) and g.render() still run unconditionally, as documented in src/editor_api.zig. Because this callback is reachable only through tick(), those rendered frames never reset the i18n frame arena, violating the promised once-per-frame lifetime and allowing editor-driven translated UI work during a paused preview to retain or consume arena storage across frames. Invoke the boundary from an unconditional frame-loop splice point rather than inside the simulation tick gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8789912 — real finding, thanks. editor_api.frame() now fires the boundary when the iteration's tick was skipped (editor_paused and !stepped_this_frame); ticked frames keep the tick()-start fire and can never double-fire (which would free the frame's strings before render). New test drives the real editor_pause/editor_step exports through both paths: two paused frames → two fires from frame(); a stepped frame → one fire from tick(), none from frame().
Studio preview renders while paused — editor_api.frame + g.render run with no tick, so the arena's once-per-frame reset never fired on exactly the frames the editor lives in. frame() now fires the boundary when this iteration's tick was skipped (editor_paused and !stepped_this_frame); ticked frames keep firing it first-thing in tick(), never twice. @hasField-gated like the camera paths for reduced fixtures. New test drives the real editor_pause/editor_step exports through both paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pbwN1ytAa8tctZMaTMjAk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/frame_boundary_test.zig`:
- Around line 152-173: Reset editor module state in this test by calling
editor_api.unbind() during setup and teardown, ensuring stepped_this_frame and
other state do not leak between tests. Keep the existing pause/step assertions
unchanged, and replace the deferred editor_pause(0) cleanup with teardown that
unbinds the editor API.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e48aaaa0-b6c9-4100-9608-bd8f313b7ec4
📒 Files selected for processing (3)
src/editor_api.zigsrc/game/loop_mixin.zigtest/frame_boundary_test.zig
🚧 Files skipped from review as they are similar to previous changes (1)
- src/game/loop_mixin.zig
The deferred editor_pause(0) discards pending steps but stepped_this_frame survives until the next unpaused shouldTick() — run one so later tests in the binary start clean (CodeRabbit on #812). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pbwN1ytAa8tctZMaTMjAk
Minor: adds public API, changes no behaviour.
`game.nativeTextureId(handle)` resolves an engine texture handle to the
BACKEND's own texture id — the value a backend-native accessor is keyed
by. It exists so game scripts stop reaching around the engine boundary
with `game.renderer.getTextureInfo(...).backend_texture.id`, which is
what a downstream UI kit had to do once gfx began minting its own keys
(labelle-gfx#326).
const backend_id = game.nativeTextureId(handle) orelse return;
const native = backend_gfx.nativeTextureHandle(backend_id);
Accepts the engine's public `u32` handle (what `loadTextureFromMemory`
and `AssetTexture` hand out) as well as an already-typed one.
`@hasDecl`-gated, so a renderer predating the seam degrades to null
rather than failing to compile — the engine still builds against gfx
older than 1.30.0.
Also carries #812's frame-boundary hook and the #811 RFC docs.
`.version` was reading 2.10.0 against a v2.11.0 tag; bumped to match.
…al gfx
Patch. v2.12.0's `game.nativeTextureId` could not compile against the
real `GfxRenderer`: the normalization keyed on a `Renderer.TextureId`
decl that only the test mock declared.
atlas_mixin.zig: struct 'GfxRendererWith(...)' has no member
named 'TextureId'
Now derives the handle type from the seam's own signature, which is
correct for any renderer. The mock is reshaped to match the real one so
the suite stops hiding this class of mismatch.
Anyone on v2.12.0 wanting the seam should take this; v2.12.0's other
contents (the #812 frame-boundary hook, #811 docs) are unaffected.
What
Adds the engine-side splice point RFC-I18N §4 needs: a frame-boundary hook the assembler-generated
main.ziguses to reset the generated i18n module'stf()frame arena once per frame.Game.frame_boundary_fn: ?*const fn () void = null— new fieldGame.setFrameBoundaryFn(callback)— registration (lifecycle mixin)tick(), before the pause gate, so it fires exactly once per frame on every frame — paused ones included (the pause menu is the i18n-heaviest screen, so the "tf() results live for the current frame" contract must hold there too)test/frame_boundary_test.zig— fires once per tick, fires on both pause paths (pausedflag andtime_scale == 0), runs before theframe_starthook, null slot is safe/clearable, plus the exact generated-main wiring shape against a mock i18n moduleThe generated main will wire it with one line:
Why this mechanism
Matches the engine's established generated-code splice idiom — a nullable function-pointer field set at startup (
gizmo_reconcile_fn,spawn_prefab_fn,active_scene_update_fn) — rather than inventing a registry. Two alternatives were rejected:frame_starthook payload: fires only on unpaused frames (wrong lifetime for a pause menu rendering translated text) and routes through the game's script-hook receivers — dispatcher ceremony for a fixed, engine-known concern.fn () voidslot — signature-identical to the generatedresetFrameArena— keeps the wiring to one line and the cost for games without i18n to one branch per frame (RFC Goal 6).RFC-I18N's frame-arena section + Open Question 1 are updated on the RFC's own branch (#811, commit 2baf381): ownership resolves as module-owned buffer, engine-owned reset.
Assembler follow-up: emit the
setFrameBoundaryFncall in the generated main when the project has alocales/directory.🤖 Generated with Claude Code
https://claude.ai/code/session_014pbwN1ytAa8tctZMaTMjAk
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.