Relevance of "Feature is not in a part" error message

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Relevance of "Feature is not in a part" error message

Postby NormandC » Sat Nov 10, 2018 8:01 pm

There is this error message that appears when you try to create a PartDesign feature from a sketch that is outside of a Body.

Feature is not in a part

In order to use this feature it needs to belong to a part object in the document.
Steps to reproduce:
  1. Create a PartDesign Body with an Additive Box
  2. Switch to the Sketcher workbench (<--- obviously the wrong work flow, I'm trying to prove a point...)
  3. Select the top face of the Box
  4. Create a sketch, select FlatFace attachment mode
  5. Sketch a circle then close
  6. Switch back to the PartDesign workbench
  7. With the sketch selected, try to create any sketch-based PartDesign feature
  8. You get the aforementioned popup error.
I located this error message here in the source code:
https://github.com/FreeCAD/FreeCAD/blob ... s.cpp#L175

Code: Select all

App::Part* getPartFor(const App::DocumentObject* obj, bool messageIfNot) {

    if(!obj)
        return nullptr;

    PartDesign::Body* body = getBodyFor(obj, false);
    if(body)
        obj = body;

    //get the part
    for(App::Part* p : obj->getDocument()->getObjectsOfType<App::Part>()) {
        if(p->hasObject(obj)) {
            return p;
        }
    }

    if (messageIfNot){
        QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Feature is not in a part"),
            QObject::tr("In order to use this feature it needs to belong to a part object in the document."));
    }

    return nullptr;
}
But a few lines above in the same file, there is this error message:
https://github.com/FreeCAD/FreeCAD/blob ... s.cpp#L142

Code: Select all

PartDesign::Body *getBodyFor(const App::DocumentObject* obj, bool messageIfNot,
                             bool autoActivate, bool assertModern)
{
    if(!obj)
        return nullptr;

    PartDesign::Body * rv = getBody(/*messageIfNot =*/false, autoActivate, assertModern);
    if (rv && rv->hasObject(obj))
        return rv;

    rv = PartDesign::Body::findBodyOf(obj);
    if (rv) {
        return rv;
    }

    if (messageIfNot){
        QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Feature is not in a body"),
            QObject::tr("In order to use this feature it needs to belong to a body object in the document."));
    }

    return nullptr;
}

In my opinion, the bit of code starting with L158 should be removed. It is problematic, because it seems to supersede the proper error message that starts at L126, and misleads the end user about the nature of the error.

As far as I know, App:Part has no relevance to PartDesign.
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Re: Relevance of "Feature is not in a part" error message

Postby NormandC » Sat Nov 10, 2018 8:22 pm

I'm trying to place myself in the shoes of a new user.

The error message tells me that it needs to belong to a part object, so I create one.
  1. Create a Part container
  2. Drop the Sketch on it (oddity: the Box gets transferred from the Body to the Part :? )
  3. Select the sketch, try to create a PartDesign feature again (say, Pocket)
  4. Same "Feature is not in a part" error. Hopefully you can understand that the user is starting to get annoyed...
  5. OK then, drop the Body over the Part container. (the Body is now inside the Part, but below the Box and the Sketch...)
  6. Repeat step 3. Now everything belongs to a part, it should go well, right? Well let's see:
  7. A new popup named "Dialog" (very self-describing wouldn't you agree?) appears saying that "You selected geometries which are not part of the active body. (...)" At this point I have no clue what to do (remember I'm a newbie), I just want to create my %?&! Pocket. So I leave the radio button "Make independent copy (recommended)" checked and click OK
  8. Getting the Pocket parameters, woohoo! Getting somewhere, right? Uh... No. ;)
  9. As soon as I press the OK button, I get an Input error pop up message that says
    No sketch support and no base shape: Please tell me where to remove the material of the pocket!
  10. At this point, the end user has no clue how to proceed, and there is probably a lot of swearing involved.
FYI this little experiment was done to try to get this last error message to show up. It was based on a frustrated user's comments on #freecad IRC from 8 days ago... On Gitter: https://gitter.im/FreeCAD/FreeCAD?at=5b ... 5bddf0598b

We probably lost that user. We need to fix this broken error message system, and better guide the end users.
Last edited by NormandC on Sat Nov 10, 2018 8:51 pm, edited 1 time in total.
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Re: Relevance of "Feature is not in a part" error message

Postby NormandC » Sat Nov 10, 2018 8:46 pm

https://github.com/FreeCAD/FreeCAD/blob ... t.cpp#L116

