From e0e1cb565af05f921665f0cf8b1c24f087dcda18 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Sun, 22 Sep 2024 07:52:13 -0400 Subject: [PATCH 1/3] Correctly build reference from selection subname --- src/App/PropertyLinks.cpp | 4 ++-- src/Gui/SoFCUnifiedSelection.cpp | 18 ++++++++++++++++++ src/Mod/Assembly/JointObject.py | 31 +++++++++++++++++++------------ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index b0e2a9a346..4fd812f8ce 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -272,7 +272,7 @@ void PropertyLinkBase::_registerElementReference(App::DocumentObject *obj, std:: return; } if (shadow.newName.empty()) { - _updateElementReference(0, obj, sub, shadow, false); + _updateElementReference(nullptr, obj, sub, shadow, false); return; } GeoFeature* geo = nullptr; @@ -283,7 +283,7 @@ void PropertyLinkBase::_registerElementReference(App::DocumentObject *obj, std:: elementName, true, GeoFeature::ElementNameType::Export, - 0, + nullptr, &element, &geo); if (!geo || !element || !element[0]) { diff --git a/src/Gui/SoFCUnifiedSelection.cpp b/src/Gui/SoFCUnifiedSelection.cpp index 87d9202d3f..d26dc956ce 100644 --- a/src/Gui/SoFCUnifiedSelection.cpp +++ b/src/Gui/SoFCUnifiedSelection.cpp @@ -580,6 +580,24 @@ bool SoFCUnifiedSelection::setSelection(const std::vector &infos, bo const char *docname = vpd->getObject()->getDocument()->getName(); auto getFullSubElementName = [vpd](std::string &subName) { + // First, remove and clean up any unnecessary elements in the subName. We are essentially + // identifying places where there is an unneeded extra level of element in the name and + // removing it. If any of the lookups fail while we do that, we just use the subname unchanged. + const char *element = Data::findElementName(subName.c_str()); + std::string::difference_type dotCount = std::count(subName.begin(), subName.end(), '.'); + if ( element != nullptr && dotCount > 1) { + auto basename = subName; + basename.resize(basename.size() - strlen(element) - 1); // chop off the end and the '.', see if still okay + const char *temp_element = Data::findElementName(basename.c_str()); + if ( element != nullptr ) { + basename.resize(basename.size() - strlen(temp_element)); // chop off the extra element + basename.append(element); // and place the end back on. + temp_element = Data::findElementName(basename.c_str()); + if ( temp_element != nullptr ) { // if this is still a legit link, then use ut. + subName = basename; + } + } + } App::ElementNamePair elementName; App::GeoFeature::resolveElement(vpd->getObject(), subName.c_str(), elementName); if (!elementName.newName.empty()) { // If we have a mapped name use it diff --git a/src/Mod/Assembly/JointObject.py b/src/Mod/Assembly/JointObject.py index 110ed3676e..a019b9f01b 100644 --- a/src/Mod/Assembly/JointObject.py +++ b/src/Mod/Assembly/JointObject.py @@ -1800,19 +1800,26 @@ class TaskAssemblyCreateJoint(QtCore.QObject): # selectionObserver stuff def addSelection(self, doc_name, obj_name, sub_name, mousePos): rootObj = App.getDocument(doc_name).getObject(obj_name) - resolved = rootObj.resolveSubElement(sub_name) - element_name_TNP = resolved[1] - element_name = resolved[2] + target_link = sub_name - # Preprocess the sub_name to remove the TNP string - # We do this because after we need to add the vertex_name as well. - # And the names will be resolved anyway after. - if len(element_name_TNP.split(".")) == 2: - names = sub_name.split(".") - names.pop(-2) # remove the TNP string - sub_name = ".".join(names) - - ref = [rootObj, [sub_name]] + # There was extra code here to try to deal with TNP at a python layer. That is to be avoided at all costs, + # and corrections have been made to the underlying c++ code in the PR that includes this comment. + # + # The original python code was flawed, but two possible correct implementation in python follow. However the + # c++ approach is better, as the python layer should not know about TNP mitigation implementation. + # + # doc_obj, new_name, old_name = rootObj.resolveSubElement(sub_name) + # doc_obj, cont_obj, sub_sub_element_name, new_name = rootObj.resolve(sub_name) + # # The subname identified by selectionObserver has extra components in it. To get the minimal reference to + # # the element we need, we must identify the extra piece sub_sub_element name and remove it, and then + # # identify the new_name ( TNP path ) and remove it, and put the old_name ( short name ) on. + # # Alternatively we could take just the element and build back up using the sub_sub_element name to get a + # # different legit path, but the first approach is preferable. + # # element_name, *path_detail = sub_name.split(".") + # # target_link = ".".join((element_name,sub_sub_element_name,old_name)) + # element_name = sub_name.replace(sub_sub_element_name, "").replace(new_name, "") + # target_link = element_name[:-1] + old_name + ref = [rootObj, [target_link]] moving_part = self.getMovingPart(ref) From d51f0f0f2ae5089a0484fb11cebdfaf732f43acd Mon Sep 17 00:00:00 2001 From: bgbsww Date: Wed, 25 Sep 2024 23:34:47 -0400 Subject: [PATCH 2/3] Return to python; simplify to work with only classic names, provide migration --- src/Gui/SoFCUnifiedSelection.cpp | 18 ---------------- src/Mod/Assembly/JointObject.py | 37 ++++++++++++++++---------------- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/Gui/SoFCUnifiedSelection.cpp b/src/Gui/SoFCUnifiedSelection.cpp index d26dc956ce..87d9202d3f 100644 --- a/src/Gui/SoFCUnifiedSelection.cpp +++ b/src/Gui/SoFCUnifiedSelection.cpp @@ -580,24 +580,6 @@ bool SoFCUnifiedSelection::setSelection(const std::vector &infos, bo const char *docname = vpd->getObject()->getDocument()->getName(); auto getFullSubElementName = [vpd](std::string &subName) { - // First, remove and clean up any unnecessary elements in the subName. We are essentially - // identifying places where there is an unneeded extra level of element in the name and - // removing it. If any of the lookups fail while we do that, we just use the subname unchanged. - const char *element = Data::findElementName(subName.c_str()); - std::string::difference_type dotCount = std::count(subName.begin(), subName.end(), '.'); - if ( element != nullptr && dotCount > 1) { - auto basename = subName; - basename.resize(basename.size() - strlen(element) - 1); // chop off the end and the '.', see if still okay - const char *temp_element = Data::findElementName(basename.c_str()); - if ( element != nullptr ) { - basename.resize(basename.size() - strlen(temp_element)); // chop off the extra element - basename.append(element); // and place the end back on. - temp_element = Data::findElementName(basename.c_str()); - if ( temp_element != nullptr ) { // if this is still a legit link, then use ut. - subName = basename; - } - } - } App::ElementNamePair elementName; App::GeoFeature::resolveElement(vpd->getObject(), subName.c_str(), elementName); if (!elementName.newName.empty()) { // If we have a mapped name use it diff --git a/src/Mod/Assembly/JointObject.py b/src/Mod/Assembly/JointObject.py index a019b9f01b..44614f34e9 100644 --- a/src/Mod/Assembly/JointObject.py +++ b/src/Mod/Assembly/JointObject.py @@ -201,6 +201,7 @@ class Joint: self.migrationScript(joint) self.migrationScript2(joint) self.migrationScript3(joint) + self.migrationScript4(joint) # First Joint Connector if not hasattr(joint, "Reference1"): @@ -530,6 +531,20 @@ class Joint: joint.Offset2 = App.Placement(current_offset, App.Rotation(current_rotation, 0, 0)) + def migrationScript4(self, joint): + if hasattr(joint, "Reference1"): + base_name, *sub_names, feature_name = joint.Reference1[1][0].split(".") + ref1 = ".".join((base_name, *sub_names[:-1], feature_name)) + base_name, *sub_names, feature_name = joint.Reference1[1][1].split(".") + ref2 = ".".join((base_name, *sub_names[:-1], feature_name)) + joint.Reference1 = (joint.Reference1[0], [ref1, ref2]) + if hasattr(joint, "Reference2"): + base_name, *sub_names, feature_name = joint.Reference2[1][0].split(".") + ref1 = ".".join((base_name, *sub_names[:-1], feature_name)) + base_name, *sub_names, feature_name = joint.Reference2[1][1].split(".") + ref2 = ".".join((base_name, *sub_names[:-1], feature_name)) + joint.Reference2 = (joint.Reference2[0], [ref1, ref2]) + def dumps(self): return None @@ -1800,25 +1815,11 @@ class TaskAssemblyCreateJoint(QtCore.QObject): # selectionObserver stuff def addSelection(self, doc_name, obj_name, sub_name, mousePos): rootObj = App.getDocument(doc_name).getObject(obj_name) - target_link = sub_name - # There was extra code here to try to deal with TNP at a python layer. That is to be avoided at all costs, - # and corrections have been made to the underlying c++ code in the PR that includes this comment. - # - # The original python code was flawed, but two possible correct implementation in python follow. However the - # c++ approach is better, as the python layer should not know about TNP mitigation implementation. - # - # doc_obj, new_name, old_name = rootObj.resolveSubElement(sub_name) - # doc_obj, cont_obj, sub_sub_element_name, new_name = rootObj.resolve(sub_name) - # # The subname identified by selectionObserver has extra components in it. To get the minimal reference to - # # the element we need, we must identify the extra piece sub_sub_element name and remove it, and then - # # identify the new_name ( TNP path ) and remove it, and put the old_name ( short name ) on. - # # Alternatively we could take just the element and build back up using the sub_sub_element name to get a - # # different legit path, but the first approach is preferable. - # # element_name, *path_detail = sub_name.split(".") - # # target_link = ".".join((element_name,sub_sub_element_name,old_name)) - # element_name = sub_name.replace(sub_sub_element_name, "").replace(new_name, "") - # target_link = element_name[:-1] + old_name + # If the sub_name that comes in has extra sections in it, remove them. + doc_obj, new_name, old_name = rootObj.resolveSubElement(sub_name) + element_name, *path_detail = sub_name.split(".") + target_link = ".".join((element_name, old_name)) ref = [rootObj, [target_link]] moving_part = self.getMovingPart(ref) From d14ca595bd72f1255591fef41239d134ce852971 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Thu, 26 Sep 2024 08:04:46 -0400 Subject: [PATCH 3/3] Refine further --- src/Mod/Assembly/JointObject.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Mod/Assembly/JointObject.py b/src/Mod/Assembly/JointObject.py index 44614f34e9..3e7adae52e 100644 --- a/src/Mod/Assembly/JointObject.py +++ b/src/Mod/Assembly/JointObject.py @@ -1816,12 +1816,12 @@ class TaskAssemblyCreateJoint(QtCore.QObject): def addSelection(self, doc_name, obj_name, sub_name, mousePos): rootObj = App.getDocument(doc_name).getObject(obj_name) - # If the sub_name that comes in has extra sections in it, remove them. - doc_obj, new_name, old_name = rootObj.resolveSubElement(sub_name) - element_name, *path_detail = sub_name.split(".") - target_link = ".".join((element_name, old_name)) + # If the sub_name that comes in has extra features in it, remove them. For example a + # Part.Body.Pad.Sketch becomes Part.Body.Sketch and a + # Body.Pad.Sketch becomes Body.sketch + base_name, *path_detail, old_name = sub_name.split(".") + target_link = ".".join((base_name, *path_detail[:-2], old_name)) ref = [rootObj, [target_link]] - moving_part = self.getMovingPart(ref) # Check if the addition is acceptable (we are not doing this in selection gate to let user move objects)