docs(ui): add design document for the Forge UI system - #580
Conversation
Adds /design/ui-system.md, a full architecture and delivery plan for a retained-mode, ECS-native UI system modelled on Unity's uGUI (Canvas / RectTransform / Graphic / EventSystem) rather than IMGUI or UI Toolkit. Covers coordinate spaces, the per-frame system pipeline and its ordering constraints, RectTransform anchor/pivot resolution, the text rendering pipeline, and the pointer interaction state machine, plus a twelve-entry decision log, a six-phase feature backlog, risks, and a testing strategy. Also documents the new /design directory in AGENTS.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| | -------------- | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------ | | ||
| | **IMGUI** | Reconstructed every frame from call order | Poor. Fights ECS — there is no entity to attach an animation, a tween, or a physics-driven wobble to. Debug tooling only. | | ||
| | **UI Toolkit** | A retained tree defined in markup + stylesheets, with its own layout engine (Yoga/flexbox) | Poor. Requires an asset pipeline, a stylesheet language, and a parser before a single button renders. Contradicts "code-only". | | ||
| | **uGUI** | A retained tree of transform nodes carrying components | **Strong.** A transform node carrying components _is_ an ECS entity. The model translates almost 1:1. | |
There was a problem hiding this comment.
uGUI is a Unity term. Let's pick something else more generic or specific to Forge.
There was a problem hiding this comment.
Agreed — fixed in 031d13b.
The model is now called an anchored rect tree throughout, which describes what it does rather than borrowing a product name. The comparison table names the alternatives generically too (immediate mode, markup + stylesheet) with a separate "Prior art" column citing Dear ImGui / the web DOM / Unity / Godot / Flash, so the lineage is still legible without any of those becoming Forge vocabulary.
uGUI now appears in exactly four places, all deliberate attribution: the summary's note on why the term was chosen, two prior-art cells, and the paragraph about which parts of Unity's implementation not to copy.
Generated by Claude Code
There was a problem hiding this comment.
Checked, and it is a bug — filed as #581. My wording here was also wrong in a way worth correcting, fixed in 854effe.
Rotation and scale are inherited, and correctly:
world.rotation = parent.world.rotation + local.rotation // correct
world.scale = parent.world.scale * local.scale // correct
world.position = parent.world.position + local.position // missing two terms
The narrower defect is that the child's local offset is never transformed by the parent before being added. It should be:
world.position = parent.world.position
+ rotate(local.position * parent.world.scale, parent.world.rotation)
Parent at the origin rotated 90°, child at local (10, 0): world position should be (0, 10), actual is (10, 0). The child's own rotation.world is correctly π/2, so the sprite spins but doesn't orbit — a turret on a rotating tank rotates fine and sits in the wrong place.
Two things I found while confirming it:
transform-system.test.tshas seven tests, none of which attach a rotation or scale component to a parent, so nothing locks the current behaviour in either direction.parent-position-system.ts,parent-rotation-system.ts, andparent-scale-system.tscarry the same additive composition, but aren't exported fromsrc/common/systems/index.tsand are referenced only by their own tests — dead code superseded bycreateTransformEcsSystem. Flagged in fix(common): parented position offsets ignore the parent's rotation and scale #581 as worth deleting alongside the fix rather than fixing in three places.
The document now treats this as a bug to be fixed rather than a constraint to design around, and DL-03's rationale no longer leans on it (the camera-based scaler is still the better option on its own merits — it keeps the scaler in one place instead of two that have to agree).
Generated by Claude Code
| - **It degrades to "just sprites".** A uGUI `Image` is a textured quad with a | ||
| tint and an optional nine-slice. Forge's `SpriteEcsComponent` is already | ||
| exactly that, nine-slice included (`src/rendering/nine-slice-options.ts`). | ||
| - **Hierarchy order is draw order**, which is trivially explainable and needs no |
There was a problem hiding this comment.
Not necessarily true, draw order of sprites are currently also influenced by camera layerMasks, camera ordering, y-positions etc.
There was a problem hiding this comment.
You're right, that bullet overclaimed — fixed in 031d13b.
It now says the model admits a single ordering rule, explicitly flagged as a property of the model and not of Forge today, followed by a new paragraph spelling out the actual chain: Renderable.category vs the camera's cullingMask gates whether a sprite draws at all; cameras composite by CameraEcsComponent.layer across distinct render targets and in query order within one destination; and within a camera, commands sort by SpriteEcsComponent.layer then by depth, which render-system.ts derives from position.world.y.
So hierarchy-order-is-draw-order is something DL-06 has to build (layout writes a pre-order index into DepthEcsComponent), and even then it only holds within a single canvas and sprite layer — the camera and culling-mask rules still sit above it, and are what keep the UI pass separate from the world pass in the first place.
Generated by Claude Code
Addresses two review comments on the UI design document. Replaces "uGUI" as the name for Forge's chosen model with the descriptive "anchored rect tree", used consistently throughout. Unity, Godot, and Flash are still cited, but only as prior art in the comparison table and where the text refers specifically to Unity's implementation - Forge no longer borrows another engine's product name as its own vocabulary. The immediate-mode and markup-and-stylesheet alternatives are likewise named by what they are rather than by Unity's product names for them. Corrects the "hierarchy order is draw order" bullet, which overclaimed. Draw order in Forge is a chain - culling mask, camera layer and query order, sprite layer, then depth from world Y - so hierarchy ordering is a property this design must build (via DL-06), not one it inherits, and it holds only within a single canvas and sprite layer. Spells that chain out explicitly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
|
|
||
| | Capability | Where | Notes | | ||
| | -------------------------------- | --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Entity hierarchy | `ParentEcsComponent`, `createTransformEcsSystem` | Top-down recursive resolve with cycle detection and a per-frame memo cache. Composition is **additive for position** and ignores parent rotation/scale. | |
There was a problem hiding this comment.
Entity hierarchy shouldn't ignore rotation and scale. If it does, it's a bug, create a github issue if that is the case.
There was a problem hiding this comment.
Confirmed a bug — filed as #581. Fixed the document's wording in 854effe. (My reply to this went onto the uGUI thread by mistake a moment ago; reposting it on the right one.)
My phrasing here was also wrong. Rotation and scale are inherited, correctly:
world.rotation = parent.world.rotation + local.rotation // correct
world.scale = parent.world.scale * local.scale // correct
world.position = parent.world.position + local.position // missing two terms
The narrower defect is that the child's local offset is never transformed by the parent before being added. It should be:
world.position = parent.world.position
+ rotate(local.position * parent.world.scale, parent.world.rotation)
Parent at the origin rotated 90°, child at local (10, 0): world position should be (0, 10), actual is (10, 0). The child's own rotation.world is correctly π/2 — so the sprite spins but doesn't orbit. A turret on a rotating tank rotates fine and sits in the wrong place.
Two things found while confirming:
transform-system.test.tshas seven tests, none of which attach a rotation or scale component to a parent — nothing locks the current behaviour in either direction.parent-position-system.ts,parent-rotation-system.ts, andparent-scale-system.tscarry the same additive composition, but aren't exported fromsrc/common/systems/index.tsand are referenced only by their own tests. Dead code superseded bycreateTransformEcsSystem; fix(common): parented position offsets ignore the parent's rotation and scale #581 flags them as worth deleting alongside the fix rather than fixing in three places.
The design no longer treats this as a constraint to work around — every diagram assumes it fixed, and DL-03's rationale was restated so it no longer leans on the bug.
Generated by Claude Code
The §4.1 table said entity hierarchy "ignores parent rotation/scale", which reads as though neither propagates. That is wrong: composeWithParent inherits rotation additively and scale multiplicatively, both correctly. What is actually broken is narrower - a child's local position offset is composed by plain addition, so it is never rotated or scaled by the parent's world transform, and a child of a rotating parent spins in place instead of orbiting. Restates the gap precisely, adds a note working through the missing terms, a concrete reproduction, and the finding that transform-system.test.ts never exercises a rotated or scaled parent so nothing locks the behaviour in. Also records that the three superseded parent-*-system.ts files carry the same composition but are unexported and referenced only by their own tests. Filed as #581. The design no longer treats this as a constraint to work around; DL-03's rationale is restated so it no longer leans on the bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| | ------------------------------------------------------------ | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | **No text rendering of any kind** | **Blocking** | `grep -ri "font\|fillText" src` returns only terrain-mesh noise. There is no glyph, no font asset, no text shaper. A UI system without text is a decoration system. | | ||
| | **No canvas-space pointer** | **Blocking** | Cursor position only exists as a side effect of `Axis2dAction` bindings inside `MouseInputSource`. Nothing exposes "where is the pointer, in canvas pixels, right now". | | ||
| | **No touch input source** | High | `AGENTS.md` advertises "Keyboard, mouse, and touch input handling"; `src/input/` has keyboard, mouse, and gamepad only. There is no `TouchInputSource`. | |
There was a problem hiding this comment.
Create an issue to update the AGENTS.md to accurately reflect the real capabilities of the input sources. Lets assume touch inputs are out of scope for the UI change.
There was a problem hiding this comment.
Done — issue is #582, and touch is out of scope as of 51dbc63.
#582 is scoped to documentation accuracy only, not to building a touch source. It also suggests auditing the rest of AGENTS.md's capability list against /src in the same pass, since this bullet was wrong twice over — claiming touch, which doesn't exist, and omitting gamepad, which does.
Changes here:
- Backlog item 0.2 (
TouchInputSource) removed. I left the numbering gap rather than renumbering, so the decision stays visible in the doc and the0.6cross-reference in the risks table stays valid. - §4.2's gap row is re-labelled
Out of scopeand points at docs(agents): AGENTS.md overstates input source capabilities (claims touch support that doesn't exist) #582 instead of implying UI should close it. - DL-07's consequences now say only the stale
getBoundingClientRectfix is a genuine prerequisite.
One design consequence worth flagging: PointerStateEcsComponent is specified source-agnostically — position, buttons, delta, scroll, with no notion of where the input came from — and MouseInputSource is simply its only writer today. So if a touch source is ever added it becomes a second writer of the same component and every UI system above it keeps working unchanged. Nothing in this design needs revisiting to add touch later; until then the UI is mouse-and-gamepad only, which its docs should state plainly rather than leave implied.
Generated by Claude Code
… issue Touch input is out of scope for the UI design. Removes backlog item 0.2 (TouchInputSource), leaving the numbering gap in place so the decision stays visible rather than being silently renumbered. DL-07 now states the consequence explicitly: PointerStateEcsComponent is specified as a source-agnostic pointer written by MouseInputSource today, so a TouchInputSource can later become another writer of the same component without any system above it changing. The stale getBoundingClientRect fix remains a genuine prerequisite and is called out as the only one. The AGENTS.md claim of touch support, which does not exist in /src/input, is tracked in #582 rather than being fixed as a side effect of this design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| | **No canvas-space pointer** | **Blocking** | Cursor position only exists as a side effect of `Axis2dAction` bindings inside `MouseInputSource`. Nothing exposes "where is the pointer, in canvas pixels, right now". | | ||
| | **No touch input source** | High | `AGENTS.md` advertises "Keyboard, mouse, and touch input handling"; `src/input/` has keyboard, mouse, and gamepad only. There is no `TouchInputSource`. | | ||
| | **Draw order is world Y** | High | `render-system.ts:98` — `const depth = entityPosition.world.y`. For UI this is actively wrong: a label near the top of a panel would draw _behind_ the panel. | | ||
| | **`DepthEcsComponent` is dead code** | Opportunity | `src/common/components/depth-component.ts` exists, has tests, and is referenced by **nothing**. It is the exact shape needed to fix the line above. | |
There was a problem hiding this comment.
Only if this is the idiomatic way to fix the problem. Is a seperate component the right move? if so, keep this in the plan. If not, explain what a better alternative is. (e.g. adding a sortDepth to the sprite component may be more ergonomic)
There was a problem hiding this comment.
You're right — switched to sortDepth in 66036dc. DL-06 now records the reversal.
I checked the perf angle before deciding, and it reinforces your suggestion rather than trading against it. buildCameraCommands runs per camera and already does three world.getComponent calls per sprite (rotation, scale, flip). A separate component makes that four — 4 × sprites × cameras — and this design adds a second camera, doubling the multiplier. sortDepth is a property read on an object already sitting in the batch array.
Two more reasons it's the better shape:
- The two halves of one sort key belong together.
SpriteEcsComponent.layeris already the primary key, and its own JSDoc says ties break "by depth (world Y position)" —sortDepthoverrides exactly that clause. Splitting them across two components means someone tuning draw order has to know about two places and can attach one without the other. - A separate component buys no composition here. Rotation and scale are meaningful on an entity with no sprite; a draw-order key is not. There's no entity that wants sort order and nothing to draw, so the split was arbitrary rather than expressive.
It stays undefined rather than taking a default of 0, so existing sprites are untouched — a 0 default would silently override world-Y sorting everywhere.
That leaves DepthEcsComponent dead with no prospective use, so the plan now deletes it (item 0.4) instead of resurrecting it.
Generated by Claude Code
There was a problem hiding this comment.
Does this affect DL-07?
| | **No touch input source** | High | `AGENTS.md` advertises "Keyboard, mouse, and touch input handling"; `src/input/` has keyboard, mouse, and gamepad only. There is no `TouchInputSource`. | | ||
| | **Draw order is world Y** | High | `render-system.ts:98` — `const depth = entityPosition.world.y`. For UI this is actively wrong: a label near the top of a panel would draw _behind_ the panel. | | ||
| | **`DepthEcsComponent` is dead code** | Opportunity | `src/common/components/depth-component.ts` exists, has tests, and is referenced by **nothing**. It is the exact shape needed to fix the line above. | | ||
| | **No rectangle concept in the transform** | Expected | Transforms are point + rotation + scale. Rects are new. | |
There was a problem hiding this comment.
What does expected mean? is it a blocker or high priority. Rect needs to be built. Saying "expected" is not helpful.
There was a problem hiding this comment.
Fair — "Expected" answered nothing. Rebuilt the table in 66036dc.
The whole column was incoherent, not just that row: it mixed urgency (Blocking/High/Medium/Minor), category (Bug, Opportunity), and scope (Out of scope, Expected). So it's now one ordered scale — Blocker > High > Medium > Low — stated above the table, plus a "Tracked as" column naming the backlog item or issue that owns each gap, so every row answers "how urgent" and "where does it get done".
On the specific row: you're right that rect needs building, and it's a Blocker, not a shrug. It now reads:
No rectangle type or rect concept in the transform — Blocker — 0.2, 1.1 — Every element in this design is a rectangle resolved against its parent's rectangle, so
Rect2andRectTransformEcsComponentare foundational and must be built first.src/math/Rect.tsexists but is a class predating theVector2class→plain-object migration, so it isn't the type to build on (DL-11).
Draw order moved from High to Blocker in the same pass — it isn't a tuning problem, it's visibly wrong output (a label draws behind its own panel).
Generated by Claude Code
| | **Draw order is world Y** | High | `render-system.ts:98` — `const depth = entityPosition.world.y`. For UI this is actively wrong: a label near the top of a panel would draw _behind_ the panel. | | ||
| | **`DepthEcsComponent` is dead code** | Opportunity | `src/common/components/depth-component.ts` exists, has tests, and is referenced by **nothing**. It is the exact shape needed to fix the line above. | | ||
| | **No rectangle concept in the transform** | Expected | Transforms are point + rotation + scale. Rects are new. | | ||
| | **No clipping/masking** | Medium | Needed for scroll views. | |
There was a problem hiding this comment.
Clipping and masking, similar to text, is useful outside of the UI module. Let's create a github issue for this and mark it out of scope for the UI module.
There was a problem hiding this comment.
Done — #583, out of scope as of 66036dc.
Agreed on the reasoning: minimap viewports, wipe transitions, fog-of-war reveals, and fill-by-reveal health bars all want rect clipping with no UI involved, so it belongs in /src/rendering next to the sprite instance-data segment.
DL-09 stays in the document as the decision record — the UI design is what surfaced the requirement and worked out that per-instance clip rects beat gl.scissor (which forces a draw-call break per mask, destroying batching for exactly the scroll-list case with the most elements) — but #583 owns the work.
One dependency worth tracking: ScrollRectEcsComponent (backlog 3.4) can't ship before #583 lands; scroll views spill past their viewport without clipping. Nothing else in the UI backlog is affected, so this doesn't gate Phases 0–2. Noted on both the issue and item 3.4.
Generated by Claude Code
| index.ts | ||
| ``` | ||
|
|
||
| **Why `text` is a sibling, not a child of `ui`.** Text is needed for damage |
There was a problem hiding this comment.
Make a github issue for text. And mark it as out-of-scope for the UI module.
There was a problem hiding this comment.
Done — #584, out of scope as of 0fe84ae. This was also my own recommendation in the open questions, so it's good to have it settled rather than left as a question.
#584 covers MSDF atlas loading, the shader, shaping, and a default shipped atlas so createLabel works with no toolchain. The sub-quad expansion refactor of render-system.ts moved with it — it only ever existed to serve text, and it was the highest-risk item in this plan (hot path, existing tests, eight-plus demos depending on nine-slice). The UI critical path no longer carries it. DL-04 and DL-05 stay here as decision records with scope banners.
That triggered a real restructure rather than a one-line edit. Phase 0 has now lost touch (#582), text (#584), and clipping (#583), so it was renumbered and reduced to five items: the canvas-space pointer, Rect2, sprite.sortDepth, deleting DepthEcsComponent, and verifying UI-camera compositing. There's a table recording what moved out and where.
The dependency I want to be explicit about rather than bury: the UI module isn't useful without text. Buttons need labels; HUDs need numbers. Layout, anchoring, hit testing, and interaction can all be built and unit-tested before #584 lands, but the module can't ship a convincing demo — or arguably a convincing release — until it does. The document now states that in the summary, in the risks table, and in the open questions, with the recommendation to build in parallel and gate the UI module's release on text rather than shipping something text-less that reads as broken.
Generated by Claude Code
…cope out clipping
Three review points.
DL-06 reversed. Draw order now comes from an optional sortDepth on
SpriteEcsComponent rather than the dead DepthEcsComponent. layer already lives
on the sprite and its JSDoc already documents the world-Y tie-break that
sortDepth overrides, so splitting one sort key across two components was
arbitrary. It is also free: buildCameraCommands runs per camera and already
does three getComponent calls per sprite, and this design adds a second camera
- a separate component made that four times sprites times cameras, where a
sprite field is a property read on an object already in the batch. A separate
component also buys no composition, since a draw-order key is meaningless on an
entity with nothing to draw. DepthEcsComponent is now slated for deletion
(item 0.11) rather than resurrection.
Gap table rebuilt. The severity column mixed urgency, category, and scope
("Blocking", "Opportunity", "Expected", "Bug"), so "Expected" answered nothing.
Replaced with one ordered priority scale plus a column naming the backlog item
or issue that owns each gap. The rect/rect-transform gap is a Blocker, not a
shrug - it is foundational and must be built first.
Clipping and masking moved out of scope, tracked in #583. Like text, it is
generally useful outside UI - minimaps, wipe transitions, fog-of-war,
fill-by-reveal bars - so it belongs in /src/rendering. DL-09 keeps the decision
record since the UI design surfaced it, and notes it blocks only ScrollRect
(3.4), not Phases 0-2.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| live aspect ratio each frame | ||
| - _Constant pixel size_ → `verticalWorldUnits = renderContext.height` | ||
|
|
||
| **Gotcha to document loudly:** `SpriteEcsComponent.pivot` is Y-**down** |
There was a problem hiding this comment.
SpriteEcsComponent.pivot should not be Y-down.
If this is the case, it's a bug. Create a github issue for this.
There was a problem hiding this comment.
Traced it, and you're right — it is Y-down, and it is a bug. Filed as #585; document updated in b61a0c0.
The derivation, since it's non-obvious:
createProjectionMatrixscales Y by-2 / height— the projection flips Y.bindSpriteInstanceDatapre-negates the entity position (buffer[POSITION_Y_OFFSET] = -position.world.y). Those two cancel, so a largerposition.world.ydraws higher. World space is Y-up. ✅sprite.vert.glslcomputes the quad's local offset in the shader, so it never gets that compensating negation — local+ypasses through the projection flip unopposed and moves down.
With pivot = (0,0): normalizedPivot = (-1,-1), pivoted = a_position + 1 spans [0,2], scaled spans [0,size]. All offsets ≥ 0, and local +y is down, so the sprite hangs below the entity — the entity sits at the sprite's top edge. Top-left, matching the JSDoc.
So pivot is the one place a rendering-internal detail leaks into the public API. Everything else Y-facing is Y-up and explicitly compensated — position.world.y, RotationEcsComponent (negated, with a comment saying it matches position), screenToWorldSpace. Pivot is the only one that isn't, and it's invisible at the default centered pivot, which is presumably how it survived.
Two things #585 flags for the same pass:
- Nine-slice region offsets.
render-system.tsaddsregion.offsetstraight ontoentityPosition.world(Y-up) while the shader pivot is Y-down. Those spaces disagree; nine-slice always rendering with a centered pivot may be masking it. - Test coverage. Given the 0.24.1 fix for pivot applying only half its offset, non-centered pivots look under-tested — worth pinning all four corners.
The design's §5.2 no longer presents sprite.pivot.y = 1 - rectTransform.pivot.y as a permanent gotcha; it's marked as pending #585, and the note says to delete that line once fixed rather than keep a compensating error.
Generated by Claude Code
Text is generally useful outside the UI module - damage numbers, dialogue, floating names, debug overlays - so like clipping it belongs in its own module rather than buried under /src/ui. Filed as #584 covering MSDF atlas loading, the shader, shaping, and a default shipped atlas. The sub-quad expansion refactor of render-system.ts moves with it. It existed to serve text, and it was the highest-risk item in this plan, so the UI critical path no longer carries it. DL-04 and DL-05 stay as decision records with scope banners pointing at #584. Phase 0 is rebuilt and renumbered: what remains is the canvas-space pointer, Rect2, sprite.sortDepth, deleting DepthEcsComponent, and verifying UI-camera compositing. A table records what moved out and where, since three separate removals had left the numbering full of holes. States the dependency plainly rather than hiding it: every phase can be built and unit-tested without text, but the module is not useful without it, so text landing should gate the UI module's first release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| B4["SpriteEcsComponent<br/>nine-sliced panel, tinted"] | ||
| B5["UiInteractableEcsComponent"] | ||
| B6["UiPointerStateEcsComponent<br/><i>written by the raycaster</i>"] | ||
| B7["ButtonEcsComponent<br/>onClick: ParameterizedForgeEvent"] |
There was a problem hiding this comment.
Would it be better to make a broader component, rather than a button specific component? Other UI elements are clickable and should have onClick events, not just buttons.
There was a problem hiding this comment.
Agreed — moved in b61a0c0, recorded as the new DL-13.
onClick and the pointer enter/exit pair now live on UiInteractableEcsComponent, which is the component that already means "this rect participates in pointer input". Toggles, sliders, scrollbar thumbs, list rows, inventory slots, cards, and close icons all get them without claiming to be buttons.
Following that through, ButtonEcsComponent had nothing left to hold — visual feedback belongs to a transition component, focus to UiFocusEcsComponent — so it's gone entirely. A button is fully described by interactable + transitions + a child label, which makes it createButton, an aggregate factory in the mould of createCamera. That's a better fit for this codebase than a component that exists only to name a preset.
Generated by Claude Code
| B3["PositionEcsComponent + DepthEcsComponent"] | ||
| B4["SpriteEcsComponent<br/>nine-sliced panel, tinted"] | ||
| B5["UiInteractableEcsComponent"] | ||
| B6["UiPointerStateEcsComponent<br/><i>written by the raycaster</i>"] |
There was a problem hiding this comment.
All components are, by definition, state. Including State in the component name, leads me to suspect there is a code smell. They may be no code smell and this is completely valid. You decide.
There was a problem hiding this comment.
You were right to be suspicious — it was a smell, and the fix was deleting the component rather than renaming it. Done in b61a0c0, folded into DL-13.
The State suffix was the tell. Splitting author-set config from system-written state into two components looked tidy, but nothing ever wants the interaction state without the interactable, or vice versa — so it was carved along the wrong seam and the name needed a filler word to justify itself.
UiInteractableEcsComponent now holds blocksRaycasts, interactable, the events, and isHovered / isPressed / isDragging / wasClickedThisFrame. That matches what the codebase already does where the two genuinely belong together: PositionEcsComponent holds author-set local beside system-written world. Interaction state is world to the interactable's local, so the system-written fields are documented as read-only to callers using the same convention.
Net effect: one component instead of three (with ButtonEcsComponent gone too), one query in the raycaster instead of two, one place to look.
I applied the same reasoning to /src/input — the pointer component is now PointerEcsComponent, not PointerStateEcsComponent.
Generated by Claude Code
…csComponent Adds DL-13, covering two related review points. Buttons are not the only clickable things. Toggles, sliders, scrollbar thumbs, list rows, inventory slots, cards and close icons all want onClick and the pointer enter/exit pair, so hanging the event surface off ButtonEcsComponent forced each of them to claim to be a button or duplicate the events. Events now live on UiInteractableEcsComponent - the component that already means "this rect participates in pointer input". That leaves nothing for a button component to hold: visual feedback belongs to a transition component and focus to UiFocusEcsComponent. A button is fully described by interactable + transitions + a child label, so it becomes createButton, an aggregate factory following the existing createCamera precedent. UiPointerStateEcsComponent is merged in as well. The State suffix was the tell - every component is state, and nothing ever wanted the interaction state without the interactable. The codebase already keeps author-set and system-written fields together where they belong: PositionEcsComponent holds local beside world. Input's pointer component is renamed PointerEcsComponent for the same reason. Also records the sprite pivot Y-axis inconsistency as issue #585 rather than documenting it as a permanent gotcha; once fixed, the compensating `1 - pivot.y` bridge in the layout system should be deleted, not kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| (`UiAnchor.topLeft`, `.stretchHorizontal`, `.stretchAll`, …) are plain frozen | ||
| constants, and are what most callers actually touch. | ||
|
|
||
| ### 5.6 Text pipeline |
There was a problem hiding this comment.
Make sure this detail is included in the text issue you create
There was a problem hiding this comment.
Added to #584 — new "Frame ordering constraints for createTextShapingEcsSystem" section.
It captures why that slot in the diagram is load-bearing rather than incidental:
- Before the render system — the render pass consumes the glyph quads, so they have to exist first.
- After anything that can change the wrap width. For text inside a UI container, the wrap width comes from the element's resolved rect, which only exists after the layout pass and
createTransformEcsSystem. Shaping earlier wraps against last frame's width — invisible until something resizes, then one frame of visibly wrong line breaks. - No layout dependency for standalone text. Damage numbers, floating names, and debug overlays have nothing constraining them, so their shaping depends only on the string, font, and size.
So the constraint is conditional on whether the text sits in a laid-out container, and the simplest correct default is to register shaping late — after layout and transform, before render — which is right for both cases. The issue points back at §5.4 for the full pipeline.
I also folded in the dirty-tracking note, since it belongs with the ordering: shaping is the one place this design deliberately doesn't recompute every frame (contrast DL-12, where everything else does), because a re-shaped paragraph per frame is real cost rather than theoretical. Re-shape only when text, font, fontSize, or the resolved wrap width actually changed.
Generated by Claude Code
| [*] --> Normal | ||
| Normal --> Hovered: pointer enters rect | ||
| Hovered --> Normal: pointer exits rect | ||
| Hovered --> Pressed: pointer down inside |
There was a problem hiding this comment.
Is there a scenario where the hovered state is skipped, the user manages to press on the very frame the cursor enters the frame?
There was a problem hiding this comment.
Yes, and the diagram was wrong to imply otherwise — fixed in ddc5445.
Input is sampled per tick, not streamed. MouseInputSource accumulates button downs and ups into per-frame sets and reset()s them each tick, so the system sees the pointer's current position plus a set of edges — never their ordering within the frame. At 60 Hz a frame is ~16.7 ms and a flick-and-click fits inside one comfortably.
More decisively: touch has no hover phase at all. Its first event is simultaneously "entered" and "down". Touch is out of scope now (#582), but DL-07 specifies the pointer source-agnostically precisely so it can be added later without revisiting this design — and that promise is only real if the state machine already tolerates a missing hover. So this isn't a rare race, it's the only path on a whole input class.
Chasing it turned up a sibling case: press and release within one tick, from a synthetic click() or a fast enough real one. Both edges survive the frame in MouseInputSource's separate down/up sets, but a system reading only "is the button currently held" sees neither and silently drops the click.
The fix is that createUiInteractionEcsSystem must not apply one transition per tick. State is derived from the tick's facts; events are derived from the delta against the previous tick:
isOver = this element was the raycast hit this tick
pressStartedHere = a pointer-down edge landed here (latched until release)
state = Disabled if !interactable | Pressed if isOver && pressStartedHere
| Hovered if isOver | Normal
onPointerEnter when !wasOver && isOver
onPointerDown when a down edge occurred && isOver
onClick when an up edge occurred && isOver && pressStartedHere
Your scenario then raises onPointerEnter and onPointerDown in the same tick, the state goes Normal → Pressed, and Hovered is never observed — nothing swallowed. I added that edge to the diagram so the path is legal rather than accidental.
The consequence for callers is now stated explicitly: treat these as sampled states, not a guaranteed sequence. A transition that only starts a press animation from a hover animation is wrong, and would fail first on touch and intermittently on fast mice. Both same-tick cases are added to the unit test plan — cheap to assert with a synthetic pointer, nearly impossible to reproduce by hand.
Generated by Claude Code
…pped The interaction state machine had no path from Normal to Pressed, which implied a press could not be observed on the same tick the pointer entered. That is wrong twice over. Input is sampled per tick, not streamed: MouseInputSource accumulates button downs and ups into per-frame sets and resets them each tick, so a system sees the current position plus a set of edges, never their intra-frame ordering. A flick-and-click fits inside one 16.7ms frame, and touch has no hover phase at all - its first event is simultaneously entered and down. Since DL-07 specifies the pointer source-agnostically so touch can be added later without revisiting the design, the state machine has to tolerate a missing hover for that promise to hold. The related case is press and release within one tick, from a synthetic or fast enough click. Both edges survive the frame in separate sets, but a system reading only "is the button held" sees neither and drops the click. Specifies the system as deriving state from each tick's facts and events from the delta against the previous tick, rather than applying one transition per tick, and adds the Normal to Pressed edge so that path is legal rather than accidental. Callers are told to treat the states as sampled, not as a guaranteed sequence. Both same-tick cases are added to the unit test plan, since they are cheap to assert synthetically and nearly impossible to reproduce by hand. Also fixes a duplicate const in the polled/evented example. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| // Polled — ECS-idiomatic, trivially unit-testable, no listener lifetime concerns. | ||
| const pointerState = world.getComponent(buttonEntity, uiPointerStateId); | ||
|
|
||
| if (pointerState?.wasClickedThisFrame) { |
There was a problem hiding this comment.
How will this work with a controller or other input source? There is no "click" on a controller. This is why the Input actions are decoupled from sources. I expect it to work that way for the UI module too.
The author needs to know that the "start" button was actioned. Not how it was actioned.
There was a problem hiding this comment.
This has the added benefit of allowing me to drive UI actions programmatically for tests and tutorials.
There was a problem hiding this comment.
You're right, and I should have followed the precedent that's already in the codebase. Restructured in 6a77d41 as DL-14.
CameraEcsComponent already does exactly what you're describing — zoomInput?: Axis1dAction, panInput?: Axis2dAction — consuming the action abstraction rather than reading keys. I built a pointer-centric model alongside it instead of following it.
onClick → onActivate, raised from either path, with the source never exposed:
- pointer — up edge inside, press started here (
createUiInteractionEcsSystem) - focus —
submitInputtriggered while focused (createUiNavigationEcsSystem)
The canvas takes submitInput / cancelInput / navigateInput as InputActions. The UI module never touches an input source. The author learns the Start button was activated, not how.
Two states stay deliberately separate, because collapsing them loses information:
isHovered— pointer-only, meaningless on a gamepad.isFocused— source-agnostic, means "the currently selected element".
Transitions key off a derived visual state merging both, so an element highlights identically whether it's moused-over or stick-focused, and no transition component needs to know which input the player is holding.
Drag events stay explicitly pointer-shaped — a drag has no controller analogue, and pretending otherwise would be worse than admitting it.
The consequence I'd flag: UiFocusEcsComponent and createUiNavigationEcsSystem move from Phase 5 polish into Phase 2 core (item 2.4). Treating focus as a late accessibility add-on was the root mistake; it's half the interaction model.
Generated by Claude Code
There was a problem hiding this comment.
Captured as a first-class consequence in DL-14, not just a footnote — it's a strong enough argument that it partly justifies the decision on its own:
UI becomes programmatically drivable for free. Because activation arrives through an
InputAction, a test or a scripted tutorial callsaction.trigger()and the UI responds exactly as it would to a real player — no synthesized DOM events, no fake pointer coordinates, no separate test-only code path.
Two things fall out that are worth naming:
- Unit tests get real interaction flows in
jsdom. Previously the testing strategy could only assert the interaction state machine against a synthetic pointer; now a test can drive a whole menu — focus, submit, cancel — through the same actions a player uses. The/e2esuite stays for what genuinely needs a real browser (actual rendered pixels, real DOM events through the real input pipeline), rather than being the only place interaction can be exercised end to end. - A tutorial can genuinely press its own buttons. Not a simulated highlight over a button plus a manual call to the handler — the actual activation path, so a scripted tutorial can't drift out of sync with what a real press does.
It also means the "does the button work with a controller" test doesn't need a controller.
Generated by Claude Code
| after the world camera. (b) A bespoke UI render pass outside the camera loop. | ||
| (c) A DOM/CSS overlay. | ||
|
|
||
| **Decision: (a).** |
There was a problem hiding this comment.
Just make sure the DX is solid. I don't want the author of a game to be bogged down by setting up 2 cameras, canvases, etc. just to get a button on screen.
There was a problem hiding this comment.
Agreed, and my previous example made it look worse than intended — fixed in 6a77d41.
Added a "minimal path" as the first thing in §6, before any of the option-heavy examples:
const canvas = createUiCanvas(world, renderContext);
const play = createButton(world, canvas, { label: 'Play' });
play.onActivate.registerListener(startGame);That's a working, clickable, gamepad-navigable button. createUiCanvas creates the canvas entity, its rect transform, the dedicated static UI camera with a transparent clear color and culling mask, and its render target — and registers the UI systems in the right order. Every option is defaulted: 1920x1080 reference resolution, scale-with-screen-size, and submit/cancel/navigate bound on the supplied InputManager. You pass an option only to change it.
The second camera in DL-01 is an implementation detail of that one call, and the doc now says so explicitly: if an author ever has to think about culling masks or render targets to put a button on screen, the design has failed. I also reworked the full menu example so the option-heavy version is clearly labelled "what it looks like when you do reach for them" rather than reading as required setup.
Added a matching goal in §2 — "two calls to a working button" — so this is a stated design constraint rather than something I assert once in a reply and drift away from later.
Generated by Claude Code
…ter click Adds DL-14. The interaction model was pointer-centric - onClick, raised only by a mouse - with gamepad navigation parked in Phase 5 polish. A controller has no cursor, no hover and no click; it has a focused element and a submit action, so that shape either excluded controllers or bolted them on as a parallel path every consumer handles twice. The engine already solved this a layer down and this design should not re-solve it differently. /src/input decouples InputAction from InputSource so game code says "jump was actioned" rather than "space was pressed", and CameraEcsComponent already consumes that directly via zoomInput/panInput rather than reading keys. The UI canvas now takes submitInput/cancelInput/navigateInput the same way and never touches an input source. onClick becomes onActivate, raised from either the pointer path or the focus path, with the source never exposed. isHovered stays pointer-only; isFocused is source-agnostic; transitions key off a derived state merging both so an element highlights the same whether moused-over or stick-focused. Drag events stay explicitly pointer-shaped, since a drag has no controller analogue. UiFocusEcsComponent and the navigation system move from Phase 5 to Phase 2 core. This also makes the UI programmatically drivable for free: because activation arrives through an InputAction, a test or scripted tutorial calls action.trigger() and the UI responds exactly as it would to a player - no synthesized DOM events and no test-only code path. Separately, addresses the DX concern that an author should not assemble cameras and canvases to show a button. Adds a "minimal path" showing the two calls that put a working, gamepad-navigable button on screen, and states that every option is defaulted and the second camera is an implementation detail of createUiCanvas. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| extra blit. | ||
| - Draw both to the canvas and rely on entity creation order. | ||
|
|
||
| Prefer the first — it is the mechanism the engine already has, it keeps UI out of |
There was a problem hiding this comment.
And if the author wanted to apply an effect to the entire screen (world + UI) how would they achieve this or would this be impossible given this set up?
Post processing and other effects are very valid for UI too.
There was a problem hiding this comment.
Not impossible, and it needs no new plumbing — but you're right that DL-01 read as though it were. Fixed in c2c9775.
The key fact I'd stated as a benefit without following through on: post-processing attaches to a render target, not to a camera. bloom-system.ts:205 dedupes with processedTargetsThisFrame.has(renderTarget), so an effect is applied once per target regardless of how many cameras drew into it. That dedupe only exists because multiple cameras per target was anticipated.
So all three scopes fall out of which target the UI camera renders into:
| Want | Configuration |
|---|---|
| World only (default) — HUD stays crisp while the world blooms | Separate renderTargets; effects attach to the world's |
| Everything, world + UI | Point the UI camera's renderTarget at the same target as the world camera. Both draw in, the render system clears it once (clearedDestinationsThisFrame), effect applied once to the composite |
| UI only — blur behind a modal, vignette a pause menu | Attach effects to the UI camera, which owns its own target |
Ordering already works for the shared case: post-processing systems are documented to register after the render system and before the present system, and both cameras have drawn by the time the render system's update returns.
The one real constraint, now written down: a shared target gives you one set of effects over both, so "bloom the world but not the UI, and tone-map everything" isn't expressible in a single pass — that needs world-on-its-own-target, effects there, then both composited into a shared target with the second effect. I've noted that as the argument for eventually letting a post-processing stack attach to the present step rather than only to a camera, and marked it out of scope for this design rather than pretending the limitation doesn't exist.
Generated by Claude Code
DL-01 presented a separate UI render target as giving "the right post-processing semantics by default", which read as though UI could never be included in a full-screen effect. That is wrong, and post-processing over UI is a legitimate thing to want. Post-processing in this engine attaches to a render target, not to a camera: bloom-system dedupes with processedTargetsThisFrame.has(renderTarget), so an effect is applied once per target regardless of how many cameras drew into it. That single fact gives all three scopes with no new plumbing - world-only via separate targets (the default), everything via pointing the UI camera at the world camera's target, and UI-only via attaching effects to the UI camera. Ordering already works for the shared case, since post-processing systems register after the render system and before the present system. Also records the one real constraint: a shared target means one set of effects over both, so bloom-the-world-but-not-the-UI combined with tone-map-everything needs two passes. Notes that as the argument for eventually attaching a post-processing stack to the present step rather than only to a camera, and marks it out of scope here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
DL-13's consequences still listed wasClickedThisFrame, which DL-14 renamed to wasActivatedThisFrame, and omitted isFocused entirely. Also notes that isFocused and wasActivatedThisFrame are written by the navigation system as well as the interaction system, so neither has a single owning system. Found by a consistency sweep over the document rather than by CI, which has not run on this branch since 031d13b. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
| `CanvasScaler` that writes a `ScaleEcsComponent` on the canvas root. (c) Layout in | ||
| raw canvas pixels and re-resolve everything on resize. | ||
|
|
||
| **Decision: (a).** |
There was a problem hiding this comment.
Does this decision change or remain as is once the parent transform bug is fixed? i.e. scale and rotation are also additively applied like position?
There was a problem hiding this comment.
Scale might be multiplicatively applied?
There was a problem hiding this comment.
The decision stands — but you've caught that I argued it badly. Answered explicitly in 2a69822 with a new "Does this decision survive #581?" subsection.
And yes to your follow-up: scale is multiplicative. The three channels compose differently, and only one of them is wrong:
| Channel | Composition today | Correct? |
|---|---|---|
| Rotation | parent.world + local — additive |
✅ nesting rotations adds angles |
| Scale | parent.world * local — multiplicative |
✅ a 2x parent with a 3x child is 6x |
| Position | parent.world + local — additive |
❌ the local offset is added raw, never scaled/rotated by the parent first |
So #581 isn't "make scale and rotation additive like position" — those two are already right. The fix makes position account for the parent's already-correct rotation and scale:
world.position = parent.world.position
+ rotate(local.position * parent.world.scale, parent.world.rotation)
What that means for DL-03: once #581 lands, option (b) stops being broken. A scaled canvas root would correctly scale its children's offsets, and sprite sizes already scale (bindSpriteInstanceData multiplies by scale.world). But (b) still loses on the arguments that were always the real ones:
- Cost — (a) changes one number on one camera; (b) dirties
position.worldandscale.worldfor every UI entity on every resize. - One coordinate space — with (a),
rectstays in reference pixels regardless of screen size, so layout, hit testing, and every rect consumer speak one language. With (b) rects live in a screen-dependent space, or you maintain both and everyone has to know which they're holding. - It breaks
isStatic(DL-12) — a subtree frozen at one scale is wrong after a resize, so the freeze would need invalidating on every resize, which is exactly the dirty-tracking complexity DL-12 avoids. - It breaks per-element scale — an author scaling a button 1.1x for a press animation would multiply against a screen-dependent base, so the same "pop" reads differently at 720p and 4K.
The honest correction, which the doc now states: leading with "(b) is broken" was lazy, because it's the one objection with an expiry date. The cost and coordinate-space arguments are load-bearing and hold either way.
Generated by Claude Code
|
|
||
| --- | ||
|
|
||
| ### DL-04 — Text uses MSDF atlases, in a sibling `/src/text` module |
There was a problem hiding this comment.
Ensure this is captured in the text issue you create
There was a problem hiding this comment.
Checked DL-04 against #584 line by line. Most of it was already there — the options table, the MSDF rationale, the atlas loader, the msdf-atlas-gen recipe, the kerning-pairs requirement, and the shipped default atlas.
One thing genuinely wasn't: the Canvas2D escape hatch. DL-04's decision is "(c) MSDF, with (a) available as a documented prototyping escape hatch", and #584 only dismissed Canvas2D as unsuitable without carrying the escape-hatch part. Now added as its own section.
The reasoning I wrote into the issue: MSDF is the shipping path, but atlas generation is a barrier at exactly the wrong moment — someone trying their own font for the first time, or throwing a debug readout on screen mid-session. So keep a fillText-to-texture path, with its limits stated plainly so nobody ships on it by accident:
- One draw call per string, no batching
- Re-uploads the texture on every change — unusable for anything per-frame (score, timer, FPS)
- Blurs when scaled
- No outline/glow/shadow
Plus a constraint worth designing to up front: both paths should share the same TextEcsComponent surface where practical, so moving a prototype onto a real atlas is a font swap rather than a rewrite. That only stays true if it's a requirement from the start.
I also strengthened the kerning note while there — missing kerning pairs produce text that's subtly wrong in a way that's hard to attribute later, which is worth flagging louder than a bullet in a list.
Generated by Claude Code
… the argument Answers directly whether the canvas-scaling decision changes once #581 lands. It does not, but the original rationale leaned on "(b) is broken" as its headline argument, which was the one objection with an expiry date. Adds a per-channel table making the composition rules explicit, since they differ and only one is wrong: rotation composes additively (correct - nesting rotations adds angles), scale composes multiplicatively (correct - a 2x parent with a 3x child is 6x), and position composes additively but omits transforming the local offset by the parent's scale and rotation first. So #581 is not "make scale and rotation additive like position"; those are already right. Records that once #581 lands, option (b) stops being broken - a scaled canvas root would scale children's offsets, and sprite sizes already scale via bindSpriteInstanceData - but still loses on the load-bearing arguments: it dirties every UI entity's world transform on resize where (a) changes one camera field, it puts rects in a screen-dependent space instead of reference pixels, it invalidates the isStatic freeze from DL-12 on every resize, and it makes a per-element press animation scale against a screen-dependent base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
The camera separation described in section 3 does affect DL-07, and the decision did not say so. Because UI renders through its own camera, one canvas-space pointer maps to two different world positions - one through each camera - and they diverge as soon as the world camera pans or zooms. So PointerEcsComponent publishes canvas pixels only and stays camera-agnostic as well as source-agnostic. It must not publish a world position, because there is no single correct one. Conversion is the caller's job and screenToWorldSpace is already camera-parameterized, so the UI raycaster converts through the UI camera and world picking converts through the world camera, with neither privileged. Also records the consistency requirement in the other direction: the raycaster must only hit-test what the UI camera can see, since an element culled by cullingMask that remains clickable produces invisible hit regions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
Yes, in a way I hadn't drawn out. Addressed in a6c083f. Because UI renders through its own camera (DL-01), a single canvas-space pointer maps to two different world positions — one through the world camera, one through the UI camera — and they diverge the moment the world camera pans or zooms. So screenToWorldSpace(screenPosition, cameraPosition, cameraZoom, width, height, pixelsPerUnit)The UI raycaster converts through the UI camera; game-world picking converts through the world camera; neither is privileged. That also keeps DL-07's original argument intact — a world-space consumer like drag-to-select or tower placement wants the world camera's conversion, which is only possible if the component stays raw. And the consistency requirement in the other direction, which is the part that would have bitten: the raycaster should only hit-test what the UI camera can actually see. An element culled from the UI camera by Generated by Claude Code |
| invisible DOM `<input>`, mirror its value into the element, and render the text | ||
| ourselves. (c) No text input in v1. | ||
|
|
||
| **Decision: (b).** |
There was a problem hiding this comment.
Text is not part of the UI feature, but text inputs can be? Thoughts?
There was a problem hiding this comment.
Maybe text inputs should be a separate issue?
There was a problem hiding this comment.
Answered in full here and here — this thread wasn't reachable through the API at the time, so those landed on the PR conversation instead. Linking back so the thread isn't left hanging.
Short version: a "text input" is three concerns — text entry (IME, soft keyboards, clipboard → input), text display (→ #584), and the focusable rect (→ UI). So it's not inconsistent, but it was the most hard-blocked item in the backlog and its hard part isn't UI at all.
Both now resolved: split out as #586, with the DOM entry bridge living in /src/input. Item 3.5 marked out of scope. Pushed in 2ffa81f and 317ef93.
Generated by Claude Code
…es out Answers why a text input belongs in this module when text rendering does not. They look inconsistent but aren't: a text input is three separable concerns. Text entry (keystrokes, caret, IME, clipboard, soft keyboard) is an input concern and almost always implies a focused widget, unlike damage numbers or world signage. Text display is identical to drawing any other text and belongs to #584. The focusable rect it hangs off is exactly what this module is. Fixes a real gap in the backlog bookkeeping while there: item 3.4 was marked blocked on #583 but 3.5 was not marked blocked on #584, despite being far more blocked. A scroll view merely looks wrong without clipping; a text field is unusable without text rendering, since you cannot see what you type. Also splits the hidden-input bridge out into /src/input, for the same reason DL-07 moved the pointer there. That bridge is where the IME and mobile-keyboard complexity lives and is reusable by anything needing text entry rather than being specific to one widget. As a result the "no DOM-backed widgets" non-goal becomes a real invariant instead of one with an exception carved out of it - the UI module now stays DOM-free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
They look inconsistent but aren't — and chasing it found a real gap in my bookkeeping. Addressed in 2ffa81f. A "text input" is three separable concerns wearing one name:
So the field belongs here. But your question exposed that I'd been sloppy: item 3.4 (ScrollRect) is marked blocked on #583, while 3.5 (text input) wasn't marked blocked on #584 — despite being far more blocked. A scroll view merely looks wrong without clipping; a text field is unusable without text rendering, because you can't see what you type. Now marked as the most hard-blocked item in the backlog. And it prompted a change I think is a genuine improvement. The hidden- The payoff: §2's "no DOM-backed widgets" non-goal becomes a real invariant instead of one with an exception carved out of it. The UI module is now DOM-free, full stop, and the one unavoidable piece of browser interop sits with the other input-source complexity where it belongs. Generated by Claude Code |
|
|
||
| ## 11. Open questions | ||
|
|
||
| 1. **Does `/src/text` ship before `/src/ui`, as its own minor release?** |
There was a problem hiding this comment.
Yes, this is the plan now. We will have a separate Text issue.
Text input follows text rendering and clipping out of the UI module. It is the strongest candidate of the three: it was the most hard-blocked item in the backlog (unusable, not merely imperfect, without #584), and its genuinely hard part - IME composition, mobile soft keyboards, clipboard, selection, accessibility - is an input concern rather than a UI one. #586 covers a text-entry primitive in /src/input wrapping a hidden DOM input, plus the thin UI component that consumes it. DL-10 stays as the decision record with a scope banner, and item 3.5 is marked out of scope. Also flags something for #584 before its API is fixed: rendering a caret and selection highlight needs per-glyph x-positions, so TextMeshEcsComponent should expose glyph advances rather than only quads. Retrofitting that later is more disruptive than including it from the start. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
Agreed — #586, done in 317ef93. It's the strongest of the three splits, for two reasons that only became clear once we'd worked through the previous question:
#586 covers the text-entry primitive in One thing I'd flag for #584 specifically, since it's cheap now and disruptive later: rendering a caret and a selection highlight needs per-glyph x-positions, so
Confirmed — that's #584, already filed and out of scope as of Where that leaves the scope splits: #583 clipping, #584 text rendering, #586 text input, plus #582 for the AGENTS.md accuracy fix. The UI module itself is now just the rect tree, layout, hit testing, interaction, and controls — which is what it should have been from the start. Generated by Claude Code |
| 3. **Which default font ships with the engine?** Needs a permissive license, good | ||
| glyph coverage, and a small atlas. The repo already vendors Fira and Aoboshi | ||
| references in `.cspell/project-words.txt`, suggesting prior art to check. | ||
| 4. **Should `Rect` be deprecated in the same release that introduces `Rect2`?** |
There was a problem hiding this comment.
Yes, and there is no need to document it as a breaking change.
Please don't call it Rect2 just Rect is fine. Replace the existing implementation.
There was a problem hiding this comment.
Done in cabf569 — full reply here (this thread wasn't reachable through the API at the time).
Rect replaced in place, no parallel type, no breaking-change note. Checking the blast radius supports replacing over deprecating: Rect is constructed nowhere in /src outside its own tests, and its only consumer is CameraEcsComponent.scissorRect, whose type name doesn't change.
One thing still needs your call, in §11: Vector2/Vec2 puts the long name on the type and the short one on the namespace, which doesn't translate — Rect has no natural abbreviation. I've proposed Rects for the static namespace; the more literal mirror would be Rectangle/Rect, but that moves Rect off the type, which may be the opposite of what you meant.
Generated by Claude Code
Per review, the plain-object rect keeps the name Rect and replaces the existing
class outright, rather than coexisting with it under a new name. Not documented
as a breaking change.
The blast radius turns out to be genuinely small, which supports replacing
rather than deprecating: Rect is constructed nowhere in /src outside its own
tests, and its only consumer is CameraEcsComponent.scissorRect, whose type name
does not change. What changes is construction and the two methods becoming
static calls.
Mirrors vector2.ts exactly - a plain interface for the data, a static-only class
for the operations with mutating helpers taking a target first argument. Keeps
{ min, max } over the current { origin, size }, since anchor math reads min/max
far more often and size is one subtraction away.
Folds in a file rename to rect.ts, since src/math/Rect.ts violates the
kebab-case convention in AGENTS.md and a full rewrite is the cheapest moment to
fix it.
Leaves one naming question open in section 11: Vector2/Vec2 puts the long name
on the type and the short one on the namespace, which does not translate since
Rect has no natural abbreviation. Rects is proposed; Rectangle/Rect is the more
literal mirror.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
Done in cabf569. DL-11 now replaces Checking the blast radius supports replacing rather than deprecating — it's smaller than I'd assumed when I wrote the coexistence plan: Shape mirrors export interface Rect {
min: Vector2;
max: Vector2;
}Kept Two things I folded in, shout if either is unwanted:
Generated by Claude Code |
Summary
Adds
/design/ui-system.md, a full architecture and delivery plan for aretained-mode, ECS-native UI system built on an anchored rect tree — a
hierarchy of rectangle-shaped elements, each anchored and pivoted against its
parent's rectangle. Deliberately not immediate-mode, and not a
markup-and-stylesheet model.
The name is descriptive rather than borrowed. The same approach appears in
Unity's uGUI, Godot's
Controlnodes, and Flash's display list; those are citedas prior art, but Forge names the model for what it does.
Documentation only. No engine code changes.
The document covers:
entities, where immediate-mode and markup-and-stylesheet models don't.
priority scale and a column naming what owns each gap.
composition of a button, the per-frame system pipeline and its ordering
constraints, RectTransform anchor/pivot resolution, the text pipeline, the
pointer interaction state machine, and the two paths into activation.
createComponentIdkeys,add-prefixed component factories, batchedcreate-prefixed system factories), opening with the minimal path — two callsto a working button.
the decision, rationale, and consequences. Two decisions were reversed during
review and record that explicitly.
Central finding
Most of what a UI module needs is not UI. Three capabilities Forge lacks
outright — text rendering, rect clipping, and a canvas-space pointer — are each
generally useful with no UI involved. Two of them are now owned separately
(#584, #583), which shrank Phase 0 from eleven items to five and moved the
plan's highest-risk item (the sub-quad renderer refactor) off the UI critical
path onto the text work it exists to serve.
The dependency worth stating plainly: UI layout, anchoring, hit testing, and
interaction can all be built and unit-tested before #584 lands, but the module
is not useful without text — buttons need labels, HUDs need numbers. Text
should gate the UI module's first release rather than its development.
Issues filed from the review of this document
Reviewing the design surfaced three genuine engine bugs and two scope splits:
AGENTS.mdoverstates input capabilities: claims touch (which doesn't exist), omits gamepad (which does)/src/rendering/src/textSpriteEcsComponent.pivotis Y-down while the rest of the engine is Y-upTwo further pre-existing problems are recorded in the document rather than as
issues, since the UI work is what fixes them:
MouseInputSourcecachesgetBoundingClientRect()in its constructor(
mouse-input-source.ts:61), so pointer coordinates are wrong after anyresize, scroll, or layout shift.
DepthEcsComponentis fully implemented and tested but referenced by nothingin
/src. DL-06 concludes it is not the right vehicle for draw order either,so the plan deletes it in favour of a
sortDepthfield on the sprite.Also documents the new
/designdirectory inAGENTS.md, including that itscontents are proposals and must never be treated as a description of current
engine behavior.
Verification checklist
Documentation-only change; the TypeScript checks are unaffected (no files under
/src,/demo, or/e2echanged).npm run check-typespasses with 0 errors — n/a, no source changesnpm testpasses — n/a, no source changesnpm run lintpasses with 0 errors — n/a, ESLint does not cover Markdownnpm run cspellpasses with 0 errors — verified locally on the changedfiles; new technical terms added to
.cspell/project-words.txtnpm run check-exportspasses — n/a, no export changesindex.ts— n/a/documentation-site/docs/docsis updated if thischange affects documented behavior — n/a, this describes a proposed
subsystem, not current behavior
Prettier formatting verified locally on both changed Markdown files, plus a
manual consistency sweep over the document (decision-log numbering, section
numbering, and module layout against referenced components).
031d13b(14:58). Neither theCI / Build and Testnor theChangelogworkflow has been instantiated for anycommit since, and the CodeQL jobs that are created never leave
queued, somergeable_stateisblockedon missing checks rather than failing ones. Allnine workflows are confirmed
state: active, so this appears to be anActions capacity or account-limit problem rather than anything in this branch.
Everything above was therefore verified by hand.
Changelog
docs.