[ Regression ? ] LinkPathArray 'Mis-Behave' ? - Carpenter Center

[vocx]

There is a rotation(to, from) that should be rotation(from, to). Fix will be along shortly.

Is this the only change? Or are there other things to consider in the placement on path functions?

Code: Select all

preRotation = App.Rotation(stdX, obj.TangentVector)   #make rotation from X to TangentVector

preRotation = App.Rotation(obj.TangentVector, stdX)   #make rotation from TangentVector to X

If you want to make only this change, then you can do it now, and I will fix my pull request accordingly. But if you have further changes to the placement on path functions, then you tell me what is better. In any case I can rebase my branch accordingly to include your new code.

[wandererfan]

Sorry, didn't see this before I submitted a PR.

Afaik, the change for prerotation is the only "geometry" related change required.

There is an issue with DiffuseColor getting lost. I included a fix for this, but I'm not sure it's in the right place.

[vocx]

...
There is an issue with DiffuseColor getting lost. I included a fix for this, but I'm not sure it's in the right place.

As I said in your pull request, the draftobjects.array.Array class should not handle any GUI property. This should be handled when the object is initially created by the make_path_array function in draftmake/make_patharray.py.

Once you assigned all properties, you can modify the view properties.

Code: Select all

# Regular properties assigned
...
obj.Count = count
obj.Align = align

# View properties assigned
if App.GuiUp:
    vobdc = obj.Base.ViewObject.DiffuseColor
        dc = vobdc
        for x in range(obj.Count - 1):
            dc =  dc + vobdc
    obj.ViewObject.DiffuseColor = dc

Maybe it's possible to put this code in the viewprovider class, in draftview/view_array.py, and call it manually. This is what the current code does already.

Code: Select all

# Call a function from the viewprovider in the make function
            if hasattr(obj.Base.ViewObject, "DiffuseColor"):
                if len(obj.Base.ViewObject.DiffuseColor) > 1:
                    obj.ViewObject.Proxy.resetColors(obj.ViewObject)

Maybe you can modify the resetColors method because it's possible it isn't considering the current state of the object.

[vocx]

This is an explanation on the note added by pull request #3586.

Currently, the draftobjects.patharray.PathArray class has two properties, PathObject and PathSubelements, to determine the object that will be the "path", and the subelements of that path. This means that the PathArray allows us to create the array using the entire object (PathObject) or only certain contiguous edges of that path (PathSubelements). For example, a polyline with three segments can create copies along the three segments, or only along the first segment, or only the first two segments, or only the last segment, or only the second and third segments, etc.

The PathObject is defined as a App::PropertyLinkGlobal, which allows us to select a single object in the property editor.

Code: Select all

obj.PathObject = App.ActiveDocument.Wire

On the other hand, PathSubelements is defined as an App::PropertyLinkSubListGlobal. This allows us to select several objects, and several subelements (edges). In my opinion, this property type is a mistake. The reason is that the way the code works, the subelements that we choose must exist inside the PathObject; however, as it is right now, we can actually pick any subelement, not necessarily inside PathObject, which would cause an error.

Correct:

Code: Select all

obj.PathSubelements = [(App.ActiveDocument.Wire, ("Edge1", "Edge2"))]  # edges of the PathObject, correct

Error:

Code: Select all

obj.PathSubelements = [(App.ActiveDocument.Wire005, ("Edge1", "Edge2")), (App.ActiveDocument.Wire006, ("Edge3", "Edge4"))]  # edges of other objects, error

In fact, a single property can be defined called, for example, PathObject, of type App::PropertyLinkSub. This property allows us to select a single object, or a single object with its subelements from the property editor. This means we can only pick edges of one valid object, and avoid the problem of having edges not belonging to that object. Also, the handling of the property is simpler because it's a simple list of two elements, instead of a list of tuples.

Code: Select all

obj.PathObject = [App.ActiveDocument.Wire, ()]  # no edges specified, uses the entire object
obj.PathObject = [App.ActiveDocument.Wire, ("Edge1", "Edge2")]  # only these edges will be used

We should address this in the future. It is not urgent right now because the current code works well, and picking edges of different objects is not intuitive so it's hard to trigger the erroneous selection, but the door is open for it.

Ideally we should migrate the PathArray objects by saving the values of PathObject (Link) and PathSubelements (LinkSubList), and then migrating them to a new PathObject (LinkSub).

Code: Select all

main = obj.PathObject
subelements = []

if obj.PathSubelements:
    subelements = obj.PathSubelements[1]

# Delete old properties
obj.removeProperty("PathObject")
obj.removeProperty("PathSubelements")

# Add new property
obj.addProperty("App::PropertyLinkSub", "PathObject")
obj.PathObject = [main, subelements]

However if this cannot be done easily, we may have to break compatibility with older files at some point.

---

Why does this problem exist? It seems that in the past there was a different syntax for defining lists of objects and their subelements.

Code: Select all

old style, obj.PathObject = [(object, "Edge1"), (object, "Edge2"), (object, "Edge3")]
new style, obj.PathObject = [object, ("Edge1", "Edge2", "Edge3")]

This was changed at some point, maybe around v0.15 and v0.16, when the Part Attachment mechanism and the new PartDesign Workbench were developed. So, it's possible that some of the Link properties didn't quite exist in their modern form, and as long as they didn't break the objects, no changes were made to the properties.

The current implementation of these properties accept both syntaxes, but only the new style is returned or printed to the terminal, so it is the preferred one.