Skip to content
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

Error in Copy/Paste node #205

Open
likemango opened this issue May 17, 2024 · 2 comments
Open

Error in Copy/Paste node #205

likemango opened this issue May 17, 2024 · 2 comments
Assignees

Comments

@likemango
Copy link

When I select a group of nodes including StartNode which can't be duplicated, then, I can paste those group of nodes. Now, StartNode been duplicated couldn't be deleted, get mess on the graph.
image

@MaksymKapelianovych
Copy link
Contributor

MaksymKapelianovych commented May 18, 2024

I did some investigation of this issue and found the root of it.
When copying selected nodes, editor allows to copy nodes if any of the nodes can be duplicated/copied. Actual checking for each node is located inside FCustomizableTextObjectFactory::ProcessBuffer. It takes node's class, then its CDO and call CanDuplicateNode(). If we look into UFlowGraphNode's implementation of CanDuplicateNode(), we will see that it requires FlowNode instance to be set, and CDO do not have it, therefore for CDO this function will always return true.
image_2024-05-18_16-23-41

I came up to 4 possible solutions to this issue:

  1. (Preffered by me as a general case) Filter out nodes, which can't be duplicated, in CopySelectedNodes. Working example:
    image_2024-05-18_16-23-41 (3)

  2. (Only applicable to Start node) Automaticaly replace Start node to CustomInput node. It would be similar behaviour to copying CustomEvent nodes in Blueprints.

  3. Modify UFlowGraphNode::CanDuplicateNode to take into account AssignedNodeClasses. It would be perfectly fine in case of single element in AssignedNodeClasses, but what if there are two classes and each have different bCanDuplicate value? Working(?) example:
    image_2024-05-18_16-23-41 (2)

  4. Store bCanDuplicate directly in UFlowGraphNode. This is not very flexible as for noncopyable custom UFlowNode user would have to specify special UFlowGraphNode with bCanDuplicate == false.

I don't know which fix would be better fit (maybe something else), so leave it to @MothDoctor to decide.

@likemango
Copy link
Author

Thanks, you have a great ideas to solve this problem, I gonna try those.

@MothDoctor MothDoctor self-assigned this Jun 29, 2024
MaksymKapelianovych added a commit to MaksymKapelianovych/FlowGraph that referenced this issue Jul 26, 2024
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants