recording: groups expanded to teach spatial membership (in sticks, out detaches)#28
Merged
Merged
Conversation
…t detaches) The groups tutorial only showed dragging a group that already contained its nodes - it never demonstrated the actual rule: membership is spatial, decided at group-drag time (FindNodesInRect over m_GroupBounds in the C++). The legacy driver also had ZERO verification, so it silently recorded a static group when the grab missed the header (a node is moved by its title band; the content box rubber-bands). Reworked: - Example: Texture starts INSIDE the group, Tint OUTSIDE. - Driver, 5 beats, each hard-verified: drag Tint IN -> a group move carries both -> drag Tint OUT -> a group move leaves Tint behind. record_check_changed proves every node that should move did; record_check_unchanged proves Tint stays put when the group moves while it is outside (the lesson, asserted). dropped=0, ok=true. - Grabs the group by its GROUP_TITLE header (the only drag handle) and nodes by their title row (center hits the right-aligned pin -> link drag). - RST rewritten: spatial membership + the header-grab rule. Depends on borisbat/dasImgui#161 (record_check_unchanged) being on dasImgui master, since node-editor CI compiles record_*.das against borisbat/dasImgui@master. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Expands the groups node-editor tutorial recording to explicitly demonstrate and assert the engine’s spatial group membership rule (node joins/leaves based on whether it lies within group bounds at group-drag time), and updates the tutorial example + documentation to match.
Changes:
- Reworks
record_groups.dasinto a 5-beat, self-verifying recording that drags Tint in/out and verifies which nodes move (including a “must not move” assertion). - Adjusts initial node placements in the groups tutorial example so one node starts inside the group and one outside.
- Rewrites the groups tutorial RST to explain spatial membership and the requirement to drag groups by the header.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/integration/record_groups.das | Reworked recording driver to demonstrate/verify spatial membership (in/out) and correct drag handles. |
| examples/tutorial/groups.das | Re-seeded node positions so the example starts with one node inside the group and one outside. |
| doc/source/tutorials/groups.rst | Updated tutorial text to explain spatial membership and header-grab behavior; added recording notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+116
to
+120
| record_check_changed(app, T_GROUP, "canvas_bbox", before_g2) | ||
| record_check_changed(app, T_TEX, "canvas_bbox", before_tex2) | ||
| // Hard self-verify the membership rule: Tint is outside now, so it MUST NOT move. | ||
| record_check_unchanged(app, T_TINT, "canvas_bbox", before_tint2) | ||
| say(app, "move again -> Texture aboard, Tint left behind", T_GROUP, |
Comment on lines
+7
to
+10
| A **group** (or comment box) is a node with no pins and an explicit content size — the | ||
| editor's tool for visually organizing a large graph. Membership is purely **spatial**: | ||
| drag a node *inside* the group's bounds and it travels with the group from then on; drag | ||
| it back *out* and the group leaves it behind. There is no "add to group" call. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the node-editor tutorial recording sweep. Expands
groupsto teach the rule it was missing.The gap
The old
groupstutorial only dragged a group that already contained its nodes — it never showed the actual behavior: membership is spatial, decided at group-drag time (FindNodesInRectoverm_GroupBoundsin the imgui-node-editor C++). Drag a node inside the bounds and it joins; drag it out and it detaches. The legacy driver also had zero verification — it would have silently recorded a static group if the grab missed (a group is moved only by its title header; pressing the content box rubber-band-selects and moves nothing).The rework
record_check_changedproves every node that should move did.record_check_unchangedproves Tint stays put when the group moves while it's outside — the lesson, asserted, not just shown.dropped=0,ok=true.GROUP_TITLEheader (the only drag handle) and nodes by their title row (the center hits the right-aligned pin → would start a link drag).Dependency
record_check_unchanged). Node-editor CI compilesrecord_*.dasagainstborisbat/dasImgui@master, so #161 must merge first or this PR's build will fail on the missing rail. No other coupling.Notes
This is its own PR (not folded into #27) so the rail-independent delete_and_select / context_menus in #27 stay mergeable now. The
.apng/ wavs / manifest / sidecar are gitignored intermediates; only the.mp4is tracked.🤖 Generated with Claude Code