★ wanayoo — archive 1999 https://github.com/paceholder/nodeeditor/pull/133Nouvelle recherche | Portail wanayoo
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 19 commits into
base: master
from

Conversation

@ReubenJCarter
Copy link

@ReubenJCarter ReubenJCarter commented Nov 15, 2017

No description provided.

@paceholder
Copy link
Owner

@paceholder paceholder commented Nov 28, 2017

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

ReubenJCarter and others added 7 commits Feb 7, 2018
Copy link

@Daguerreo Daguerreo left a comment

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.

QString modelName = nodeJson["model"].toObject()["name"].toString();
auto dataModel = registry().create(modelName);

if (!dataModel)

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

Consider this scenario:

Scene S1 with a set of registered models M1.
Scene S2 with a set of registered models M2.

where M1 != M2 or M2 subset of M1.

If you try to copy and paste a node from S1 to S2 which not belong to M2 it will throw.
A more correct approach would be returning with doing anything with a simple warning message.

@@ -55,7 +55,8 @@ class NODE_EDITOR_PUBLIC Connection

QJsonObject
save() const override;


QJsonObject copyWithNewID(QUuid in, QUuid out);

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

Indentation in headers is not consisteng

}

void FlowScene::Undo()
{
std::cout << "Undo" << std::endl;

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

Remove cout. Same in Redo()

@@ -491,6 +535,11 @@ saveToMemory() const
}


void FlowScene::saveToClipBoard()
{

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

Why is it empty?

@@ -122,6 +143,10 @@ class NODE_EDITOR_PUBLIC FlowScene
std::unordered_map<QUuid, SharedConnection> _connections;
std::unordered_map<QUuid, UniqueNode> _nodes;
std::shared_ptr<DataModelRegistry> _registry;

int historyInx;

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

private member should start with _

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

QString toolTipText();

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

Should be const and inline as other getters

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


void NodeDataModel::setToolTipText(QString text)

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

change to const QString& text

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

Node&restoreNode(QJsonObject const& nodeJson);

QUuid pasteNode(QJsonObject &json);

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

make it const reference

_scene->UpdateHistory();
}

void FlowView::duplicateSelectedNode()

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

a lot of implicit conversion warning float-double


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

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

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());

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

unused nodeInId and nodeOutId

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

This comment has been minimized.

@Daguerreo

Daguerreo Apr 19, 2019

unused variable

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

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.