Code: Select all

    } catch (const Base::Exception&) {
        return new App::DocumentObjectExecReturn("No sketch support and no base shape: Please tell me where to remove the material of the pocket!");
}
https://github.com/FreeCAD/FreeCAD/blob ... e.cpp#L951

Code: Select all

    } catch (const Base::Exception&) {
            return new App::DocumentObjectExecReturn("No sketch support and no base shape: Please tell me where to remove the material of the hole!");
}
I propose that both error messages be changed to (native English speakers, please advise on wording):

Input error

The requested feature cannot be created. The reason may be either:
  • The active Body does not contain a base shape, so there is no material to be removed.
  • The selected sketch does not belong to the active Body.
Is it possible to format the text as a bullet list?

There is no need to mention sketch support. When a sketch belongs to a PartDesign Body, sketch support is irrelevant. I'm guessing this part of the error message predates v0.17.
User avatar
DeepSOIC
Posts: 6852
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Relevance of "Feature is not in a part" error message

Postby DeepSOIC » Sat Nov 10, 2018 11:43 pm

NormandC wrote:
Sat Nov 10, 2018 8:01 pm
my opinion, the bit of code starting with L158 should be removed. It is problematic,
Yes. It's some sort of strange code created somewhere in the era of early partdesign-next. Needs a fix.

NormandC wrote:
Sat Nov 10, 2018 8:22 pm
2. Drop the Sketch on it (oddity: the Box gets transferred from the Body to the Part :? )
I also don't like it. This behavior was probably created to make moving things across containers easier and less prone to hidden inconsistencies. The way it works makes me curse.

It was in the last (I think) pull requests by ickby regarding containers. I really really dislike a few things about it, particularly this one, along with a strange way Group-in-Part works.

That was one of the reasons I didn't continue ActiveContainer branch. I missed the problems of these changes before they got merged, and continuing activecontainer would make me redesign this system, effectively reverting ickby's work. I'm lost on how to proceed.
User avatar
DeepSOIC
Posts: 6852
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Relevance of "Feature is not in a part" error message

Postby DeepSOIC » Sun Nov 11, 2018 12:11 am

NormandC wrote:
Sat Nov 10, 2018 8:46 pm
There is no need to mention sketch support. When a sketch belongs to a PartDesign Body, sketch support is irrelevant. I'm guessing this part of the error message predates v0.17.
FreeCAD still (in theory) supports recomputing v0.16 projects, that's why Pad/Pocket still care about sketch support. But removing that from the error message sounds like a good idea, newbie doesn't need to know about all this legacy mess.
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Re: Relevance of "Feature is not in a part" error message

Postby NormandC » Sun Nov 11, 2018 5:05 am

Thanks DeepSOIC.

DeepSOIC wrote:
Sat Nov 10, 2018 11:43 pm
Yes. It's some sort of strange code created somewhere in the era of early partdesign-next. Needs a fix.
Am I naive in thinking that all that is needed, is to simply delete the App::Part stuff between L149 and L180?

DeepSOIC wrote:
Sun Nov 11, 2018 12:11 am
FreeCAD still (in theory) supports recomputing v0.16 projects, that's why Pad/Pocket still care about sketch support.
Doh! You're right. I had already forgotten about v0.16 somewhat still being active. But the more time passes, the less relevance it has. We hardly see people using v0.16 anymore, apart from people who can't get v0.16 because of their old OS, or the very few people who hang on to the old version.
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: Relevance of "Feature is not in a part" error message

Postby abdullah » Sun Dec 09, 2018 1:14 pm

I think that the most important decision to actually know how to proceed is to decide what the part container will become in the future.

If I am not wrong, most of the advanced users have already "given up" the pure theoretical (or theological) idea that an individual part is within a part container, that bodies are not necessarily separate parts, but that, in a general case, several bodies, each of them limited by the continuous solid paradigm are combined using boolean operations to produce a part. I think this might be because in practice one body = one part in 99,9% of the cases. Having hierarchical levels, that today serve no purpose, is seen as not useful and bloating the tree. Which I can understand and even share. But that is today with the functionality of today.

The other issue is that we have no clue yet, or at least I have no clue, how assemblies will look like. For example, whether they will operate on part containers or not. This could make what today is just seen as useless very valuable.

The effect of changing the code today without knowing what comes tomorrow, is similar to the problem of the support (of the sketch) above, in 0.19 we will have to live with some degree of backwards compatibility for 0.16, 0.17, 0.18 input files.

I do see a need for relevant message to the users. I think it is mandatory to review this for... well follow Normand's link to glitter... Many of these messages are worthless to the user and they mask the real information to be conveyed that could be helpful for him. They may be still very useful for the report view.

