Unify element storage in Graph Editor classes#2756
Unify element storage in Graph Editor classes#2756jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
Conversation
This changelist unifies element storage and replaces direct data references with accessor methods in the `UiNode`, `UiPin`, and `UiEdge` classes, providing better encapsulation, and addressing edge cases where data copies within these classes could fall out of sync. Fixes AcademySoftwareFoundation#1261 and related issues after user edits.
There was a problem hiding this comment.
Could the new getNode() function for the pins be called something like getUiNode() instead of just getNode() since that already exists? I think having pin->getNode()->getNode() is a bit hard to read.
There was a problem hiding this comment.
Good catch, and I missed that! I've now renamed UiPin::getNode to UiPin::getUiNode for clarity.
| // Remove interface value in order to set the default of the input | ||
| pin->_input->setConnectedInterfaceName(mx::EMPTY_STRING); | ||
| setDefaults(pin->_input); | ||
| setDefaults(_graphNodes[upNode]->getInput()); |
There was a problem hiding this comment.
I'm curious why the node input no longer needs to set to the default as well?
There was a problem hiding this comment.
Ah, good question. That was an opportunistic fix that I found during testing of the graph editor, where we were unintentionally clearing the defaults of upstream inputs. I believe this new version of the logic is slightly better, and in my local testing I get more consistent behavior when disconnecting and reconnecting pins.
There was a problem hiding this comment.
Ah copy that! These changes look great. Thanks @jstone-lucasfilm
jstone-lucasfilm
merged commit 2e64ebb
into
AcademySoftwareFoundation:main
2 participants
This changelist unifies element storage and replaces direct data references with accessor methods in the
UiNode,UiPin, andUiEdgeclasses, providing better encapsulation, and addressing edge cases where data copies within these classes could fall out of sync.Fixes #1261 and related issues after user edits.