When you say current pop-up, do you mean the one in the screenshot in the OP? That's produced by PartDesignGui::getBody(), which I'm asking not to produce a message in this case. The message in the OP needs to stay around for other usages, but the new one needs a different message. -Ian-simonvanderveldt wrote:Where does the text in the current pop-up come from? I expected a changed message but I only see additions.
Automatically create body?
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
Re: Automatically create body?
-
- Posts: 62
- Joined: Tue Mar 14, 2017 2:11 pm
Re: Automatically create body?
Yeah, that's the one I meant. Thanks for the clarification, obviously makes senseian.rees wrote:When you say current pop-up, do you mean the one in the screenshot in the OP? That's produced by PartDesignGui::getBody(), which I'm asking not to produce a message in this case. The message in the OP needs to stay around for other usages, but the new one needs a different message. -Ian-simonvanderveldt wrote:Where does the text in the current pop-up come from? I expected a changed message but I only see additions.
Re: Automatically create body?
Yep, I'll do that in a few hours. Thanks!DeepSOIC wrote:Add it for primitives too, I think. Thus, it may be a good idea to wrap the code you added to new-sketch command into a standalone routine, like "autoNewBody()"...ian.rees wrote:How does this look? https://github.com/ianrrees/FreeCAD_tin ... -auto-body -Ian-
Re: Automatically create body?
The automatically created body should be undo/redo-able, too.
Re: Automatically create body?
Good catch, thanks! I'll try to get to this after work today. -Ian-wmayer wrote:The automatically created body should be undo/redo-able, too.
Re: Automatically create body?
DeepSOIC - I'm having a little trouble understanding some of the logic in PartDesign/Gui/Command.cpp CmdPartDesignNewSketch::activated().
Basically, I would like to to delay the creation of the new Sketch as far as possible, ideally until near to the openCommand(), so we don't have to sprinkle abortCommand() calls everywhere that the method might return early. I think some of the major branches are impossible in the case of a newly-created Body. For example, I guess (FaceFilter.match() || PlaneFilter.match()) won't ever be true?
The current PR is at https://github.com/FreeCAD/FreeCAD/pull/667 - I added a couple TODOs around a note at the top of CmdPartDesignNewSketch::activated()
Any chance you have some time to take a look? Thanks! -Ian-
Basically, I would like to to delay the creation of the new Sketch as far as possible, ideally until near to the openCommand(), so we don't have to sprinkle abortCommand() calls everywhere that the method might return early. I think some of the major branches are impossible in the case of a newly-created Body. For example, I guess (FaceFilter.match() || PlaneFilter.match()) won't ever be true?
The current PR is at https://github.com/FreeCAD/FreeCAD/pull/667 - I added a couple TODOs around a note at the top of CmdPartDesignNewSketch::activated()
Any chance you have some time to take a look? Thanks! -Ian-
- DeepSOIC
- Veteran
- Posts: 7896
- Joined: Fri Aug 29, 2014 12:45 am
- Location: used to be Saint-Petersburg, Russia
Re: Automatically create body?
Unfortunately I don't quite understand, even after scrolling through the changes of your PR. So here's my attempt to answer, maybe way off from what you wanted to getian.rees wrote:DeepSOIC - I'm having a little trouble understanding some of the logic in PartDesign/Gui/Command.cpp CmdPartDesignNewSketch::activated().
Basically, I would like to to delay the creation of the new Sketch as far as possible, ideally until near to the openCommand(), so we don't have to sprinkle abortCommand() calls everywhere that the method might return early. I think some of the major branches are impossible in the case of a newly-created Body. For example, I guess (FaceFilter.match() || PlaneFilter.match()) won't ever be true?
Often with complicated logic, I use exceptions to cancel the process, rather than returns. Then, an exception handler around the whole logic calls the abortCommand(), and maybe does other cleanup.
If you want to include body creation into "new sketch" transaction, I'd just recommend to create an independent transaction for body creation, and don't bother any further. Or is transaction management of sketch creation itself broken, and you are attempting to fix it?
- DeepSOIC
- Veteran
- Posts: 7896
- Joined: Fri Aug 29, 2014 12:45 am
- Location: used to be Saint-Petersburg, Russia
Re: Automatically create body?
Not sure. Creation of body will likely purge selection, so probably yes. But I imagine the situation:ian.rees wrote:I think some of the major branches are impossible in the case of a newly-created Body. For example, I guess (FaceFilter.match() || PlaneFilter.match()) won't ever be true?
1. Part Box
2. Select face of Box
2. PartDesign Sketch
In this case, a user generally expects a sketch created on a face, which can then be padded to enlarge the box, etc. Transplanting this to a new workflow, this should happen:
1. New body
2. Body.BaseFeature = Box
3. new sketch
4. Sketch.Support = Box.Face1
Done.
Re: Automatically create body?
Thanks, I'll take another stab at it tonight hopefully. Most of that PR is just refactoring the commands for adding additive/subtractive primitives so the new logic isn't repeated in 7 nearly identical places. On that front - I realised that we shouldn't be making new Body objects for subtractive primitives, because that would imply that it's OK to have a shape which is is not in a Body, that the new primitive will be subtracted from. For additive primitives, I think it does make sense to make a new Body as discussed.
The part I'm not understanding is all in CmdPartDesignNewSketch::activated(). And, the issue is mostly that I haven't got a good understanding of all the sketch creation modes.
The part I'm not understanding is all in CmdPartDesignNewSketch::activated(). And, the issue is mostly that I haven't got a good understanding of all the sketch creation modes.
But, that's the textbook use of goto!DeepSOIC wrote: Often with complicated logic, I use exceptions to cancel the process, rather than returns. Then, an exception handler around the whole logic calls the abortCommand(), and maybe does other cleanup.
- DeepSOIC
- Veteran
- Posts: 7896
- Joined: Fri Aug 29, 2014 12:45 am
- Location: used to be Saint-Petersburg, Russia
Re: Automatically create body?
I don't mind using gotos as long as they make code more readable, and help avoid repetitions, of course. And I even did it in FC code once, and I think it was almost merged. But it caused interesting compilation problems on Linux (related to jumps over variable initializations). Then I switched to exception approach.ian.rees wrote:But, that's the textbook use of goto!
And using exceptions is more interesting than plain gotos, because I can raise an exception deeper in stack. And a side benefit of gracefully handling unexpected failures...
Anyway, it's up to you.
I'll try gather it up.ian.rees wrote:And, the issue is mostly that I haven't got a good understanding of all the sketch creation modes.