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

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

Closed
wants to merge 1 commit into from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Aug 24, 2019

Yet another attempt at node copy-pasting. This time it allows to copy nodes outside editor, thus between editor instances or into anything that takes text. It's achieved by converting node data to text and parsing as scene on paste.

Closes #3720
Closes #7605
Closes #13380
Supersedes #19327

Caveats:

  • doesn't allow to cut/copy more than one node
  • uses intermediate file to get scene text data for clipboard (and to paste it)
  • discards any instancing

Unfortunately, I had to apply sort of a hack for converting the node data. I'm saving a file in a cache directory and then load it as text and copy into clipboard. For pasting, I'm creating a temporary file with clipboard contents and instance it into scene. The reason for this is that ResourceSaver/Loader can only work on files and don't operate on strings in memory. Perfectly, it should be changed, but could be done in another PR probably (it works and this feature waited long enough anyways).

Here's a video showcase: https://youtu.be/5vqNqaOtrTI

@KoBeWi KoBeWi force-pushed the totally_not_a_hack branch from fd3aa3b to c5c327a Compare August 24, 2019 02:27
editor/scene_tree_dock.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

CC @swarnimarun who implemented the WIP #19327.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 28, 2019

Ok, so we had a talk about this PR on the IRC, and @reduz says this feature is too much and you don't need copy-paste outside the current project (and that it can apparently be done with a plugin). The problem with this implementation is that it doesn't care about node/resource pointers, so e.g. copy-paste inside one scene doesn't work exactly like duplicate, which is to be expected.

That being said, I'll just close this PR in favor of #19327 (which hopefully works more correctly, but I haven't tested it)

EDIT:
Heh, that PR died too.

@swarnimarun
Copy link
Contributor

This one actually seemed better than whatever mess I had created an year ago... :(

@swarnimarun
Copy link
Contributor

Went through the code pretty much what I would have wanted to do for the PR. What was the problem?

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 29, 2019

What was the problem?

I explained briefly in the closing post. Basically, when you copy-paste a node in the same scene, it has to work exactly the same like duplicating the node. Which means you have to keep resource references (so they are not duplicated if local to scene) and some other internal stuff, because "users will expect this". Which was not the case with my PR.

@swarnimarun
Copy link
Contributor

swarnimarun commented Aug 31, 2019

Aah makes sense so probably something more like my old PR. But better would be what we need.

Better as in, without all the crashes, memleaks and stuff... :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants