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

Added duplicate selection (CTRL+D), undo and redo (CTRL+Z/Y) with scene history. #133

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

ReubenJCarter
Copy link

No description provided.

@paceholder
Copy link
Owner

My laptop is in repair and I was busy with our small baby last weeks.
I will come back to it as soon as I can.

Thanks
Dmitry

Copy link

@Daguerreo Daguerreo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indenting is not consistent with the project. Copy and Pasting a node from a scene to another should not throw in case the node is not registered in second scene but just not performing the operation.

@@ -126,6 +128,8 @@ class NODE_EDITOR_PUBLIC NodeDataModel
virtual
NodePainterDelegate* painterDelegate() const { return nullptr; }

QString toolTipText();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be const and inline as other getters

@@ -39,3 +40,15 @@ setNodeStyle(NodeStyle const& style)
{
_nodeStyle = style;
}


void NodeDataModel::setToolTipText(QString text)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to const QString& text

@@ -54,6 +59,10 @@ class NODE_EDITOR_PUBLIC FlowScene

Node&restoreNode(QJsonObject const& nodeJson);

QUuid pasteNode(QJsonObject &json);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it const reference

_scene->UpdateHistory();
}

void FlowView::duplicateSelectedNode()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of implicit conversion warning float-double

src/FlowView.cpp Outdated

//create nodes
std::vector<Node*> createdNodes;
std::vector<Node*> couterpartNode;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe "counterPart" ?

@@ -123,6 +132,20 @@ restoreConnection(QJsonObject const &connectionJson)
return createConnection(*nodeIn, portIndexIn, *nodeOut, portIndexOut);
}

void FlowScene::pasteConnection(QJsonObject const &connectionJson, QUuid newIn, QUuid newOut)
{
QUuid nodeInId = QUuid(connectionJson["in_id"].toString());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused nodeInId and nodeOutId

src/FlowView.cpp Outdated
if(j >=0 && k>=0 && j < couterpartNode.size() && k < couterpartNode.size())
{
auto connection = _scene->createConnection(*createdNodes[j], portIndexIn, *createdNodes[k], portIndexOut);
auto& connectionRef = connection;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused variable

@paceholder
Copy link
Owner

This pull request has a plenty of unrelated features.
Some of them seemed to be half-baked.

Nevertheless, I took some coding samples from this work and implemented an Undo/Redo system and a Ctrl+D node duplication mechanism.

I'll continlue looking through the code to take more features for master

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

Successfully merging this pull request may close these issues.

5 participants