Closed ketoo closed 4 years ago
Thanks for the contribution! 👍 Some thoughts:
EditorContextMoveToNode
is a nice idea! Haven't yet personally had node graphs grow that large, but this probably helps out in that case 🙂 SetNodeOriginPos
made me realize that SetNodePos
isn't clear about what the position is! I think this change is a good addition, but we could name things better. Maybe SetNodeGridSpacePos
and SetNodeScreenSpacePos
?SetNodeDraggable
-- it looks like you want to lock certain nodes in place? What sort of use-case do you have in mind?Thanks for the contribution! 👍 Some thoughts:
EditorContextMoveToNode
is a nice idea! Haven't yet personally had node graphs grow that large, but this probably helps out in that case 🙂SetNodeOriginPos
made me realize thatSetNodePos
isn't clear about what the position is! I think this change is a good addition, but we could name things better. MaybeSetNodeGridSpacePos
andSetNodeScreenSpacePos
?SetNodeDraggable
-- it looks like you want to lock certain nodes in place? What sort of use-case do you have in mind?
Thanks for your feedback.
For EditorContextMoveToNode, it's about the user experience, just to do a quick way to let the user can focus on which node now editing (Normally we have a tree controller to list all nodes, the user could click the tree node.).
For SetNodeOriginPos, I will modify the name under your advice and submit a new PR. Is that should be SetNodeScreenSpacePos?
For SetNodeDraggable, we will use it in the case as below: sometimes, we just want to display some items with the node system. For instance, we show some objects which come from other system but the viewer dont have the permission to modify the position as the position come from other systems too.
For SetNodeOriginPos, I will modify the name under your advice and submit a new PR. Is that should be SetNodeScreenSpacePos?
Your SetNodeOriginPos
would be named SetNodeGridSpacePos
since the node origin is implicitly assumed to be relative to the grid/canvas origin and your function sets this value without any transformations 🙂 It's probably something that I should document in the code a bit better. You can update this PR with the updated name if you like!
I can update the name of existing function 👍
For SetNodeDraggable, we will use it in the case as below: sometimes, we just want to display some items with the node system. For instance, we show some objects which come from other system but the viewer dont have the permission to modify the position as the position come from other systems too.
Ok, sounds like a useful feature then!
Merged it, and made a few formatting shanges of my own 🙂
Merged it, and made a few formatting shanges of my own 🙂
Thanks.
add draggable feature for node add interface to set the node's origin position