I think this allowOverrideViewProviderName() function should be added to ViewProvider, because whether it is safe for use as an override or not is really determined by the corresponding ViewProvider's implementation. Maybe shorten the name to just allowOverride(). This function can be called right after view provider creation.wmayer wrote: ↑Fri Sep 13, 2019 10:56 pmgit commit 503f987cf fixes the most urgent issue. With the method allowOverrideViewProviderName() all document objects by default don't allow to override its view provider type.
Yes, it's true that a DocumentObject can't really decide if its ViewProvider has this flexibility or not, only the ViewProvider itself knows it. When I realized the consequences of git commit ff1d1cd341 I was not really sure what the intended behaviour is supposed to be. Is it really to allow to couple an arbitrary ViewProvider with a DocumentObject or was the actual intention to allow that a Python proxy can implement the method getViewProviderName().I think this allowOverrideViewProviderName() function should be added to ViewProvider, because whether it is safe for use as an override or not is really determined by the corresponding ViewProvider's implementation. Maybe shorten the name to just allowOverride(). This function can be called right after view provider creation.
OK, so then it must be checked inside Gui::Document::slotNewObject() whether the created view provider is compatible and if not an instance of the type delivered with getViewProviderName() must be created.Edit: even better, define the function as allowOverride(pObj) and let the view provider decide whether the object is supported or not. Currently, Gui::ViewProviderLink and PartGui::ViewProviderPartExt can accept any object.
I'll submit a PR for that soon.
I have submitted this pull request, PR#2553. Compiles, runs, and passes tests on my local machine. I'll monitor the travis builds and update as necessary.ezzieyguywuf wrote: ↑Wed Sep 04, 2019 8:47 pmwell then, I will begin work on a PR that incorporates these concepts.
Code: Select all
# Load a project with a body and select a sub-feature
Gui.Selection.getSelection() # has only one element
selection=[]
for i in App.ActiveDocument.Objects:
if Gui.Selection.isSelected(i):
selection.append(i)
selection # has two elements
When using None it raises an error and says it expects a str, not None.docName - document name. None means the active document, and '*' means all document
The default argument is True but here you can define three different modes which somehow makes no sense. The default of resolve should be 1 (or 2) but not True.resolve - whether to resolve the subname references.
0: do not resolve, 1: resolve, 2: resolve with element map
This is in order to disambiguate the same object in different placement because of it appears under different parents, for example, a child in a group, and the same child in a link to the same group.wmayer wrote: ↑Sun Oct 13, 2019 11:00 amIMO, the basic problem is the new concept that document objects that are children of certain objects with grouping behaviour (e.g. Body or Part but not DocumentObjectGroup) are considered as sub-elements. Now when querying with SelectionSingleton::isSelected() whether an object is selected it automatically checks if claimed objects are selected and if yes returns true.
The design goal is to add the new object path information to the selection while making it completely transparent to legacy code, if the implementation is correct, that is. The problem here is a bug in the implementation. There is a way to distinguish sub-object from sub-element (Face, Edge, Vertex etc.) in the path. Each sub object name must ends with a dot, the sub-element must contain no dot. So a selection path (i.e. the SubName) with sub-object but no sub-element will end with a dot. To support legacy code, Gui.Selection will auto resolve to the last sub-object, and store any sub-element inside SubName just like before.This behaviour might be OK for certain use cases but it's a regression for other uses cases which leads to undesired side-effects as shown in the referenced topic.
Yes, it would be better to add new API to obtained the selection path. But in this case, for the existing APIs, while the call signature can stay the same, the implementation still needs to be changed to incorporate the new concept.General remark:
When adding new features it's IMO best to not simply change the implementation of the existing API but add new methods instead and adjust those code parts that should benefit from the changes. This way it can be excluded to introduce any regressions like this in other parts of the code.
Thanks!PR submitted here
Does this logic apply to any object that implements the group logic or only those that have their own placement? I ask because for document object group I didn't notice the issue but for Part container and Body.This is in order to disambiguate the same object in different placement because of it appears under different parents, for example, a child in a group, and the same child in a link to the same group.
OK.The problem here is a bug in the implementation.
After a bit of searching I found it in getSubObjects() of DocumentObject and the overridden extensionGetSubObjects() methods.There is a way to distinguish sub-object from sub-element (Face, Edge, Vertex etc.) in the path. Each sub object name must ends with a dot, the sub-element must contain no dot.
I see.To support legacy code, Gui.Selection will auto resolve to the last sub-object, and store any sub-element inside SubName just like before.
Let me check after integrating your fix.BTW, I looked at MeshPart Tessellation code. It seems to use its own tree widget to offer selection if the user does not select anything in the treeview. Do you think it is still needed? I can easily add hierarchical selection (object in group), and non Part::Feature object support using Part::getShape(), but the extra tree widget is going to cause some confusion, because it displays no hierarchy even though it is a tree. Shall I just remove it?
It's gone now: git commit 1f5f00e655Let me check after integrating your fix.
There are actually two types of object path in play here. One is entirely in the App namespace, that relies on DocumentObject::getSubObject() API to resolve the path. The default implementation is to find the first dot in the given subname path, resolve the first level sub-object, and then calls its getSubObject() to resolve the rest of the path. BTW, getSubObjects() has a related but different purpose. getSubObjects() is used to enumerate sub-objects, similar to claimChildren() but in App namespace. It is mostly used for exporting object hierarchy, such as STEP export.