More helpful "need active body" message

[jnxd]

In a combobox you can use currentIndex() to find the currently selected item.

https://doc.qt.io/qt-5/qcombobox.html#currentIndex-prop

So, thing is, that was what I was using, but as suggested I switched to a listview. Now there is an equivalent function currentRow(), but the problem is the default for that is 0 when nothing is selected (as opposed to -1 for the combobox). Because of this, if I press OK without selecting anything, it still creates a new body (the 0th row option).

I'm now working around this by checking if the current row is selected, as so

Code: Select all

        int idx = dlg.bodySelect->currentRow();
        if (!dlg.bodySelect->item(idx)->isSelected()) {
            return activeBody; // this is null at this point
        }

To get the body that contains the active object currently selected:

Code: Select all

PartDesign::Body* bodyOfActiveObject = PartDesign::Body::findBodyOf(doc->getActiveObject());

That's one neat snippet!

Can you help out?

TheMarkster already said it all (except I guess that 'activeObject' can be different from the selected item in some cases).
I quickly read the thread, and should say I would prefer a list rather than a combo box too. Especially if double-click is supported.

I would like to know what those cases are, but the active object should be fine to get a working default most of the times.

I'll see what I can do about the double click, but I'm not much experienced with Qt.

[openBrain]

I would like to know what those cases are, but the active object should be fine to get a working default most of the times.

This is IMO rare in usage, but really easy to produce :
* Create a new doc
* Go to PartDesign WB
* Click "Create a new sketch", select any plane and leave Sketcher immediately -> A sketch inside a Body is created
* Got to Part WB
* Add a Cylinder
* Go in the tree view and select the previously created sketch (without editing it)
* Enter in Python Console 'App.ActiveDocument.ActiveObject' -> Answer is 'Cylinder'

'ActiveObject' is absolutely not defined as being the selected object, but basically 'the last modified one'.

I'll see what I can do about the double click, but I'm not much experienced with Qt.

I can help to some extent.

[jnxd]

'ActiveObject' is absolutely not defined as being the selected object, but basically 'the last modified one'.

I think I get it. Tried using the active object, but it clearly wasn't working well enough. So went for using the selection.

I can help to some extent.

Can you point me somewhere? Tried to use QObject::connect(), but can't seem to get the right usage...

Code: Select all

    QObject::connect(dlg.bodySelect, SIGNAL(itemDoubleClicked(QListWidgetItem *)),
                     dia, SLOT(accept()));

[openBrain]

Can you point me somewhere? Tried to use QObject::connect(), but can't seem to get the right usage...

Code: Select all

    QObject::connect(dlg.bodySelect, SIGNAL(itemDoubleClicked(QListWidgetItem *)),
                     dia, SLOT(accept()));

Can you point to some real code on your GitHub or tell what error you get ?

[jnxd]

Can you point me somewhere? Tried to use QObject::connect(), but can't seem to get the right usage...

Code: Select all

    QObject::connect(dlg.bodySelect, SIGNAL(itemDoubleClicked(QListWidgetItem *)),
                     dia, SLOT(accept()));

Can you point to some real code on your GitHub or tell what error you get ?

Almost immediately found out I had to use &dia instead of dia . The code is here: https://github.com/AjinkyaDahale/FreeCA ... s.cpp#L170

[openBrain]

Almost immediately found out I had to use &dia instead of dia . The code is here: https://github.com/AjinkyaDahale/FreeCA ... s.cpp#L170

Great.

[TheMarkster]

'ActiveObject' is absolutely not defined as being the selected object, but basically 'the last modified one'.

Thanks. You learn something new everyday if you pay attention.

[TheMarkster]

In practice, in many cases it would probably be better to simply offer a yes/no dialog asking the user whether to make the body containing the selected object the active body rather than offering a list of bodies to choose from.

Example: the document contains 2 bodies: body and body001 and 2 sketches: sketch and sketch001, in body and body001, respectively.

Body
--> Sketch

Body001
--> Sketch001

Neither body is active. The user selects Sketch001, and clicks the Pad icon. We should not be offering him to put the Pad in Body since the sketch he selected is in Body001. Just offer to make Body001 the active body in a yes/no dialog. Same could be done if Body is active. The user could asked if he wants to make Body001 active instead.

It is only when a new object is being created, such as a new sketch or datum plane or primitive, and when there is no active body, should a list of existing bodies be presented, along with the option of creating a new body.

[jnxd]

After some squashing and related shenanigans, here's the PR: https://github.com/FreeCAD/FreeCAD/pull/4949.

[jnxd]

Body
--> Sketch

Body001
--> Sketch001

Neither body is active. The user selects Sketch001, and clicks the Pad icon. We should not be offering him to put the Pad in Body since the sketch he selected is in Body001. Just offer to make Body001 the active body in a yes/no dialog. Same could be done if Body is active. The user could asked if he wants to make Body001 active instead.

I guess that makes sense. But I really was hoping this would be a quick fix when I started on Monday, and now it's Friday . I think I'm going to call this at this point (maybe adding it to datum objects in the next commit), and maybe we can brainstorm on further logic before I or someone else digs their hands back in.