-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Several improvements for copy/pasting nodes: #219
base: 5.x
Are you sure you want to change the base?
Several improvements for copy/pasting nodes: #219
Conversation
edited
- UFlowNode_Start now is substituted with UFlowNode_CustomInput when duplicating or copy/pasting. Error in Copy/Paste node #205
- Reset EventName in UFlowNode_CustomInput after duplicating or copy/pasting
- Fix copy/pasting comments. Copy Pasting comments broken in 5.x #215
- CanUserDeleteNode and CanDuplicateNode now use FlowNode's CDO if NodeInstance is NULL (always when pasting). This will prevent pasting nodes that cannot be duplicated, but are copied with other duplicable nodes.
- Fix pasting nodes into FlowAsset when FlowAsset cannot accept such nodes (node or asset class is denied).
- If among selected nodes there are nodes nodes that cannot be deleted, they will stay in graph as is and all "deletable" nodes will be deleted (currently none of the nodes will be deleted in such case)
- Improve duplicating and copy/pasting of AddOns (as a reference was taken AIGraphEditor code)
- duplicated AddOn is pasted on the same level as selected AddOn
- allow pasting AddOns only if there is one SelectedNode
- check if SelectedNode for paste can accept AddOn
- fix copying AddOns if both AddOn and its ParentNode are selected
- fix copying AddOns if there are other GraphNodes selected (not AddOns)
1. UFlowNode_Start now is substituted with UFlowNode_CustomInput when duplicating or copy/pasting. MothCocoon#205 2. Reset EventName in UFlowNode_CustomInput after duplicating or copy/pasting 3. Fix copy/pasting comments. MothCocoon#215 4. CanUserDeleteNode and CanDuplicateNode now use FlowNode's CDO if NodeInstance is NULL (always when pasting). This will prevent pasting nodes that cannot be duplicated, but are copied with other duplicable nodes. 5. Improve duplicating and copy/pasting of AddOns (as a reference was taken AIGraphEditor code) - duplicated AddOn is pasted on the same level as selected AddOn - allow pasting AddOns only if there is one SelectedNode - check if SelectedNode for paste can accept AddOn - fix copying AddOns if both AddOn and its ParentNode are selected - fix copying AddOns if there are other GraphNodes selected (not AddOns)
For this issue I went with second option (Substitute UFlowNode_Start with UFlowNode_CustomInput) #205. Please let me know if you prefer different solution. @MothDoctor @LindyHopperGT please take a look, especially AddOn part) |
I am just now catching-up on this. I have a few thoughts. I think there's a version of #3 that uses
If both are null when I favor the #3 solution generally, because it is simplest. If we can detect the illegal operation (that is, trying to copy a set of nodes, that includes an uncopyable node) and then reject the entire copy operation as a result; we can prevent any further need to handle the complexities of copying some, but not all, the nodes and what that means. I don't like the #2 solution that changes the node type, that is a strange semantic choice that doesn't follow from the concept of a 'copy request'. I think, if we do allow the copy (which I prefer to reject the whole thing, if we can), then the #1 solution of filtering out the uncopyable nodes makes the most sense. -- Take these comments with a disclaimer of me having only about 20mins of researching and thinking about this problem, first thing in the morning ... so I may be missing some nuance(s). |
Solution №3 is implemented as a general one for all nodes, but I thought that it would be nice to implement №2 for Start node and to create CutomInput node if user deliberately copied Start node, maybe for some debug purposes. But it is not necessary to have this feature, we can either make it optional by enabling/disabling in FlowGraphSettings or completely remove it |
UFlowNodeBase* Node = NodeInstanceClass->GetDefaultObject<UFlowNodeBase>(); | ||
return Node->bCanDuplicate; | ||
} | ||
return Super::CanDuplicateNode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to allow duplicating node if NodeInstance and NodeInstanceClass are invalid? This node is clearly malformed in this case. Maybe just return false?