[WIP] Copy Paste feature for Scene Tree Dock #19327

swarnimarun · 2018-06-02T06:49:07Z

There are a few issues with creating a proper copy and paste some also cause of my limited skill but I created this PR just for the basic Copy-Paste feature.
I have tried my best to use the existing API rather than add new things to the system to bloat it up.
Hopefully there isn't any problems, this is my first serious PR. Sorry in advance for wasting anyone's time.

I also added some new icons hope no one minds. I am not sure if there is a design philosophy behind the designs but I tried to maintain some uniformity.

The caveats with it are,

Inter-scene copy-paste is not there.
Root can't be copied.
And finally if you change scene or close it you lose the copied data.

~~I would really like to fix the problems with the inter-scene stuff.~~
Figured that there is only way that is to save data of the nodes separately and use it to create nodes in the specific scene but this will require creating more than I did like to do now.
So I will work on that later. For this I don't think much more can be done.

Calinou · 2018-06-02T12:03:16Z

editor/scene_tree_dock.cpp

+			if (copied_nodes.empty()) {
+				current_option = -1;
+				accept->get_ok()->set_text(TTR("I see..."));
+				accept->set_text(TTR("Nothing in copy. Sorry Dude. Copy Something First."));


Normal sentence case should be used in dialogs (don't capitalize every word). Also, I would advise making it simpler and dropping the "Sorry Dude"; something like "Clipboard is empty, nothing to copy." should be enough.

Calinou · 2018-06-02T12:03:47Z

editor/scene_tree_dock.cpp

+			if (copied_nodes.empty()) {
+				current_option = -1;
+				accept->get_ok()->set_text(TTR("I see..."));
+				accept->set_text(TTR("Nothing in copy. Sorry Dude. Copy Something First."));


Same remark as above.

I forgot to change it sorry. It was just place holder back then.

swarnimarun · 2018-06-02T13:12:18Z

Fixed the problems and squashed everything together.

Zylann · 2018-06-02T21:29:55Z

I thought this form of copy paste should use the system clipboard? (Would allow way more use cases)

fracteed · 2018-06-03T02:25:33Z

We really need a copy/paste that will work between scenes and on hierarchies. I seem to remember another PR from last year that did a similar thing to this one, and only pasted within the scene. Not sure how this is any better than just using the existing duplication functionality?

swarnimarun · 2018-06-03T05:09:23Z

@Zylann I know that but then again you will have to convert the nodes to actual nodes rather than reference cause references can't be saved if the original is closed, that means we have to implement conversion of references to instances and then store them on clipboard.
But is that the correct method I am thinking. And so I realized that Blender has inter-instance copy-paste so it should be possible by reference even after the instance(even whole application is closed(all instances of blender)) have been closed(huge meshes can also be copied) but how?

I am rather new to programming on actual software. Mostly hackerrank. Realized that how much more real programmers need to work.

Although I too want to implement it but the problem is I am currently trying to figure out blenders implementation of it which is rather perfect even allowing for inter-instance copy paste.

@fracteed There are 2 advantages of this feature,

First, the copy stays in the copy, paste it as many times as you like.
-Second, this makes the duplicate and move little faster.
-Third, from first 2, there is a keep transform and a don't keep transform option which makes everything a little faster.

And I am not asking for this to become the copy-paste feature for Godot.
I created this for trying to understand the API. many people knowing that it's in a scene and limited asked me to make a PR so I did. I got in, it got in else I will make another proper one after a few weeks or more of understanding Blender and some other tools which have lots of awesome features. Not to forget Inkscape.

I probably be back to visit the feature once I figure out the implementation that is used in Blender, which should be similar. Until then if it's the unanimous decision to close it then close it I shall.

fracteed · 2018-06-03T07:44:36Z

@swarnimarun don't let my comments deter you in the slightest, as I applaud any effort in this direction! Hopefully one of the more experienced devs can help you develop this into a cross scene copy/paste which is sorely needed :) Not sure if you looked at #14810 as it seems similar?

Zylann · 2018-06-03T11:24:03Z

@swarnimarun yeah I think it was mentionned somewhere it should work using serialization, a bit like scn. Using refs can be dangerous even within the same instance.

swarnimarun · 2018-06-03T13:49:46Z

@Zylann Was thinking of that, will be looking into it.

@fracteed Yeah saw that PR now, it's not useful cause it's quite like mine just seems to be using node_path rather than actual copy and paste.

Figuring out a proper implementation should not be that hard but well it's just that I experience level is quite low so it will be me running around in circles until I figure things out. But those things can't stop me, might(more like will) slow me down though.
Probably I am gonna dive into something that I understand more and once I figure things out I would return create a new proper PR then.
This will be mostly be a reminder....

eon-s · 2018-06-04T04:46:52Z

I wonder if this can be a bit like the copy params feature of the inspector, but copy a "tree branch" of parameters instead.

swarnimarun · 2018-06-04T06:12:34Z

@eon-s Yeah I thought the same thing, but the problem with that is we also have to be able to copy whole trees no matter the complexity and copying the params for all of them not a really good idea, so creating a new feature to do it is inevitable.

akien-mga · 2018-07-26T08:34:08Z

So we have three WIP PRs for this kind of feature, we'll have to decide which one to focus on and eventually merge:

Scene Tree Dock: added ability to cut/paste nodes. Scene Tree Dock: added ability to cut/paste nodes. #13309 by @Krakean
Add "Paste From Node Path" to scene tree Add "Paste From Node Path" to scene tree #14810 by @poke1024
Basic Copy Paste feature [WIP] Copy Paste feature for Scene Tree Dock #19327 by @swarnimarun

reduz · 2018-07-27T13:01:27Z

editor/scene_tree_dock.cpp

@@ -1599,6 +1731,7 @@ void SceneTreeDock::replace_node(Node *p_node, Node *p_by_node) {
 void SceneTreeDock::set_edited_scene(Node *p_scene) {

 	edited_scene = p_scene;
+	copied_nodes.clear();


It feels to me this will memleak, you should do a proper function to get rid of the nodes when you no longer need them

Copying/Pasting nodes between scenes does look like a desired feature

you may also want to call the clear clipboard funciton on exit (NOTIFICATION_EXIT_TREE)

But only if node stuff is being copied. Why not copy serialized data instead? Would work across scenes, even if nodes are deleted, and perhaps even across instances.

I side with @Zylann, serializing is the right way to go IMHO. BTW the reason I only copied note paths in #14810 was because keeping track of which Node* in an array are still valid for copying after some time (and scene changes) is a nightmare.

I created this PR while I was learning basics.

And, yeah @Zylann and @poke1024, I get that serialisation will be best.

I was earlier planning to write a function like _parse_nodes(to serialize) from the Scene State,
And probably just keep the signals between the copied nodes and ignore the rest.
After that I could de-serialise on paste and the rest was simple.

I was still waiting for the right time to implement it, as it's a little new for me. And as, this will likely result in me redoing the whole copy paste feature.
And I was tight on time these days, as I just got busy with my College, just got in.

And from what I have been told, I think that without multi-scene or multi-instance copy-paste it's useless.

There is not really any risk in copying the nodes and keeping them in memory. Nodes are completely disabled while outside the scene tree. You can even copy them across scenes fine.
There really is no need to do serialization.

@reduz there is if another unrelated part of the editor destroys them. If you delete them or close the scene for example.

eon-s · 2018-07-28T20:20:13Z

I ask here to not add more noise to the review, about the serialization, won't it be better for the use of regular clipboard and keep cut/copy there even with the engine closed?

poke1024 · 2018-08-04T11:20:52Z

@eon-s As a user: yes! As developer: might be tricky, as the serialized content might be large depending on the kind of tree/sub trees you copy and cannot keep refcounted objects without copying them. Ever got that dialog box from Excel "your clipboard is very large, do you really want to export it to the system clipboard?" Adds quite more complexity to serialization I guess.

swarnimarun · 2019-05-21T17:38:10Z

Gonna be reworking on this so probably gonna be a bit of noise here.

akien-mga · 2019-08-28T09:51:44Z

There's a new take on this feature in #31616 - should we review both implementations in parallel, or do you think that #31616 is more advanced and should supersede this one?

swarnimarun · 2019-08-29T14:38:43Z

Closing this for now. Will be redoing this one as a few things have changed since and this one doesn't even work anymore...

Calinou requested changes Jun 2, 2018

View reviewed changes

swarnimarun force-pushed the copy-paste branch from 5f583fe to 7834067 Compare June 2, 2018 13:11

mhilbrunner added enhancement topic:editor labels Jun 4, 2018

mhilbrunner added this to the 3.1 milestone Jun 4, 2018

akien-mga added feature proposal usability and removed enhancement labels Jul 26, 2018

reduz reviewed Jul 27, 2018

View reviewed changes

This was referenced Jul 27, 2018

Add "Paste From Node Path" to scene tree #14810

Closed

Scene Tree Dock: added ability to cut/paste nodes. #13309

Closed

swarnimarun changed the title ~~Basic Copy Paste feature~~ [WIP] Copy Paste feature for Scene Tree Dock Aug 13, 2018

akien-mga modified the milestones: 3.1, 3.2 Dec 9, 2018

swarnimarun force-pushed the copy-paste branch from 7834067 to 74eebc7 Compare May 21, 2019 17:37

Adding copy and paste feature [ci skip]

d122a59

swarnimarun force-pushed the copy-paste branch from 74eebc7 to d122a59 Compare May 21, 2019 17:51

KoBeWi mentioned this pull request Aug 24, 2019

Add copy/paste nodes across scenes and outside editor #31616

Closed

swarnimarun closed this Aug 29, 2019

akien-mga added the archived label Aug 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Copy Paste feature for Scene Tree Dock #19327

[WIP] Copy Paste feature for Scene Tree Dock #19327

swarnimarun commented Jun 2, 2018 •

edited

Loading

Calinou Jun 2, 2018

Calinou Jun 2, 2018

swarnimarun Jun 2, 2018 •

edited

Loading

swarnimarun commented Jun 2, 2018

Zylann commented Jun 2, 2018

fracteed commented Jun 3, 2018

swarnimarun commented Jun 3, 2018

fracteed commented Jun 3, 2018

Zylann commented Jun 3, 2018

swarnimarun commented Jun 3, 2018

eon-s commented Jun 4, 2018

swarnimarun commented Jun 4, 2018

akien-mga commented Jul 26, 2018

reduz Jul 27, 2018

reduz Jul 27, 2018

Zylann Jul 27, 2018 •

edited

Loading

poke1024 Jul 28, 2018

swarnimarun Jul 28, 2018 •

edited

Loading

reduz Jul 28, 2018

Zylann Jul 28, 2018 •

edited

Loading

eon-s commented Jul 28, 2018

poke1024 commented Aug 4, 2018

swarnimarun commented May 21, 2019

akien-mga commented Aug 28, 2019

swarnimarun commented Aug 29, 2019

[WIP] Copy Paste feature for Scene Tree Dock #19327

[WIP] Copy Paste feature for Scene Tree Dock #19327

Conversation

swarnimarun commented Jun 2, 2018 • edited Loading

The caveats with it are,

Calinou Jun 2, 2018

Choose a reason for hiding this comment

Calinou Jun 2, 2018

Choose a reason for hiding this comment

swarnimarun Jun 2, 2018 • edited Loading

Choose a reason for hiding this comment

swarnimarun commented Jun 2, 2018

Zylann commented Jun 2, 2018

fracteed commented Jun 3, 2018

swarnimarun commented Jun 3, 2018

fracteed commented Jun 3, 2018

Zylann commented Jun 3, 2018

swarnimarun commented Jun 3, 2018

eon-s commented Jun 4, 2018

swarnimarun commented Jun 4, 2018

akien-mga commented Jul 26, 2018

reduz Jul 27, 2018

Choose a reason for hiding this comment

reduz Jul 27, 2018

Choose a reason for hiding this comment

Zylann Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

poke1024 Jul 28, 2018

Choose a reason for hiding this comment

swarnimarun Jul 28, 2018 • edited Loading

Choose a reason for hiding this comment

reduz Jul 28, 2018

Choose a reason for hiding this comment

Zylann Jul 28, 2018 • edited Loading

Choose a reason for hiding this comment

eon-s commented Jul 28, 2018

poke1024 commented Aug 4, 2018

swarnimarun commented May 21, 2019

akien-mga commented Aug 28, 2019

swarnimarun commented Aug 29, 2019

swarnimarun commented Jun 2, 2018 •

edited

Loading

swarnimarun Jun 2, 2018 •

edited

Loading

Zylann Jul 27, 2018 •

edited

Loading

swarnimarun Jul 28, 2018 •

edited

Loading

Zylann Jul 28, 2018 •

edited

Loading