Skip to content

Commit a7b8299

Browse files
committed
Further improved code style consistency; Added extensive additional checks in NodeArea::LoadFromJson, which now returns a bool indicating success; Added GetConnectionCount() and GetRerouteConnectionCount() functions to NodeArea; Fixed an issue where NodeArea::TryToConnect could incorrectly connect two output sockets; Introduces a mechanism to validate the structure of nodes loaded from JSON against their current C++ class definition, specifically checking for mismatches in the number of input and output sockets, incorrect nodes will now be omitted; BREAKING The signature of Node::FromJson has changed from virtual void FromJson(Json::Value Json) to virtual bool FromJson(Json::Value Json), any classes inheriting from Node and overriding FromJson must update their signature and return true on success or false on failure (e.g., validation failure).
1 parent 0ad22c2 commit a7b8299

8 files changed

Lines changed: 366 additions & 120 deletions

File tree

SubSystems/VisualNodeArea/VisualNodeArea.cpp

Lines changed: 160 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -359,27 +359,37 @@ void NodeArea::CopyNodesTo(NodeArea* SourceNodeArea, NodeArea* TargetNodeArea)
359359
}
360360
}
361361

362-
void NodeArea::LoadFromJson(std::string JsonText)
362+
bool NodeArea::LoadFromJson(std::string JsonText)
363363
{
364364
if (JsonText.find("{") == std::string::npos || JsonText.find("}") == std::string::npos || JsonText.find(":") == std::string::npos)
365-
return;
365+
return false;
366366

367-
Json::Value root;
368-
JSONCPP_STRING err;
369-
Json::CharReaderBuilder builder;
367+
Json::Value Root;
368+
JSONCPP_STRING Error;
369+
Json::CharReaderBuilder Builder;
370370

371-
const std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
372-
if (!reader->parse(JsonText.c_str(), JsonText.c_str() + JsonText.size(), &root, &err))
373-
return;
371+
const std::unique_ptr<Json::CharReader> Reader(Builder.newCharReader());
372+
if (!Reader->parse(JsonText.c_str(), JsonText.c_str() + JsonText.size(), &Root, &Error))
373+
return false;
374374

375-
if (!root.isMember("nodes"))
376-
return;
375+
if (!Root.isMember("nodes"))
376+
return false;
377+
378+
if (!Root["nodes"].isObject())
379+
return false;
377380

378381
std::unordered_map<std::string, Node*> LoadedNodes;
379-
std::vector<Json::String> NodesList = root["nodes"].getMemberNames();
382+
std::vector<Json::String> NodesList = Root["nodes"].getMemberNames();
380383
for (size_t i = 0; i < NodesList.size(); i++)
381384
{
382-
std::string NodeType = root["nodes"][std::to_string(i)]["nodeType"].asCString();
385+
std::string NodeKey = std::to_string(i);
386+
if (!Root["nodes"][NodeKey].isMember("nodeType"))
387+
continue;
388+
389+
if (!Root["nodes"][NodeKey]["nodeType"].isString())
390+
continue;
391+
392+
std::string NodeType = Root["nodes"][NodeKey]["nodeType"].asCString();
383393
Node* NewNode = NODE_FACTORY.CreateNode(NodeType);
384394
if (NewNode == nullptr)
385395
{
@@ -393,7 +403,14 @@ void NodeArea::LoadFromJson(std::string JsonText)
393403
}
394404
}
395405

396-
NewNode->FromJson(root["nodes"][std::to_string(i)]);
406+
bool bResult = NewNode->FromJson(Root["nodes"][NodeKey]);
407+
if (!bResult)
408+
{
409+
// TO-DO: Implement a more robust user notification system (e.g., logging, UI warning).
410+
delete NewNode;
411+
NewNode = nullptr;
412+
continue;
413+
}
397414

398415
if (NewNode != nullptr)
399416
{
@@ -402,102 +419,173 @@ void NodeArea::LoadFromJson(std::string JsonText)
402419
}
403420
}
404421