Now, irrespective to what I wrote above. I have taken a look to the code and where it is used. One of the usages is in this infamous dialog on whether upon copy the geometry should result in an independent copy, a dependent copy or a crosslink. I never now what to pick at that point. I do not know if there is anybody that is using the different options. Probably, this should also be answered as well before trying to see which is the best solution.

If I would have to point to a solution at this moment with what I know, probably I would substitute these messages with a system of exceptions. Because I have no clue about all the possible different usages of these functions and what is relevant in which context, probably creating specific exception types, throwing them and deciding at a higher level, at the point where the function was actually called, what to do and which information to throw to the report view and what information is in addition sensible to be presented to the user in the report view or as a pop up.

EDIT: BTW, we may also think how to enforcing the out of scope error messages in v0.19. There is such a warning at the moment the sketch is created in the workflow above...

Code: Select all

Sketcher::SketchObject / Sketch: Links go out of the allowed scope
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Re: Relevance of "Feature is not in a part" error message

Postby NormandC » Sun Dec 09, 2018 8:56 pm

abdullah wrote:
Sun Dec 09, 2018 1:14 pm
If I am not wrong, most of the advanced users have already "given up" the pure theoretical (or theological) idea that an individual part is within a part container, that bodies are not necessarily separate parts, but that, in a general case, several bodies, each of them limited by the continuous solid paradigm are combined using boolean operations to produce a part.
The intent of ickby was clear in creating the Part container: it was supposed to be the basis for an assembly container. Since he is no longer developing either PartDesign and Assembly workbenches, I suppose this could yet change depending on who takes up the mantle.

And a general consensus among regulars has emerged, as far as I can tell: most of the issues regarding the PD Body would be solved if the single solid rule was lifted.

abdullah wrote:
Sun Dec 09, 2018 1:14 pm
EDIT: BTW, we may also think how to enforcing the out of scope error messages in v0.19.
IIRC originally, ickby was enforcing the out of scope rule. I believe wmayer made it more lax before the v0.17 release, or it would have been very, very painful for many users used to mix Part and PartDesign.
triplus
Posts: 8605
Joined: Mon Dec 12, 2011 4:45 pm

Re: Relevance of "Feature is not in a part" error message

Postby triplus » Sun Dec 09, 2018 11:00 pm

Hi @abdullah

I am not all that interested in the original discussion. Such in-depth discussion (you started) likely deserves a separate thread. Especially as you have expressed some plans to work on PartDesign workbench in the future.
abdullah wrote:
Sun Dec 09, 2018 1:14 pm
One of the usages is in this infamous dialog on whether upon copy the geometry should result in an independent copy, a dependent copy or a crosslink. I never now what to pick at that point. I do not know if there is anybody that is using the different options. Probably, this should also be answered as well before trying to see which is the best solution.


This is a nice example, on how things are not all that optimal yet. In general i feel that @ickby was more or less alone, working on such big project. He did a good job and i guess it's just impossible to iron out everything and to meet all end user expectations. What i would therefore propose is when you plan to work on something. To select a small scope and open a discussion about it. Before doing any work. As if we will try to tackle everything in one go. You will fell under the same pressure as @ickby did. Gigantic expectations and just not enough resources available to tackle it. Your experience should be pleasant and rewarding too. Hopefully small scope tasks will attract developers like @ickby. To again find motivation to discuss things. Assembly 3 effort has become gigantic too. And it will likely need to be tackled in scopes. Therefore i see some opinion sharing from that side too.
User avatar
DeepSOIC
Posts: 6852
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Relevance of "Feature is not in a part" error message

Postby DeepSOIC » Mon Dec 10, 2018 12:12 am

NormandC wrote:
Sun Dec 09, 2018 8:56 pm
IIRC originally, ickby was enforcing the out of scope rule. I believe wmayer made it more lax before the v0.17 release, or it would have been very, very painful for many users used to mix Part and PartDesign.
More or less yes.

The reason of the out-of-scope rule was like so. Most tools don't account for placements of containers. It was not clear if we want that to be taken into account or not, and if it's possible at all. So this hard block was introduced, to allow picking any behavior in the future without collapsing everyone's projects which relied upon the current behavior.

That block made working in Part wb (and others, except PD) while sorting stuff into Part containers, impossible.. well, not impossible, but exceptionally annoying. So I created a thread stating these problems, and there were basically two quick solution: relax the restriction, or hide Part container creation tools. Werner (I think) picked to relax the restriction.