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.

Sign up for GitHub

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

Jump to bottom

Error in Copy/Paste node #205

Closed
likemango opened this issue May 17, 2024 · 3 comments
Closed

Error in Copy/Paste node #205

likemango opened this issue May 17, 2024 · 3 comments
Assignees
@MothDoctor

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 •
edited
Loading

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
@MaksymKapelianovych
Several improvements for copy/pasting nodes:
e35333e 
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)
@MaksymKapelianovych MaksymKapelianovych mentioned this issue Jul 26, 2024
Open
MothDoctor added a commit that referenced this issue Dec 15, 2024
@MothDoctor
#205 preventing duplication of unique nodes like Start Node
19857cd
@MothDoctor
Copy link
Contributor

Hey @MaksymKapelianovych thanks for investigating this issue.

I went with option 4 (slightly adjusted after testing) since I assume that the editor code should know about the specializations of specific nodes. And yes, usually Flow Node class doesn't disable duplicating. It only occurs for super specific nodes like "Start", so this solution shouldn't cause issues :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MothDoctor MothDoctor

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@MothDoctor @likemango @MaksymKapelianovych