405-
std::vector<Json::String> ConnectionsList = root["connections"].getMemberNames();
406-
for (size_t i = 0; i < ConnectionsList.size(); i++)
422+
if (Root.isMember("connections"))
407423
{
408-
std::string InSocketID = root["connections"][ConnectionsList[i]]["in"]["socket_ID"].asCString();
409-
std::string InNodeID = root["connections"][ConnectionsList[i]]["in"]["node_ID"].asCString();
424+
if (!Root["connections"].isObject())
425+
return false;
426+
427+
std::vector<Json::String> ConnectionsList = Root["connections"].getMemberNames();
428+
for (size_t i = 0; i < ConnectionsList.size(); i++)
429+
{
430+
if (!Root["connections"][ConnectionsList[i]].isObject())
431+
return false;
432+
const Json::Value& ConnectionData = Root["connections"][ConnectionsList[i]];
433+
434+
// TO-DO: Implement a more robust user notification system (e.g., logging, UI warning).
435+
if (!ConnectionData.isMember("in") || !ConnectionData.isMember("out") || !ConnectionData["in"].isObject() || !ConnectionData["out"].isObject())
436+
continue;
410437

411-
std::string OutSocketID = root["connections"][ConnectionsList[i]]["out"]["socket_ID"].asCString();
412-
std::string OutNodeID = root["connections"][ConnectionsList[i]]["out"]["node_ID"].asCString();
438+
if (!ConnectionData["in"].isMember("socket_ID") || !ConnectionData["in"]["socket_ID"].isString() ||
439+
!ConnectionData["in"].isMember("node_ID") || !ConnectionData["in"]["node_ID"].isString() ||
440+
!ConnectionData["out"].isMember("socket_ID") || !ConnectionData["out"]["socket_ID"].isString() ||
441+
!ConnectionData["out"].isMember("node_ID") || !ConnectionData["out"]["node_ID"].isString())
442+
{
443+
continue;
444+
}
445+
446+
std::string InSocketID = ConnectionData["in"]["socket_ID"].asCString();
447+
std::string InNodeID = ConnectionData["in"]["node_ID"].asCString();
413448

414-
if (LoadedNodes.find(OutNodeID) != LoadedNodes.end() && LoadedNodes.find(InNodeID) != LoadedNodes.end())
415-
if (!TryToConnect(LoadedNodes[OutNodeID], OutSocketID, LoadedNodes[InNodeID], InSocketID))
449+
std::string OutSocketID = ConnectionData["out"]["socket_ID"].asCString();
450+
std::string OutNodeID = ConnectionData["out"]["node_ID"].asCString();
451+
452+
// TO-DO: Implement a more robust user notification system (e.g., logging, UI warning).
453+
if (LoadedNodes.find(InNodeID) == LoadedNodes.end() || LoadedNodes.find(OutNodeID) == LoadedNodes.end())
416454
continue;
417455

418-
Connection* NewConnection = Connections.back();
456+
if (LoadedNodes.find(OutNodeID) != LoadedNodes.end() && LoadedNodes.find(InNodeID) != LoadedNodes.end())
457+
if (!TryToConnect(LoadedNodes[OutNodeID], OutSocketID, LoadedNodes[InNodeID], InSocketID))
458+
continue;
419459

420-
// First pass to fill information that does not depend on other reroutes.
421-
std::vector<Json::String> RerouteList = root["connections"][ConnectionsList[i]]["reroute_connections"].getMemberNames();
422-
for (size_t j = 0; j < RerouteList.size(); j++)
423-
{
424-
RerouteNode* NewReroute = new RerouteNode();
425-
std::string ID = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["reroute_ID"].asCString();
426-
NewReroute->ID = ID;
427-
NewReroute->Parent = NewConnection;
460+
Connection* NewConnection = Connections.back();
428461

429-
NewReroute->Position.x = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["position_x"].asFloat();
430-
NewReroute->Position.y = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["position_y"].asFloat();
462+
if (NewConnection == nullptr)
463+
continue;
431464

432-
NewConnection->RerouteNodes.push_back(NewReroute);
433-
}
465+
if (!ConnectionData.isMember("reroute_connections"))
466+
continue;
434467

435-
// Second pass to fill pointers.
436-
for (size_t j = 0; j < RerouteList.size(); j++)
437-
{
438-
std::string BeginSocketID = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["begin_socket_ID"].asCString();
439-
if (!BeginSocketID.empty())
468+
// First pass to fill information that does not depend on other reroutes.
469+
std::vector<Json::String> RerouteList = ConnectionData["reroute_connections"].getMemberNames();
470+
for (size_t j = 0; j < RerouteList.size(); j++)
440471
{
441-
NodeSocket* BeginSocket = NewConnection->Out;
442-
if (BeginSocketID == BeginSocket->GetID())
443-
NewConnection->RerouteNodes[j]->BeginSocket = BeginSocket;
444-
}
472+
const Json::Value& CurrentReroute = ConnectionData["reroute_connections"][std::to_string(j)];
473+
if (!CurrentReroute.isMember("reroute_ID") || !CurrentReroute.isMember("position_x") || !CurrentReroute.isMember("position_y"))
474+
continue;
445475

446-
std::string EndSocketID = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["end_socket_ID"].asCString();
447-
if (!EndSocketID.empty())
448-
{
449-
NodeSocket* EndSocket = NewConnection->In;
450-
if (EndSocketID == EndSocket->GetID())
451-
NewConnection->RerouteNodes[j]->EndSocket = EndSocket;
476+
if (!CurrentReroute["reroute_ID"].isString() || !CurrentReroute["position_x"].isNumeric() || !CurrentReroute["position_y"].isNumeric())
477+
continue;
478+
479+
RerouteNode* NewReroute = new RerouteNode();
480+
NewReroute->ID = CurrentReroute["reroute_ID"].asCString();
481+
NewReroute->Parent = NewConnection;
482+
483+
NewReroute->Position.x = CurrentReroute["position_x"].asFloat();
484+
NewReroute->Position.y = CurrentReroute["position_y"].asFloat();
485+
486+
NewConnection->RerouteNodes.push_back(NewReroute);
452487
}
453488

454-
std::string BeginRerouteID = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["begin_reroute_ID"].asCString();
455-
if (!BeginRerouteID.empty())
489+
// Second pass to fill pointers.
490+
for (size_t j = 0; j < RerouteList.size(); j++)
456491
{
457-
RerouteNode* BeginReroute = nullptr;
458-
for (size_t k = 0; k < NewConnection->RerouteNodes.size(); k++)
492+
const Json::Value& CurrentReroute = ConnectionData["reroute_connections"][std::to_string(j)];
493+
if (!CurrentReroute.isMember("begin_socket_ID") || !CurrentReroute.isMember("end_socket_ID") || !CurrentReroute.isMember("begin_reroute_ID") || !CurrentReroute.isMember("end_reroute_ID"))
494+
continue;
495+
496+
if (!CurrentReroute["begin_socket_ID"].isString() || !CurrentReroute["end_socket_ID"].isString() || !CurrentReroute["begin_reroute_ID"].isString() || !CurrentReroute["end_reroute_ID"].isString())
497+
continue;
498+
499+
std::string BeginSocketID = CurrentReroute["begin_socket_ID"].asCString();
500+
if (!BeginSocketID.empty())
501+
{
502+
NodeSocket* BeginSocket = NewConnection->Out;
503+
if (BeginSocketID == BeginSocket->GetID())
504+
NewConnection->RerouteNodes[j]->BeginSocket = BeginSocket;
505+
}
506+
507+
std::string EndSocketID = CurrentReroute["end_socket_ID"].asCString();
508+
if (!EndSocketID.empty())
509+
{
510+
NodeSocket* EndSocket = NewConnection->In;
511+
if (EndSocketID == EndSocket->GetID())
512+
NewConnection->RerouteNodes[j]->EndSocket = EndSocket;
513+
}
514+
515+
std::string BeginRerouteID = CurrentReroute["begin_reroute_ID"].asCString();
516+
if (!BeginRerouteID.empty())
459517
{
460-
if (BeginRerouteID == NewConnection->RerouteNodes[k]->ID)
461-
BeginReroute = NewConnection->RerouteNodes[k];
518+
RerouteNode* BeginReroute = nullptr;
519+
for (size_t k = 0; k < NewConnection->RerouteNodes.size(); k++)
520+
{
521+
if (BeginRerouteID == NewConnection->RerouteNodes[k]->ID)
522+
BeginReroute = NewConnection->RerouteNodes[k];
523+
}
524+
525+
if (BeginReroute != nullptr)
526+
NewConnection->RerouteNodes[j]->BeginReroute = BeginReroute;
462527
}
463528

464-
if (BeginReroute != nullptr)
465-
NewConnection->RerouteNodes[j]->BeginReroute = BeginReroute;
529+
std::string EndRerouteID = CurrentReroute["end_reroute_ID"].asCString();
530+
if (!EndRerouteID.empty())
531+
{
532+
RerouteNode* EndReroute = nullptr;
533+
for (size_t k = 0; k < NewConnection->RerouteNodes.size(); k++)
534+
{
535+
if (EndRerouteID == NewConnection->RerouteNodes[k]->ID)
536+
EndReroute = NewConnection->RerouteNodes[k];
537+
}
538+
539+
if (EndReroute != nullptr)
540+
NewConnection->RerouteNodes[j]->EndReroute = EndReroute;
541+
}
466542
}
467543

468-
std::string EndRerouteID = root["connections"][ConnectionsList[i]]["reroute_connections"][std::to_string(j)]["end_reroute_ID"].asCString();
469-
if (!EndRerouteID.empty())
544+
for (size_t i = 0; i < NewConnection->RerouteNodes.size(); i++)
470545
{
471-
RerouteNode* EndReroute = nullptr;
472-
for (size_t k = 0; k < NewConnection->RerouteNodes.size(); k++)
546+
// If any of the reroute nodes are invalid, we need to delete all of them.
547+
if (!IsRerouteNodeValid(NewConnection->RerouteNodes[i]))
473548
{
474-
if (EndRerouteID == NewConnection->RerouteNodes[k]->ID)
475-
EndReroute = NewConnection->RerouteNodes[k];
549+
for (size_t j = 0; j < NewConnection->RerouteNodes.size(); j++)
550+
{
551+
delete NewConnection->RerouteNodes[j];
552+
NewConnection->RerouteNodes.erase(NewConnection->RerouteNodes.begin() + j, NewConnection->RerouteNodes.begin() + j + 1);
553+
j--;
554+
}
555+
break;
476556
}
477-
478-
if (EndReroute != nullptr)
479-
NewConnection->RerouteNodes[j]->EndReroute = EndReroute;
480557
}
481558
}
482559
}
483560

484-
if (root.isMember("GroupComments"))
561+
if (Root.isMember("GroupComments"))
485562
{
486-
std::vector<Json::String> GroupCommentsList = root["GroupComments"].getMemberNames();
563+
if (!Root["GroupComments"].isObject())
564+
return false;
565+
566+
std::vector<Json::String> GroupCommentsList = Root["GroupComments"].getMemberNames();
487567
for (size_t i = 0; i < GroupCommentsList.size(); i++)
488568
{
569+
if (!Root["GroupComments"][std::to_string(i)].isObject())
570+
return false;
571+
489572
GroupComment* NewGroupComment = new GroupComment();
490-
NewGroupComment->FromJson(root["GroupComments"][std::to_string(i)]);
573+
NewGroupComment->FromJson(Root["GroupComments"][std::to_string(i)]);
491574
AddGroupComment(NewGroupComment);
492575
}
493576
}
494577

495-
if (root.isMember("renderOffset"))
578+
if (Root.isMember("renderOffset"))
496579
{
497-
float OffsetX = root["renderOffset"]["x"].asFloat();
498-
float OffsetY = root["renderOffset"]["y"].asFloat();
580+
if (!Root["renderOffset"].isMember("x") || !Root["renderOffset"].isMember("y"))
581+
return false;
582+
583+
float OffsetX = Root["renderOffset"]["x"].asFloat();
584+
float OffsetY = Root["renderOffset"]["y"].asFloat();
499585
SetRenderOffset(ImVec2(OffsetX, OffsetY));
500586
}
587+
588+
return true;
501589
}
502590

503591
void NodeArea::LoadFromFile(const char* FileName)

SubSystems/VisualNodeArea/VisualNodeArea.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ namespace VisNodeSys
7777

7878
std::string ToJson() const;
7979
void SaveToFile(const char* FileName) const;
80-
void LoadFromJson(std::string JsonText);
80+
bool LoadFromJson(std::string JsonText);
8181
void LoadFromFile(const char* FileName);
8282
void SaveNodesToFile(const char* FileName, std::vector<Node*> Nodes);
8383

@@ -140,6 +140,8 @@ namespace VisNodeSys
140140
ImVec2 GetRenderedViewCenter() const;
141141

142142
// *********************** Connections ************************
143+
size_t GetConnectionCount() const;
144+
143145
bool TryToConnect(const Node* OutNode, size_t OutNodeSocketIndex, const Node* InNode, size_t InNodeSocketIndex);
144146
bool TryToConnect(const Node* OutNode, std::string OutSocketID, const Node* InNode, std::string InSocketID);
145147

@@ -156,6 +158,8 @@ namespace VisNodeSys
156158

157159
bool GetConnectionStyle(Node* Node, bool bOutputSocket, size_t SocketIndex, ConnectionStyle& Style) const;
158160
void SetConnectionStyle(Node* Node, bool bOutputSocket, size_t SocketIndex, ConnectionStyle NewStyle);
161+
162+
size_t GetRerouteConnectionCount() const;
159163
private:
160164
struct SocketEvent
161165
{
@@ -297,6 +301,7 @@ namespace VisNodeSys
297301
void ConnectionsDoubleMouseClick();
298302
std::vector<ConnectionSegment> GetConnectionSegments(const Connection* Connection) const;
299303
bool AddRerouteNode(Connection* Connection, size_t SegmentToDivide, ImVec2 Position);
304+
bool IsRerouteNodeValid(const RerouteNode* RerouteNode);
300305
bool IsMouseOverConnection(Connection* Connection, const int Steps, const float MaxDistance, ImVec2* CollisionPoint = nullptr);
301306
bool IsMouseOverSegment(ImVec2 Begin, ImVec2 End, const int Steps, const float MaxDistance, ImVec2* CollisionPoint = nullptr);
302307
bool IsPointInRegion(const ImVec2& Point, const ImVec2& RegionMin, const ImVec2& RegionMax);

SubSystems/VisualNodeArea/VisualNodeAreaInput.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ void NodeArea::KeyboardInputUpdate()
672672
}
673673
}
674674

675-
static bool WasCopiedToClipboard = false;
675+
static bool bWasCopiedToClipboard = false;
676676
if (ImGui::IsKeyDown(ImGuiKey_LeftCtrl) || ImGui::IsKeyDown(ImGuiKey_RightCtrl))
677677
{
678678
if (ImGui::IsKeyDown(ImGuiKey_C))
@@ -686,18 +686,18 @@ void NodeArea::KeyboardInputUpdate()
686686
}
687687
else if (ImGui::IsKeyDown(ImGuiKey_V))
688688
{
689-
if (!WasCopiedToClipboard)
689+
if (!bWasCopiedToClipboard)
690690
{
691-
WasCopiedToClipboard = true;
691+
bWasCopiedToClipboard = true;
692692

693693
const std::string NodesToImport = NODE_CORE.GetClipboardText();
694-
Json::Value data;
694+
Json::Value Data;
695695

696-
JSONCPP_STRING err;
697-
const Json::CharReaderBuilder builder;
696+
JSONCPP_STRING Error;
697+
const Json::CharReaderBuilder Builder;
698698

699-
const std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
700-
if (!reader->parse(NodesToImport.c_str(), NodesToImport.c_str() + NodesToImport.size(), &data, &err))
699+
const std::unique_ptr<Json::CharReader> Reader(Builder.newCharReader());
700+
if (!Reader->parse(NodesToImport.c_str(), NodesToImport.c_str() + NodesToImport.size(), &Data, &Error))
701701
return;
702702

703703
NodeArea* NewNodeArea = new NodeArea();
@@ -760,7 +760,7 @@ void NodeArea::KeyboardInputUpdate()
760760
}
761761

762762
if (!ImGui::IsKeyDown(ImGuiKey_V))
763-
WasCopiedToClipboard = false;
763+
bWasCopiedToClipboard = false;
764764
}
765765

766766
Node* NodeArea::GetHovered() const

0 commit comments

Comments
 (0)